Skip to content

refactor: simplify handling of reused event sources #1513

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

Closed
wants to merge 69 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
6e42e53
feat: improvements on caching and dependent resources
csviri Jul 27, 2022
7e3119c
wip
csviri Jul 27, 2022
ffc150b
fix
csviri Jul 27, 2022
7b16504
kubernetes dependent resource configuration
csviri Jul 28, 2022
7b442bc
IT fix
csviri Jul 28, 2022
f5fd957
fixed ITs
csviri Jul 29, 2022
d9d4a95
index based discriminator
csviri Jul 29, 2022
924f162
IT fix
csviri Jul 29, 2022
7eb4eb7
wip
csviri Jul 29, 2022
1f6b8b9
fixes from rebase from next
csviri Aug 26, 2022
c359557
fix after rebase
csviri Sep 5, 2022
e9c8c7e
event source provider to context
csviri Sep 6, 2022
2042fd8
todo fixes
csviri Sep 16, 2022
8f04146
remove void discriminator
csviri Sep 27, 2022
bdd2d96
rebase on next
csviri Sep 27, 2022
378f463
fix: bulk creation of dependent resource directly in abstract resource
csviri Sep 8, 2022
210034a
wip
csviri Sep 9, 2022
7a56e1c
wip
csviri Sep 9, 2022
bc4709c
wip to start IT
csviri Sep 9, 2022
2e96cd9
fixes, progress
csviri Sep 9, 2022
6892077
wp
csviri Sep 12, 2022
9d8055b
matcher
csviri Sep 12, 2022
e222735
test passes
csviri Sep 12, 2022
e55592d
bulk dependent resource to an interface
csviri Sep 13, 2022
b0a7227
wip
csviri Sep 14, 2022
88f0053
test improvement
csviri Sep 14, 2022
fe33446
note
csviri Sep 14, 2022
65705d4
wip
csviri Sep 15, 2022
58c4096
rebase on next
csviri Sep 20, 2022
6a4109b
increates test timeout
csviri Sep 20, 2022
5a3ac2d
comment
csviri Sep 20, 2022
50c6a0f
fix format
csviri Sep 20, 2022
b955396
wip
csviri Sep 20, 2022
069290d
delete, other improvements
csviri Sep 21, 2022
018e8c5
manage tests, refactored ITs
csviri Sep 21, 2022
a596b00
additionl IT
csviri Sep 21, 2022
bd6b9e7
external resource
csviri Sep 21, 2022
50a50b9
external resource IT
csviri Sep 22, 2022
3d07b2e
docs
csviri Sep 22, 2022
fe3bd1c
feat: optional event source from dependent resources
csviri Sep 16, 2022
efd2324
fix test
csviri Sep 22, 2022
ab1e980
wip
csviri Sep 23, 2022
34850d1
wip
csviri Sep 23, 2022
fc618d7
wip
csviri Sep 26, 2022
72ef5b6
wip
csviri Sep 26, 2022
5927d96
format
csviri Sep 27, 2022
6c87c40
fix
csviri Sep 27, 2022
b910139
IT skeleton
csviri Sep 27, 2022
ef67469
IT managed dependent resourc
csviri Sep 27, 2022
d80feed
IT improvement
csviri Sep 27, 2022
a6bee16
wip external resource handling
csviri Sep 27, 2022
94dff83
fix shortname
csviri Sep 27, 2022
5f6356e
IT
csviri Sep 27, 2022
a488572
feat: decouple event source from cache + list discriminator (#1378)
csviri Sep 28, 2022
7c66c05
bulk dependent resources (#1448)
csviri Sep 29, 2022
adef026
Merge branch 'next' into optional-eventsource-on-dr
csviri Sep 29, 2022
66882bc
fixes after merge
csviri Sep 29, 2022
4792f60
feat: decouple event source from cache + list discriminator (#1378)
csviri Sep 28, 2022
99f4b10
bulk dependent resources (#1448)
csviri Sep 29, 2022
d403291
Merge branch 'next' into optional-eventsource-on-dr
csviri Oct 3, 2022
ba64ed8
fixes after merge
csviri Oct 3, 2022
5a912a5
controller fix
csviri Oct 3, 2022
4576677
merge fixes
csviri Oct 3, 2022
55c3796
automatic event source selection
csviri Oct 3, 2022
964cf5a
fix
csviri Oct 3, 2022
9bad894
proper setting event source
csviri Oct 3, 2022
b86b6f7
fix: avoid NPE if no dependent resources are passed
metacosm Oct 3, 2022
b99847d
refactor: simplify handling of reused event sources
metacosm Oct 3, 2022
6fa1b5d
docs: javadoc fix
csviri Oct 4, 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,11 @@ 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),
Constants.NO_VALUE_SET.equals(dependent.eventSource()) ? null
: dependent.eventSource());
specsMap.put(name, spec);
}

Expand All @@ -258,10 +258,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,17 +287,17 @@ 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;
String eventSourceNameToUse = null;
if (kubeDependent != null) {
if (!Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES,
kubeDependent.namespaces())) {
namespaces = Set.of(kubeDependent.namespaces());
configuredNS = true;
}

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 +311,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 @@ -174,7 +174,7 @@ private void replaceConfig(String name, Object newConfig, DependentResourceSpec<
namedDependentResourceSpecs.put(name,
new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name,
current.getDependsOn(), current.getReadyCondition(), current.getReconcileCondition(),
current.getDeletePostCondition()));
current.getDeletePostCondition(), current.getEventSourceName().orElse(null)));
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -220,7 +220,8 @@ public ControllerConfiguration<R> build() {
KubernetesDependentResourceConfig c) {
return new DependentResourceSpec(spec.getDependentResourceClass(),
c.setNamespaces(namespaces), name, spec.getDependsOn(), spec.getReadyCondition(),
spec.getReconcileCondition(), spec.getDeletePostCondition());
spec.getReconcileCondition(), spec.getDeletePostCondition(),
(String) spec.getEventSourceName().orElse(null));
}

