Skip to content

Decouple event source from cache + list discriminator #1378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d33df68
fix: PerResourcePollingEventSourceIT integration test race condition …
csviri Sep 28, 2022
3bb2d45
feat: improvements on caching and dependent resources
csviri Jul 27, 2022
741ac0f
wip
csviri Jul 27, 2022
a5b2bb6
fix
csviri Jul 27, 2022
7cbc297
kubernetes dependent resource configuration
csviri Jul 28, 2022
fbac50f
IT fix
csviri Jul 28, 2022
5fe2013
fixed ITs
csviri Jul 29, 2022
2828706
index based discriminator
csviri Jul 29, 2022
fed11e8
IT fix
csviri Jul 29, 2022
6b392e8
wip
csviri Jul 29, 2022
364e73d
fixes from rebase from next
csviri Aug 26, 2022
342ab86
fix after rebase
csviri Sep 5, 2022
9a16aa1
event source provider to context
csviri Sep 6, 2022
c7b2d2e
todo fixes
csviri Sep 16, 2022
fc01f94
remove void discriminator
csviri Sep 27, 2022
9aa1890
rebase on next
csviri Sep 27, 2022
e355a1e
refactor: clean-up
metacosm Sep 27, 2022
98f0378
fix: wrong reference
metacosm Sep 27, 2022
18e7ce5
refactor: extract more generic instantiateIfNotDefault method
metacosm Sep 27, 2022
4f9de03
feat: add isSameResource method
metacosm Aug 29, 2022
f5079bf
refactor: use isSameResource method
metacosm Sep 27, 2022
af9d162
refactor: clean-up
metacosm Sep 27, 2022
569bed4
feat: re-introduce getSecondaryResource and simplify
metacosm Sep 27, 2022
fa0bfb7
fix: typos and wrong logged information
metacosm Sep 27, 2022
e5a98ff
refactor: remove useless AbstractCachingDependentResource class
metacosm Aug 29, 2022
c689843
refactor: use new fabric8 resource access pattern
metacosm Sep 28, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.*;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
Expand Down Expand Up @@ -54,9 +53,8 @@ public AnnotationControllerConfiguration(Reconciler<P> reconciler) {
this.reconciler = reconciler;
this.annotation = reconciler.getClass().getAnnotation(ControllerConfiguration.class);
if (annotation == null) {
throw new OperatorException(
"Missing mandatory @" + ControllerConfiguration.class.getSimpleName() +
" annotation for reconciler: " + reconciler);
throw new OperatorException("Missing mandatory @" + CONTROLLER_CONFIG_ANNOTATION +
" annotation for reconciler: " + reconciler);
}
}

Expand Down Expand Up @@ -247,9 +245,9 @@ public List<DependentResourceSpec> getDependentResources() {
final var context = "DependentResource of type '" + dependentType.getName() + "'";
spec = new DependentResourceSpec(dependentType, config, name,
Set.of(dependent.dependsOn()),
instantiateConditionIfNotDefault(dependent.readyPostcondition(), context),
instantiateConditionIfNotDefault(dependent.reconcilePrecondition(), context),
instantiateConditionIfNotDefault(dependent.deletePostcondition(), context));
instantiateIfNotDefault(dependent.readyPostcondition(), Condition.class, context),
instantiateIfNotDefault(dependent.reconcilePrecondition(), Condition.class, context),
instantiateIfNotDefault(dependent.deletePostcondition(), Condition.class, context));
specsMap.put(name, spec);
}

Expand All @@ -258,10 +256,10 @@ public List<DependentResourceSpec> getDependentResources() {
return specs;
}

