From 75dc2eabbd47afe228f9984c4c50475666b05da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 19 Sep 2023 15:04:02 +0200 Subject: [PATCH 01/24] rename condition to activationCondition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/api/config/BaseConfigurationService.java | 1 + .../api/config/dependent/DependentResourceSpec.java | 10 +++++++++- .../operator/api/reconciler/dependent/Dependent.java | 2 ++ .../dependent/workflow/DefaultManagedWorkflow.java | 1 + .../dependent/workflow/DependentResourceNode.java | 6 ++++-- 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index ae836f2a72..d908f52ebe 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -230,6 +230,7 @@ private static List dependentResources( Utils.instantiate(dependent.readyPostcondition(), Condition.class, context), Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context), Utils.instantiate(dependent.deletePostcondition(), Condition.class, context), + Utils.instantiate(dependent.activationCondition(), Condition.class, context), eventSourceName); specsMap.put(dependentName, spec); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java index 58fd9ace4b..1fcd0709fb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java @@ -22,18 +22,21 @@ public class DependentResourceSpec { private final Condition deletePostCondition; + private final Condition activationCondition; + private final String useEventSourceWithName; public DependentResourceSpec(Class> dependentResourceClass, String name, Set dependsOn, Condition readyCondition, Condition reconcileCondition, Condition deletePostCondition, - String useEventSourceWithName) { + Condition activationCondition, String useEventSourceWithName) { this.dependentResourceClass = dependentResourceClass; this.name = name; this.dependsOn = dependsOn; this.readyCondition = readyCondition; this.reconcileCondition = reconcileCondition; this.deletePostCondition = deletePostCondition; + this.activationCondition = activationCondition; this.useEventSourceWithName = useEventSourceWithName; } @@ -87,6 +90,11 @@ public Condition getDeletePostCondition() { return deletePostCondition; } + @SuppressWarnings("rawtypes") + public Condition getActivationCondition() { + return activationCondition; + } + public Optional getUseEventSourceWithName() { return Optional.ofNullable(useEventSourceWithName); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java index 78e9ee4581..e8084cc6c9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java @@ -50,6 +50,8 @@ */ Class deletePostcondition() default Condition.class; + Class activationCondition() default Condition.class; + /** * The list of named dependents that need to be reconciled before this one can be. * diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java index e5b89f6c80..13d51b1759 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java @@ -81,6 +81,7 @@ public Workflow

resolve(KubernetesClient client, spec.getReconcileCondition(), spec.getDeletePostCondition(), spec.getReadyCondition(), + spec.getActivationCondition(), resolve(spec, client, configuration)); alreadyResolved.put(node.getName(), node); spec.getDependsOn() diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java index 1b12970f48..476d87765e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java @@ -16,19 +16,21 @@ public class DependentResourceNode { private Condition reconcilePrecondition; private Condition deletePostcondition; private Condition readyPostcondition; + private Condition activationCondition; private final DependentResource dependentResource; DependentResourceNode(DependentResource dependentResource) { - this(getNameFor(dependentResource), null, null, null, dependentResource); + this(getNameFor(dependentResource), null, null, null, null, dependentResource); } public DependentResourceNode(String name, Condition reconcilePrecondition, Condition deletePostcondition, Condition readyPostcondition, - DependentResource dependentResource) { + Condition activationCondition, DependentResource dependentResource) { this.name = name; this.reconcilePrecondition = reconcilePrecondition; this.deletePostcondition = deletePostcondition; this.readyPostcondition = readyPostcondition; + this.activationCondition = activationCondition; this.dependentResource = dependentResource; } From 4bebcf3879bea351f853e59a690878f0608b047b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 19 Sep 2023 14:35:28 +0200 Subject: [PATCH 02/24] feat: platform condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../processing/dependent/workflow/ManagedWorkflowTestUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java index 25c0ad139b..b314b5b112 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java @@ -21,7 +21,7 @@ public class ManagedWorkflowTestUtils { @SuppressWarnings("unchecked") public static DependentResourceSpec createDRS(String name, String... dependOns) { return new DependentResourceSpec(EmptyTestDependentResource.class, name, Set.of(dependOns), - null, null, null, null); + null, null, null, null, null); } public static DependentResourceSpec createDRSWithTraits(String name, From 6c5ff08d43bffd48e7dbfa427f6ce30b48aff509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 24 Oct 2023 14:33:19 +0200 Subject: [PATCH 03/24] progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../dependent/workflow/DependentResourceNode.java | 8 ++++++++ .../dependent/workflow/WorkflowBuilder.java | 5 +++++ .../operator/processing/event/EventSourceManager.java | 11 +++++++++++ .../processing/event/EventSourceRetriever.java | 6 ++++++ 4 files changed, 30 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java index 476d87765e..39a9ba6fe5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DependentResourceNode.java @@ -65,6 +65,10 @@ public Optional> getDeletePostcondition() { return Optional.ofNullable(deletePostcondition); } + public Optional> getActivationCondition() { + return Optional.ofNullable(activationCondition); + } + void setReconcilePrecondition(Condition reconcilePrecondition) { this.reconcilePrecondition = reconcilePrecondition; } @@ -73,6 +77,10 @@ void setDeletePostcondition(Condition cleanupCondition) { this.deletePostcondition = cleanupCondition; } + void setActivationCondition(Condition activationCondition) { + this.activationCondition = activationCondition; + } + public Optional> getReadyPostcondition() { return Optional.ofNullable(readyPostcondition); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java index 0eb176f3aa..a7729859a8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java @@ -58,6 +58,11 @@ public WorkflowBuilder

withDeletePostcondition(Condition deletePostcondition) return this; } + public WorkflowBuilder

withActivationCondition(Condition activationCondition) { + currentNode.setActivationCondition(activationCondition); + return this; + } + DependentResourceNode getNodeByDependentResource(DependentResource dependentResource) { // first check by name final var node = diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 29785eda01..4bcdb01b47 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -231,6 +231,17 @@ public List> getResourceEventSourcesFor(Class d return eventSources.getEventSources(dependentType); } + @Override + public void dynamicallyRegisterEventSource(String name, EventSource eventSource) { + // todo not that other thread should wait for syncing (with start() within synchronized this is + // automatically ensured) + } + + @Override + public void dynamicallyDeRegisterEventSource(String name) { + // todo + } + /** * @deprecated Use {@link #getResourceEventSourceFor(Class)} instead * diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index a37f35531f..1f9f0b5f3a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -3,6 +3,7 @@ import java.util.List; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource; public interface EventSourceRetriever

{ @@ -15,4 +16,9 @@ default ResourceEventSource getResourceEventSourceFor(Class depende List> getResourceEventSourcesFor(Class dependentType); + // todo javadocs + // this will be an idempotent synchronized operation + void dynamicallyRegisterEventSource(String name, EventSource eventSource); + + void dynamicallyDeRegisterEventSource(String name); } From cb1f913bb3123496cb605a4bf6b13f37435c65ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 24 Oct 2023 16:32:05 +0200 Subject: [PATCH 04/24] progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/Controller.java | 9 +++++-- .../workflow/WorkflowReconcileExecutor.java | 27 ++++++++++++++++++- .../processing/event/EventSourceManager.java | 6 +++++ .../event/EventSourceRetriever.java | 4 +++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index e8d15671bf..8163d19a71 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -76,6 +76,7 @@ public class Controller

private final GroupVersionKind associatedGVK; private final EventProcessor

eventProcessor; private final ControllerHealthInfo controllerHealthInfo; + private final EventSourceContext

eventSourceContext; public Controller(Reconciler

reconciler, ControllerConfiguration

configuration, @@ -98,9 +99,9 @@ public Controller(Reconciler

reconciler, eventProcessor = new EventProcessor<>(eventSourceManager, configurationService); eventSourceManager.postProcessDefaultEventSourcesAfterProcessorInitializer(); controllerHealthInfo = new ControllerHealthInfo(eventSourceManager); - final var context = new EventSourceContext<>( + eventSourceContext = new EventSourceContext<>( eventSourceManager.getControllerResourceEventSource(), configuration, kubernetesClient); - initAndRegisterEventSources(context); + initAndRegisterEventSources(eventSourceContext); configurationService.getMetrics().controllerRegistered(this); } @@ -440,4 +441,8 @@ public EventProcessor

getEventProcessor() { public ExecutorServiceManager getExecutorServiceManager() { return getConfiguration().getConfigurationService().getExecutorServiceManager(); } + + public EventSourceContext

eventSourceContext() { + return eventSourceContext; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index b1ef16c7b0..2d1a4a4397 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -64,13 +64,38 @@ private synchronized void handleReconcile(DependentResourceNode depend boolean reconcileConditionMet = isConditionMet(dependentResourceNode.getReconcilePrecondition(), dependentResourceNode.getDependentResource()); - if (!reconcileConditionMet) { + boolean activationConditionMet = isConditionMet(dependentResourceNode.getActivationCondition(), + dependentResourceNode.getDependentResource()); + + registerEventSourceForActivationCondition(activationConditionMet, dependentResourceNode); + + if (!reconcileConditionMet || !activationConditionMet) { handleReconcileConditionNotMet(dependentResourceNode); } else { submit(dependentResourceNode, new NodeReconcileExecutor<>(dependentResourceNode), RECONCILE); } } + private void registerEventSourceForActivationCondition(boolean activationConditionMet, + DependentResourceNode dependentResourceNode) { + if (dependentResourceNode.getActivationCondition().isPresent()) { + if (activationConditionMet) { + // todo create issue for v5 to return name also from DependentResource.eventSource + var eventSource = + dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() + .eventSourceContexForDynamicRegistration()); + + eventSource.ifPresent(es -> { + // todo check if event source with the name not exists yet, if does do not register. + context.eventSourceRetriever() + .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es); + }); + } else { + // todo deregister event source + } + } + } + private synchronized void handleDelete(DependentResourceNode dependentResourceNode) { log.debug("Submitting for delete: {}", dependentResourceNode); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 4bcdb01b47..097e517ba1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -17,6 +17,7 @@ import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -242,6 +243,11 @@ public void dynamicallyDeRegisterEventSource(String name) { // todo } + @Override + public EventSourceContext

eventSourceContexForDynamicRegistration() { + return controller.eventSourceContext(); + } + /** * @deprecated Use {@link #getResourceEventSourceFor(Class)} instead * diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index 1f9f0b5f3a..9f9aa0c760 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -3,6 +3,7 @@ import java.util.List; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource; @@ -21,4 +22,7 @@ default ResourceEventSource getResourceEventSourceFor(Class depende void dynamicallyRegisterEventSource(String name, EventSource eventSource); void dynamicallyDeRegisterEventSource(String name); + + EventSourceContext

eventSourceContexForDynamicRegistration(); + } From 488f6f3bdd8986797074732bfae4725ec35177df Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 14 Nov 2023 15:21:24 +0100 Subject: [PATCH 05/24] impl proress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../dependent/DependentResource.java | 1 + .../workflow/WorkflowCleanupExecutor.java | 6 ++++- .../workflow/WorkflowReconcileExecutor.java | 23 ++++++++++--------- .../processing/event/EventSourceManager.java | 16 +++++++++---- .../event/EventSourceRetriever.java | 1 + .../processing/event/EventSources.java | 7 +++++- 6 files changed, 36 insertions(+), 18 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index 8230dc4cf9..f451ace5df 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -31,6 +31,7 @@ public interface DependentResource { */ Class resourceType(); + // todo recreate the event source because of re-active use case? /** * Dependent resources are designed to by default provide event sources. There are cases where * they might not: diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 5426eff1aa..7e905e4837 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -68,7 +68,11 @@ protected void doRun(DependentResourceNode dependentResourceNode, DependentResource dependentResource) { var deletePostCondition = dependentResourceNode.getDeletePostcondition(); - if (dependentResource.isDeletable()) { + // todo test + var active = + isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource); + + if (dependentResource.isDeletable() && active) { ((Deleter

) dependentResource).delete(primary, context); deleteCalled.add(dependentResourceNode); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index 2d1a4a4397..f3c1794b59 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -76,22 +76,23 @@ private synchronized void handleReconcile(DependentResourceNode depend } } + // todo IT test re-registration private void registerEventSourceForActivationCondition(boolean activationConditionMet, DependentResourceNode dependentResourceNode) { if (dependentResourceNode.getActivationCondition().isPresent()) { + var eventSource = + dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() + .eventSourceContexForDynamicRegistration()); + if (activationConditionMet) { - // todo create issue for v5 to return name also from DependentResource.eventSource - var eventSource = - dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() - .eventSourceContexForDynamicRegistration()); - - eventSource.ifPresent(es -> { - // todo check if event source with the name not exists yet, if does do not register. - context.eventSourceRetriever() - .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es); - }); + + var es = eventSource.orElseThrow(); + context.eventSourceRetriever() + .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es); + } else { - // todo deregister event source + context.eventSourceRetriever() + .dynamicallyDeRegisterEventSource(dependentResourceNode.getName()); } } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 097e517ba1..824c24e276 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -233,14 +233,20 @@ public List> getResourceEventSourcesFor(Class d } @Override - public void dynamicallyRegisterEventSource(String name, EventSource eventSource) { - // todo not that other thread should wait for syncing (with start() within synchronized this is - // automatically ensured) + public synchronized void dynamicallyRegisterEventSource(String name, EventSource eventSource) { + if (eventSources.existing(name, eventSource) != null) { + return; + } + registerEventSource(name, eventSource); + eventSource.start(); } @Override - public void dynamicallyDeRegisterEventSource(String name) { - // todo + public synchronized void dynamicallyDeRegisterEventSource(String name) { + EventSource es = eventSources.remove(name); + if (es != null) { + es.stop(); + } } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index 9f9aa0c760..c431154c7f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -19,6 +19,7 @@ default ResourceEventSource getResourceEventSourceFor(Class depende // todo javadocs // this will be an idempotent synchronized operation + // todo check if event source with the name not exists yet, if does do not register. void dynamicallyRegisterEventSource(String name, EventSource eventSource); void dynamicallyDeRegisterEventSource(String name); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java index 63c94b1b6a..b53b92e122 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java @@ -75,7 +75,7 @@ public void clear() { sources.clear(); } - private NamedEventSource existing(String name, EventSource source) { + public NamedEventSource existing(String name, EventSource source) { final var eventSources = sources.get(keyFor(source)); if (eventSources == null || eventSources.isEmpty()) { return null; @@ -183,4 +183,9 @@ public List> getEventSources(Class dependentTyp .map(es -> (ResourceEventSource) es) .collect(Collectors.toList()); } + + public EventSource remove(String name) { + var optionalMap = sources.values().stream().filter(m -> m.containsKey(name)).findFirst(); + return optionalMap.map(m -> m.remove(name)).orElse(null); + } } From ffe9627b2a60c8d13097ab65040481c3b649a9f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 14 Nov 2023 16:45:44 +0100 Subject: [PATCH 06/24] informer restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../source/informer/InformerEventSource.java | 6 ++--- .../informer/ManagedInformerEventSource.java | 23 +++++++++++-------- .../ControllerResourceEventSourceTest.java | 2 +- .../informer/InformerEventSourceTest.java | 7 +++--- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index f900e602ce..435e5e92cb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -312,16 +312,16 @@ private boolean acceptedByDeleteFilters(R resource, boolean b) { public void setConfigurationService(ConfigurationService configurationService) { super.setConfigurationService(configurationService); - super.addIndexers(indexerBuffer); + super.addIndexers(indexerBuffer); // todo check indexerBuffer = new HashMap<>(); } @Override public void addIndexers(Map>> indexers) { - if (indexerBuffer == null) { + if (isRunning()) { throw new OperatorException("Cannot add indexers after InformerEventSource started."); } - indexerBuffer.putAll(indexers); + indexerBuffer.putAll(indexers); // todo check } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index d030e7a8f4..18513b2181 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -1,9 +1,6 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -36,7 +33,11 @@ public abstract class ManagedInformerEventSource, Configurable { private static final Logger log = LoggerFactory.getLogger(ManagedInformerEventSource.class); - private final InformerManager cache; + private InformerManager cache; + private boolean parseResourceVersions; + private ConfigurationService configurationService; + private C configuration; + private Map>> indexers = new HashMap<>(); protected TemporaryResourceCache temporaryResourceCache; protected MixedOperation, Resource> client; @@ -44,9 +45,9 @@ protected ManagedInformerEventSource( MixedOperation, Resource> client, C configuration, boolean parseResourceVersions) { super(configuration.getResourceClass()); + this.parseResourceVersions = parseResourceVersions; this.client = client; - temporaryResourceCache = new TemporaryResourceCache<>(this, parseResourceVersions); - this.cache = new InformerManager<>(client, configuration, this); + this.configuration = configuration; } @Override @@ -77,6 +78,10 @@ public void changeNamespaces(Set namespaces) { @Override public void start() { + temporaryResourceCache = new TemporaryResourceCache<>(this, parseResourceVersions); + this.cache = new InformerManager<>(client, configuration, this); + cache.setConfigurationService(configurationService); + cache.addIndexers(indexers); manager().start(); super.start(); } @@ -129,7 +134,7 @@ void setTemporalResourceCache(TemporaryResourceCache temporaryResourceCache) @Override public void addIndexers(Map>> indexers) { - cache.addIndexers(indexers); + this.indexers.putAll(indexers); } @Override @@ -175,6 +180,6 @@ public String toString() { } public void setConfigurationService(ConfigurationService configurationService) { - cache.setConfigurationService(configurationService); + this.configurationService = configurationService; } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java index 87ec3f9eba..64f0993139 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java @@ -37,7 +37,7 @@ class ControllerResourceEventSourceTest extends @BeforeEach public void setup() { - setUpSource(new ControllerResourceEventSource<>(testController), false, + setUpSource(new ControllerResourceEventSource<>(testController), true, new BaseConfigurationService()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index 0881cccaf2..ce3c52076d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -55,15 +55,16 @@ void setup() { when(informerConfiguration.getResourceClass()).thenReturn(Deployment.class); informerEventSource = new InformerEventSource<>(informerConfiguration, clientMock); - informerEventSource.setTemporalResourceCache(temporaryResourceCacheMock); - informerEventSource.setEventHandler(eventHandlerMock); - + informerEventSource.setEventHandler(eventHandlerMock); + informerEventSource.setConfigurationService(new BaseConfigurationService()); SecondaryToPrimaryMapper secondaryToPrimaryMapper = mock(SecondaryToPrimaryMapper.class); when(informerConfiguration.getSecondaryToPrimaryMapper()) .thenReturn(secondaryToPrimaryMapper); when(secondaryToPrimaryMapper.toPrimaryResourceIDs(any())) .thenReturn(Set.of(ResourceID.fromResource(testDeployment()))); + informerEventSource.start(); + informerEventSource.setTemporalResourceCache(temporaryResourceCacheMock); } @Test From a7a0d0a84cdd934a281613c35d3025f929ecb20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 14 Nov 2023 17:00:20 +0100 Subject: [PATCH 07/24] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/source/informer/ManagedInformerEventSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 18513b2181..2964fe313c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -169,7 +169,7 @@ public ResourceConfiguration getInformerConfiguration() { @Override public C configuration() { - return manager().configuration(); + return configuration; } @Override From b993ef56655347c832f5f3cc1019101b39f7b741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 15 Nov 2023 16:56:02 +0100 Subject: [PATCH 08/24] Simple Integration Test passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/Controller.java | 3 +- .../dependent/workflow/DefaultWorkflow.java | 11 ++++ .../dependent/workflow/Workflow.java | 4 ++ .../workflow/WorkflowReconcileExecutor.java | 31 +++++++----- operator-framework/pom.xml | 5 ++ .../WorkflowActivationConditionIT.java | 50 +++++++++++++++++++ .../ConfigMapDependentResource.java | 30 +++++++++++ .../RouteDependentResource.java | 27 ++++++++++ ...flowActivationConditionCustomResource.java | 17 +++++++ ...WorkflowActivationConditionReconciler.java | 21 ++++++++ .../WorkflowActivationConditionSpec.java | 14 ++++++ .../isOpenShiftCondition.java | 19 +++++++ 12 files changed, 218 insertions(+), 14 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/ConfigMapDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/RouteDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 8163d19a71..70069148c3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -237,7 +237,8 @@ public void initAndRegisterEventSources(EventSourceContext