public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,20 @@ public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {

private final Condition<?, ?> deletePostCondition;

private final String eventSourceName;

public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
String name, Set<String> dependsOn, Condition<?, ?> readyCondition,
Condition<?, ?> reconcileCondition, Condition<?, ?> deletePostCondition) {
Condition<?, ?> reconcileCondition, Condition<?, ?> deletePostCondition,
String eventSourceName) {
this.dependentResourceClass = dependentResourceClass;
this.dependentResourceConfig = dependentResourceConfig;
this.name = name;
this.dependsOn = dependsOn;
this.readyCondition = readyCondition;
this.reconcileCondition = reconcileCondition;
this.deletePostCondition = deletePostCondition;
this.eventSourceName = eventSourceName;
}

public Class<T> getDependentResourceClass() {
Expand Down Expand Up @@ -89,4 +93,8 @@ public Condition getReconcileCondition() {
public Condition getDeletePostCondition() {
return deletePostCondition;
}

public Optional<String> getEventSourceName() {
return Optional.ofNullable(eventSourceName);
}
}
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
Expand Up @@ -3,6 +3,8 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.processing.event.EventSourceManager;
import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
import io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache;

Expand All @@ -13,14 +15,14 @@
*/
public class EventSourceContext<P extends HasMetadata> {

private final IndexerResourceCache<P> primaryCache;
private final EventSourceManager<P> eventSourceManager;
private final ControllerConfiguration<P> controllerConfiguration;
private final KubernetesClient client;

public EventSourceContext(IndexerResourceCache<P> primaryCache,
public EventSourceContext(EventSourceManager<P> eventSourceManager,
ControllerConfiguration<P> controllerConfiguration,
KubernetesClient client) {
this.primaryCache = primaryCache;
this.eventSourceManager = eventSourceManager;
this.controllerConfiguration = controllerConfiguration;
this.client = client;
}
Expand All @@ -31,7 +33,7 @@ public EventSourceContext(IndexerResourceCache<P> primaryCache,
* @return the primary resource cache
*/
public IndexerResourceCache<P> getPrimaryCache() {
return primaryCache;
return eventSourceManager.getControllerResourceEventSource();
}

/**
Expand All @@ -54,4 +56,8 @@ public ControllerConfiguration<P> getControllerConfiguration() {
public KubernetesClient getClient() {
return client;
}

public EventSourceRetriever<P> getEventSourceRetriever() {
return eventSourceManager;
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package io.javaoperatorsdk.operator.api.reconciler;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceAware;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;

/**
Expand Down Expand Up @@ -39,6 +43,21 @@ static Map<String, EventSource> nameEventSources(EventSource... eventSources) {
return eventSourceMap;
}

@SuppressWarnings("unchecked,rawtypes")
static <K extends HasMetadata> Map<String, EventSource> nameEventSourcesFromDependentResource(
EventSourceContext<K> context, DependentResource... dependentResources) {
if (dependentResources != null) {
Map<String, EventSource> eventSourceMap = new HashMap<>(dependentResources.length);
Arrays.stream(dependentResources)
.filter(EventSourceAware.class::isInstance)
.forEach(esa -> ((EventSourceAware<?, K>) esa).eventSource(context)
.ifPresent(es -> eventSourceMap.put(generateNameFor(es), es)));
return eventSourceMap;
} else {
return Collections.emptyMap();
}
}

/**
* This is for the use case when the event sources are not access explicitly by name in the
* reconciler.
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
Expand Up @@ -57,4 +57,13 @@
* one can be
*/
String[] dependsOn() default {};

/**
* Setting here a name of the event source means that dependent resource will use an event source
* registered with that name. So won't create one. This is helpful if more dependent resources
* created for the same type, and want to share a common event source.
*
* @return event source name (if any) provided by the dependent resource should be used.
*/
String eventSource() default NO_VALUE_SET;
}
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
Loading