protected Condition<?, ?> instantiateConditionIfNotDefault(Class<? extends Condition> condition,
protected <T> T instantiateIfNotDefault(Class<? extends T> toInstantiate, Class<T> defaultClass,
String context) {
if (condition != Condition.class) {
return instantiateAndConfigureIfNeeded(condition, Condition.class, context);
if (!defaultClass.equals(toInstantiate)) {
return instantiateAndConfigureIfNeeded(toInstantiate, defaultClass, context);
}
return null;
}
Expand All @@ -287,6 +285,7 @@ private Object createKubernetesResourceConfig(Class<? extends DependentResource>
OnUpdateFilter<? extends HasMetadata> onUpdateFilter = null;
OnDeleteFilter<? extends HasMetadata> onDeleteFilter = null;
GenericFilter<? extends HasMetadata> genericFilter = null;
ResourceDiscriminator<?, ? extends HasMetadata> resourceDiscriminator = null;
if (kubeDependent != null) {
if (!Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES,
kubeDependent.namespaces())) {
Expand All @@ -297,7 +296,6 @@ private Object createKubernetesResourceConfig(Class<? extends DependentResource>
final var fromAnnotation = kubeDependent.labelSelector();
labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;


final var context =
KUBE_DEPENDENT_NAME + " annotation on " + dependentType.getName() + " DependentResource";
onAddFilter = createFilter(kubeDependent.onAddFilter(), OnAddFilter.class, context)
Expand All @@ -311,10 +309,15 @@ private Object createKubernetesResourceConfig(Class<? extends DependentResource>
genericFilter =
createFilter(kubeDependent.genericFilter(), GenericFilter.class, context)
.orElse(null);

resourceDiscriminator =
instantiateIfNotDefault(kubeDependent.resourceDiscriminator(),
ResourceDiscriminator.class, context);
}

config =
new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS, onAddFilter,
new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS,
resourceDiscriminator, onAddFilter,
onUpdateFilter, onDeleteFilter, genericFilter);

return config;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,27 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;

public interface Context<P extends HasMetadata> {

Optional<RetryInfo> getRetryInfo();

default <T> Optional<T> getSecondaryResource(Class<T> expectedType) {
return getSecondaryResource(expectedType, null);
default <R> Optional<R> getSecondaryResource(Class<R> expectedType) {
return getSecondaryResource(expectedType, (String) null);
}

<T> Set<T> getSecondaryResources(Class<T> expectedType);
<R> Set<R> getSecondaryResources(Class<R> expectedType);

<T> Optional<T> getSecondaryResource(Class<T> expectedType, String eventSourceName);
@Deprecated(forRemoval = true)
<R> Optional<R> getSecondaryResource(Class<R> expectedType, String eventSourceName);

<R> Optional<R> getSecondaryResource(Class<R> expectedType,
ResourceDiscriminator<R, P> discriminator);

ControllerConfiguration<P> getControllerConfiguration();

ManagedDependentResourceContext managedDependentResourceContext();

EventSourceRetriever<P> eventSourceRetriever();
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
import io.javaoperatorsdk.operator.processing.Controller;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;

public class DefaultContext<P extends HasMetadata> implements Context<P> {

Expand Down Expand Up @@ -47,6 +48,12 @@ public <T> Optional<T> getSecondaryResource(Class<T> expectedType, String eventS
.getSecondaryResource(primaryResource);
}

@Override
public <R> Optional<R> getSecondaryResource(Class<R> expectedType,
ResourceDiscriminator<R, P> discriminator) {
return discriminator.distinguish(expectedType, primaryResource, this);
}

@Override
public ControllerConfiguration<P> getControllerConfiguration() {
return controllerConfiguration;
Expand All @@ -57,6 +64,11 @@ public ManagedDependentResourceContext managedDependentResourceContext() {
return defaultManagedDependentResourceContext;
}

@Override
public EventSourceRetriever<P> eventSourceRetriever() {
return controller.getEventSourceManager();
}

public DefaultContext<P> setRetryInfo(RetryInfo retryInfo) {
this.retryInfo = retryInfo;
return this;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.javaoperatorsdk.operator.api.reconciler;

import java.util.Optional;

import io.fabric8.kubernetes.api.model.HasMetadata;

public interface ResourceDiscriminator<R, P extends HasMetadata> {

Optional<R> distinguish(Class<R> resource, P primary, Context<P> context);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package io.javaoperatorsdk.operator.api.reconciler;

import java.util.Optional;
import java.util.function.Function;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.processing.event.ResourceID;

public class ResourceIDMatcherDiscriminator<R extends HasMetadata, P extends HasMetadata>
implements ResourceDiscriminator<R, P> {

private final Function<P, ResourceID> mapper;

public ResourceIDMatcherDiscriminator(Function<P, ResourceID> mapper) {
this.mapper = mapper;
}

@Override
public Optional<R> distinguish(Class<R> resource, P primary, Context<P> context) {
var resourceID = mapper.apply(primary);
return context.getSecondaryResources(resource).stream()
.filter(resourceID::isSameResource)
.findFirst();
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent;

import java.util.Optional;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.ResourceOwner;

/**
* An interface to implement and provide dependent resource support.
*
* @param <R> the dependent resource type
* @param <P> the associated primary resource type
*/
public interface DependentResource<R, P extends HasMetadata> extends ResourceOwner<R, P> {
public interface DependentResource<R, P extends HasMetadata> {

/**
* Reconciles the dependent resource given the desired primary state
Expand All @@ -21,6 +22,17 @@ public interface DependentResource<R, P extends HasMetadata> extends ResourceOwn
*/
ReconcileResult<R> reconcile(P primary, Context<P> context);

/**
* Retrieves the resource type associated with this DependentResource
*
* @return the resource type associated with this DependentResource
*/
Class<R> resourceType();

default Optional<R> getSecondaryResource(P primary, Context<P> context) {
return Optional.empty();
}

/**
* Computes a default name for the specified DependentResource class
*
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package io.javaoperatorsdk.operator.processing.dependent;

import java.util.Optional;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.Ignore;
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
import io.javaoperatorsdk.operator.processing.event.ResourceID;
Expand All @@ -21,6 +24,8 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
protected Creator<R, P> creator;
protected Updater<R, P> updater;

private ResourceDiscriminator<R, P> resourceDiscriminator;

@SuppressWarnings("unchecked")
public AbstractDependentResource() {
creator = creatable ? (Creator<R, P>) this : null;
Expand All @@ -29,7 +34,7 @@ public AbstractDependentResource() {

@Override
public ReconcileResult<R> reconcile(P primary, Context<P> context) {
var maybeActual = getSecondaryResource(primary);
Optional<R> maybeActual = getSecondaryResource(primary, context);
if (creatable || updatable) {
if (maybeActual.isEmpty()) {
if (creatable) {
Expand Down Expand Up @@ -62,6 +67,11 @@ public ReconcileResult<R> reconcile(P primary, Context<P> context) {
return ReconcileResult.noOperation(maybeActual.orElse(null));
}

public Optional<R> getSecondaryResource(P primary, Context<P> context) {
return resourceDiscriminator == null ? context.getSecondaryResource(resourceType())
: resourceDiscriminator.distinguish(resourceType(), primary, context);
}

private void throwIfNull(R desired, P primary, String descriptor) {
if (desired == null) {
throw new DependentResourceException(
Expand Down Expand Up @@ -118,4 +128,13 @@ protected R desired(P primary, Context<P> context) {
throw new IllegalStateException(
"desired method must be implemented if this DependentResource can be created and/or updated");
}

public void setResourceDiscriminator(
ResourceDiscriminator<R, P> resourceDiscriminator) {
this.resourceDiscriminator = resourceDiscriminator;
}

public ResourceDiscriminator<R, P> getResourceDiscriminator() {
return resourceDiscriminator;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@ public abstract class AbstractEventSourceHolderDependentResource<R, P extends Ha
implements EventSourceProvider<P> {

private T eventSource;
private final Class<R> resourceType;
private boolean isCacheFillerEventSource;
protected OnAddFilter<R> onAddFilter;
protected OnUpdateFilter<R> onUpdateFilter;
protected OnDeleteFilter<R> onDeleteFilter;
protected GenericFilter<R> genericFilter;

protected AbstractEventSourceHolderDependentResource(Class<R> resourceType) {
this.resourceType = resourceType;
}

public EventSource initEventSource(EventSourceContext<P> context) {
// some sub-classes (e.g. KubernetesDependentResource) can have their event source created
Expand All @@ -42,6 +46,11 @@ public EventSource initEventSource(EventSourceContext<P> context) {
return eventSource;
}

@Override
public Class<R> resourceType() {
return resourceType;
}

protected abstract T createEventSource(EventSourceContext<P> context);

protected void setEventSource(T eventSource) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Ignore;
import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource;
import io.javaoperatorsdk.operator.processing.event.source.CacheKeyMapper;
import io.javaoperatorsdk.operator.processing.event.source.ExternalResourceCachingEventSource;

@Ignore
public abstract class AbstractPollingDependentResource<R, P extends HasMetadata>
extends AbstractCachingDependentResource<R, P> implements CacheKeyMapper<R> {
extends
AbstractEventSourceHolderDependentResource<R, P, ExternalResourceCachingEventSource<R, P>>
implements CacheKeyMapper<R> {

public static final int DEFAULT_POLLING_PERIOD = 5000;
private long pollingPeriod;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public AbstractSimpleDependentResource(UpdatableCache<R> cache) {
}

@Override
public Optional<R> getSecondaryResource(HasMetadata primaryResource) {
public Optional<R> getSecondaryResource(P primaryResource, Context<P> context) {
return cache.get(ResourceID.fromResource(primaryResource));
}

Expand Down
Loading