context) { } // register created event sources - final var dependentResourcesByName = managedWorkflow.getDependentResourcesByName(); + final var dependentResourcesByName = + managedWorkflow.getDependentResourcesByNameWithoutActivationCondition(); final var size = dependentResourcesByName.size(); if (size > 0) { dependentResourcesByName.forEach((key, dependentResource) -> { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java index b2b8716470..d31f42419d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java @@ -147,4 +147,15 @@ public Map getDependentResourcesByName() { .forEach((name, node) -> resources.put(name, node.getDependentResource())); return resources; } + + public Map getDependentResourcesByNameWithoutActivationCondition() { + final var resources = new HashMap(dependentResourceNodes.size()); + dependentResourceNodes + .forEach((name, node) -> { + if (node.getActivationCondition().isEmpty()) { + resources.put(name, node.getDependentResource()); + } + }); + return resources; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java index c06f17b7d8..fd2acac234 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java @@ -42,4 +42,8 @@ default boolean isEmpty() { default Map getDependentResourcesByName() { return Collections.emptyMap(); } + + default Map getDependentResourcesByNameWithoutActivationCondition() { + return Collections.emptyMap(); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index f3c1794b59..1041d10cef 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -70,22 +70,19 @@ private synchronized void handleReconcile(DependentResourceNode depend registerEventSourceForActivationCondition(activationConditionMet, dependentResourceNode); if (!reconcileConditionMet || !activationConditionMet) { - handleReconcileConditionNotMet(dependentResourceNode); + handleReconcileConditionNotMet(dependentResourceNode, !activationConditionMet); } else { submit(dependentResourceNode, new NodeReconcileExecutor<>(dependentResourceNode), RECONCILE); } } - // todo IT test re-registration private void registerEventSourceForActivationCondition(boolean activationConditionMet, DependentResourceNode dependentResourceNode) { if (dependentResourceNode.getActivationCondition().isPresent()) { var eventSource = dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() .eventSourceContexForDynamicRegistration()); - if (activationConditionMet) { - var es = eventSource.orElseThrow(); context.eventSourceRetriever() .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es); @@ -97,7 +94,8 @@ private void registerEventSourceForActivationCondition(boolean activationCon } } - private synchronized void handleDelete(DependentResourceNode dependentResourceNode) { + private synchronized void handleDelete(DependentResourceNode dependentResourceNode, + boolean activationConditionNotMet) { log.debug("Submitting for delete: {}", dependentResourceNode); if (alreadyVisited(dependentResourceNode) @@ -109,7 +107,8 @@ private synchronized void handleDelete(DependentResourceNode dependentResourceNo return; } - submit(dependentResourceNode, new NodeDeleteExecutor<>(dependentResourceNode), DELETE); + submit(dependentResourceNode, + new NodeDeleteExecutor<>(dependentResourceNode, activationConditionNotMet), DELETE); } private boolean allDependentsDeletedAlready(DependentResourceNode dependentResourceNode) { @@ -157,8 +156,12 @@ protected void doRun(DependentResourceNode dependentResourceNode, private class NodeDeleteExecutor extends NodeExecutor { - private NodeDeleteExecutor(DependentResourceNode dependentResourceNode) { + private boolean activationConditionNotMet; + + private NodeDeleteExecutor(DependentResourceNode dependentResourceNode, + boolean activationConditionNotMet) { super(dependentResourceNode, WorkflowReconcileExecutor.this); + this.activationConditionNotMet = activationConditionNotMet; } @Override @@ -170,13 +173,13 @@ protected void doRun(DependentResourceNode dependentResourceNode, // GarbageCollected status is irrelevant here, as this method is only called when a // precondition does not hold, // a deleter should be deleted even if it is otherwise garbage collected - if (dependentResource instanceof Deleter) { + if (dependentResource instanceof Deleter && !activationConditionNotMet) { ((Deleter

) dependentResource).delete(primary, context); } boolean deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); if (deletePostConditionMet) { markAsVisited(dependentResourceNode); - handleDependentDeleted(dependentResourceNode); + handleDependentDeleted(dependentResourceNode, activationConditionNotMet); } else { // updating alreadyVisited needs to be the last operation otherwise could lead to a race // condition in handleDelete condition checks @@ -187,11 +190,11 @@ protected void doRun(DependentResourceNode dependentResourceNode, } private synchronized void handleDependentDeleted( - DependentResourceNode dependentResourceNode) { + DependentResourceNode dependentResourceNode, boolean activationConditionNotMet) { dependentResourceNode.getDependsOn().forEach(dr -> { log.debug("Handle deleted for: {} with dependent: {} primaryID: {}", dr, dependentResourceNode, primaryID); - handleDelete(dr); + handleDelete(dr, activationConditionNotMet); }); } @@ -206,10 +209,12 @@ private synchronized void handleDependentsReconcile( } - private void handleReconcileConditionNotMet(DependentResourceNode dependentResourceNode) { + private void handleReconcileConditionNotMet(DependentResourceNode dependentResourceNode, + boolean activationConditionNotMet) { Set bottomNodes = new HashSet<>(); markDependentsForDelete(dependentResourceNode, bottomNodes); - bottomNodes.forEach(this::handleDelete); + bottomNodes.forEach( + dependentResourceNode1 -> handleDelete(dependentResourceNode1, activationConditionNotMet)); } private void markDependentsForDelete(DependentResourceNode dependentResourceNode, diff --git a/operator-framework/pom.xml b/operator-framework/pom.xml index d5b8a5371b..2d22e7037b 100644 --- a/operator-framework/pom.xml +++ b/operator-framework/pom.xml @@ -64,6 +64,11 @@ crd-generator-apt test + + + io.fabric8 + openshift-client-api + org.apache.logging.log4j log4j-slf4j-impl diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java new file mode 100644 index 0000000000..38b6bed16a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java @@ -0,0 +1,50 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.workflowactivationcondition.WorkflowActivationConditionCustomResource; +import io.javaoperatorsdk.operator.sample.workflowactivationcondition.WorkflowActivationConditionReconciler; +import io.javaoperatorsdk.operator.sample.workflowactivationcondition.WorkflowActivationConditionSpec; + +import static io.javaoperatorsdk.operator.sample.workflowactivationcondition.ConfigMapDependentResource.DATA_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class WorkflowActivationConditionIT { + + public static final String TEST_RESOURCE_NAME = "test1"; + public static final String TEST_DATA = "test data"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(WorkflowActivationConditionReconciler.class) + .build(); + + @Test + void routeIsNotCreated() { + extension.create(testResource()); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE_NAME); + assertThat(cm).isNotNull(); + assertThat(cm.getData()).containsEntry(DATA_KEY, TEST_DATA); + }); + } + + @Test + WorkflowActivationConditionCustomResource testResource() { + var res = new WorkflowActivationConditionCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE_NAME) + .build()); + res.setSpec(new WorkflowActivationConditionSpec()); + res.getSpec().setValue(TEST_DATA); + return res; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/ConfigMapDependentResource.java new file mode 100644 index 0000000000..7433c31c73 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/ConfigMapDependentResource.java @@ -0,0 +1,30 @@ +package io.javaoperatorsdk.operator.sample.workflowactivationcondition; + +import java.util.Map; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class ConfigMapDependentResource + extends CRUDKubernetesDependentResource { + + public static final String DATA_KEY = "data"; + + public ConfigMapDependentResource() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(WorkflowActivationConditionCustomResource primary, + Context context) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + configMap.setData(Map.of(DATA_KEY, primary.getSpec().getValue())); + return configMap; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/RouteDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/RouteDependentResource.java new file mode 100644 index 0000000000..de5d52f76a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/RouteDependentResource.java @@ -0,0 +1,27 @@ +package io.javaoperatorsdk.operator.sample.workflowactivationcondition; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.openshift.api.model.Route; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class RouteDependentResource + extends CRUDKubernetesDependentResource { + + public RouteDependentResource() { + super(Route.class); + } + + @Override + protected Route desired(WorkflowActivationConditionCustomResource primary, + Context context) { + // basically does not matter since this should not be called + Route route = new Route(); + route.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + + return route; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionCustomResource.java new file mode 100644 index 0000000000..63d4bc343c --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionCustomResource.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.sample.workflowactivationcondition; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("wac") +public class WorkflowActivationConditionCustomResource + extends CustomResource + implements Namespaced { + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java new file mode 100644 index 0000000000..b9332f5d44 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java @@ -0,0 +1,21 @@ +package io.javaoperatorsdk.operator.sample.workflowactivationcondition; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; + +@ControllerConfiguration(dependents = { + @Dependent(type = ConfigMapDependentResource.class), + @Dependent(type = RouteDependentResource.class, + activationCondition = isOpenShiftCondition.class) +}) +public class WorkflowActivationConditionReconciler + implements Reconciler { + + @Override + public UpdateControl reconcile( + WorkflowActivationConditionCustomResource resource, + Context context) { + + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionSpec.java new file mode 100644 index 0000000000..826fe04958 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionSpec.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample.workflowactivationcondition; + +public class WorkflowActivationConditionSpec { + + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java new file mode 100644 index 0000000000..0f76f8a52a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.sample.workflowactivationcondition; + +import io.fabric8.openshift.api.model.Route; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + +public class isOpenShiftCondition + implements Condition { + @Override + public boolean isMet( + DependentResource dependentResource, + WorkflowActivationConditionCustomResource primary, + Context context) { + + return context.getClient().getApiGroups().getGroups().stream() + .anyMatch(g -> g.getName().equals("route.openshift.io")); + } +} From 0b9879efd4b35035e56d4c6f8a10fd808cc01e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 15 Nov 2023 18:54:14 +0100 Subject: [PATCH 09/24] integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../WorkflowMultipleActivationIT.java | 119 ++++++++++++++++++ .../ActivationCondition.java | 20 +++ .../ConfigMapDependentResource.java | 31 +++++ .../SecretDependentResource.java | 31 +++++ ...kflowMultipleActivationCustomResource.java | 17 +++ .../WorkflowMultipleActivationReconciler.java | 34 +++++ .../WorkflowMultipleActivationSpec.java | 14 +++ 7 files changed, 266 insertions(+) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ActivationCondition.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ConfigMapDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/SecretDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationSpec.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java new file mode 100644 index 0000000000..0d844a36e6 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java @@ -0,0 +1,119 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Secret; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.workflowmultipleactivation.*; + +import static io.javaoperatorsdk.operator.sample.workflowactivationcondition.ConfigMapDependentResource.DATA_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class WorkflowMultipleActivationIT { + + public static final String INITIAL_DATA = "initial data"; + public static final String TEST_RESOURCE = "test1"; + public static final String CHANGED_VALUE = "changed value"; + public static final int POLL_DELAY = 150; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(WorkflowMultipleActivationReconciler.class) + .build(); + + @Test + void deactivatingAndReactivatingDependent() { + ActivationCondition.MET = true; + var cr1 = extension.create(testResource()); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + var secret = extension.get(Secret.class, TEST_RESOURCE); + assertThat(cm).isNotNull(); + assertThat(secret).isNotNull(); + assertThat(cm.getData()).containsEntry(DATA_KEY, INITIAL_DATA); + }); + + extension.delete(cr1); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + assertThat(cm).isNull(); + }); + + ActivationCondition.MET = false; + cr1 = extension.create(testResource()); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + var secret = extension.get(Secret.class, TEST_RESOURCE); + assertThat(cm).isNull(); + assertThat(secret).isNotNull(); + }); + + ActivationCondition.MET = true; + cr1.getSpec().setValue(CHANGED_VALUE); + extension.replace(cr1); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + assertThat(cm).isNotNull(); + assertThat(cm.getData()).containsEntry(DATA_KEY, CHANGED_VALUE); + }); + + ActivationCondition.MET = false; + cr1.getSpec().setValue(INITIAL_DATA); + extension.replace(cr1); + + await().pollDelay(Duration.ofMillis(POLL_DELAY)).untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + assertThat(cm).isNotNull(); + // data not changed + assertThat(cm.getData()).containsEntry(DATA_KEY, CHANGED_VALUE); + }); + + var numOfReconciliation = + extension.getReconcilerOfType(WorkflowMultipleActivationReconciler.class) + .getNumberOfReconciliationExecution(); + var actualCM = extension.get(ConfigMap.class, TEST_RESOURCE); + actualCM.getData().put("data2", "additionaldata"); + extension.replace(actualCM); + await().pollDelay(Duration.ofMillis(POLL_DELAY)).untilAsserted(() -> { + // change in config map does not induce reconciliation if inactive (thus informer is not + // present) + assertThat(extension.getReconcilerOfType(WorkflowMultipleActivationReconciler.class) + .getNumberOfReconciliationExecution()).isEqualTo(numOfReconciliation); + }); + + extension.delete(cr1); + await().pollDelay(Duration.ofMillis(POLL_DELAY)).untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + assertThat(cm).isNotNull(); + }); + } + + + + // @Test + void simpleConcurrencyTest() { + // todo + } + + WorkflowMultipleActivationCustomResource testResource() { + var res = new WorkflowMultipleActivationCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(TEST_RESOURCE) + .build()); + res.setSpec(new WorkflowMultipleActivationSpec()); + res.getSpec().setValue(INITIAL_DATA); + return res; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ActivationCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ActivationCondition.java new file mode 100644 index 0000000000..c8be8467df --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ActivationCondition.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator.sample.workflowmultipleactivation; + +import io.fabric8.openshift.api.model.Route; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + +public class ActivationCondition + implements Condition { + + public static volatile boolean MET = true; + + @Override + public boolean isMet( + DependentResource dependentResource, + WorkflowMultipleActivationCustomResource primary, + Context context) { + return MET; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ConfigMapDependentResource.java new file mode 100644 index 0000000000..e245cb222d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/ConfigMapDependentResource.java @@ -0,0 +1,31 @@ +package io.javaoperatorsdk.operator.sample.workflowmultipleactivation; + +import java.util.Map; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDNoGCKubernetesDependentResource; + +public class ConfigMapDependentResource + extends + CRUDNoGCKubernetesDependentResource { + + public static final String DATA_KEY = "data"; + + public ConfigMapDependentResource() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(WorkflowMultipleActivationCustomResource primary, + Context context) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + configMap.setData(Map.of(DATA_KEY, primary.getSpec().getValue())); + return configMap; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/SecretDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/SecretDependentResource.java new file mode 100644 index 0000000000..decd5a346d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/SecretDependentResource.java @@ -0,0 +1,31 @@ +package io.javaoperatorsdk.operator.sample.workflowmultipleactivation; + +import java.util.Base64; +import java.util.Map; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Secret; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class SecretDependentResource + extends CRUDKubernetesDependentResource { + + public SecretDependentResource() { + super(Secret.class); + } + + @Override + protected Secret desired(WorkflowMultipleActivationCustomResource primary, + Context context) { + // basically does not matter since this should not be called + Secret secret = new Secret(); + secret.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + secret.setData(Map.of("data", + Base64.getEncoder().encodeToString(primary.getSpec().getValue().getBytes()))); + return secret; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationCustomResource.java new file mode 100644 index 0000000000..9c642e9635 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationCustomResource.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.sample.workflowmultipleactivation; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("mwac") +public class WorkflowMultipleActivationCustomResource + extends CustomResource + implements Namespaced { + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationReconciler.java new file mode 100644 index 0000000000..8277e7f8e7 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationReconciler.java @@ -0,0 +1,34 @@ +package io.javaoperatorsdk.operator.sample.workflowmultipleactivation; + +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; + +@ControllerConfiguration(dependents = { + @Dependent(type = ConfigMapDependentResource.class, + activationCondition = ActivationCondition.class), + @Dependent(type = SecretDependentResource.class) +}) +public class WorkflowMultipleActivationReconciler + implements Reconciler { + + private final AtomicInteger numberOfReconciliationExecution = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + WorkflowMultipleActivationCustomResource resource, + Context context) { + + numberOfReconciliationExecution.incrementAndGet(); + + return UpdateControl.noUpdate(); + } + + public int getNumberOfReconciliationExecution() { + return numberOfReconciliationExecution.get(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationSpec.java new file mode 100644 index 0000000000..b342bb331f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowmultipleactivation/WorkflowMultipleActivationSpec.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample.workflowmultipleactivation; + +public class WorkflowMultipleActivationSpec { + + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} From a22808b6e5c8c3639799f5394999979a4f1bc219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 11:25:07 +0100 Subject: [PATCH 10/24] workflow cleanup tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../dependent/DependentResource.java | 1 - .../workflow/WorkflowCleanupExecutor.java | 10 +- .../AbstractWorkflowExecutorTest.java | 4 +- .../workflow/WorkflowCleanupExecutorTest.java | 91 ++++++++++++++++++- .../WorkflowReconcileExecutorTest.java | 52 +++++------ .../WorkflowMultipleActivationIT.java | 7 -- 6 files changed, 117 insertions(+), 48 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index f451ace5df..8230dc4cf9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -31,7 +31,6 @@ public interface DependentResource { */ Class resourceType(); - // todo recreate the event source because of re-active use case? /** * Dependent resources are designed to by default provide event sources. There are cases where * they might not: diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 7e905e4837..86fddaa321 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -68,7 +68,6 @@ protected void doRun(DependentResourceNode dependentResourceNode, DependentResource dependentResource) { var deletePostCondition = dependentResourceNode.getDeletePostcondition(); - // todo test var active = isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource); @@ -76,7 +75,14 @@ protected void doRun(DependentResourceNode dependentResourceNode, ((Deleter

) dependentResource).delete(primary, context); deleteCalled.add(dependentResourceNode); } - boolean deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); + + boolean deletePostConditionMet; + // todo test + if (active) { + deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); + } else { + deletePostConditionMet = true; + } if (deletePostConditionMet) { markAsVisited(dependentResourceNode); handleDependentCleaned(dependentResourceNode); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java index e8b26184c4..6cfdb3dc63 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java @@ -25,9 +25,9 @@ public class AbstractWorkflowExecutorTest { protected GarbageCollectedDeleter gcDeleter = new GarbageCollectedDeleter("GC_DELETER"); @SuppressWarnings("rawtypes") - protected final Condition noMetDeletePostCondition = (primary, secondary, context) -> false; + protected final Condition notMetCondition = (primary, secondary, context) -> false; @SuppressWarnings("rawtypes") - protected final Condition metDeletePostCondition = (primary, secondary, context) -> true; + protected final Condition metCondition = (primary, secondary, context) -> true; protected List executionHistory = Collections.synchronizedList(new ArrayList<>()); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java index 64fdc6e6f7..ce335fba04 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java @@ -21,6 +21,7 @@ class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest { protected TestDeleterDependent dd1 = new TestDeleterDependent("DR_DELETER_1"); protected TestDeleterDependent dd2 = new TestDeleterDependent("DR_DELETER_2"); protected TestDeleterDependent dd3 = new TestDeleterDependent("DR_DELETER_3"); + protected TestDeleterDependent dd4 = new TestDeleterDependent("DR_DELETER_4"); @SuppressWarnings("unchecked") Context mockContext = mock(Context.class); @@ -76,7 +77,7 @@ void dontDeleteIfDependentErrored() { void cleanupConditionTrivialCase() { var workflow = new WorkflowBuilder() .addDependentResource(dd1) - .addDependentResource(dd2).dependsOn(dd1).withDeletePostcondition(noMetDeletePostCondition) + .addDependentResource(dd2).dependsOn(dd1).withDeletePostcondition(notMetCondition) .build(); var res = workflow.cleanup(new TestCustomResource(), mockContext); @@ -91,7 +92,7 @@ void cleanupConditionTrivialCase() { void cleanupConditionMet() { var workflow = new WorkflowBuilder() .addDependentResource(dd1) - .addDependentResource(dd2).dependsOn(dd1).withDeletePostcondition(metDeletePostCondition) + .addDependentResource(dd2).dependsOn(dd1).withDeletePostcondition(metCondition) .build(); var res = workflow.cleanup(new TestCustomResource(), mockContext); @@ -105,12 +106,10 @@ void cleanupConditionMet() { @Test void cleanupConditionDiamondWorkflow() { - TestDeleterDependent dd4 = new TestDeleterDependent("DR_DELETER_4"); - var workflow = new WorkflowBuilder() .addDependentResource(dd1) .addDependentResource(dd2).dependsOn(dd1) - .addDependentResource(dd3).dependsOn(dd1).withDeletePostcondition(noMetDeletePostCondition) + .addDependentResource(dd3).dependsOn(dd1).withDeletePostcondition(notMetCondition) .addDependentResource(dd4).dependsOn(dd2, dd3) .build(); @@ -141,4 +140,86 @@ void dontDeleteIfGarbageCollected() { Assertions.assertThat(res.getDeleteCalledOnDependents()).isEmpty(); } + @Test + void ifDependentActiveDependentNormallyDeleted() { + var workflow = new WorkflowBuilder() + .addDependentResource(dd1) + .addDependentResource(dd2).dependsOn(dd1) + .addDependentResource(dd3).dependsOn(dd1) + .withActivationCondition(metCondition) + .addDependentResource(dd4).dependsOn(dd2, dd3) + .build(); + + var res = workflow.cleanup(new TestCustomResource(), mockContext); + + assertThat(executionHistory) + .reconciledInOrder(dd4, dd2, dd1) + .reconciledInOrder(dd4, dd3, dd1); + + Assertions.assertThat(res.getDeleteCalledOnDependents()).containsExactlyInAnyOrder(dd4, dd3, + dd2, dd1); + } + + @Test + void ifDependentActiveDeletePostConditionIsChecked() { + var workflow = new WorkflowBuilder() + .addDependentResource(dd1) + .addDependentResource(dd2).dependsOn(dd1) + .addDependentResource(dd3).dependsOn(dd1) + .withDeletePostcondition(notMetCondition) + .withActivationCondition(metCondition) + .addDependentResource(dd4).dependsOn(dd2, dd3) + .build(); + + var res = workflow.cleanup(new TestCustomResource(), mockContext); + + assertThat(executionHistory) + .reconciledInOrder(dd4, dd2) + .reconciledInOrder(dd4, dd3) + .notReconciled(dr1); + + Assertions.assertThat(res.getDeleteCalledOnDependents()).containsExactlyInAnyOrder(dd4, dd3, + dd2); + Assertions.assertThat(res.getErroredDependents()).isEmpty(); + Assertions.assertThat(res.getPostConditionNotMetDependents()).containsExactlyInAnyOrder(dd3); + } + + @Test + void ifDependentInactiveDeleteIsNotCalled() { + var workflow = new WorkflowBuilder() + .addDependentResource(dd1) + .addDependentResource(dd2).dependsOn(dd1) + .addDependentResource(dd3).dependsOn(dd1) + .withActivationCondition(notMetCondition) + .addDependentResource(dd4).dependsOn(dd2, dd3) + .build(); + + var res = workflow.cleanup(new TestCustomResource(), mockContext); + + assertThat(executionHistory) + .reconciledInOrder(dd4, dd2, dd1); + + Assertions.assertThat(res.getDeleteCalledOnDependents()).containsExactlyInAnyOrder(dd4, + dd2, dd1); + } + + @Test + void ifDependentInactiveDeletePostConditionNotChecked() { + var workflow = new WorkflowBuilder() + .addDependentResource(dd1) + .addDependentResource(dd2).dependsOn(dd1) + .addDependentResource(dd3).dependsOn(dd1) + .withDeletePostcondition(notMetCondition) + .withActivationCondition(notMetCondition) + .addDependentResource(dd4).dependsOn(dd2, dd3) + .build(); + + var res = workflow.cleanup(new TestCustomResource(), mockContext); + + assertThat(executionHistory) + .reconciledInOrder(dd4, dd2, dd1); + + Assertions.assertThat(res.getPostConditionNotMetDependents()).isEmpty(); + } + } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java index b3c4ea35e9..69822fbd98 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java @@ -19,16 +19,6 @@ @SuppressWarnings("rawtypes") class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { - - - private final Condition met_reconcile_condition = (primary, secondary, context) -> true; - private final Condition not_met_reconcile_condition = (primary, secondary, context) -> false; - - private final Condition metReadyCondition = - (primary, secondary, context) -> true; - private final Condition notMetReadyCondition = - (primary, secondary, context) -> false; - @SuppressWarnings("unchecked") Context mockContext = mock(Context.class); ExecutorService executorService = Executors.newCachedThreadPool(); @@ -194,9 +184,9 @@ void onlyOneDependsOnErroredResourceNotReconciled() { @Test void simpleReconcileCondition() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReconcilePrecondition(not_met_reconcile_condition) - .addDependentResource(dr2).withReconcilePrecondition(met_reconcile_condition) - .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(dr1).withReconcilePrecondition(notMetCondition) + .addDependentResource(dr2).withReconcilePrecondition(metCondition) + .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .build(); var res = workflow.reconcile(new TestCustomResource(), mockContext); @@ -213,7 +203,7 @@ void triangleOnceConditionNotMet() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) .addDependentResource(dr2).dependsOn(dr1) - .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .dependsOn(dr1) .build(); @@ -232,11 +222,11 @@ void reconcileConditionTransitiveDelete() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) .addDependentResource(dr2).dependsOn(dr1) - .withReconcilePrecondition(not_met_reconcile_condition) + .withReconcilePrecondition(notMetCondition) .addDependentResource(drDeleter).dependsOn(dr2) - .withReconcilePrecondition(met_reconcile_condition) + .withReconcilePrecondition(metCondition) .addDependentResource(drDeleter2).dependsOn(drDeleter) - .withReconcilePrecondition(met_reconcile_condition) + .withReconcilePrecondition(metCondition) .build(); var res = workflow.reconcile(new TestCustomResource(), mockContext); @@ -257,9 +247,9 @@ void reconcileConditionAlsoErrorDependsOn() { var workflow = new WorkflowBuilder() .addDependentResource(drError) - .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .addDependentResource(drDeleter2).dependsOn(drError, drDeleter) - .withReconcilePrecondition(met_reconcile_condition) + .withReconcilePrecondition(metCondition) .withThrowExceptionFurther(false) .build(); @@ -280,7 +270,7 @@ void reconcileConditionAlsoErrorDependsOn() { void oneDependsOnConditionNotMet() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) - .addDependentResource(dr2).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(dr2).withReconcilePrecondition(notMetCondition) .addDependentResource(drDeleter).dependsOn(dr1, dr2) .build(); @@ -300,7 +290,7 @@ void deletedIfReconcileConditionNotMet() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) .addDependentResource(drDeleter).dependsOn(dr1) - .withReconcilePrecondition(not_met_reconcile_condition) + .withReconcilePrecondition(notMetCondition) .addDependentResource(drDeleter2).dependsOn(dr1, drDeleter) .build(); @@ -323,7 +313,7 @@ void deleteDoneInReverseOrder() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) - .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .dependsOn(dr1) .addDependentResource(drDeleter2).dependsOn(drDeleter) .addDependentResource(drDeleter3).dependsOn(drDeleter) @@ -349,10 +339,10 @@ void diamondDeleteWithPostConditionInMiddle() { TestDeleterDependent drDeleter4 = new TestDeleterDependent("DR_DELETER_4"); var workflow = new WorkflowBuilder() - .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .addDependentResource(drDeleter2).dependsOn(drDeleter) .addDependentResource(drDeleter3).dependsOn(drDeleter) - .withDeletePostcondition(noMetDeletePostCondition) + .withDeletePostcondition(this.notMetCondition) .addDependentResource(drDeleter4).dependsOn(drDeleter3, drDeleter2) .build(); @@ -373,7 +363,7 @@ void diamondDeleteErrorInMiddle() { TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3"); var workflow = new WorkflowBuilder() - .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .addDependentResource(drDeleter2).dependsOn(drDeleter) .addDependentResource(errorDD).dependsOn(drDeleter) .addDependentResource(drDeleter3).dependsOn(errorDD, drDeleter2) @@ -394,7 +384,7 @@ void diamondDeleteErrorInMiddle() { @Test void readyConditionTrivialCase() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReadyPostcondition(metReadyCondition) + .addDependentResource(dr1).withReadyPostcondition(metCondition) .addDependentResource(dr2).dependsOn(dr1) .build(); @@ -410,7 +400,7 @@ void readyConditionTrivialCase() { @Test void readyConditionNotMetTrivialCase() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReadyPostcondition(notMetReadyCondition) + .addDependentResource(dr1).withReadyPostcondition(notMetCondition) .addDependentResource(dr2).dependsOn(dr1) .build(); @@ -429,7 +419,7 @@ void readyConditionNotMetInOneParent() { TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReadyPostcondition(notMetReadyCondition) + .addDependentResource(dr1).withReadyPostcondition(notMetCondition) .addDependentResource(dr2) .addDependentResource(dr3).dependsOn(dr1, dr2) .build(); @@ -449,7 +439,7 @@ void diamondShareWithReadyCondition() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) - .addDependentResource(dr2).dependsOn(dr1).withReadyPostcondition(notMetReadyCondition) + .addDependentResource(dr2).dependsOn(dr1).withReadyPostcondition(notMetCondition) .addDependentResource(dr3).dependsOn(dr1) .addDependentResource(dr4).dependsOn(dr2, dr3) .build(); @@ -469,7 +459,7 @@ void diamondShareWithReadyCondition() { @Test void garbageCollectedResourceIsDeletedIfReconcilePreconditionDoesNotHold() { var workflow = new WorkflowBuilder() - .addDependentResource(gcDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(gcDeleter).withReconcilePrecondition(notMetCondition) .build(); var res = workflow.reconcile(new TestCustomResource(), mockContext); @@ -481,7 +471,7 @@ void garbageCollectedResourceIsDeletedIfReconcilePreconditionDoesNotHold() { @Test void garbageCollectedDeepResourceIsDeletedIfReconcilePreconditionDoesNotHold() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReconcilePrecondition(not_met_reconcile_condition) + .addDependentResource(dr1).withReconcilePrecondition(notMetCondition) .addDependentResource(gcDeleter).dependsOn(dr1) .build(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java index 0d844a36e6..9b402b7ca7 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java @@ -99,13 +99,6 @@ void deactivatingAndReactivatingDependent() { }); } - - - // @Test - void simpleConcurrencyTest() { - // todo - } - WorkflowMultipleActivationCustomResource testResource() { var res = new WorkflowMultipleActivationCustomResource(); res.setMetadata(new ObjectMetaBuilder() From 319b7356d77734e3f7960332e12ce025aab5649e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 15:16:50 +0100 Subject: [PATCH 11/24] workflow tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../workflow/AbstractWorkflowExecutor.java | 2 +- .../workflow/WorkflowCleanupExecutor.java | 1 - .../workflow/WorkflowReconcileExecutor.java | 88 ++++++++------ .../WorkflowReconcileExecutorTest.java | 114 ++++++++++++++++-- 4 files changed, 157 insertions(+), 48 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index 595341add7..fc4ef12822 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -46,12 +46,12 @@ public AbstractWorkflowExecutor(Workflow

workflow, P primary, Context

cont protected synchronized void waitForScheduledExecutionsToRun() { while (true) { try { - this.wait(); if (noMoreExecutionsScheduled()) { break; } else { logger().warn("Notified but still resources under execution. This should not happen."); } + this.wait(); } catch (InterruptedException e) { if (noMoreExecutionsScheduled()) { logger().debug("interrupted, no more executions for: {}", primaryID); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 86fddaa321..74ae47100e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -77,7 +77,6 @@ protected void doRun(DependentResourceNode dependentResourceNode, } boolean deletePostConditionMet; - // todo test if (active) { deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); } else { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index 1041d10cef..d1fbfc5845 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -62,27 +62,29 @@ private synchronized void handleReconcile(DependentResourceNode depend return; } - boolean reconcileConditionMet = isConditionMet(dependentResourceNode.getReconcilePrecondition(), - dependentResourceNode.getDependentResource()); boolean activationConditionMet = isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode.getDependentResource()); + registerOrDeregisterEventSourceBasedOnActivation(activationConditionMet, dependentResourceNode); - registerEventSourceForActivationCondition(activationConditionMet, dependentResourceNode); - + boolean reconcileConditionMet = true; + if (activationConditionMet) { + reconcileConditionMet = isConditionMet(dependentResourceNode.getReconcilePrecondition(), + dependentResourceNode.getDependentResource()); + } if (!reconcileConditionMet || !activationConditionMet) { - handleReconcileConditionNotMet(dependentResourceNode, !activationConditionMet); + handleReconcileOrActivationConditionNotMet(dependentResourceNode, activationConditionMet); } else { submit(dependentResourceNode, new NodeReconcileExecutor<>(dependentResourceNode), RECONCILE); } } - private void registerEventSourceForActivationCondition(boolean activationConditionMet, + private void registerOrDeregisterEventSourceBasedOnActivation(boolean activationConditionMet, DependentResourceNode dependentResourceNode) { if (dependentResourceNode.getActivationCondition().isPresent()) { - var eventSource = - dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() - .eventSourceContexForDynamicRegistration()); if (activationConditionMet) { + var eventSource = + dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever() + .eventSourceContexForDynamicRegistration()); var es = eventSource.orElseThrow(); context.eventSourceRetriever() .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es); @@ -94,8 +96,7 @@ private void registerEventSourceForActivationCondition(boolean activationCon } } - private synchronized void handleDelete(DependentResourceNode dependentResourceNode, - boolean activationConditionNotMet) { + private synchronized void handleDelete(DependentResourceNode dependentResourceNode) { log.debug("Submitting for delete: {}", dependentResourceNode); if (alreadyVisited(dependentResourceNode) @@ -108,7 +109,7 @@ private synchronized void handleDelete(DependentResourceNode dependentResourceNo } submit(dependentResourceNode, - new NodeDeleteExecutor<>(dependentResourceNode, activationConditionNotMet), DELETE); + new NodeDeleteExecutor<>(dependentResourceNode), DELETE); } private boolean allDependentsDeletedAlready(DependentResourceNode dependentResourceNode) { @@ -156,12 +157,8 @@ protected void doRun(DependentResourceNode dependentResourceNode, private class NodeDeleteExecutor extends NodeExecutor { - private boolean activationConditionNotMet; - - private NodeDeleteExecutor(DependentResourceNode dependentResourceNode, - boolean activationConditionNotMet) { + private NodeDeleteExecutor(DependentResourceNode dependentResourceNode) { super(dependentResourceNode, WorkflowReconcileExecutor.this); - this.activationConditionNotMet = activationConditionNotMet; } @Override @@ -170,16 +167,24 @@ protected void doRun(DependentResourceNode dependentResourceNode, DependentResource dependentResource) { var deletePostCondition = dependentResourceNode.getDeletePostcondition(); - // GarbageCollected status is irrelevant here, as this method is only called when a - // precondition does not hold, - // a deleter should be deleted even if it is otherwise garbage collected - if (dependentResource instanceof Deleter && !activationConditionNotMet) { - ((Deleter

) dependentResource).delete(primary, context); + var activationConditionMet = dependentResourceNode.getActivationCondition() + .map(c -> c.isMet(dependentResource, primary, context)) + .orElse(true); + + boolean deletePostConditionMet = true; + if (Boolean.TRUE.equals(activationConditionMet)) { + // GarbageCollected status is irrelevant here, as this method is only called when a + // precondition does not hold, + // a deleter should be deleted even if it is otherwise garbage collected + if (dependentResource instanceof Deleter) { + ((Deleter

) dependentResource).delete(primary, context); + } + deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); } - boolean deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); + if (deletePostConditionMet) { markAsVisited(dependentResourceNode); - handleDependentDeleted(dependentResourceNode, activationConditionNotMet); + handleDependentDeleted(dependentResourceNode); } else { // updating alreadyVisited needs to be the last operation otherwise could lead to a race // condition in handleDelete condition checks @@ -190,11 +195,11 @@ protected void doRun(DependentResourceNode dependentResourceNode, } private synchronized void handleDependentDeleted( - DependentResourceNode dependentResourceNode, boolean activationConditionNotMet) { + DependentResourceNode dependentResourceNode) { dependentResourceNode.getDependsOn().forEach(dr -> { log.debug("Handle deleted for: {} with dependent: {} primaryID: {}", dr, dependentResourceNode, primaryID); - handleDelete(dr, activationConditionNotMet); + handleDelete(dr); }); } @@ -209,22 +214,35 @@ private synchronized void handleDependentsReconcile( } - private void handleReconcileConditionNotMet(DependentResourceNode dependentResourceNode, - boolean activationConditionNotMet) { + private void handleReconcileOrActivationConditionNotMet( + DependentResourceNode dependentResourceNode, + boolean activationConditionMet) { Set bottomNodes = new HashSet<>(); - markDependentsForDelete(dependentResourceNode, bottomNodes); + markDependentsForDelete(dependentResourceNode, bottomNodes, activationConditionMet); bottomNodes.forEach( - dependentResourceNode1 -> handleDelete(dependentResourceNode1, activationConditionNotMet)); + dependentResourceNode1 -> handleDelete(dependentResourceNode1)); } private void markDependentsForDelete(DependentResourceNode dependentResourceNode, - Set bottomNodes) { - markedForDelete.add(dependentResourceNode); + Set bottomNodes, boolean activationConditionMet) { + // this is a check so the activation condition is not evaluated twice, + // so if the activation condition was false, this node is not meant to be deleted. var dependents = dependentResourceNode.getParents(); - if (dependents.isEmpty()) { - bottomNodes.add(dependentResourceNode); + if (activationConditionMet) { + markedForDelete.add(dependentResourceNode); + if (dependents.isEmpty()) { + bottomNodes.add(dependentResourceNode); + } else { + dependents.forEach(d -> markDependentsForDelete(d, bottomNodes, true)); + } } else { - dependents.forEach(d -> markDependentsForDelete(d, bottomNodes)); + // this is for an edge case when there is only one resource but that is not active + markAsVisited(dependentResourceNode); + if (dependents.isEmpty()) { + handleNodeExecutionFinish(dependentResourceNode); + } else { + dependents.forEach(d -> markDependentsForDelete(d, bottomNodes, true)); + } } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java index 69822fbd98..099d5fad2b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java @@ -9,12 +9,12 @@ import io.javaoperatorsdk.operator.AggregatedOperatorException; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; @SuppressWarnings("rawtypes") class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { @@ -23,9 +23,13 @@ class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { Context mockContext = mock(Context.class); ExecutorService executorService = Executors.newCachedThreadPool(); + TestDependent dr3 = new TestDependent("DR_3"); + TestDependent dr4 = new TestDependent("DR_4"); + @BeforeEach void setup() { when(mockContext.getWorkflowExecutorService()).thenReturn(executorService); + when(mockContext.eventSourceRetriever()).thenReturn(mock(EventSourceRetriever.class)); } @Test @@ -60,7 +64,6 @@ void reconciliationWithSimpleDependsOn() { @Test void reconciliationWithTwoTheDependsOns() { - TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() .addDependentResource(dr1) @@ -80,9 +83,6 @@ void reconciliationWithTwoTheDependsOns() { @Test void diamondShareWorkflowReconcile() { - TestDependent dr3 = new TestDependent("DR_3"); - TestDependent dr4 = new TestDependent("DR_4"); - var workflow = new WorkflowBuilder() .addDependentResource(dr1) .addDependentResource(dr2).dependsOn(dr1) @@ -142,7 +142,6 @@ void dependentsOnErroredResourceNotReconciled() { @Test void oneBranchErrorsOtherCompletes() { - TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() .addDependentResource(dr1) @@ -416,7 +415,6 @@ void readyConditionNotMetTrivialCase() { @Test void readyConditionNotMetInOneParent() { - TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() .addDependentResource(dr1).withReadyPostcondition(notMetCondition) @@ -434,9 +432,6 @@ void readyConditionNotMetInOneParent() { @Test void diamondShareWithReadyCondition() { - TestDependent dr3 = new TestDependent("DR_3"); - TestDependent dr4 = new TestDependent("DR_4"); - var workflow = new WorkflowBuilder() .addDependentResource(dr1) .addDependentResource(dr2).dependsOn(dr1).withReadyPostcondition(notMetCondition) @@ -481,4 +476,101 @@ void garbageCollectedDeepResourceIsDeletedIfReconcilePreconditionDoesNotHold() { assertThat(executionHistory).deleted(gcDeleter); } + @Test + void notReconciledIfActivationConditionNotMet() { + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withActivationCondition(notMetCondition) + .addDependentResource(dr2) + .build(); + var res = workflow.reconcile(new TestCustomResource(), mockContext); + + assertThat(executionHistory).reconciled(dr2).notReconciled(dr1); + Assertions.assertThat(res.getErroredDependents()).isEmpty(); + Assertions.assertThat(res.getReconciledDependents()).contains(dr2); + } + + @Test + void dependentsOnANonActiveDependentNotReconciled() { + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withActivationCondition(notMetCondition) + .addDependentResource(dr2) + .addDependentResource(dr3).dependsOn(dr1) + .build(); + var res = workflow.reconcile(new TestCustomResource(), mockContext); + + assertThat(executionHistory).reconciled(dr2).notReconciled(dr1, dr3); + Assertions.assertThat(res.getErroredDependents()).isEmpty(); + Assertions.assertThat(res.getReconciledDependents()).contains(dr2); + } + + @Test + void readyConditionNotCheckedOnNonActiveDependent() { + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withActivationCondition(notMetCondition) + .withReadyPostcondition(notMetCondition) + .addDependentResource(dr2) + .addDependentResource(dr3).dependsOn(dr1) + .build(); + + var res = workflow.reconcile(new TestCustomResource(), mockContext); + + Assertions.assertThat(res.getNotReadyDependents()).isEmpty(); + } + + @Test + void reconcilePreconditionNotCheckedOnNonActiveDependent() { + var precondition = mock(Condition.class); + + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withActivationCondition(notMetCondition) + .withReconcilePrecondition(precondition) + .build(); + + workflow.reconcile(new TestCustomResource(), mockContext); + + verify(precondition, never()).isMet(any(), any(), any()); + } + + @Test + void deletesDependentsOfNonActiveDependentButNotTheNonActive() { + TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); + TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_2"); + + var workflow = new WorkflowBuilder() + .addDependentResource(dr1).withActivationCondition(notMetCondition) + .addDependentResource(drDeleter).dependsOn(dr1) + .addDependentResource(drDeleter2).dependsOn(drDeleter) + .withActivationCondition(notMetCondition) + .addDependentResource(drDeleter3).dependsOn(drDeleter2) + .build(); + + var res = workflow.reconcile(new TestCustomResource(), mockContext); + + Assertions.assertThat(res.getReconciledDependents()).isEmpty(); + assertThat(executionHistory).deleted(drDeleter, drDeleter3) + .notReconciled(dr1, + drDeleter2); + } + + @Test + void activationConditionOnlyCalledOnceOnDeleteDependents() { + TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); + var condition = mock(Condition.class); + when(condition.isMet(any(), any(), any())).thenReturn(false); + + var workflow = new WorkflowBuilder() + .addDependentResource(drDeleter).withActivationCondition(condition) + .addDependentResource(drDeleter2).dependsOn(drDeleter) + .build(); + + workflow.reconcile(new TestCustomResource(), mockContext); + + assertThat(executionHistory).deleted(drDeleter2); + verify(condition, times(1)).isMet(any(), any(), any()); + } + } From 1271067bcbed9e6b66dcfa8aac08cc8e2dce54e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 15:25:08 +0100 Subject: [PATCH 12/24] additional corner case test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../workflow/WorkflowCleanupExecutorTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java index ce335fba04..3b34dcf458 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java @@ -222,4 +222,16 @@ void ifDependentInactiveDeletePostConditionNotChecked() { Assertions.assertThat(res.getPostConditionNotMetDependents()).isEmpty(); } + @Test + void singleInactiveDependent() { + var workflow = new WorkflowBuilder() + .addDependentResource(dd1) + .withActivationCondition(notMetCondition) + .build(); + + workflow.cleanup(new TestCustomResource(), mockContext); + + assertThat(executionHistory).notReconciled(dd1); + } + } From 41a80642c6f001c7279862a23a6caf54f28acfc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 16:07:12 +0100 Subject: [PATCH 13/24] timeout increase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/EventSourceRetriever.java | 39 +++++++++++++++++-- .../source/informer/InformerEventSource.java | 23 ----------- .../informer/ManagedInformerEventSource.java | 4 ++ .../WorkflowMultipleActivationIT.java | 2 +- 4 files changed, 41 insertions(+), 27 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index c431154c7f..8ef6d47887 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -17,11 +17,44 @@ default ResourceEventSource getResourceEventSourceFor(Class depende List> getResourceEventSourcesFor(Class dependentType); - // todo javadocs - // this will be an idempotent synchronized operation - // todo check if event source with the name not exists yet, if does do not register. + /** + * Registers (and starts) event source dynamically during the reconciliation. if there is an event + * source registered already with the selected name it will just skip the registration. + *

+ * Normally this is not needed, just in very special cases. Like when you are implementing an + * operator that dynamically decides what resource it will watch or not. This is especially + * important when the platform might or might not have such resources, and even that decision can + * be done when registering event sources using the standard way. In other words, use this as a + * last effort. + *

+ *

+ * This method will block until the event source is synced (in case of InformersEventSource-s + * mostly). + *

+ *

+ * In case multiple reconciliations will happen concurrently, there will be nore more event + * sources with same name registered, always only one. + *

+ * + * @param name of the event source + * @param eventSource to register + */ void dynamicallyRegisterEventSource(String name, EventSource eventSource); + /** + * De-registers (and stops) an event source dynamically. If there is no event source with the + * target name method will just return. + *

+ * The method call will block until the event source is de-registered and stopped. If multiple + * reconciliations are calling the method concurrently all will be blocked until the event source + * if not de-registered. + *

+ *

+ * This method is ment only to be used for dynamically registered event sources. + *

+ * + * @param name of the event source + */ void dynamicallyDeRegisterEventSource(String name); EventSourceContext

eventSourceContexForDynamicRegistration(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 435e5e92cb..a9bc2d308a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -1,7 +1,6 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; import java.util.*; -import java.util.function.Function; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -10,8 +9,6 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; -import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.processing.event.Event; @@ -76,7 +73,6 @@ public class InformerEventSource // we need direct control for the indexer to propagate the just update resource also to the index private final PrimaryToSecondaryIndex primaryToSecondaryIndex; private final PrimaryToSecondaryMapper

primaryToSecondaryMapper; - private Map>> indexerBuffer = new HashMap<>(); private final String id = UUID.randomUUID().toString(); public InformerEventSource( @@ -305,25 +301,6 @@ private boolean acceptedByDeleteFilters(R resource, boolean b) { (genericFilter == null || genericFilter.accept(resource)); } - - // Since this event source instance is created by the user, the ConfigurationService is actually - // injected after it is registered. Some of the subcomponents are initialized at that time here. - @Override - public void setConfigurationService(ConfigurationService configurationService) { - super.setConfigurationService(configurationService); - - super.addIndexers(indexerBuffer); // todo check - indexerBuffer = new HashMap<>(); - } - - @Override - public void addIndexers(Map>> indexers) { - if (isRunning()) { - throw new OperatorException("Cannot add indexers after InformerEventSource started."); - } - indexerBuffer.putAll(indexers); // todo check - } - /** * Add an annotation to the resource so that the subsequent will be omitted * diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 2964fe313c..7a1a6ba310 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -13,6 +13,7 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; @@ -134,6 +135,9 @@ void setTemporalResourceCache(TemporaryResourceCache temporaryResourceCache) @Override public void addIndexers(Map>> indexers) { + if (isRunning()) { + throw new OperatorException("Cannot add indexers after InformerEventSource started."); + } this.indexers.putAll(indexers); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java index 9b402b7ca7..e5333ffe4f 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java @@ -20,7 +20,7 @@ public class WorkflowMultipleActivationIT { public static final String INITIAL_DATA = "initial data"; public static final String TEST_RESOURCE = "test1"; public static final String CHANGED_VALUE = "changed value"; - public static final int POLL_DELAY = 150; + public static final int POLL_DELAY = 300; @RegisterExtension LocallyRunOperatorExtension extension = From 65bb301713de0d963a6717405d95df5b5cc7c1e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 16:14:01 +0100 Subject: [PATCH 14/24] concurrency IT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../WorkflowMultipleActivationIT.java | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java index e5333ffe4f..a7f93935ee 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowMultipleActivationIT.java @@ -18,7 +18,8 @@ public class WorkflowMultipleActivationIT { public static final String INITIAL_DATA = "initial data"; - public static final String TEST_RESOURCE = "test1"; + public static final String TEST_RESOURCE1 = "test1"; + public static final String TEST_RESOURCE2 = "test2"; public static final String CHANGED_VALUE = "changed value"; public static final int POLL_DELAY = 300; @@ -34,8 +35,8 @@ void deactivatingAndReactivatingDependent() { var cr1 = extension.create(testResource()); await().untilAsserted(() -> { - var cm = extension.get(ConfigMap.class, TEST_RESOURCE); - var secret = extension.get(Secret.class, TEST_RESOURCE); + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); + var secret = extension.get(Secret.class, TEST_RESOURCE1); assertThat(cm).isNotNull(); assertThat(secret).isNotNull(); assertThat(cm.getData()).containsEntry(DATA_KEY, INITIAL_DATA); @@ -44,7 +45,7 @@ void deactivatingAndReactivatingDependent() { extension.delete(cr1); await().untilAsserted(() -> { - var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); assertThat(cm).isNull(); }); @@ -52,8 +53,8 @@ void deactivatingAndReactivatingDependent() { cr1 = extension.create(testResource()); await().untilAsserted(() -> { - var cm = extension.get(ConfigMap.class, TEST_RESOURCE); - var secret = extension.get(Secret.class, TEST_RESOURCE); + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); + var secret = extension.get(Secret.class, TEST_RESOURCE1); assertThat(cm).isNull(); assertThat(secret).isNotNull(); }); @@ -63,7 +64,7 @@ void deactivatingAndReactivatingDependent() { extension.replace(cr1); await().untilAsserted(() -> { - var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); assertThat(cm).isNotNull(); assertThat(cm.getData()).containsEntry(DATA_KEY, CHANGED_VALUE); }); @@ -73,7 +74,7 @@ void deactivatingAndReactivatingDependent() { extension.replace(cr1); await().pollDelay(Duration.ofMillis(POLL_DELAY)).untilAsserted(() -> { - var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); assertThat(cm).isNotNull(); // data not changed assertThat(cm.getData()).containsEntry(DATA_KEY, CHANGED_VALUE); @@ -82,7 +83,7 @@ void deactivatingAndReactivatingDependent() { var numOfReconciliation = extension.getReconcilerOfType(WorkflowMultipleActivationReconciler.class) .getNumberOfReconciliationExecution(); - var actualCM = extension.get(ConfigMap.class, TEST_RESOURCE); + var actualCM = extension.get(ConfigMap.class, TEST_RESOURCE1); actualCM.getData().put("data2", "additionaldata"); extension.replace(actualCM); await().pollDelay(Duration.ofMillis(POLL_DELAY)).untilAsserted(() -> { @@ -94,19 +95,43 @@ void deactivatingAndReactivatingDependent() { extension.delete(cr1); await().pollDelay(Duration.ofMillis(POLL_DELAY)).untilAsserted(() -> { - var cm = extension.get(ConfigMap.class, TEST_RESOURCE); + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); assertThat(cm).isNotNull(); }); } - WorkflowMultipleActivationCustomResource testResource() { + WorkflowMultipleActivationCustomResource testResource(String name) { var res = new WorkflowMultipleActivationCustomResource(); res.setMetadata(new ObjectMetaBuilder() - .withName(TEST_RESOURCE) + .withName(name) .build()); res.setSpec(new WorkflowMultipleActivationSpec()); res.getSpec().setValue(INITIAL_DATA); return res; } + WorkflowMultipleActivationCustomResource testResource() { + return testResource(TEST_RESOURCE1); + } + + WorkflowMultipleActivationCustomResource testResource2() { + return testResource(TEST_RESOURCE2); + } + + @Test + void simpleConcurrencyTest() { + ActivationCondition.MET = true; + extension.create(testResource()); + extension.create(testResource2()); + + await().untilAsserted(() -> { + var cm = extension.get(ConfigMap.class, TEST_RESOURCE1); + var cm2 = extension.get(ConfigMap.class, TEST_RESOURCE2); + assertThat(cm).isNotNull(); + assertThat(cm2).isNotNull(); + assertThat(cm.getData()).containsEntry(DATA_KEY, INITIAL_DATA); + assertThat(cm2.getData()).containsEntry(DATA_KEY, INITIAL_DATA); + }); + } + } From e4d3c4cf322f9847fc61a821911fbfcad2d1f232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 17:13:47 +0100 Subject: [PATCH 15/24] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- docs/documentation/workflows.md | 31 +++++++++++++------ .../api/reconciler/dependent/Dependent.java | 13 ++++++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/docs/documentation/workflows.md b/docs/documentation/workflows.md index c676207855..df43bad7a9 100644 --- a/docs/documentation/workflows.md +++ b/docs/documentation/workflows.md @@ -35,6 +35,12 @@ reconciliation process. proceeding until the condition checking whether the DR is ready holds true - **Delete postcondition** - is a condition on a given DR to check if the reconciliation of dependents can proceed after the DR is supposed to have been deleted +- **Activation condition** - is a special condition, what is mean to describe if the DR + should be used in the workflow. Typical use-case is, to differentiate if the actual + platform is Openshift or vanilla Kubernetes, therefore users can make sure that no + informers are registered for resources witch are platform specific (like Route on Openshift) - + since that would result in an error. But can be used do check like if CertMager is installed or not, + and define the behavior based on that. ## Defining Workflows @@ -66,6 +72,7 @@ will only consider the `ConfigMap` deleted until that post-condition becomes `tr @Dependent(type = ConfigMapDependentResource.class, reconcilePrecondition = ConfigMapReconcileCondition.class, deletePostcondition = ConfigMapDeletePostCondition.class, + activationCondition = ConfigMapActivationCondition.class, dependsOn = DEPLOYMENT_NAME) }) public class SampleWorkflowReconciler implements Reconciler, @@ -165,7 +172,7 @@ executed if a resource is marked for deletion. ## Common Principles - **As complete as possible execution** - when a workflow is reconciled, it tries to reconcile as - many resources as possible. Thus if an error happens or a ready condition is not met for a + many resources as possible. Thus, if an error happens or a ready condition is not met for a resources, all the other independent resources will be still reconciled. This is the opposite to a fail-fast approach. The assumption is that eventually in this way the overall state will converge faster towards the desired state than would be the case if the reconciliation was @@ -187,12 +194,12 @@ demonstrated using examples: 2. Root nodes, i.e. nodes in the graph that do not depend on other nodes are reconciled first, in a parallel manner. 2. A DR is reconciled if it does not depend on any other DRs, or *ALL* the DRs it depends on are - reconciled and ready. If a DR defines a reconcile pre-condition, then this condition must - become `true` before the DR is reconciled. + reconciled and ready. If a DR defines a reconcile pre-condition and/or an activationCondition, + then these condition must become `true` before the DR is reconciled. 2. A DR is considered *ready* if it got successfully reconciled and any ready post-condition it might define is `true`. -3. If a DR's reconcile pre-condition is not met, this DR is deleted. All of the DRs that depend - on the dependent resource being considered are also recursively deleted. This implies that +3. If a DR's reconcile pre-condition is not met, this DR is deleted. All the DRs that depend + on the dependent resource are also recursively deleted. This implies that DRs are deleted in reverse order compared the one in which they are reconciled. The reason for this behavior is (Will make a more detailed blog post about the design decision, much deeper than the reference documentation) @@ -202,11 +209,15 @@ demonstrated using examples: idempotency (i.e. with the same input state, we should have the same output state), from this follows that if the condition doesn't hold `true` anymore, the associated resource needs to be deleted because the resource shouldn't exist/have been created. -4. For a DR to be deleted by a workflow, it needs to implement the `Deleter` interface, in which - case its `delete` method will be called, unless it also implements the `GarbageCollected` - interface. If a DR doesn't implement `Deleter` it is considered as automatically deleted. If - a delete post-condition exists for this DR, it needs to become `true` for the workflow to - consider the DR as successfully deleted. +4. if a DR's activation condition is not met, it won't be reconciled or deleted. If + other DR's depend on it, those will be recursively deleted in a same way as for reconcile pre-condition. + Event sources for a dependent resource with activation condition are registered/de-registered dynamically, + thus during the reconciliation. +5. For a DR to be deleted by a workflow, it needs to implement the `Deleter` interface, in which + case its `delete` method will be called, unless it also implements the `GarbageCollected` + interface. If a DR doesn't implement `Deleter` it is considered as automatically deleted. If + a delete post-condition exists for this DR, it needs to become `true` for the workflow to + consider the DR as successfully deleted. ### Samples diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java index e8084cc6c9..e26576eb03 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java @@ -50,6 +50,19 @@ */ Class deletePostcondition() default Condition.class; + /** + *

+ * If the condition is not met, the dependent resource won't be used. That is, no event sources + * will be registered for the dependent, and won't be reconciled or deleted. If other dependents + * are still "depend on" this resource, those still will be deleted when needed. Exactly the same + * way as for the reconcilePrecondition. + *

+ *

+ * This condition is evaluated dynamically, thus on every reconciliation as other conditions. Thus + * it's result can change dynamically, therefore the event source that the dependent resource + * provides are registered or de-registered dynamically during the reconciliation. + *

+ */ Class activationCondition() default Condition.class; /** From 833d656e8948c2a96896a3f9c5d7c1c5ba83a1b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 17:18:31 +0100 Subject: [PATCH 16/24] naming + docs on IT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/WorkflowActivationConditionIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java index 38b6bed16a..3913263c83 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java @@ -25,8 +25,9 @@ public class WorkflowActivationConditionIT { .withReconciler(WorkflowActivationConditionReconciler.class) .build(); + // Without activation condition this would fail / there would be errors. @Test - void routeIsNotCreated() { + void reconciledOnVanillaKubernetesDespiteRouteInWorkflow() { extension.create(testResource()); await().untilAsserted(() -> { From e6058ae7a055c251408fae12aa2696539d546065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 17:26:42 +0100 Subject: [PATCH 17/24] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/WorkflowActivationConditionIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java index 3913263c83..38a11e0438 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationConditionIT.java @@ -37,8 +37,7 @@ void reconciledOnVanillaKubernetesDespiteRouteInWorkflow() { }); } - @Test - WorkflowActivationConditionCustomResource testResource() { + private WorkflowActivationConditionCustomResource testResource() { var res = new WorkflowActivationConditionCustomResource(); res.setMetadata(new ObjectMetaBuilder() .withName(TEST_RESOURCE_NAME) From 8d9a77814f42e9d9534452a1c4943c7354bc2c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 16 Nov 2023 17:59:22 +0100 Subject: [PATCH 18/24] IT changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../workflowactivationcondition/isOpenShiftCondition.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java index 0f76f8a52a..e02d47060c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java @@ -12,8 +12,7 @@ public boolean isMet( DependentResource dependentResource, WorkflowActivationConditionCustomResource primary, Context context) { - - return context.getClient().getApiGroups().getGroups().stream() - .anyMatch(g -> g.getName().equals("route.openshift.io")); + // we are testing if the reconciliation still works on Kubernetes, so this always false; + return false; } } From 884a22522a298e8aac7cbdaa5620aa8fcafa4272 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 17 Nov 2023 09:59:00 +0100 Subject: [PATCH 19/24] doc: improve wording [skip ci] Signed-off-by: Chris Laprun --- docs/documentation/workflows.md | 39 ++++++++++--------- .../api/reconciler/dependent/Dependent.java | 19 +++++---- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/docs/documentation/workflows.md b/docs/documentation/workflows.md index df43bad7a9..fed30afb25 100644 --- a/docs/documentation/workflows.md +++ b/docs/documentation/workflows.md @@ -35,12 +35,14 @@ reconciliation process. proceeding until the condition checking whether the DR is ready holds true - **Delete postcondition** - is a condition on a given DR to check if the reconciliation of dependents can proceed after the DR is supposed to have been deleted -- **Activation condition** - is a special condition, what is mean to describe if the DR - should be used in the workflow. Typical use-case is, to differentiate if the actual - platform is Openshift or vanilla Kubernetes, therefore users can make sure that no - informers are registered for resources witch are platform specific (like Route on Openshift) - - since that would result in an error. But can be used do check like if CertMager is installed or not, - and define the behavior based on that. +- **Activation condition** - is a special condition meant to specify under which condition the DR is used in the + workflow. A typical use-case for this feature is to only activate some dependents depending on the presence of + optional resources / features on the target cluster. Without this activation condition, JOSDK would attempt to + register an informer for these optional resources, which would cause an error in the case where the resource is + missing. With this activation condition, you can now conditionally register informers depending on whether the + condition holds or not. This is a very useful feature when your operator needs to handle different flavors of the + platform (e.g. OpenShift vs plain Kubernetes) and/or change its behavior based on the availability of optional + resources / features (e.g. CertManager, a specific Ingress controller, etc.). ## Defining Workflows @@ -193,12 +195,12 @@ demonstrated using examples: `depends-on` relations. 2. Root nodes, i.e. nodes in the graph that do not depend on other nodes are reconciled first, in a parallel manner. -2. A DR is reconciled if it does not depend on any other DRs, or *ALL* the DRs it depends on are - reconciled and ready. If a DR defines a reconcile pre-condition and/or an activationCondition, +3. A DR is reconciled if it does not depend on any other DRs, or *ALL* the DRs it depends on are + reconciled and ready. If a DR defines a reconcile pre-condition and/or an activation condition, then these condition must become `true` before the DR is reconciled. -2. A DR is considered *ready* if it got successfully reconciled and any ready post-condition it +4. A DR is considered *ready* if it got successfully reconciled and any ready post-condition it might define is `true`. -3. If a DR's reconcile pre-condition is not met, this DR is deleted. All the DRs that depend +5. If a DR's reconcile pre-condition is not met, this DR is deleted. All the DRs that depend on the dependent resource are also recursively deleted. This implies that DRs are deleted in reverse order compared the one in which they are reconciled. The reason for this behavior is (Will make a more detailed blog post about the design decision, much deeper @@ -209,15 +211,14 @@ demonstrated using examples: idempotency (i.e. with the same input state, we should have the same output state), from this follows that if the condition doesn't hold `true` anymore, the associated resource needs to be deleted because the resource shouldn't exist/have been created. -4. if a DR's activation condition is not met, it won't be reconciled or deleted. If - other DR's depend on it, those will be recursively deleted in a same way as for reconcile pre-condition. - Event sources for a dependent resource with activation condition are registered/de-registered dynamically, - thus during the reconciliation. -5. For a DR to be deleted by a workflow, it needs to implement the `Deleter` interface, in which - case its `delete` method will be called, unless it also implements the `GarbageCollected` - interface. If a DR doesn't implement `Deleter` it is considered as automatically deleted. If - a delete post-condition exists for this DR, it needs to become `true` for the workflow to - consider the DR as successfully deleted. +6. If a DR's activation condition is not met, it won't be reconciled or deleted. If other DR's depend on it, those will + be recursively deleted in a way similar to reconcile pre-conditions. Event sources for a dependent resource with + activation condition are registered/de-registered dynamically, thus during the reconciliation. +7. For a DR to be deleted by a workflow, it needs to implement the `Deleter` interface, in which + case its `delete` method will be called, unless it also implements the `GarbageCollected` + interface. If a DR doesn't implement `Deleter` it is considered as automatically deleted. If + a delete post-condition exists for this DR, it needs to become `true` for the workflow to + consider the DR as successfully deleted. ### Samples diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java index e26576eb03..754e2c85be 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Dependent.java @@ -52,15 +52,20 @@ /** *

- * If the condition is not met, the dependent resource won't be used. That is, no event sources - * will be registered for the dependent, and won't be reconciled or deleted. If other dependents - * are still "depend on" this resource, those still will be deleted when needed. Exactly the same - * way as for the reconcilePrecondition. + * A condition that needs to become true for the dependent to even be considered as part of the + * workflow. This is useful for dependents that represent optional resources on the cluster and + * might not be present. In this case, a reconcile pre-condition is not enough because in that + * situation, the associated informer will still be registered. With this activation condition, + * the associated event source will only be registered if the condition is met. Otherwise, this + * behaves like a reconcile pre-condition in the sense that dependents, that depend on this one, + * will only get created if the condition is met and will get deleted if the condition becomes + * false. *

*

- * This condition is evaluated dynamically, thus on every reconciliation as other conditions. Thus - * it's result can change dynamically, therefore the event source that the dependent resource - * provides are registered or de-registered dynamically during the reconciliation. + * As other conditions, this gets evaluated at the beginning of every reconciliation, which means + * that it allows to react to optional resources becoming available on the cluster as the operator + * runs. More specifically, this means that the associated event source can get dynamically + * registered or de-registered during reconciliation. *

*/ Class activationCondition() default Condition.class; From b3f3f4ac3f38866b90ba141cb6c003392e3f716a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 17 Nov 2023 10:45:14 +0100 Subject: [PATCH 20/24] fix: warning Signed-off-by: Chris Laprun --- .../operator/processing/dependent/workflow/Workflow.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java index fd2acac234..839844256e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Workflow.java @@ -43,6 +43,7 @@ default Map getDependentResourcesByName() { return Collections.emptyMap(); } + @SuppressWarnings("rawtypes") default Map getDependentResourcesByNameWithoutActivationCondition() { return Collections.emptyMap(); } From 689ec0f133cd10015f88a54b226bb74d9da81a6b Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 17 Nov 2023 13:51:06 +0100 Subject: [PATCH 21/24] doc: more improvements Signed-off-by: Chris Laprun --- .../workflow/WorkflowReconcileExecutor.java | 3 +- .../event/EventSourceRetriever.java | 36 ++++++++++--------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index d1fbfc5845..ce380ab88b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -219,8 +219,7 @@ private void handleReconcileOrActivationConditionNotMet( boolean activationConditionMet) { Set bottomNodes = new HashSet<>(); markDependentsForDelete(dependentResourceNode, bottomNodes, activationConditionMet); - bottomNodes.forEach( - dependentResourceNode1 -> handleDelete(dependentResourceNode1)); + bottomNodes.forEach(this::handleDelete); } private void markDependentsForDelete(DependentResourceNode dependentResourceNode, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index 8ef6d47887..94fb463828 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -18,22 +18,23 @@ default ResourceEventSource getResourceEventSourceFor(Class depende List> getResourceEventSourcesFor(Class dependentType); /** - * Registers (and starts) event source dynamically during the reconciliation. if there is an event - * source registered already with the selected name it will just skip the registration. + * Registers (and starts) the specified {@link EventSource} dynamically during the reconciliation. + * If an EventSource is already registered with the specified name, the registration will be + * ignored. *

- * Normally this is not needed, just in very special cases. Like when you are implementing an - * operator that dynamically decides what resource it will watch or not. This is especially - * important when the platform might or might not have such resources, and even that decision can - * be done when registering event sources using the standard way. In other words, use this as a - * last effort. + * This is only needed when your operator needs to adapt dynamically based on optional resources + * that may or may not be present on the target cluster. Even in this situation, it should be + * possible to make these decisions at when the "regular" EventSources are registered so this + * method should not typically be called directly but rather by the framework to support + * activation conditions of dependents, for example. *

*

- * This method will block until the event source is synced (in case of InformersEventSource-s - * mostly). + * This method will block until the event source is synced, if needed (as is the case for + * {@link io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource}). *

*

- * In case multiple reconciliations will happen concurrently, there will be nore more event - * sources with same name registered, always only one. + * Should multiple reconciliations happen concurrently, only one EventSource with the specified + * name will ever be registered. *

* * @param name of the event source @@ -42,15 +43,16 @@ default ResourceEventSource getResourceEventSourceFor(Class depende void dynamicallyRegisterEventSource(String name, EventSource eventSource); /** - * De-registers (and stops) an event source dynamically. If there is no event source with the - * target name method will just return. + * De-registers (and stops) the {@link EventSource} associated with the specified name. If no such + * source exists, this method will do nothing. *

- * The method call will block until the event source is de-registered and stopped. If multiple - * reconciliations are calling the method concurrently all will be blocked until the event source - * if not de-registered. + * This method will block until the event source is de-registered and stopped. If multiple + * reconciliations happen concurrently, all will be blocked until the event source is + * de-registered. *

*

- * This method is ment only to be used for dynamically registered event sources. + * This method is meant only to be used for dynamically registered event sources and should not be + * typically called directly. *

* * @param name of the event source From ba6d31f0ecd36e7cabd18f7253d70f3cf7cee14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 20 Nov 2023 10:12:25 +0100 Subject: [PATCH 22/24] changes from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- operator-framework/pom.xml | 1 + .../{isOpenShiftCondition.java => IsOpenShiftCondition.java} | 2 +- .../WorkflowActivationConditionReconciler.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/{isOpenShiftCondition.java => IsOpenShiftCondition.java} (95%) diff --git a/operator-framework/pom.xml b/operator-framework/pom.xml index 2d22e7037b..943acc11d2 100644 --- a/operator-framework/pom.xml +++ b/operator-framework/pom.xml @@ -68,6 +68,7 @@ io.fabric8 openshift-client-api + test org.apache.logging.log4j diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/IsOpenShiftCondition.java similarity index 95% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/IsOpenShiftCondition.java index e02d47060c..79434409b8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/isOpenShiftCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/IsOpenShiftCondition.java @@ -5,7 +5,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; -public class isOpenShiftCondition +public class IsOpenShiftCondition implements Condition { @Override public boolean isMet( diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java index b9332f5d44..33db3043ba 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcondition/WorkflowActivationConditionReconciler.java @@ -6,7 +6,7 @@ @ControllerConfiguration(dependents = { @Dependent(type = ConfigMapDependentResource.class), @Dependent(type = RouteDependentResource.class, - activationCondition = isOpenShiftCondition.class) + activationCondition = IsOpenShiftCondition.class) }) public class WorkflowActivationConditionReconciler implements Reconciler { From 81c26582fe33255e49e05b54a24d09cd76e11929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 20 Nov 2023 10:42:52 +0100 Subject: [PATCH 23/24] fixes from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../dependent/workflow/AbstractWorkflowExecutor.java | 3 +++ .../dependent/workflow/WorkflowReconcileExecutor.java | 6 +----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index fc4ef12822..6aab0f1f39 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -46,6 +46,9 @@ public AbstractWorkflowExecutor(Workflow

workflow, P primary, Context

cont protected synchronized void waitForScheduledExecutionsToRun() { while (true) { try { + // in case when workflow just contains non-activated dependents, + // it needs to be checked first if there are already no executions + // scheduled at the beginning. if (noMoreExecutionsScheduled()) { break; } else { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index ce380ab88b..27b5ad45da 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -167,12 +167,8 @@ protected void doRun(DependentResourceNode dependentResourceNode, DependentResource dependentResource) { var deletePostCondition = dependentResourceNode.getDeletePostcondition(); - var activationConditionMet = dependentResourceNode.getActivationCondition() - .map(c -> c.isMet(dependentResource, primary, context)) - .orElse(true); - boolean deletePostConditionMet = true; - if (Boolean.TRUE.equals(activationConditionMet)) { + if (isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource)) { // GarbageCollected status is irrelevant here, as this method is only called when a // precondition does not hold, // a deleter should be deleted even if it is otherwise garbage collected From 62d722409f746479f320c8dc4fbfe7638b34027a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 20 Nov 2023 10:45:52 +0100 Subject: [PATCH 24/24] javadoc for registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/event/EventSourceRetriever.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index 94fb463828..67f149f5cd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -20,7 +20,8 @@ default ResourceEventSource getResourceEventSourceFor(Class depende /** * Registers (and starts) the specified {@link EventSource} dynamically during the reconciliation. * If an EventSource is already registered with the specified name, the registration will be - * ignored. + * ignored. It is the user's responsibility to handle the naming correctly, thus to not try to + * register different event source with same name that is already registered. *

* This is only needed when your operator needs to adapt dynamically based on optional resources * that may or may not be present on the target cluster. Even in this situation, it should be