From 2d3c1118fb45f880616ce69170e1d0667c2e6b51 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 26 May 2022 13:49:28 +0200 Subject: [PATCH 01/35] feat: annotation config for workflow --- .../AnnotationControllerConfiguration.java | 11 ++++- .../dependent/DependentResourceSpec.java | 46 +++++++++++++++++++ .../api/reconciler/dependent/Dependent.java | 13 ++++++ .../reconciler/dependent/VoidCondition.java | 14 ++++++ 4 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index c86c3f95b6..dabae34942 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -166,12 +166,15 @@ public List getDependentResources() { } final var name = getName(dependent, dependentType); - final var spec = specsMap.get(name); + var spec = specsMap.get(name); if (spec != null) { throw new IllegalArgumentException( "A DependentResource named: " + name + " already exists: " + spec); } - specsMap.put(name, new DependentResourceSpec(dependentType, config, name)); + spec = new DependentResourceSpec(dependentType, config, name); + spec.setDependsOn(Set.of(dependent.dependsOn())); + addConditions(spec,dependent); + specsMap.put(name, spec); } specs = specsMap.values().stream().collect(Collectors.toUnmodifiableList()); @@ -179,6 +182,10 @@ public List getDependentResources() { return specs; } + private void addConditions(DependentResourceSpec spec, Dependent dependent) { + // todo + } + private String getName(Dependent dependent, Class dependentType) { var name = dependent.name(); if (name.isBlank()) { 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 40a7db53c9..905d43206f 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 @@ -2,8 +2,10 @@ import java.util.Objects; import java.util.Optional; +import java.util.Set; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; public class DependentResourceSpec, C> { @@ -13,6 +15,14 @@ public class DependentResourceSpec, C> { private final String name; + private Set dependsOn; + + private Condition readyCondition; + + private Condition reconcileCondition; + + private Condition deletePostCondition; + public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig, String name) { this.dependentResourceClass = dependentResourceClass; @@ -55,4 +65,40 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(name); } + + public Set getDependsOn() { + return dependsOn; + } + + public DependentResourceSpec setDependsOn(Set dependsOn) { + this.dependsOn = dependsOn; + return this; + } + + public Condition getReadyCondition() { + return readyCondition; + } + + public DependentResourceSpec setReadyCondition(Condition readyCondition) { + this.readyCondition = readyCondition; + return this; + } + + public Condition getReconcileCondition() { + return reconcileCondition; + } + + public DependentResourceSpec setReconcileCondition(Condition reconcileCondition) { + this.reconcileCondition = reconcileCondition; + return this; + } + + public Condition getDeletePostCondition() { + return deletePostCondition; + } + + public DependentResourceSpec setDeletePostCondition(Condition deletePostCondition) { + this.deletePostCondition = deletePostCondition; + return this; + } } 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 25c7418445..9cefd7abcd 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 @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator.api.reconciler.dependent; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET; public @interface Dependent { @@ -8,4 +10,15 @@ Class type(); String name() default NO_VALUE_SET; + + @SuppressWarnings("rawtypes") + Class readyCondition() default VoidCondition.class; + + @SuppressWarnings("rawtypes") + Class reconcileCondition() default VoidCondition.class; + + @SuppressWarnings("rawtypes") + Class deletePostCondition() default VoidCondition.class; + + String[] dependsOn() default {}; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java new file mode 100644 index 0000000000..7f4faf0ed1 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/VoidCondition.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.api.reconciler.dependent; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; + +/** Used as default value for Condition in annotations */ +@SuppressWarnings("rawtypes") +public class VoidCondition implements Condition { + @Override + public boolean isMet(DependentResource dependentResource, HasMetadata primary, Context context) { + throw new IllegalStateException("This is a placeholder class, should not be called"); + } +} From 7c40e1bd7d31bb939c8fc8bcee65387a4b5d2b69 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 30 May 2022 16:24:58 +0200 Subject: [PATCH 02/35] wip --- .../AnnotationControllerConfiguration.java | 26 +++++++- .../operator/processing/ManagedWorkflow.java | 66 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index dabae34942..2f3f5d3bad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config; +import java.lang.reflect.InvocationTargetException; import java.time.Duration; import java.util.Arrays; import java.util.Collections; @@ -19,9 +20,11 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.VoidCondition; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters; @@ -173,6 +176,7 @@ public List getDependentResources() { } spec = new DependentResourceSpec(dependentType, config, name); spec.setDependsOn(Set.of(dependent.dependsOn())); + // todo tests addConditions(spec,dependent); specsMap.put(name, spec); } @@ -182,8 +186,28 @@ public List getDependentResources() { return specs; } + @SuppressWarnings("unchecked") private void addConditions(DependentResourceSpec spec, Dependent dependent) { - // todo + if (dependent.deletePostCondition() != VoidCondition.class) { + spec.setDeletePostCondition(instantiateCondition(dependent.deletePostCondition())); + } + if (dependent.readyCondition() != VoidCondition.class) { + spec.setReadyCondition(instantiateCondition(dependent.readyCondition())); + } + if (dependent.reconcileCondition() != VoidCondition.class) { + spec.setReconcileCondition(instantiateCondition(dependent.reconcileCondition())); + } + } + + private Condition instantiateCondition(Class condition) { + try { + return condition.getDeclaredConstructor().newInstance(); + } catch (InstantiationException + | IllegalAccessException + | InvocationTargetException + | NoSuchMethodException e) { + throw new OperatorException(e); + } } private String getName(Dependent dependent, Class dependentType) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java new file mode 100644 index 0000000000..92a5b0bf3a --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java @@ -0,0 +1,66 @@ +package io.javaoperatorsdk.operator.processing; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow; +import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; + +import java.util.List; + +@SuppressWarnings("rawtypes") +class ManagedWorkflow

{ + + private Workflow

workflow; + private final boolean isCleaner; + + public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { + workflow = toWorkFlow(client,dependentResourceSpecs); + + isCleaner = checkIfCleaner(); + } + + // todo + private boolean checkIfCleaner() { + return false; + } + + // todo + public boolean isCleaner() { + return isCleaner; + } + + private Workflow

toWorkFlow(KubernetesClient client, List dependentResourceSpecs) { + List orderedDependentResources = orderDependentsToBeAdded(dependentResourceSpecs); + + var w = new WorkflowBuilder

(); + return w.build(); + } + + private List orderDependentsToBeAdded(List dependentResourceSpecs) { + return null; + } + + private DependentResource createAndConfigureFrom(DependentResourceSpec spec, + KubernetesClient client) { + final var dependentResource = + ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); + + if (dependentResource instanceof KubernetesClientAware) { + ((KubernetesClientAware) dependentResource).setKubernetesClient(client); + } + + if (dependentResource instanceof DependentResourceConfigurator) { + final var configurator = (DependentResourceConfigurator) dependentResource; + spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); + } + return dependentResource; + } + + + +} From 5c62702144bc4973445fd85c1451461ebf156815 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 31 May 2022 14:31:15 +0200 Subject: [PATCH 03/35] impl skeleton --- .../AnnotationControllerConfiguration.java | 22 +-- .../dependent/DependentResourceSpec.java | 6 +- .../operator/processing/Controller.java | 173 +++++------------- .../operator/processing/ManagedWorkflow.java | 127 +++++++++---- .../dependent/workflow/Workflow.java | 7 + .../workflow/builder/DependentBuilder.java | 10 +- 6 files changed, 164 insertions(+), 181 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 2f3f5d3bad..9088b80560 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -177,7 +177,7 @@ public List getDependentResources() { spec = new DependentResourceSpec(dependentType, config, name); spec.setDependsOn(Set.of(dependent.dependsOn())); // todo tests - addConditions(spec,dependent); + addConditions(spec, dependent); specsMap.put(name, spec); } @@ -188,20 +188,20 @@ public List getDependentResources() { @SuppressWarnings("unchecked") private void addConditions(DependentResourceSpec spec, Dependent dependent) { - if (dependent.deletePostCondition() != VoidCondition.class) { - spec.setDeletePostCondition(instantiateCondition(dependent.deletePostCondition())); - } - if (dependent.readyCondition() != VoidCondition.class) { - spec.setReadyCondition(instantiateCondition(dependent.readyCondition())); - } - if (dependent.reconcileCondition() != VoidCondition.class) { - spec.setReconcileCondition(instantiateCondition(dependent.reconcileCondition())); - } + if (dependent.deletePostCondition() != VoidCondition.class) { + spec.setDeletePostCondition(instantiateCondition(dependent.deletePostCondition())); + } + if (dependent.readyCondition() != VoidCondition.class) { + spec.setReadyCondition(instantiateCondition(dependent.readyCondition())); + } + if (dependent.reconcileCondition() != VoidCondition.class) { + spec.setReconcileCondition(instantiateCondition(dependent.reconcileCondition())); + } } private Condition instantiateCondition(Class condition) { try { - return condition.getDeclaredConstructor().newInstance(); + return condition.getDeclaredConstructor().newInstance(); } catch (InstantiationException | IllegalAccessException | InvocationTargetException 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 905d43206f..ac79f36be7 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 @@ -17,11 +17,11 @@ public class DependentResourceSpec, C> { private Set dependsOn; - private Condition readyCondition; + private Condition readyCondition; - private Condition reconcileCondition; + private Condition reconcileCondition; - private Condition deletePostCondition; + private Condition deletePostCondition; public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig, String name) { 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 f60dffcc33..2251ce4f0d 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 @@ -1,7 +1,6 @@ package io.javaoperatorsdk.operator.processing; import java.util.*; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,17 +15,10 @@ import io.javaoperatorsdk.operator.*; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; import io.javaoperatorsdk.operator.api.reconciler.*; -import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; -import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; -import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; -import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; -import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceException; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE; @@ -42,12 +34,10 @@ public class Controller

private final ControllerConfiguration

configuration; private final KubernetesClient kubernetesClient; private final EventSourceManager

eventSourceManager; - private final LinkedHashMap dependents; private final boolean contextInitializer; - private final boolean hasDeleterDependents; private final boolean isCleaner; private final Metrics metrics; - + private final ManagedWorkflow

managedWorkflow; public Controller(Reconciler

reconciler, ControllerConfiguration

configuration, @@ -58,92 +48,46 @@ public Controller(Reconciler

reconciler, this.metrics = Optional.ofNullable(ConfigurationServiceProvider.instance().getMetrics()) .orElse(Metrics.NOOP); contextInitializer = reconciler instanceof ContextInitializer; - eventSourceManager = new EventSourceManager<>(this); - - final var hasDeleterHolder = new boolean[] {false}; - final var specs = configuration.getDependentResources(); - final var specsSize = specs.size(); - if (specsSize == 0) { - dependents = new LinkedHashMap<>(); - } else { - final Map dependentsHolder = new LinkedHashMap<>(specsSize); - specs.forEach(drs -> { - final var dependent = createAndConfigureFrom(drs, kubernetesClient); - // check if dependent implements Deleter to record that fact - if (!hasDeleterHolder[0] && dependent instanceof Deleter - && !(dependent instanceof GarbageCollected)) { - hasDeleterHolder[0] = true; - } - dependentsHolder.put(drs.getName(), dependent); - }); - dependents = new LinkedHashMap<>(dependentsHolder); - } - - hasDeleterDependents = hasDeleterHolder[0]; isCleaner = reconciler instanceof Cleaner; - } - - @SuppressWarnings("rawtypes") - private DependentResource createAndConfigureFrom(DependentResourceSpec spec, - KubernetesClient client) { - final var dependentResource = - ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); - - if (dependentResource instanceof KubernetesClientAware) { - ((KubernetesClientAware) dependentResource).setKubernetesClient(client); - } - - if (dependentResource instanceof DependentResourceConfigurator) { - final var configurator = (DependentResourceConfigurator) dependentResource; - spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); - } - return dependentResource; - } - - private void initContextIfNeeded(P resource, Context

context) { - if (contextInitializer) { - ((ContextInitializer

) reconciler).initContext(resource, context); - } + managedWorkflow = + new ManagedWorkflow<>(kubernetesClient, configuration.getDependentResources()); } @Override public DeleteControl cleanup(P resource, Context

context) { try { - return metrics - .timeControllerExecution( - new ControllerExecution<>() { - @Override - public String name() { - return "cleanup"; - } - - @Override - public String controllerName() { - return configuration.getName(); - } - - @Override - public String successTypeName(DeleteControl deleteControl) { - return deleteControl.isRemoveFinalizer() ? "delete" : "finalizerNotRemoved"; - } - - @Override - public DeleteControl execute() { - initContextIfNeeded(resource, context); - if (hasDeleterDependents) { - dependents.values().stream() - .filter(d -> d instanceof Deleter && !(d instanceof GarbageCollected)) - .map(Deleter.class::cast) - .forEach(deleter -> deleter.delete(resource, context)); - } - if (isCleaner) { - return ((Cleaner

) reconciler).cleanup(resource, context); - } else { - return DeleteControl.defaultDelete(); - } - } - }); + return metrics.timeControllerExecution( + new ControllerExecution<>() { + @Override + public String name() { + return "cleanup"; + } + + @Override + public String controllerName() { + return configuration.getName(); + } + + @Override + public String successTypeName(DeleteControl deleteControl) { + return deleteControl.isRemoveFinalizer() ? "delete" : "finalizerNotRemoved"; + } + + @Override + public DeleteControl execute() { + initContextIfNeeded(resource, context); + if (managedWorkflow.isCleaner()) { + // todo use result + managedWorkflow.cleanup(resource, context); + } + if (isCleaner) { + return ((Cleaner

) reconciler).cleanup(resource, context); + } else { + return DeleteControl.defaultDelete(); + } + } + }); } catch (Exception e) { throw new OperatorException(e); } @@ -178,53 +122,24 @@ public String successTypeName(UpdateControl

result) { @Override public UpdateControl

execute() throws Exception { initContextIfNeeded(resource, context); - final var exceptions = new ArrayList(dependents.size()); - dependents.forEach((name, dependent) -> { - try { - final var reconcileResult = dependent.reconcile(resource, context); - context.managedDependentResourceContext().setReconcileResult(name, - reconcileResult); - log.info("Reconciled dependent '{}' -> {}", name, reconcileResult.getOperation()); - } catch (Exception e) { - final var message = e.getMessage(); - exceptions.add(new ManagedDependentResourceException( - name, "Error reconciling dependent '" + name + "': " + message, e)); - } - }); - - if (!exceptions.isEmpty()) { - throw new AggregatedOperatorException("One or more DependentResource(s) failed:\n" + - exceptions.stream() - .map(Controller.this::createExceptionInformation) - .collect(Collectors.joining("\n")), - exceptions); + if (!managedWorkflow.isEmptyWorkflow()) { + // todo use result + managedWorkflow.reconcile(resource, context); } - return reconciler.reconcile(resource, context); } }); } - private String createExceptionInformation(Exception e) { - final var exceptionLocation = Optional.ofNullable(e.getCause()) - .map(Throwable::getStackTrace) - .filter(stackTrace -> stackTrace.length > 0) - .map(stackTrace -> { - int i = 0; - while (i < stackTrace.length) { - final var moduleName = stackTrace[i].getModuleName(); - if (!"java.base".equals(moduleName)) { - return " at: " + stackTrace[i].toString(); - } - i++; - } - return ""; - }); - return "\t\t- " + e.getMessage() + exceptionLocation.orElse(""); + private void initContextIfNeeded(P resource, Context

context) { + if (contextInitializer) { + ((ContextInitializer

) reconciler).initContext(resource, context); + } } public void initAndRegisterEventSources(EventSourceContext

context) { - dependents.entrySet().stream() + managedWorkflow + .getDependentResourceByName().entrySet().stream() .filter(drEntry -> drEntry.getValue() instanceof EventSourceProvider) .forEach(drEntry -> { final var provider = (EventSourceProvider) drEntry.getValue(); @@ -374,6 +289,6 @@ public void stop() { } public boolean useFinalizer() { - return isCleaner || hasDeleterDependents; + return isCleaner || managedWorkflow.isCleaner(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java index 92a5b0bf3a..190dd2b9fa 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java @@ -1,66 +1,119 @@ package io.javaoperatorsdk.operator.processing; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowExecutionResult; import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; -import java.util.List; - @SuppressWarnings("rawtypes") class ManagedWorkflow

{ - private Workflow

workflow; - private final boolean isCleaner; + private final Workflow

workflow; + private final boolean isCleaner; + private final boolean isEmptyWorkflow; + private final Map dependentResourceByName; - public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { - workflow = toWorkFlow(client,dependentResourceSpecs); + public ManagedWorkflow(KubernetesClient client, + List dependentResourceSpecs) { + var orderedSpecs = orderDependentsToBeAdded(dependentResourceSpecs); + dependentResourceByName = orderedSpecs + .stream().collect(Collectors.toMap(DependentResourceSpec::getName, + spec -> createAndConfigureFrom(spec, client))); - isCleaner = checkIfCleaner(); - } - // todo - private boolean checkIfCleaner() { - return false; - } + workflow = toWorkFlow(client, orderedSpecs); + isCleaner = checkIfCleaner(); + isEmptyWorkflow = workflow.getDependentResources().isEmpty(); + } - // todo - public boolean isCleaner() { - return isCleaner; - } + public WorkflowExecutionResult reconcile(P primary, Context

context) { + return workflow.reconcile(primary, context); + } - private Workflow

toWorkFlow(KubernetesClient client, List dependentResourceSpecs) { - List orderedDependentResources = orderDependentsToBeAdded(dependentResourceSpecs); + public WorkflowCleanupResult cleanup(P primary, Context

context) { + return workflow.cleanup(primary, context); + } - var w = new WorkflowBuilder

(); - return w.build(); - } - private List orderDependentsToBeAdded(List dependentResourceSpecs) { - return null; + private boolean checkIfCleaner() { + for (var dr : workflow.getDependentResources()) { + if (dr instanceof Deleter && !(dr instanceof GarbageCollected)) { + return true; + } + } + return false; + } + + + @SuppressWarnings("unchecked") + private Workflow

toWorkFlow(KubernetesClient client, + List dependentResourceSpecs) { + var orderedSpecs = orderDependentsToBeAdded(dependentResourceSpecs); + + var workflow = new WorkflowBuilder

(); + orderedSpecs.forEach(spec -> { + var drBuilder = + workflow.addDependent(dependentResourceByName.get(spec.getName())).dependsOn( + (Set) spec.getDependsOn() + .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); + drBuilder.withDeletePostCondition(spec.getDeletePostCondition()); + drBuilder.withReconcileCondition(spec.getReconcileCondition()); + drBuilder.withReadyCondition(spec.getReadyCondition()); + }); + return workflow.build(); + } + + // todo + private List orderDependentsToBeAdded( + List dependentResourceSpecs) { + return dependentResourceSpecs; + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private DependentResource createAndConfigureFrom(DependentResourceSpec spec, + KubernetesClient client) { + final var dependentResource = + ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); + + if (dependentResource instanceof KubernetesClientAware) { + ((KubernetesClientAware) dependentResource).setKubernetesClient(client); } - private DependentResource createAndConfigureFrom(DependentResourceSpec spec, - KubernetesClient client) { - final var dependentResource = - ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); - - if (dependentResource instanceof KubernetesClientAware) { - ((KubernetesClientAware) dependentResource).setKubernetesClient(client); - } - - if (dependentResource instanceof DependentResourceConfigurator) { - final var configurator = (DependentResourceConfigurator) dependentResource; - spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); - } - return dependentResource; + if (dependentResource instanceof DependentResourceConfigurator) { + final var configurator = (DependentResourceConfigurator) dependentResource; + spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); } + return dependentResource; + } + + public boolean isCleaner() { + return isCleaner; + } + public boolean isEmptyWorkflow() { + return isEmptyWorkflow; + } + public Set getDependentResources() { + return workflow.getDependentResources(); + } + public Map getDependentResourceByName() { + return dependentResourceByName; + } } 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 df7ca1bd31..419be51cfb 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 @@ -4,10 +4,12 @@ import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; /** * Dependents definition: so if B depends on A, the B is dependent of A. @@ -99,4 +101,9 @@ Set getBottomLevelResource() { ExecutorService getExecutorService() { return executorService; } + + public Set getDependentResources() { + return dependentResourceNodes.stream().map(DependentResourceNode::getDependentResource) + .collect(Collectors.toSet()); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java index 56feec0ae1..07191fe961 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java @@ -1,5 +1,9 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow.builder; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; @@ -16,7 +20,7 @@ public DependentBuilder(WorkflowBuilder

workflowBuilder, DependentResourceNod this.node = node; } - public DependentBuilder

dependsOn(DependentResource... dependentResources) { + public DependentBuilder

dependsOn(Set dependentResources) { for (var dependentResource : dependentResources) { var dependsOn = workflowBuilder.getNodeByDependentResource(dependentResource); node.addDependsOnRelation(dependsOn); @@ -24,6 +28,10 @@ public DependentBuilder

dependsOn(DependentResource... dependentResources) { return this; } + public DependentBuilder

dependsOn(DependentResource... dependentResources) { + return dependsOn(new HashSet<>(Arrays.asList(dependentResources))); + } + public DependentBuilder

withReconcileCondition(Condition reconcileCondition) { node.setReconcileCondition(reconcileCondition); return this; From e344e77a51bbde8cd692d5d961b4c5ecdffbef5a Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 31 May 2022 16:48:01 +0200 Subject: [PATCH 04/35] wip --- .../operator/processing/Controller.java | 1 + .../workflow}/ManagedWorkflow.java | 13 +-- .../workflow/ManagedWorkflowUtils.java | 98 +++++++++++++++++++ 3 files changed, 102 insertions(+), 10 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/{ => dependent/workflow}/ManagedWorkflow.java (88%) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.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 2251ce4f0d..a64ef116e5 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 @@ -19,6 +19,7 @@ import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; +import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflow; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java similarity index 88% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 190dd2b9fa..623dc2abfe 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.processing; +package io.javaoperatorsdk.operator.processing.dependent.workflow; import java.util.List; import java.util.Map; @@ -15,13 +15,10 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; -import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow; -import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; -import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowExecutionResult; import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; @SuppressWarnings("rawtypes") -class ManagedWorkflow

{ +public class ManagedWorkflow

{ private final Workflow

workflow; private final boolean isCleaner; @@ -30,7 +27,7 @@ class ManagedWorkflow

{ public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { - var orderedSpecs = orderDependentsToBeAdded(dependentResourceSpecs); + var orderedSpecs = ManagedWorkflowUtils.orderAndDetectCycles(dependentResourceSpecs); dependentResourceByName = orderedSpecs .stream().collect(Collectors.toMap(DependentResourceSpec::getName, spec -> createAndConfigureFrom(spec, client))); @@ -109,10 +106,6 @@ public boolean isEmptyWorkflow() { return isEmptyWorkflow; } - public Set getDependentResources() { - return workflow.getDependentResources(); - } - public Map getDependentResourceByName() { return dependentResourceByName; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java new file mode 100644 index 0000000000..c522d9947c --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java @@ -0,0 +1,98 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.*; +import java.util.stream.Collectors; + +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; + +@SuppressWarnings({"rawtypes", "unchecked"}) +public class ManagedWorkflowUtils { + + public void checkForNameDuplication(List dependentResourceSpecs) { + if (dependentResourceSpecs.size() <= 1) { + return; + } + var nameList = + dependentResourceSpecs.stream().map(DependentResourceSpec::getName) + .sorted().collect(Collectors.toList()); + for (int i = 0; i < nameList.size() - 1; i++) { + if (nameList.get(i).equals(nameList.get(i + 1))) { + throw new OperatorException("Duplicate dependent resource name:" + nameList.get(i)); + } + } + } + + /** + * Throws also exception if there is a cycle in the dependencies. + * + * @param dependentResourceSpecs list of specs + * @return top-bottom ordered resources that can be added safely to workflow + * + */ + public static List orderAndDetectCycles( + List dependentResourceSpecs) { + + List res = new ArrayList<>(dependentResourceSpecs.size()); + Set alreadySelected = new HashSet<>(); + Map> dependOnIndex = + createDependOnIndex(dependentResourceSpecs); + Map nameToDR = dependentResourceSpecs.stream() + .collect(Collectors.toMap(DependentResourceSpec::getName, s -> s)); + Set selectedLastIteration = + getTopDependentResources(dependentResourceSpecs); + + while (!selectedLastIteration.isEmpty()) { + res.addAll(selectedLastIteration); + alreadySelected.addAll(selectedLastIteration); + Set newAdds = new HashSet<>(); + selectedLastIteration.forEach(dr -> dependOnIndex.get(dr.getName()).forEach(ndr -> { + if (allDependsOnsAlreadySelected(ndr, alreadySelected, nameToDR, dr.getName())) { + newAdds.add(ndr); + } + })); + selectedLastIteration = newAdds; + } + if (res.size() != dependentResourceSpecs.size()) { + var cycleNames = dependentResourceSpecs.stream().filter(dr -> !alreadySelected.contains(dr)) + .map(dr -> dr.getName()) + .collect(Collectors.toList()); + // could provide improved message where the exact cycles are made visible + throw new OperatorException( + "Cycle between in dependent resources. Involved resource names: " + cycleNames); + } + + return res; + } + + private static boolean allDependsOnsAlreadySelected(DependentResourceSpec dr, + Set alreadySelected, + Map nameToDR, + String alreadyPresentName) { + for (var name : dr.getDependsOn()) { + if (name.equals(alreadyPresentName)) + continue; + if (!alreadySelected.contains(nameToDR.get(name))) { + return false; + } + } + return true; + } + + private static Set getTopDependentResources( + List dependentResourceSpecs) { + return dependentResourceSpecs.stream().filter(r -> r.getDependsOn().isEmpty()) + .collect(Collectors.toSet()); + } + + private static Map> createDependOnIndex( + List dependentResourceSpecs) { + Map> dependsOnSpec = new HashMap<>(); + dependentResourceSpecs.forEach(dr -> dr.getDependsOn().forEach(name -> { + dependsOnSpec.computeIfAbsent((String) name, n -> new ArrayList<>()); + dependsOnSpec.get(name).add(dr); + })); + return dependsOnSpec; + } + +} From 7ca2a9f5beb79d1d97f5ac93d7b10187221fcd85 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 1 Jun 2022 11:25:21 +0200 Subject: [PATCH 05/35] untit test for workflow utils --- .../dependent/workflow/ManagedWorkflow.java | 1 + .../workflow/ManagedWorkflowUtils.java | 28 +++-- .../workflow/ManagedWorkflowUtilsTest.java | 105 ++++++++++++++++++ 3 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 623dc2abfe..97c52dac1e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -27,6 +27,7 @@ public class ManagedWorkflow

{ public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { + ManagedWorkflowUtils.checkForNameDuplication(dependentResourceSpecs); var orderedSpecs = ManagedWorkflowUtils.orderAndDetectCycles(dependentResourceSpecs); dependentResourceByName = orderedSpecs .stream().collect(Collectors.toMap(DependentResourceSpec::getName, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java index c522d9947c..18fc4f98ab 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java @@ -7,9 +7,9 @@ import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; @SuppressWarnings({"rawtypes", "unchecked"}) -public class ManagedWorkflowUtils { +class ManagedWorkflowUtils { - public void checkForNameDuplication(List dependentResourceSpecs) { + public static void checkForNameDuplication(List dependentResourceSpecs) { if (dependentResourceSpecs.size() <= 1) { return; } @@ -46,25 +46,29 @@ public static List orderAndDetectCycles( res.addAll(selectedLastIteration); alreadySelected.addAll(selectedLastIteration); Set newAdds = new HashSet<>(); - selectedLastIteration.forEach(dr -> dependOnIndex.get(dr.getName()).forEach(ndr -> { - if (allDependsOnsAlreadySelected(ndr, alreadySelected, nameToDR, dr.getName())) { - newAdds.add(ndr); - } - })); + selectedLastIteration.forEach(dr -> { + var dependsOn = dependOnIndex.get(dr.getName()); + if (dependsOn == null) + dependsOn = Collections.emptyList(); + dependsOn.forEach(ndr -> { + if (allDependsOnsAlreadySelected(ndr, alreadySelected, nameToDR, dr.getName())) { + newAdds.add(ndr); + } + }); + }); selectedLastIteration = newAdds; } + if (res.size() != dependentResourceSpecs.size()) { - var cycleNames = dependentResourceSpecs.stream().filter(dr -> !alreadySelected.contains(dr)) - .map(dr -> dr.getName()) - .collect(Collectors.toList()); // could provide improved message where the exact cycles are made visible throw new OperatorException( - "Cycle between in dependent resources. Involved resource names: " + cycleNames); + "Cycle(s) between dependent resources."); } - return res; } + + private static boolean allDependsOnsAlreadySelected(DependentResourceSpec dr, Set alreadySelected, Map nameToDR, diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java new file mode 100644 index 0000000000..3104e5df71 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java @@ -0,0 +1,105 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.stream.Collectors; + +import org.assertj.core.data.Index; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@SuppressWarnings("rawtypes") +class ManagedWorkflowUtilsTest { + + + public static final String NAME_2 = "name2"; + public static final String NAME_1 = "name1"; + + @Test + void trivialCasesNameDuplicates() { + ManagedWorkflowUtils.checkForNameDuplication(Collections.emptyList()); + ManagedWorkflowUtils.checkForNameDuplication(List.of(createDRS(NAME_1))); + ManagedWorkflowUtils.checkForNameDuplication(List.of(createDRS(NAME_1), createDRS(NAME_2))); + } + + @Test + void checkFindsDuplicates() { + Assertions.assertThrows(OperatorException.class, () -> ManagedWorkflowUtils + .checkForNameDuplication(List.of(createDRS(NAME_2), createDRS(NAME_2)))); + + Assertions.assertThrows(OperatorException.class, + () -> ManagedWorkflowUtils.checkForNameDuplication(List.of(createDRS(NAME_1), + createDRS(NAME_2), + createDRS(NAME_2)))); + } + + @Test + void orderingTrivialCases() { + assertThat(ManagedWorkflowUtils.orderAndDetectCycles(List.of(createDRS(NAME_1)))) + .map(DependentResourceSpec::getName).containsExactly(NAME_1); + + assertThat(ManagedWorkflowUtils + .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1)))) + .map(DependentResourceSpec::getName).containsExactly(NAME_1, NAME_2); + } + + @Test + void orderingDiamondShape() { + String NAME_3 = "name3"; + String NAME_4 = "name4"; + + var res = ManagedWorkflowUtils + .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1), + createDRS(NAME_3, NAME_1), createDRS(NAME_4, NAME_2, NAME_3))) + .stream().map(DependentResourceSpec::getName).collect(Collectors.toList()); + + assertThat(res) + .containsExactlyInAnyOrder(NAME_1, NAME_2, NAME_3, NAME_4) + .contains(NAME_1, Index.atIndex(0)) + .contains(NAME_4, Index.atIndex(3)); + } + + @Test + void detectsCyclesTrivialCases() { + String NAME_3 = "name3"; + Assertions.assertThrows(OperatorException.class, () -> ManagedWorkflowUtils + .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1, NAME_2)))); + Assertions.assertThrows(OperatorException.class, + () -> ManagedWorkflowUtils + .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1, NAME_3), + createDRS(NAME_3, NAME_2)))); + } + + @Test + void detectsCycleOnSubTree() { + String NAME_3 = "name3"; + String NAME_4 = "name4"; + Assertions.assertThrows(OperatorException.class, + () -> ManagedWorkflowUtils.orderAndDetectCycles(List.of(createDRS(NAME_1), + createDRS(NAME_2, NAME_1), + createDRS(NAME_3, NAME_1, NAME_4), + createDRS(NAME_4, NAME_3)))); + } + + private DependentResourceSpec createDRS(String name, String... dependOns) { + var drcMock = createDRS(name); + when(drcMock.getDependsOn()).thenReturn(new HashSet(Arrays.asList(dependOns))); + return drcMock; + } + + private DependentResourceSpec createDRS(String name) { + var drc = mock(DependentResourceSpec.class); + when(drc.getName()).thenReturn(name); + return drc; + } + +} From 6a91a5de62141792b1808821a079719d1e30ac4e Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 1 Jun 2022 12:41:35 +0200 Subject: [PATCH 06/35] workflow api integration --- .../operator/api/reconciler/Cleaner.java | 2 +- .../api/reconciler/DefaultContext.java | 8 +- ...efaultManagedDependentResourceContext.java | 97 +++++++++++++++++++ .../ManagedDependentResourceContext.java | 93 ++---------------- .../operator/processing/Controller.java | 83 ++++++++-------- .../dependent/workflow/ManagedWorkflow.java | 18 ++-- 6 files changed, 160 insertions(+), 141 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Cleaner.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Cleaner.java index e2e70a246d..2a53d5730b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Cleaner.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Cleaner.java @@ -5,7 +5,7 @@ public interface Cleaner

{ /** - * Note that this method turns on automatic finalizer usage. + * This method turns on automatic finalizer usage. * * The implementation should delete the associated component(s). This method is called when an * object is marked for deletion. After it's executed the custom resource finalizer is diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 0b6d90065b..9891d358d4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -6,6 +6,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.Controller; @@ -15,14 +16,14 @@ public class DefaultContext

implements Context

{ private final Controller

controller; private final P primaryResource; private final ControllerConfiguration

controllerConfiguration; - private final ManagedDependentResourceContext managedDependentResourceContext; + private final DefaultManagedDependentResourceContext defaultManagedDependentResourceContext; public DefaultContext(RetryInfo retryInfo, Controller

controller, P primaryResource) { this.retryInfo = retryInfo; this.controller = controller; this.primaryResource = primaryResource; this.controllerConfiguration = controller.getConfiguration(); - this.managedDependentResourceContext = new ManagedDependentResourceContext(); + this.defaultManagedDependentResourceContext = new DefaultManagedDependentResourceContext(); } @Override @@ -52,8 +53,9 @@ public ControllerConfiguration

getControllerConfiguration() { return controllerConfiguration; } + @Override public ManagedDependentResourceContext managedDependentResourceContext() { - return managedDependentResourceContext; + return defaultManagedDependentResourceContext; } public DefaultContext

setRetryInfo(RetryInfo retryInfo) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java new file mode 100644 index 0000000000..df8ec21561 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java @@ -0,0 +1,97 @@ +package io.javaoperatorsdk.operator.api.reconciler.dependent.managed; + +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; + +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowExecutionResult; + +/** + * Contextual information related to {@link DependentResource} either to retrieve the actual + * implementations to interact with them or to pass information between them and/or the reconciler + */ +@SuppressWarnings("rawtypes") +public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext { + + private WorkflowExecutionResult workflowExecutionResult; + private WorkflowCleanupResult workflowCleanupResult; + private final ConcurrentHashMap attributes = new ConcurrentHashMap(); + + /** + * Retrieve a contextual object, if it exists and is of the specified expected type, associated + * with the specified key. Contextual objects can be used to pass data between the reconciler and + * dependent resources and are scoped to the current reconciliation. + * + * @param key the key identifying which contextual object to retrieve + * @param expectedType the class representing the expected type of the contextual object + * @param the type of the expected contextual object + * @return an Optional containing the contextual object or {@link Optional#empty()} if no such + * object exists or doesn't match the expected type + */ + @Override + public Optional get(Object key, Class expectedType) { + return Optional.ofNullable(attributes.get(key)) + .filter(expectedType::isInstance) + .map(expectedType::cast); + } + + /** + * Associates the specified contextual value to the specified key. If the value is {@code null}, + * the semantics of this operation is defined as removing the mapping associated with the + * specified key. + * + * @param key the key identifying which contextual object to add or remove from the context + * @param value the value to add to the context or {@code null} to remove an existing entry + * associated with the specified key + * @return an Optional containing the previous value associated with the key or + * {@link Optional#empty()} if none existed + */ + @Override + @SuppressWarnings("unchecked") + public T put(Object key, T value) { + if (value == null) { + return (T) Optional.ofNullable(attributes.remove(key)); + } + return (T) Optional.ofNullable(attributes.put(key, value)); + } + + /** + * Retrieves the value associated with the key or fail with an exception if none exists. + * + * @param key the key identifying which contextual object to retrieve + * @param expectedType the expected type of the value to retrieve + * @param the type of the expected contextual object + * @return the contextual object value associated with the specified key + * @see #get(Object, Class) + */ + @Override + @SuppressWarnings("unused") + public T getMandatory(Object key, Class expectedType) { + return get(key, expectedType).orElseThrow(() -> new IllegalStateException( + "Mandatory attribute (key: " + key + ", type: " + expectedType.getName() + + ") is missing or not of the expected type")); + } + + public DefaultManagedDependentResourceContext setWorkflowExecutionResult( + WorkflowExecutionResult workflowExecutionResult) { + this.workflowExecutionResult = workflowExecutionResult; + return this; + } + + public DefaultManagedDependentResourceContext setWorkflowCleanupResult( + WorkflowCleanupResult workflowCleanupResult) { + this.workflowCleanupResult = workflowCleanupResult; + return this; + } + + @Override + public Optional getWorkflowExecutionResult() { + return Optional.ofNullable(workflowExecutionResult); + } + + @Override + public Optional getWorkflowCleanupResult() { + return Optional.ofNullable(workflowCleanupResult); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java index 0d0b4c1412..598f56b076 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java @@ -1,97 +1,20 @@ package io.javaoperatorsdk.operator.api.reconciler.dependent.managed; -import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowExecutionResult; -/** - * Contextual information related to {@link DependentResource} either to retrieve the actual - * implementations to interact with them or to pass information between them and/or the reconciler - */ -@SuppressWarnings("rawtypes") -public class ManagedDependentResourceContext { +public interface ManagedDependentResourceContext { + Optional get(Object key, Class expectedType); - private final Map reconcileResults = new ConcurrentHashMap<>(); - private final ConcurrentHashMap attributes = new ConcurrentHashMap(); - - /** - * Retrieve a contextual object, if it exists and is of the specified expected type, associated - * with the specified key. Contextual objects can be used to pass data between the reconciler and - * dependent resources and are scoped to the current reconciliation. - * - * @param key the key identifying which contextual object to retrieve - * @param expectedType the class representing the expected type of the contextual object - * @param the type of the expected contextual object - * @return an Optional containing the contextual object or {@link Optional#empty()} if no such - * object exists or doesn't match the expected type - */ - public Optional get(Object key, Class expectedType) { - return Optional.ofNullable(attributes.get(key)) - .filter(expectedType::isInstance) - .map(expectedType::cast); - } - - /** - * Associates the specified contextual value to the specified key. If the value is {@code null}, - * the semantics of this operation is defined as removing the mapping associated with the - * specified key. - * - * @param key the key identifying which contextual object to add or remove from the context - * @param value the value to add to the context or {@code null} to remove an existing entry - * associated with the specified key - * @return an Optional containing the previous value associated with the key or - * {@link Optional#empty()} if none existed - */ @SuppressWarnings("unchecked") - public Optional put(Object key, Object value) { - if (value == null) { - return Optional.ofNullable(attributes.remove(key)); - } - return Optional.ofNullable(attributes.put(key, value)); - } + T put(Object key, T value); - /** - * Retrieves the value associated with the key or fail with an exception if none exists. - * - * @param key the key identifying which contextual object to retrieve - * @param expectedType the expected type of the value to retrieve - * @param the type of the expected contextual object - * @return the contextual object value associated with the specified key - * @see #get(Object, Class) - */ @SuppressWarnings("unused") - public T getMandatory(Object key, Class expectedType) { - return get(key, expectedType).orElseThrow(() -> new IllegalStateException( - "Mandatory attribute (key: " + key + ", type: " + expectedType.getName() - + ") is missing or not of the expected type")); - } + T getMandatory(Object key, Class expectedType); - /** - * Retrieve the {@link ReconcileResult}, if it exists, associated with the - * {@link DependentResource} associated with the specified name - * - * @param name the name of the {@link DependentResource} for which we want to retrieve a - * {@link ReconcileResult} - * @return an Optional containing the reconcile result or {@link Optional#empty()} if no such - * result is available - */ - @SuppressWarnings({"rawtypes", "unused"}) - public Optional getReconcileResult(String name) { - return Optional.ofNullable(reconcileResults.get(name)); - } + Optional getWorkflowExecutionResult(); - /** - * Set the {@link ReconcileResult} for the specified {@link DependentResource} implementation. - * - * @param name the name of the {@link DependentResource} for which we want to set the - * {@link ReconcileResult} - * @param reconcileResult the reconcile result associated with the specified - * {@link DependentResource} - */ - public void setReconcileResult(String name, ReconcileResult reconcileResult) { - reconcileResults.put(name, reconcileResult); - } + Optional getWorkflowCleanupResult(); } 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 a64ef116e5..13cdd6904c 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 @@ -19,6 +19,7 @@ import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflow; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; @@ -55,6 +56,45 @@ public Controller(Reconciler

reconciler, new ManagedWorkflow<>(kubernetesClient, configuration.getDependentResources()); } + @Override + public UpdateControl

reconcile(P resource, Context

context) throws Exception { + return metrics.timeControllerExecution( + new ControllerExecution<>() { + @Override + public String name() { + return "reconcile"; + } + + @Override + public String controllerName() { + return configuration.getName(); + } + + @Override + public String successTypeName(UpdateControl

result) { + String successType = "resource"; + if (result.isUpdateStatus()) { + successType = "status"; + } + if (result.isUpdateResourceAndStatus()) { + successType = "both"; + } + return successType; + } + + @Override + public UpdateControl

execute() throws Exception { + initContextIfNeeded(resource, context); + if (!managedWorkflow.isEmptyWorkflow()) { + var res = managedWorkflow.reconcile(resource, context); + ((DefaultManagedDependentResourceContext) context).setWorkflowExecutionResult(res); + res.throwAggregateExceptionIfErrorsPresent(); + } + return reconciler.reconcile(resource, context); + } + }); + } + @Override public DeleteControl cleanup(P resource, Context

context) { try { @@ -79,8 +119,9 @@ public String successTypeName(DeleteControl deleteControl) { public DeleteControl execute() { initContextIfNeeded(resource, context); if (managedWorkflow.isCleaner()) { - // todo use result - managedWorkflow.cleanup(resource, context); + var res = managedWorkflow.cleanup(resource, context); + ((DefaultManagedDependentResourceContext) context).setWorkflowCleanupResult(res); + res.throwAggregateExceptionIfErrorsPresent(); } if (isCleaner) { return ((Cleaner

) reconciler).cleanup(resource, context); @@ -94,44 +135,6 @@ public DeleteControl execute() { } } - @Override - public UpdateControl

reconcile(P resource, Context

context) throws Exception { - return metrics.timeControllerExecution( - new ControllerExecution<>() { - @Override - public String name() { - return "reconcile"; - } - - @Override - public String controllerName() { - return configuration.getName(); - } - - @Override - public String successTypeName(UpdateControl

result) { - String successType = "resource"; - if (result.isUpdateStatus()) { - successType = "status"; - } - if (result.isUpdateResourceAndStatus()) { - successType = "both"; - } - return successType; - } - - @Override - public UpdateControl

execute() throws Exception { - initContextIfNeeded(resource, context); - if (!managedWorkflow.isEmptyWorkflow()) { - // todo use result - managedWorkflow.reconcile(resource, context); - } - return reconciler.reconcile(resource, context); - } - }); - } - private void initContextIfNeeded(P resource, Context

context) { if (contextInitializer) { ((ContextInitializer

) reconciler).initContext(resource, context); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 97c52dac1e..07341f8c55 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -57,30 +57,24 @@ private boolean checkIfCleaner() { return false; } - @SuppressWarnings("unchecked") private Workflow

toWorkFlow(KubernetesClient client, - List dependentResourceSpecs) { - var orderedSpecs = orderDependentsToBeAdded(dependentResourceSpecs); + List orderedResourceSpecs) { - var workflow = new WorkflowBuilder

(); - orderedSpecs.forEach(spec -> { + var w = new WorkflowBuilder

(); + w.withThrowExceptionFurther(false); + orderedResourceSpecs.forEach(spec -> { var drBuilder = - workflow.addDependent(dependentResourceByName.get(spec.getName())).dependsOn( + w.addDependent(dependentResourceByName.get(spec.getName())).dependsOn( (Set) spec.getDependsOn() .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); drBuilder.withDeletePostCondition(spec.getDeletePostCondition()); drBuilder.withReconcileCondition(spec.getReconcileCondition()); drBuilder.withReadyCondition(spec.getReadyCondition()); }); - return workflow.build(); + return w.build(); } - // todo - private List orderDependentsToBeAdded( - List dependentResourceSpecs) { - return dependentResourceSpecs; - } @SuppressWarnings({"rawtypes", "unchecked"}) private DependentResource createAndConfigureFrom(DependentResourceSpec spec, From 6916bb418538b0f6ad5d32fa42af16f4634da8cb Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 1 Jun 2022 13:03:15 +0200 Subject: [PATCH 07/35] fixes --- .../api/config/dependent/DependentResourceSpec.java | 4 ++++ .../io/javaoperatorsdk/operator/processing/Controller.java | 6 ++++-- .../dependent/workflow/WorkflowCleanupResult.java | 4 ++-- .../dependent/workflow/WorkflowExecutionResult.java | 4 ++-- 4 files changed, 12 insertions(+), 6 deletions(-) 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 ac79f36be7..6b0d985f8c 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 @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config.dependent; +import java.util.HashSet; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -67,6 +68,9 @@ public int hashCode() { } public Set getDependsOn() { + if (dependsOn == null) { + dependsOn = new HashSet<>(0); + } return dependsOn; } 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 13cdd6904c..2de09bb2fb 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 @@ -87,7 +87,8 @@ public UpdateControl

execute() throws Exception { initContextIfNeeded(resource, context); if (!managedWorkflow.isEmptyWorkflow()) { var res = managedWorkflow.reconcile(resource, context); - ((DefaultManagedDependentResourceContext) context).setWorkflowExecutionResult(res); + ((DefaultManagedDependentResourceContext) context.managedDependentResourceContext()) + .setWorkflowExecutionResult(res); res.throwAggregateExceptionIfErrorsPresent(); } return reconciler.reconcile(resource, context); @@ -120,7 +121,8 @@ public DeleteControl execute() { initContextIfNeeded(resource, context); if (managedWorkflow.isCleaner()) { var res = managedWorkflow.cleanup(resource, context); - ((DefaultManagedDependentResourceContext) context).setWorkflowCleanupResult(res); + ((DefaultManagedDependentResourceContext) context.managedDependentResourceContext()) + .setWorkflowCleanupResult(res); res.throwAggregateExceptionIfErrorsPresent(); } if (isCleaner) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupResult.java index c724376d78..f52774b9ac 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupResult.java @@ -45,8 +45,8 @@ public WorkflowCleanupResult setErroredDependents( return this; } - public boolean postConditionsNotMet() { - return !postConditionNotMetDependents.isEmpty(); + public boolean allPostConditionsMet() { + return postConditionNotMetDependents.isEmpty(); } public boolean erroredDependentsExists() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java index 87c23e422c..08baaffad0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java @@ -67,8 +67,8 @@ private AggregatedOperatorException createFinalException() { new ArrayList<>(erroredDependents.values())); } - public boolean notReadyDependentsExists() { - return !notReadyDependents.isEmpty(); + public boolean allDependentResourcesReady() { + return notReadyDependents.isEmpty(); } public boolean erroredDependentsExists() { From b45a0e3f790c759bd384a658d4ee96bda7a01de9 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 1 Jun 2022 16:57:29 +0200 Subject: [PATCH 08/35] it fix --- .../config/ControllerConfigurationOverrider.java | 13 +++++++++---- .../workflow/WorkflowReconcileExecutor.java | 8 +++++++- .../operator/processing/event/ResourceID.java | 9 +++++++++ .../DependentResourceCrossRefReconciler.java | 6 ++++-- .../operator/sample/MySQLSchemaReconciler.java | 5 +++-- .../sample/dependent/SecretDependentResource.java | 2 +- 6 files changed, 33 insertions(+), 10 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index d81a7fcb03..af5059e11b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -140,11 +140,10 @@ public ControllerConfiguration build() { // if the spec has a config and it's a KubernetesDependentResourceConfig, update the // namespaces if needed, otherwise, just return the existing spec final Optional maybeConfig = spec.getDependentResourceConfiguration(); - final Class drClass = drsEntry.getValue().getDependentResourceClass(); return maybeConfig.filter(KubernetesDependentResourceConfig.class::isInstance) .map(KubernetesDependentResourceConfig.class::cast) .filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured)) - .map(c -> updateSpec(drsEntry.getKey(), drClass, c)) + .map(c -> updateSpec(drsEntry.getKey(), spec, c)) .orElse(drsEntry.getValue()); }).collect(Collectors.toUnmodifiableList()); @@ -164,9 +163,15 @@ public ControllerConfiguration build() { } @SuppressWarnings({"rawtypes", "unchecked"}) - private DependentResourceSpec updateSpec(String name, Class drClass, + private DependentResourceSpec updateSpec(String name, DependentResourceSpec spec, KubernetesDependentResourceConfig c) { - return new DependentResourceSpec(drClass, c.setNamespaces(namespaces), name); + var res = new DependentResourceSpec(spec.getDependentResourceClass(), + c.setNamespaces(namespaces), name); + res.setReadyCondition(spec.getReadyCondition()); + res.setReconcileCondition(spec.getReconcileCondition()); + res.setDeletePostCondition(spec.getDeletePostCondition()); + res.setDependsOn(spec.getDependsOn()); + return res; } public static ControllerConfigurationOverrider override( 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 f32e3b83b8..df1d27c620 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 @@ -17,6 +17,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; +import io.javaoperatorsdk.operator.processing.event.ResourceID; @SuppressWarnings({"rawtypes", "unchecked"}) public class WorkflowReconcileExecutor

{ @@ -160,7 +161,12 @@ private NodeReconcileExecutor(DependentResourceNode dependentResourceNode) public void run() { try { DependentResource dependentResource = dependentResourceNode.getDependentResource(); - + if (log.isDebugEnabled()) { + log.debug( + "Reconciling {} for primary: {}", + dependentResourceNode, + ResourceID.fromResource(primary)); + } ReconcileResult reconcileResult = dependentResource.reconcile(primary, context); reconcileResults.put(dependentResource, reconcileResult); reconciled.add(dependentResourceNode); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceID.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceID.java index 612cac8e41..7baeab6a4b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceID.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceID.java @@ -61,9 +61,18 @@ public int hashCode() { @Override public String toString() { + return toString(name, namespace); + } + + public static String toString(HasMetadata resource) { + return toString(resource.getMetadata().getName(), resource.getMetadata().getNamespace()); + } + + private static String toString(String name, String namespace) { return "ResourceID{" + "name='" + name + '\'' + ", namespace='" + namespace + '\'' + '}'; } + } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java index d6eedc26dc..d211f31f28 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java @@ -13,8 +13,10 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; @ControllerConfiguration(dependents = { - @Dependent(type = DependentResourceCrossRefReconciler.SecretDependentResource.class), - @Dependent(type = DependentResourceCrossRefReconciler.ConfigMapDependentResource.class)}) + @Dependent(name = "secret", + type = DependentResourceCrossRefReconciler.SecretDependentResource.class), + @Dependent(type = DependentResourceCrossRefReconciler.ConfigMapDependentResource.class, + dependsOn = "secret")}) public class DependentResourceCrossRefReconciler implements Reconciler, ErrorStatusHandler { diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java index 08b65162fd..05187aea2b 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java @@ -21,8 +21,9 @@ @ControllerConfiguration( dependents = { - @Dependent(type = SecretDependentResource.class), - @Dependent(type = SchemaDependentResource.class, name = SchemaDependentResource.NAME) + @Dependent(type = SecretDependentResource.class, name = SecretDependentResource.NAME), + @Dependent(type = SchemaDependentResource.class, name = SchemaDependentResource.NAME, + dependsOn = SecretDependentResource.NAME) }) public class MySQLSchemaReconciler implements Reconciler, ErrorStatusHandler { diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java index 043b50a6cc..1aa2ad62e5 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java @@ -17,7 +17,7 @@ public class SecretDependentResource extends KubernetesDependentResource implements Creator, SecondaryToPrimaryMapper { - + public static final String NAME = "secret"; public static final String SECRET_SUFFIX = "-secret"; public static final String SECRET_FORMAT = "%s" + SECRET_SUFFIX; public static final String USERNAME_FORMAT = "%s-user"; From 6af09ff9edbc996f2b7b62693661774650861d46 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 1 Jun 2022 17:10:36 +0200 Subject: [PATCH 09/35] IT fix --- .../OrderedManagedDependentTestReconciler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/orderedmanageddependent/OrderedManagedDependentTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/orderedmanageddependent/OrderedManagedDependentTestReconciler.java index 8ca8f0651d..f7172ca44d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/orderedmanageddependent/OrderedManagedDependentTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/orderedmanageddependent/OrderedManagedDependentTestReconciler.java @@ -16,8 +16,8 @@ @ControllerConfiguration( namespaces = Constants.WATCH_CURRENT_NAMESPACE, dependents = { - @Dependent(type = ConfigMapDependentResource1.class), - @Dependent(type = ConfigMapDependentResource2.class) + @Dependent(type = ConfigMapDependentResource1.class, name = "cm1"), + @Dependent(type = ConfigMapDependentResource2.class, dependsOn = "cm1") }) public class OrderedManagedDependentTestReconciler implements Reconciler, From 88d81d0005b920af3f5b28064b06f98615c208a7 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 09:52:46 +0200 Subject: [PATCH 10/35] renaming --- .../DefaultManagedDependentResourceContext.java | 12 ++++++------ .../managed/ManagedDependentResourceContext.java | 4 ++-- .../dependent/workflow/ManagedWorkflow.java | 2 +- .../processing/dependent/workflow/Workflow.java | 2 +- .../workflow/WorkflowReconcileExecutor.java | 16 ++++++++-------- ...nResult.java => WorkflowReconcileResult.java} | 10 +++++----- 6 files changed, 23 insertions(+), 23 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/{WorkflowExecutionResult.java => WorkflowReconcileResult.java} (88%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java index df8ec21561..d6e716e2c6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java @@ -5,7 +5,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; -import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowExecutionResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult; /** * Contextual information related to {@link DependentResource} either to retrieve the actual @@ -14,7 +14,7 @@ @SuppressWarnings("rawtypes") public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext { - private WorkflowExecutionResult workflowExecutionResult; + private WorkflowReconcileResult workflowReconcileResult; private WorkflowCleanupResult workflowCleanupResult; private final ConcurrentHashMap attributes = new ConcurrentHashMap(); @@ -74,8 +74,8 @@ public T getMandatory(Object key, Class expectedType) { } public DefaultManagedDependentResourceContext setWorkflowExecutionResult( - WorkflowExecutionResult workflowExecutionResult) { - this.workflowExecutionResult = workflowExecutionResult; + WorkflowReconcileResult workflowReconcileResult) { + this.workflowReconcileResult = workflowReconcileResult; return this; } @@ -86,8 +86,8 @@ public DefaultManagedDependentResourceContext setWorkflowCleanupResult( } @Override - public Optional getWorkflowExecutionResult() { - return Optional.ofNullable(workflowExecutionResult); + public Optional getWorkflowExecutionResult() { + return Optional.ofNullable(workflowReconcileResult); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java index 598f56b076..3290a446a3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java @@ -3,7 +3,7 @@ import java.util.Optional; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; -import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowExecutionResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult; public interface ManagedDependentResourceContext { Optional get(Object key, Class expectedType); @@ -14,7 +14,7 @@ public interface ManagedDependentResourceContext { @SuppressWarnings("unused") T getMandatory(Object key, Class expectedType); - Optional getWorkflowExecutionResult(); + Optional getWorkflowExecutionResult(); Optional getWorkflowCleanupResult(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 07341f8c55..59a766d1c4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -39,7 +39,7 @@ public ManagedWorkflow(KubernetesClient client, isEmptyWorkflow = workflow.getDependentResources().isEmpty(); } - public WorkflowExecutionResult reconcile(P primary, Context

context) { + public WorkflowReconcileResult reconcile(P primary, Context

context) { return workflow.reconcile(primary, context); } 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 419be51cfb..bdd4e3cd7c 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 @@ -48,7 +48,7 @@ public Workflow(Set dependentResourceNodes, int globalPar this(dependentResourceNodes, Executors.newFixedThreadPool(globalParallelism), true); } - public WorkflowExecutionResult reconcile(P primary, Context

context) { + public WorkflowReconcileResult reconcile(P primary, Context

context) { WorkflowReconcileExecutor

workflowReconcileExecutor = new WorkflowReconcileExecutor<>(this, primary, context); var result = workflowReconcileExecutor.reconcile(); 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 df1d27c620..075993ebbe 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 @@ -50,7 +50,7 @@ public WorkflowReconcileExecutor(Workflow

workflow, P primary, Context

con this.workflow = workflow; } - public synchronized WorkflowExecutionResult reconcile() { + public synchronized WorkflowReconcileResult reconcile() { for (DependentResourceNode dependentResourceNode : workflow .getTopLevelDependentResources()) { handleReconcile(dependentResourceNode); @@ -286,18 +286,18 @@ private boolean hasErroredParent( .anyMatch(exceptionsDuringExecution::containsKey); } - private WorkflowExecutionResult createReconcileResult() { - WorkflowExecutionResult workflowExecutionResult = new WorkflowExecutionResult(); - workflowExecutionResult.setErroredDependents(exceptionsDuringExecution + private WorkflowReconcileResult createReconcileResult() { + WorkflowReconcileResult workflowReconcileResult = new WorkflowReconcileResult(); + workflowReconcileResult.setErroredDependents(exceptionsDuringExecution .entrySet().stream() .collect(Collectors.toMap(e -> e.getKey().getDependentResource(), Map.Entry::getValue))); - workflowExecutionResult.setNotReadyDependents(notReady.stream() + workflowReconcileResult.setNotReadyDependents(notReady.stream() .map(DependentResourceNode::getDependentResource) .collect(Collectors.toList())); - workflowExecutionResult.setReconciledDependents(reconciled.stream() + workflowReconcileResult.setReconciledDependents(reconciled.stream() .map(DependentResourceNode::getDependentResource).collect(Collectors.toList())); - workflowExecutionResult.setReconcileResults(reconcileResults); - return workflowExecutionResult; + workflowReconcileResult.setReconcileResults(reconcileResults); + return workflowReconcileResult; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java similarity index 88% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java index 08baaffad0..3c75873920 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowExecutionResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java @@ -9,7 +9,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; @SuppressWarnings("rawtypes") -public class WorkflowExecutionResult { +public class WorkflowReconcileResult { private List reconciledDependents; private List notReadyDependents; @@ -20,7 +20,7 @@ public Map getErroredDependents() { return erroredDependents; } - public WorkflowExecutionResult setErroredDependents( + public WorkflowReconcileResult setErroredDependents( Map erroredDependents) { this.erroredDependents = erroredDependents; return this; @@ -30,7 +30,7 @@ public List getReconciledDependents() { return reconciledDependents; } - public WorkflowExecutionResult setReconciledDependents( + public WorkflowReconcileResult setReconciledDependents( List reconciledDependents) { this.reconciledDependents = reconciledDependents; return this; @@ -40,7 +40,7 @@ public List getNotReadyDependents() { return notReadyDependents; } - public WorkflowExecutionResult setNotReadyDependents( + public WorkflowReconcileResult setNotReadyDependents( List notReadyDependents) { this.notReadyDependents = notReadyDependents; return this; @@ -50,7 +50,7 @@ public Map getReconcileResults() { return reconcileResults; } - public WorkflowExecutionResult setReconcileResults( + public WorkflowReconcileResult setReconcileResults( Map reconcileResults) { this.reconcileResults = reconcileResults; return this; From 9fb2ffb5b49587c3406d649f818c10481efb6026 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 11:47:43 +0200 Subject: [PATCH 11/35] refactors, tests --- .../dependent/workflow/ManagedWorkflow.java | 61 ++++--------------- ...Utils.java => ManagedWorkflowSupport.java} | 57 ++++++++++++++--- ...t.java => ManagedWorkflowSupportTest.java} | 42 +++++-------- .../workflow/ManagedWorkflowTest.java | 60 ++++++++++++++++++ .../workflow/ManagedWorkflowTestUtils.java | 20 ++++++ 5 files changed, 156 insertions(+), 84 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/{ManagedWorkflowUtils.java => ManagedWorkflowSupport.java} (56%) rename operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/{ManagedWorkflowUtilsTest.java => ManagedWorkflowSupportTest.java} (64%) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 59a766d1c4..f062ec903f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -2,20 +2,15 @@ import java.util.List; import java.util.Map; -import java.util.Set; import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; -import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; -import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; -import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; @SuppressWarnings("rawtypes") public class ManagedWorkflow

{ @@ -27,16 +22,21 @@ public class ManagedWorkflow

{ public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { - ManagedWorkflowUtils.checkForNameDuplication(dependentResourceSpecs); - var orderedSpecs = ManagedWorkflowUtils.orderAndDetectCycles(dependentResourceSpecs); - dependentResourceByName = orderedSpecs - .stream().collect(Collectors.toMap(DependentResourceSpec::getName, - spec -> createAndConfigureFrom(spec, client))); + this(client, dependentResourceSpecs, new ManagedWorkflowSupport<>()); + } + ManagedWorkflow(KubernetesClient client, + List dependentResourceSpecs, + ManagedWorkflowSupport

managedWorkflowSupport) { + managedWorkflowSupport.checkForNameDuplication(dependentResourceSpecs); + dependentResourceByName = dependentResourceSpecs + .stream().collect(Collectors.toMap(DependentResourceSpec::getName, + spec -> managedWorkflowSupport.createAndConfigureFrom(spec, client))); - workflow = toWorkFlow(client, orderedSpecs); + isEmptyWorkflow = dependentResourceSpecs.isEmpty(); + workflow = + managedWorkflowSupport.toWorkflow(client, dependentResourceSpecs, dependentResourceByName); isCleaner = checkIfCleaner(); - isEmptyWorkflow = workflow.getDependentResources().isEmpty(); } public WorkflowReconcileResult reconcile(P primary, Context

context) { @@ -47,7 +47,6 @@ public WorkflowCleanupResult cleanup(P primary, Context

context) { return workflow.cleanup(primary, context); } - private boolean checkIfCleaner() { for (var dr : workflow.getDependentResources()) { if (dr instanceof Deleter && !(dr instanceof GarbageCollected)) { @@ -57,42 +56,6 @@ private boolean checkIfCleaner() { return false; } - @SuppressWarnings("unchecked") - private Workflow

toWorkFlow(KubernetesClient client, - List orderedResourceSpecs) { - - var w = new WorkflowBuilder

(); - w.withThrowExceptionFurther(false); - orderedResourceSpecs.forEach(spec -> { - var drBuilder = - w.addDependent(dependentResourceByName.get(spec.getName())).dependsOn( - (Set) spec.getDependsOn() - .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); - drBuilder.withDeletePostCondition(spec.getDeletePostCondition()); - drBuilder.withReconcileCondition(spec.getReconcileCondition()); - drBuilder.withReadyCondition(spec.getReadyCondition()); - }); - return w.build(); - } - - - @SuppressWarnings({"rawtypes", "unchecked"}) - private DependentResource createAndConfigureFrom(DependentResourceSpec spec, - KubernetesClient client) { - final var dependentResource = - ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); - - if (dependentResource instanceof KubernetesClientAware) { - ((KubernetesClientAware) dependentResource).setKubernetesClient(client); - } - - if (dependentResource instanceof DependentResourceConfigurator) { - final var configurator = (DependentResourceConfigurator) dependentResource; - spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); - } - return dependentResource; - } - public boolean isCleaner() { return isCleaner; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java similarity index 56% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index 18fc4f98ab..97ccadf47a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -3,13 +3,20 @@ import java.util.*; import java.util.stream.Collectors; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; +import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; +import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; @SuppressWarnings({"rawtypes", "unchecked"}) -class ManagedWorkflowUtils { +class ManagedWorkflowSupport

{ - public static void checkForNameDuplication(List dependentResourceSpecs) { + public void checkForNameDuplication(List dependentResourceSpecs) { if (dependentResourceSpecs.size() <= 1) { return; } @@ -23,6 +30,42 @@ public static void checkForNameDuplication(List dependent } } + @SuppressWarnings("unchecked") + public Workflow

toWorkflow(KubernetesClient client, + List dependentResourceSpecs, + Map dependentResourceByName) { + var orderedResourceSpecs = orderAndDetectCycles(dependentResourceSpecs); + var w = new WorkflowBuilder

(); + w.withThrowExceptionFurther(false); + orderedResourceSpecs.forEach(spec -> { + var drBuilder = + w.addDependent(dependentResourceByName.get(spec.getName())).dependsOn( + (Set) spec.getDependsOn() + .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); + drBuilder.withDeletePostCondition(spec.getDeletePostCondition()); + drBuilder.withReconcileCondition(spec.getReconcileCondition()); + drBuilder.withReadyCondition(spec.getReadyCondition()); + }); + return w.build(); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + public DependentResource createAndConfigureFrom(DependentResourceSpec spec, + KubernetesClient client) { + final var dependentResource = + ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); + + if (dependentResource instanceof KubernetesClientAware) { + ((KubernetesClientAware) dependentResource).setKubernetesClient(client); + } + + if (dependentResource instanceof DependentResourceConfigurator) { + final var configurator = (DependentResourceConfigurator) dependentResource; + spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); + } + return dependentResource; + } + /** * Throws also exception if there is a cycle in the dependencies. * @@ -30,7 +73,7 @@ public static void checkForNameDuplication(List dependent * @return top-bottom ordered resources that can be added safely to workflow * */ - public static List orderAndDetectCycles( + public List orderAndDetectCycles( List dependentResourceSpecs) { List res = new ArrayList<>(dependentResourceSpecs.size()); @@ -67,9 +110,7 @@ public static List orderAndDetectCycles( return res; } - - - private static boolean allDependsOnsAlreadySelected(DependentResourceSpec dr, + private boolean allDependsOnsAlreadySelected(DependentResourceSpec dr, Set alreadySelected, Map nameToDR, String alreadyPresentName) { @@ -83,13 +124,13 @@ private static boolean allDependsOnsAlreadySelected(DependentResourceSpec dr, return true; } - private static Set getTopDependentResources( + private Set getTopDependentResources( List dependentResourceSpecs) { return dependentResourceSpecs.stream().filter(r -> r.getDependsOn().isEmpty()) .collect(Collectors.toSet()); } - private static Map> createDependOnIndex( + private Map> createDependOnIndex( List dependentResourceSpecs) { Map> dependsOnSpec = new HashMap<>(); dependentResourceSpecs.forEach(dr -> dr.getDependsOn().forEach(name -> { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java similarity index 64% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index 3104e5df71..19c79e6485 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -1,8 +1,6 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.stream.Collectors; @@ -13,41 +11,42 @@ import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import static io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowTestUtils.createDRS; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; @SuppressWarnings("rawtypes") -class ManagedWorkflowUtilsTest { +class ManagedWorkflowSupportTest { public static final String NAME_2 = "name2"; public static final String NAME_1 = "name1"; + ManagedWorkflowSupport managedWorkflowSupport = new ManagedWorkflowSupport(); + @Test void trivialCasesNameDuplicates() { - ManagedWorkflowUtils.checkForNameDuplication(Collections.emptyList()); - ManagedWorkflowUtils.checkForNameDuplication(List.of(createDRS(NAME_1))); - ManagedWorkflowUtils.checkForNameDuplication(List.of(createDRS(NAME_1), createDRS(NAME_2))); + managedWorkflowSupport.checkForNameDuplication(Collections.emptyList()); + managedWorkflowSupport.checkForNameDuplication(List.of(createDRS(NAME_1))); + managedWorkflowSupport.checkForNameDuplication(List.of(createDRS(NAME_1), createDRS(NAME_2))); } @Test void checkFindsDuplicates() { - Assertions.assertThrows(OperatorException.class, () -> ManagedWorkflowUtils + Assertions.assertThrows(OperatorException.class, () -> managedWorkflowSupport .checkForNameDuplication(List.of(createDRS(NAME_2), createDRS(NAME_2)))); Assertions.assertThrows(OperatorException.class, - () -> ManagedWorkflowUtils.checkForNameDuplication(List.of(createDRS(NAME_1), + () -> managedWorkflowSupport.checkForNameDuplication(List.of(createDRS(NAME_1), createDRS(NAME_2), createDRS(NAME_2)))); } @Test void orderingTrivialCases() { - assertThat(ManagedWorkflowUtils.orderAndDetectCycles(List.of(createDRS(NAME_1)))) + assertThat(managedWorkflowSupport.orderAndDetectCycles(List.of(createDRS(NAME_1)))) .map(DependentResourceSpec::getName).containsExactly(NAME_1); - assertThat(ManagedWorkflowUtils + assertThat(managedWorkflowSupport .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1)))) .map(DependentResourceSpec::getName).containsExactly(NAME_1, NAME_2); } @@ -57,7 +56,7 @@ void orderingDiamondShape() { String NAME_3 = "name3"; String NAME_4 = "name4"; - var res = ManagedWorkflowUtils + var res = managedWorkflowSupport .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1), createDRS(NAME_3, NAME_1), createDRS(NAME_4, NAME_2, NAME_3))) .stream().map(DependentResourceSpec::getName).collect(Collectors.toList()); @@ -71,10 +70,10 @@ void orderingDiamondShape() { @Test void detectsCyclesTrivialCases() { String NAME_3 = "name3"; - Assertions.assertThrows(OperatorException.class, () -> ManagedWorkflowUtils + Assertions.assertThrows(OperatorException.class, () -> managedWorkflowSupport .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1, NAME_2)))); Assertions.assertThrows(OperatorException.class, - () -> ManagedWorkflowUtils + () -> managedWorkflowSupport .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1, NAME_3), createDRS(NAME_3, NAME_2)))); } @@ -84,22 +83,11 @@ void detectsCycleOnSubTree() { String NAME_3 = "name3"; String NAME_4 = "name4"; Assertions.assertThrows(OperatorException.class, - () -> ManagedWorkflowUtils.orderAndDetectCycles(List.of(createDRS(NAME_1), + () -> managedWorkflowSupport.orderAndDetectCycles(List.of(createDRS(NAME_1), createDRS(NAME_2, NAME_1), createDRS(NAME_3, NAME_1, NAME_4), createDRS(NAME_4, NAME_3)))); } - private DependentResourceSpec createDRS(String name, String... dependOns) { - var drcMock = createDRS(name); - when(drcMock.getDependsOn()).thenReturn(new HashSet(Arrays.asList(dependOns))); - return drcMock; - } - - private DependentResourceSpec createDRS(String name) { - var drc = mock(DependentResourceSpec.class); - when(drc.getName()).thenReturn(name); - return drc; - } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java new file mode 100644 index 0000000000..2ed8b40185 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java @@ -0,0 +1,60 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.List; +import java.util.Set; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; + +import static io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowTestUtils.createDRS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@SuppressWarnings({"rawtypes", "unchecked"}) +class ManagedWorkflowTest { + + ManagedWorkflowSupport managedWorkflowSupportMock = mock(ManagedWorkflowSupport.class); + KubernetesClient kubernetesClientMock = mock(KubernetesClient.class); + + @Test + void checksIfWorkflowEmpty() { + var mockWorkflow = mock(Workflow.class); + when(managedWorkflowSupportMock.toWorkflow(any(), any(), any())).thenReturn(mockWorkflow); + when(managedWorkflowSupportMock.createAndConfigureFrom(any(), any())) + .thenReturn(mock(DependentResource.class)); + assertThat(managedWorkflow().isEmptyWorkflow()).isTrue(); + + when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class))); + assertThat(managedWorkflow(createDRS("name1")).isEmptyWorkflow()).isFalse(); + } + + @Test + void isCleanerIfAtLeastOneDRIsDeleterAndNoGC() { + var mockWorkflow = mock(Workflow.class); + when(managedWorkflowSupportMock.toWorkflow(any(), any(), any())).thenReturn(mockWorkflow); + when(managedWorkflowSupportMock.createAndConfigureFrom(any(), any())) + .thenReturn(mock(DependentResource.class)); + when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class))); + + assertThat(managedWorkflow(createDRS("name1")).isCleaner()).isFalse(); + + when(mockWorkflow.getDependentResources()).thenReturn( + Set.of(mock(DependentResource.class, withSettings().extraInterfaces(Deleter.class)))); + assertThat(managedWorkflow(createDRS("name1")).isCleaner()).isTrue(); + + when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class, + withSettings().extraInterfaces(Deleter.class, GarbageCollected.class)))); + assertThat(managedWorkflow(createDRS("name1")).isCleaner()).isFalse(); + } + + ManagedWorkflow managedWorkflow(DependentResourceSpec... specs) { + return new ManagedWorkflow(kubernetesClientMock, List.of(specs), managedWorkflowSupportMock); + } + +} 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 new file mode 100644 index 0000000000..0d70ef3971 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTestUtils.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.Arrays; +import java.util.HashSet; + +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@SuppressWarnings("rawtypes") +public class ManagedWorkflowTestUtils { + + public static DependentResourceSpec createDRS(String name, String... dependOns) { + var drcMock = mock(DependentResourceSpec.class); + when(drcMock.getName()).thenReturn(name); + when(drcMock.getDependsOn()).thenReturn(new HashSet(Arrays.asList(dependOns))); + return drcMock; + } +} From 6e6d47afd85bd4a489ebad786b3c04ff183430e4 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 12:00:41 +0200 Subject: [PATCH 12/35] wip --- .../dependent/workflow/ManagedWorkflowTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java index 2ed8b40185..b953640d8f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java @@ -19,6 +19,8 @@ @SuppressWarnings({"rawtypes", "unchecked"}) class ManagedWorkflowTest { + public static final String NAME = "name"; + ManagedWorkflowSupport managedWorkflowSupportMock = mock(ManagedWorkflowSupport.class); KubernetesClient kubernetesClientMock = mock(KubernetesClient.class); @@ -31,7 +33,7 @@ void checksIfWorkflowEmpty() { assertThat(managedWorkflow().isEmptyWorkflow()).isTrue(); when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class))); - assertThat(managedWorkflow(createDRS("name1")).isEmptyWorkflow()).isFalse(); + assertThat(managedWorkflow(createDRS(NAME)).isEmptyWorkflow()).isFalse(); } @Test @@ -42,15 +44,15 @@ void isCleanerIfAtLeastOneDRIsDeleterAndNoGC() { .thenReturn(mock(DependentResource.class)); when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class))); - assertThat(managedWorkflow(createDRS("name1")).isCleaner()).isFalse(); + assertThat(managedWorkflow(createDRS(NAME)).isCleaner()).isFalse(); when(mockWorkflow.getDependentResources()).thenReturn( Set.of(mock(DependentResource.class, withSettings().extraInterfaces(Deleter.class)))); - assertThat(managedWorkflow(createDRS("name1")).isCleaner()).isTrue(); + assertThat(managedWorkflow(createDRS(NAME)).isCleaner()).isTrue(); when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class, withSettings().extraInterfaces(Deleter.class, GarbageCollected.class)))); - assertThat(managedWorkflow(createDRS("name1")).isCleaner()).isFalse(); + assertThat(managedWorkflow(createDRS(NAME)).isCleaner()).isFalse(); } ManagedWorkflow managedWorkflow(DependentResourceSpec... specs) { From 8e08769f5c4e6faa08aedb6cffe67adfdf1665f4 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 14:05:22 +0200 Subject: [PATCH 13/35] additional unit tests --- .../workflow/DependentResourceNode.java | 11 +- .../dependent/workflow/ManagedWorkflow.java | 2 +- .../workflow/ManagedWorkflowSupport.java | 11 +- .../workflow/builder/WorkflowBuilder.java | 2 +- .../operator/api/config/UtilsTest.java | 3 +- .../dependent/EmptyTestDependentResource.java | 31 ++++ .../workflow/ManagedWorkflowSupportTest.java | 32 ++++- .../workflow/ManagedWorkflowTest.java | 4 +- .../workflow/ManagedWorkflowTestUtils.java | 3 + .../workflow/WorkflowCleanupExecutorTest.java | 37 ++--- .../WorkflowReconcileExecutorTest.java | 135 +++++++++--------- .../dependent/workflow/WorkflowTest.java | 13 +- .../WebPageDependentsWorkflowReconciler.java | 8 +- 13 files changed, 176 insertions(+), 116 deletions(-) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java 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 973afd4ff6..99d82a7885 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 @@ -3,7 +3,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; @@ -58,13 +57,9 @@ public void addDependsOnRelation(DependentResourceNode node) { @Override public String toString() { - return "{" - + parents.stream().map(p -> p.dependentResource.toString()) - .collect(Collectors.joining(", ", "[", "]->")) - + "(" + dependentResource + ")" - + dependsOn.stream().map(d -> d.dependentResource.toString()) - .collect(Collectors.joining(", ", "->[", "]")) - + '}'; + return "DependentResourceNode{" + + "dependentResource=" + dependentResource + + '}'; } public DependentResourceNode setReconcileCondition( diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index f062ec903f..3b045417cf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -35,7 +35,7 @@ public ManagedWorkflow(KubernetesClient client, isEmptyWorkflow = dependentResourceSpecs.isEmpty(); workflow = - managedWorkflowSupport.toWorkflow(client, dependentResourceSpecs, dependentResourceByName); + managedWorkflowSupport.createWorkflow(dependentResourceSpecs, dependentResourceByName); isCleaner = checkIfCleaner(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index 97ccadf47a..a0a083702d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -31,20 +31,19 @@ public void checkForNameDuplication(List dependentResourc } @SuppressWarnings("unchecked") - public Workflow

toWorkflow(KubernetesClient client, - List dependentResourceSpecs, + public Workflow

createWorkflow(List dependentResourceSpecs, Map dependentResourceByName) { var orderedResourceSpecs = orderAndDetectCycles(dependentResourceSpecs); var w = new WorkflowBuilder

(); w.withThrowExceptionFurther(false); orderedResourceSpecs.forEach(spec -> { var drBuilder = - w.addDependent(dependentResourceByName.get(spec.getName())).dependsOn( + w.addDependentResource(dependentResourceByName.get(spec.getName())).dependsOn( (Set) spec.getDependsOn() .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); - drBuilder.withDeletePostCondition(spec.getDeletePostCondition()); - drBuilder.withReconcileCondition(spec.getReconcileCondition()); - drBuilder.withReadyCondition(spec.getReadyCondition()); + drBuilder.withDeletePostCondition(spec.getDeletePostCondition()) + .withReconcileCondition(spec.getReconcileCondition()) + .withReadyCondition(spec.getReadyCondition()); }); return w.build(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/WorkflowBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/WorkflowBuilder.java index 270dbd0261..14d1b96f76 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/WorkflowBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/WorkflowBuilder.java @@ -18,7 +18,7 @@ public class WorkflowBuilder

{ private final Set> dependentResourceNodes = new HashSet<>(); private boolean throwExceptionAutomatically = THROW_EXCEPTION_AUTOMATICALLY_DEFAULT; - public DependentBuilder

addDependent(DependentResource dependentResource) { + public DependentBuilder

addDependentResource(DependentResource dependentResource) { DependentResourceNode node = new DependentResourceNode<>(dependentResource); dependentResourceNodes.add(node); return new DependentBuilder<>(this, node); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java index 05ab4d1258..c9346134a3 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java @@ -8,6 +8,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; +import io.javaoperatorsdk.operator.processing.dependent.EmptyTestDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -84,7 +85,7 @@ void getsFirstTypeArgumentFromExtendedClass() { @Test void getsFirstTypeArgumentFromInterface() { - assertThat(Utils.getFirstTypeArgumentFromInterface(TestDependentResource.class)) + assertThat(Utils.getFirstTypeArgumentFromInterface(EmptyTestDependentResource.class)) .isEqualTo(Deployment.class); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java new file mode 100644 index 0000000000..aa75849051 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/EmptyTestDependentResource.java @@ -0,0 +1,31 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import java.util.Optional; + +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +public class EmptyTestDependentResource + implements DependentResource { + + @Override + public ReconcileResult reconcile(TestCustomResource primary, + Context context) { + return null; + } + + @Override + public Optional getSecondaryResource(TestCustomResource primaryResource) { + return Optional.empty(); + } + + @Override + public Class resourceType() { + return Deployment.class; + } + +} + diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index 19c79e6485..5537cf2150 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -8,18 +8,22 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import static io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowTestUtils.createDRS; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; @SuppressWarnings("rawtypes") class ManagedWorkflowSupportTest { - - public static final String NAME_2 = "name2"; public static final String NAME_1 = "name1"; + public static final String NAME_2 = "name2"; + public static final String NAME_3 = "name3"; + public static final String NAME_4 = "name4"; ManagedWorkflowSupport managedWorkflowSupport = new ManagedWorkflowSupport(); @@ -80,8 +84,7 @@ void detectsCyclesTrivialCases() { @Test void detectsCycleOnSubTree() { - String NAME_3 = "name3"; - String NAME_4 = "name4"; + Assertions.assertThrows(OperatorException.class, () -> managedWorkflowSupport.orderAndDetectCycles(List.of(createDRS(NAME_1), createDRS(NAME_2, NAME_1), @@ -89,5 +92,26 @@ void detectsCycleOnSubTree() { createDRS(NAME_4, NAME_3)))); } + @Test + void createsWorkflow() { + var specs = List.of(createDRS(NAME_1), + createDRS(NAME_2, NAME_1), + createDRS(NAME_3, NAME_1), + createDRS(NAME_4, NAME_3, NAME_2)); + + var drByName = specs + .stream().collect(Collectors.toMap(DependentResourceSpec::getName, + spec -> managedWorkflowSupport.createAndConfigureFrom(spec, + mock(KubernetesClient.class)))); + + var workflow = managedWorkflowSupport.createWorkflow(specs, drByName); + + assertThat(workflow.getDependentResources()).containsExactlyInAnyOrder(drByName.values() + .toArray(new DependentResource[0])); + assertThat(workflow.getTopLevelDependentResources()) + .map(DependentResourceNode::getDependentResource).containsExactly(drByName.get(NAME_1)); + assertThat(workflow.getBottomLevelResource()).map(DependentResourceNode::getDependentResource) + .containsExactly(drByName.get(NAME_4)); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java index b953640d8f..2deb1dbeb8 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java @@ -27,7 +27,7 @@ class ManagedWorkflowTest { @Test void checksIfWorkflowEmpty() { var mockWorkflow = mock(Workflow.class); - when(managedWorkflowSupportMock.toWorkflow(any(), any(), any())).thenReturn(mockWorkflow); + when(managedWorkflowSupportMock.createWorkflow(any(), any())).thenReturn(mockWorkflow); when(managedWorkflowSupportMock.createAndConfigureFrom(any(), any())) .thenReturn(mock(DependentResource.class)); assertThat(managedWorkflow().isEmptyWorkflow()).isTrue(); @@ -39,7 +39,7 @@ void checksIfWorkflowEmpty() { @Test void isCleanerIfAtLeastOneDRIsDeleterAndNoGC() { var mockWorkflow = mock(Workflow.class); - when(managedWorkflowSupportMock.toWorkflow(any(), any(), any())).thenReturn(mockWorkflow); + when(managedWorkflowSupportMock.createWorkflow(any(), any())).thenReturn(mockWorkflow); when(managedWorkflowSupportMock.createAndConfigureFrom(any(), any())) .thenReturn(mock(DependentResource.class)); when(mockWorkflow.getDependentResources()).thenReturn(Set.of(mock(DependentResource.class))); 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 0d70ef3971..cca36a2fe3 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 @@ -4,6 +4,7 @@ import java.util.HashSet; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.processing.dependent.EmptyTestDependentResource; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -13,8 +14,10 @@ public class ManagedWorkflowTestUtils { public static DependentResourceSpec createDRS(String name, String... dependOns) { var drcMock = mock(DependentResourceSpec.class); + when(drcMock.getDependentResourceClass()).thenReturn(EmptyTestDependentResource.class); when(drcMock.getName()).thenReturn(name); when(drcMock.getDependsOn()).thenReturn(new HashSet(Arrays.asList(dependOns))); return drcMock; } + } 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 2488059beb..b819456a28 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 @@ -19,10 +19,10 @@ class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest { @Test void cleanUpDiamondWorkflow() { var workflow = new WorkflowBuilder() - .addDependent(dd1).build() - .addDependent(dr1).dependsOn(dd1).build() - .addDependent(dd2).dependsOn(dd1).build() - .addDependent(dd3).dependsOn(dr1, dd2).build() + .addDependentResource(dd1).build() + .addDependentResource(dr1).dependsOn(dd1).build() + .addDependentResource(dd2).dependsOn(dd1).build() + .addDependentResource(dd3).dependsOn(dr1, dd2).build() .build(); var res = workflow.cleanup(new TestCustomResource(), null); @@ -38,10 +38,10 @@ void cleanUpDiamondWorkflow() { @Test void dontDeleteIfDependentErrored() { var workflow = new WorkflowBuilder() - .addDependent(dd1).build() - .addDependent(dd2).dependsOn(dd1).build() - .addDependent(dd3).dependsOn(dd2).build() - .addDependent(errorDD).dependsOn(dd2).build() + .addDependentResource(dd1).build() + .addDependentResource(dd2).dependsOn(dd1).build() + .addDependentResource(dd3).dependsOn(dd2).build() + .addDependentResource(errorDD).dependsOn(dd2).build() .withThrowExceptionFurther(false) .build(); @@ -60,8 +60,9 @@ void dontDeleteIfDependentErrored() { @Test void cleanupConditionTrivialCase() { var workflow = new WorkflowBuilder() - .addDependent(dd1).build() - .addDependent(dd2).dependsOn(dd1).withDeletePostCondition(noMetDeletePostCondition).build() + .addDependentResource(dd1).build() + .addDependentResource(dd2).dependsOn(dd1).withDeletePostCondition(noMetDeletePostCondition) + .build() .build(); var res = workflow.cleanup(new TestCustomResource(), null); @@ -75,8 +76,9 @@ void cleanupConditionTrivialCase() { @Test void cleanupConditionMet() { var workflow = new WorkflowBuilder() - .addDependent(dd1).build() - .addDependent(dd2).dependsOn(dd1).withDeletePostCondition(metDeletePostCondition).build() + .addDependentResource(dd1).build() + .addDependentResource(dd2).dependsOn(dd1).withDeletePostCondition(metDeletePostCondition) + .build() .build(); var res = workflow.cleanup(new TestCustomResource(), null); @@ -93,10 +95,11 @@ void cleanupConditionDiamondWorkflow() { TestDeleterDependent dd4 = new TestDeleterDependent("DR_DELETER_4"); var workflow = new WorkflowBuilder() - .addDependent(dd1).build() - .addDependent(dd2).dependsOn(dd1).build() - .addDependent(dd3).dependsOn(dd1).withDeletePostCondition(noMetDeletePostCondition).build() - .addDependent(dd4).dependsOn(dd2, dd3).build() + .addDependentResource(dd1).build() + .addDependentResource(dd2).dependsOn(dd1).build() + .addDependentResource(dd3).dependsOn(dd1).withDeletePostCondition(noMetDeletePostCondition) + .build() + .addDependentResource(dd4).dependsOn(dd2, dd3).build() .build(); var res = workflow.cleanup(new TestCustomResource(), null); @@ -116,7 +119,7 @@ void cleanupConditionDiamondWorkflow() { void dontDeleteIfGarbageCollected() { GarbageCollectedDeleter gcDel = new GarbageCollectedDeleter("GC_DELETER"); var workflow = new WorkflowBuilder() - .addDependent(gcDel).build() + .addDependentResource(gcDel).build() .build(); var res = workflow.cleanup(new TestCustomResource(), null); 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 b0d210b07e..ecf562890d 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 @@ -28,8 +28,8 @@ class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { @Test void reconcileTopLevelResources() { var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -42,8 +42,8 @@ void reconcileTopLevelResources() { @Test void reconciliationWithSimpleDependsOn() { var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -60,9 +60,9 @@ void reconciliationWithTwoTheDependsOns() { TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() - .addDependent(dr3).dependsOn(dr1).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() + .addDependentResource(dr3).dependsOn(dr1).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -81,10 +81,10 @@ void diamondShareWorkflowReconcile() { TestDependent dr4 = new TestDependent("DR_4"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() - .addDependent(dr3).dependsOn(dr1).build() - .addDependent(dr4).dependsOn(dr3).dependsOn(dr2).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() + .addDependentResource(dr3).dependsOn(dr1).build() + .addDependentResource(dr4).dependsOn(dr3).dependsOn(dr2).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -103,7 +103,7 @@ void diamondShareWorkflowReconcile() { @Test void exceptionHandlingSimpleCases() { var workflow = new WorkflowBuilder() - .addDependent(drError).build() + .addDependentResource(drError).build() .withThrowExceptionFurther(false) .build(); @@ -121,9 +121,9 @@ void exceptionHandlingSimpleCases() { @Test void dependentsOnErroredResourceNotReconciled() { var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(drError).dependsOn(dr1).build() - .addDependent(dr2).dependsOn(drError).build() + .addDependentResource(dr1).build() + .addDependentResource(drError).dependsOn(dr1).build() + .addDependentResource(dr2).dependsOn(drError).build() .withThrowExceptionFurther(false) .build(); @@ -142,10 +142,10 @@ void oneBranchErrorsOtherCompletes() { TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(drError).dependsOn(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() - .addDependent(dr3).dependsOn(dr2).build() + .addDependentResource(dr1).build() + .addDependentResource(drError).dependsOn(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() + .addDependentResource(dr3).dependsOn(dr2).build() .withThrowExceptionFurther(false) .build(); @@ -162,9 +162,9 @@ void oneBranchErrorsOtherCompletes() { @Test void onlyOneDependsOnErroredResourceNotReconciled() { var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(drError).build() - .addDependent(dr2).dependsOn(drError, dr1).build() + .addDependentResource(dr1).build() + .addDependentResource(drError).build() + .addDependentResource(dr2).dependsOn(drError, dr1).build() .withThrowExceptionFurther(false) .build(); @@ -181,9 +181,9 @@ void onlyOneDependsOnErroredResourceNotReconciled() { @Test void simpleReconcileCondition() { var workflow = new WorkflowBuilder() - .addDependent(dr1).withReconcileCondition(not_met_reconcile_condition).build() - .addDependent(dr2).withReconcileCondition(met_reconcile_condition).build() - .addDependent(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(dr1).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(dr2).withReconcileCondition(met_reconcile_condition).build() + .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -198,9 +198,10 @@ void simpleReconcileCondition() { @Test void triangleOnceConditionNotMet() { var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() - .addDependent(drDeleter).withReconcileCondition(not_met_reconcile_condition).dependsOn(dr1) + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() + .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition) + .dependsOn(dr1) .build() .build(); @@ -217,12 +218,14 @@ void reconcileConditionTransitiveDelete() { TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).withReconcileCondition(not_met_reconcile_condition) + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1) + .withReconcileCondition(not_met_reconcile_condition) .build() - .addDependent(drDeleter).dependsOn(dr2).withReconcileCondition(met_reconcile_condition) + .addDependentResource(drDeleter).dependsOn(dr2) + .withReconcileCondition(met_reconcile_condition) .build() - .addDependent(drDeleter2).dependsOn(drDeleter) + .addDependentResource(drDeleter2).dependsOn(drDeleter) .withReconcileCondition(met_reconcile_condition).build() .build(); @@ -243,9 +246,9 @@ void reconcileConditionAlsoErrorDependsOn() { TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); var workflow = new WorkflowBuilder() - .addDependent(drError).build() - .addDependent(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() - .addDependent(drDeleter2).dependsOn(drError, drDeleter) + .addDependentResource(drError).build() + .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter2).dependsOn(drError, drDeleter) .withReconcileCondition(met_reconcile_condition) .build() .withThrowExceptionFurther(false) @@ -267,9 +270,9 @@ void reconcileConditionAlsoErrorDependsOn() { @Test void oneDependsOnConditionNotMet() { var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).withReconcileCondition(not_met_reconcile_condition).build() - .addDependent(drDeleter).dependsOn(dr1, dr2).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter).dependsOn(dr1, dr2).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -286,10 +289,11 @@ void oneDependsOnConditionNotMet() { void deletedIfReconcileConditionNotMet() { TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(drDeleter).dependsOn(dr1).withReconcileCondition(not_met_reconcile_condition) + .addDependentResource(dr1).build() + .addDependentResource(drDeleter).dependsOn(dr1) + .withReconcileCondition(not_met_reconcile_condition) .build() - .addDependent(drDeleter2).dependsOn(dr1, drDeleter).build() + .addDependentResource(drDeleter2).dependsOn(dr1, drDeleter).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -310,12 +314,13 @@ void deleteDoneInReverseOrder() { TestDeleterDependent drDeleter4 = new TestDeleterDependent("DR_DELETER_4"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(drDeleter).withReconcileCondition(not_met_reconcile_condition).dependsOn(dr1) + .addDependentResource(dr1).build() + .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition) + .dependsOn(dr1) .build() - .addDependent(drDeleter2).dependsOn(drDeleter).build() - .addDependent(drDeleter3).dependsOn(drDeleter).build() - .addDependent(drDeleter4).dependsOn(drDeleter3).build() + .addDependentResource(drDeleter2).dependsOn(drDeleter).build() + .addDependentResource(drDeleter3).dependsOn(drDeleter).build() + .addDependentResource(drDeleter4).dependsOn(drDeleter3).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -337,11 +342,11 @@ void diamondDeleteWithPostConditionInMiddle() { TestDeleterDependent drDeleter4 = new TestDeleterDependent("DR_DELETER_4"); var workflow = new WorkflowBuilder() - .addDependent(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() - .addDependent(drDeleter2).dependsOn(drDeleter).build() - .addDependent(drDeleter3).dependsOn(drDeleter) + .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter2).dependsOn(drDeleter).build() + .addDependentResource(drDeleter3).dependsOn(drDeleter) .withDeletePostCondition(noMetDeletePostCondition).build() - .addDependent(drDeleter4).dependsOn(drDeleter3, drDeleter2).build() + .addDependentResource(drDeleter4).dependsOn(drDeleter3, drDeleter2).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -361,10 +366,10 @@ void diamondDeleteErrorInMiddle() { TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3"); var workflow = new WorkflowBuilder() - .addDependent(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() - .addDependent(drDeleter2).dependsOn(drDeleter).build() - .addDependent(errorDD).dependsOn(drDeleter).build() - .addDependent(drDeleter3).dependsOn(errorDD, drDeleter2).build() + .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter2).dependsOn(drDeleter).build() + .addDependentResource(errorDD).dependsOn(drDeleter).build() + .addDependentResource(drDeleter3).dependsOn(errorDD, drDeleter2).build() .withThrowExceptionFurther(false) .build(); @@ -382,8 +387,8 @@ void diamondDeleteErrorInMiddle() { @Test void readyConditionTrivialCase() { var workflow = new WorkflowBuilder() - .addDependent(dr1).withReadyCondition(metReadyCondition).build() - .addDependent(dr2).dependsOn(dr1).build() + .addDependentResource(dr1).withReadyCondition(metReadyCondition).build() + .addDependentResource(dr2).dependsOn(dr1).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -398,8 +403,8 @@ void readyConditionTrivialCase() { @Test void readyConditionNotMetTrivialCase() { var workflow = new WorkflowBuilder() - .addDependent(dr1).withReadyCondition(notMetReadyCondition).build() - .addDependent(dr2).dependsOn(dr1).build() + .addDependentResource(dr1).withReadyCondition(notMetReadyCondition).build() + .addDependentResource(dr2).dependsOn(dr1).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -417,9 +422,9 @@ void readyConditionNotMetInOneParent() { TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() - .addDependent(dr1).withReadyCondition(notMetReadyCondition).build() - .addDependent(dr2).build() - .addDependent(dr3).dependsOn(dr1, dr2).build() + .addDependentResource(dr1).withReadyCondition(notMetReadyCondition).build() + .addDependentResource(dr2).build() + .addDependentResource(dr3).dependsOn(dr1, dr2).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -436,10 +441,10 @@ void diamondShareWithReadyCondition() { TestDependent dr4 = new TestDependent("DR_4"); var workflow = new WorkflowBuilder() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).withReadyCondition(notMetReadyCondition).build() - .addDependent(dr3).dependsOn(dr1).build() - .addDependent(dr4).dependsOn(dr2, dr3).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).withReadyCondition(notMetReadyCondition).build() + .addDependentResource(dr3).dependsOn(dr1).build() + .addDependentResource(dr4).dependsOn(dr2, dr3).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java index 72a27ed305..01b8bc619c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java @@ -10,7 +10,6 @@ import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; @SuppressWarnings("rawtypes") @@ -23,9 +22,9 @@ void calculatesTopLevelResources() { var independentDR = mock(DependentResource.class); var workflow = new WorkflowBuilder() - .addDependent(independentDR).build() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() + .addDependentResource(independentDR).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() .build(); Set topResources = @@ -43,9 +42,9 @@ void calculatesBottomLevelResources() { var independentDR = mock(DependentResource.class); Workflow workflow = new WorkflowBuilder() - .addDependent(independentDR).build() - .addDependent(dr1).build() - .addDependent(dr2).dependsOn(dr1).build() + .addDependentResource(independentDR).build() + .addDependentResource(dr1).build() + .addDependentResource(dr2).dependsOn(dr1).build() .build(); Set bottomResources = diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java index 2ced5a0f3d..2d8d84f7f9 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java @@ -44,10 +44,10 @@ public class WebPageDependentsWorkflowReconciler public WebPageDependentsWorkflowReconciler(KubernetesClient kubernetesClient) { initDependentResources(kubernetesClient); workflow = new WorkflowBuilder() - .addDependent(configMapDR).build() - .addDependent(deploymentDR).build() - .addDependent(serviceDR).build() - .addDependent(ingressDR).withReconcileCondition(new IngressCondition()).build() + .addDependentResource(configMapDR).build() + .addDependentResource(deploymentDR).build() + .addDependentResource(serviceDR).build() + .addDependentResource(ingressDR).withReconcileCondition(new IngressCondition()).build() .build(); } From 0e3693b0e0f095482ac5fbed14f59f810da5f87d Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 14:12:56 +0200 Subject: [PATCH 14/35] format --- .../dependent/workflow/ManagedWorkflowSupportTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index 5537cf2150..346c53b1d0 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -52,7 +52,7 @@ void orderingTrivialCases() { assertThat(managedWorkflowSupport .orderAndDetectCycles(List.of(createDRS(NAME_2, NAME_1), createDRS(NAME_1)))) - .map(DependentResourceSpec::getName).containsExactly(NAME_1, NAME_2); + .map(DependentResourceSpec::getName).containsExactly(NAME_1, NAME_2); } @Test From 01b6ffdd8810ae6ceddb909e4705749133ea2962 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 14:42:52 +0200 Subject: [PATCH 15/35] updated samples --- .../operator/sample/ExposedIngressCondition.java | 14 ++++++++++++++ .../WebPageDependentsWorkflowReconciler.java | 13 +++---------- .../sample/WebPageManagedDependentsReconciler.java | 4 +++- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java new file mode 100644 index 0000000000..218d1f8ca2 --- /dev/null +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample; + +import io.fabric8.kubernetes.api.model.networking.v1.Ingress; +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 ExposedIngressCondition implements Condition { + @Override + public boolean isMet(DependentResource dependentResource, WebPage primary, + Context context) { + return primary.getSpec().getExposed(); + } +} diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java index 2d8d84f7f9..b5765ed784 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java @@ -12,10 +12,8 @@ import io.fabric8.kubernetes.api.model.networking.v1.Ingress; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.reconciler.*; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; -import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow; import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; import io.javaoperatorsdk.operator.processing.event.source.EventSource; @@ -47,7 +45,8 @@ public WebPageDependentsWorkflowReconciler(KubernetesClient kubernetesClient) { .addDependentResource(configMapDR).build() .addDependentResource(deploymentDR).build() .addDependentResource(serviceDR).build() - .addDependentResource(ingressDR).withReconcileCondition(new IngressCondition()).build() + .addDependentResource(ingressDR).withReconcileCondition(new ExposedIngressCondition()) + .build() .build(); } @@ -90,12 +89,6 @@ private void initDependentResources(KubernetesClient client) { }); } - static class IngressCondition implements Condition { - @Override - public boolean isMet(DependentResource dependentResource, WebPage primary, - Context context) { - return primary.getSpec().getExposed(); - } - } + } diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java index ac89ebd269..cf2e2e0025 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java @@ -20,7 +20,9 @@ dependents = { @Dependent(type = ConfigMapDependentResource.class), @Dependent(type = DeploymentDependentResource.class), - @Dependent(type = ServiceDependentResource.class) + @Dependent(type = ServiceDependentResource.class), + @Dependent(type = IngressDependentResource.class, + reconcileCondition = ExposedIngressCondition.class) }) public class WebPageManagedDependentsReconciler implements Reconciler, ErrorStatusHandler { From 8d26095f37fd373928801df9e9fdce02f9e8ca99 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Jun 2022 16:01:16 +0200 Subject: [PATCH 16/35] IT wip --- .../operator/WorkflowAllFeatureIT.java | 8 +++++ ...ntAnnotationSecondaryMapperReconciler.java | 4 +-- .../DependentResourceCrossRefReconciler.java | 7 +++-- .../workflowallfeature/PrivateKeySecret.java | 25 ++++++++++++++++ .../PublicKeyConfigMap.java | 29 +++++++++++++++++++ .../WorkflowAllFeatureCustomResource.java | 15 ++++++++++ .../WorkflowAllFeatureReconciler.java | 28 ++++++++++++++++++ .../WorkflowAllFeatureSpec.java | 19 ++++++++++++ .../WorkflowAllFeatureStatus.java | 5 ++++ 9 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java new file mode 100644 index 0000000000..6e98cb4feb --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java @@ -0,0 +1,8 @@ +package io.javaoperatorsdk.operator; + +public class WorkflowAllFeatureIT { + + // todo test features + // todo test empty dependents (separate IT) + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperReconciler.java index 2608b8373c..ec4a2c86b9 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentannotationsecondarymapper/DependentAnnotationSecondaryMapperReconciler.java @@ -13,8 +13,8 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; -@ControllerConfiguration(dependents = {@Dependent( - type = DependentAnnotationSecondaryMapperReconciler.ConfigMapDependentResource.class)}) +@ControllerConfiguration(dependents = @Dependent( + type = DependentAnnotationSecondaryMapperReconciler.ConfigMapDependentResource.class)) public class DependentAnnotationSecondaryMapperReconciler implements Reconciler, TestExecutionInfoProvider { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java index d211f31f28..bb319741b3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentresourcecrossref/DependentResourceCrossRefReconciler.java @@ -12,15 +12,18 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; +import static io.javaoperatorsdk.operator.sample.dependentresourcecrossref.DependentResourceCrossRefReconciler.SECRET_NAME; + @ControllerConfiguration(dependents = { - @Dependent(name = "secret", + @Dependent(name = SECRET_NAME, type = DependentResourceCrossRefReconciler.SecretDependentResource.class), @Dependent(type = DependentResourceCrossRefReconciler.ConfigMapDependentResource.class, - dependsOn = "secret")}) + dependsOn = SECRET_NAME)}) public class DependentResourceCrossRefReconciler implements Reconciler, ErrorStatusHandler { + public static final String SECRET_NAME = "secret"; private final AtomicInteger numberOfExecutions = new AtomicInteger(0); private volatile boolean errorHappened = false; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java new file mode 100644 index 0000000000..680b6d90f0 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java @@ -0,0 +1,25 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +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.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class PrivateKeySecret + extends CRUDKubernetesDependentResource { + + public PrivateKeySecret(Class resourceType) { + super(Secret.class); + } + + @Override + protected Secret desired(WorkflowAllFeatureCustomResource primary, + Context context) { + Secret secret = new Secret(); + secret.setMetadata(new ObjectMetaBuilder().build()); + + + return secret; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java new file mode 100644 index 0000000000..b93e6d0da2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import java.util.Map; + +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.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class PublicKeyConfigMap + extends CRUDKubernetesDependentResource { + + public PublicKeyConfigMap() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(WorkflowAllFeatureCustomResource primary, + Context context) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder().withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()).build()); + var relatedSecret = context.getSecondaryResource(Secret.class); + configMap.setData(Map.of("publicKey", + "Public --- " + relatedSecret.orElseThrow().getData().get("createdKey") + " ---")); + return configMap; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java new file mode 100644 index 0000000000..b478381094 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +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("waf") +public class WorkflowAllFeatureCustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java new file mode 100644 index 0000000000..a130abf3b8 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java @@ -0,0 +1,28 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +@ControllerConfiguration( + +) +public class WorkflowAllFeatureReconciler + implements Reconciler, TestExecutionInfoProvider { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + WorkflowAllFeatureCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java new file mode 100644 index 0000000000..9047df821a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +public class WorkflowAllFeatureSpec { + + private String inputData; + private boolean secretApproved = false; + private boolean createConfigMap = false; + + + public String getInputData() { + return inputData; + } + + public void setInputData(String inputData) { + this.inputData = inputData; + } + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java new file mode 100644 index 0000000000..5188ec8bf9 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java @@ -0,0 +1,5 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +public class WorkflowAllFeatureStatus { + +} From c8fe3515e4a38cab1e0030908d1fa695fea02e74 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 3 Jun 2022 16:52:45 +0200 Subject: [PATCH 17/35] IT wip --- ...efaultManagedDependentResourceContext.java | 2 +- .../ManagedDependentResourceContext.java | 2 +- .../operator/processing/Controller.java | 28 +++++-- .../CRUDNoGCKubernetesDependentResource.java | 16 ++++ .../workflow/WorkflowCleanupExecutor.java | 2 +- .../StandaloneDependentResourceIT.java | 2 +- .../operator/WorkflowAllFeatureIT.java | 75 ++++++++++++++++++- .../StandaloneDependentTestReconciler.java | 4 +- .../ConfigMapDeletePostCondition.java | 17 +++++ .../ConfigMapDependentResource.java | 52 +++++++++++++ .../ConfigMapReconcileCondition.java | 17 +++++ .../DeploymentDependentResource.java | 29 +++++++ .../DeploymentReadyCondition.java | 20 +++++ .../workflowallfeature/PrivateKeySecret.java | 25 ------- .../PublicKeyConfigMap.java | 29 ------- .../WorkflowAllFeatureCustomResource.java | 3 + .../WorkflowAllFeatureReconciler.java | 51 ++++++++++--- .../WorkflowAllFeatureSpec.java | 14 ++-- .../WorkflowAllFeatureStatus.java | 10 +++ .../nginx-deployment.yaml | 0 20 files changed, 312 insertions(+), 86 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java rename operator-framework/src/test/resources/io/javaoperatorsdk/operator/{sample/standalonedependent => }/nginx-deployment.yaml (100%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java index d6e716e2c6..6ef3113306 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java @@ -86,7 +86,7 @@ public DefaultManagedDependentResourceContext setWorkflowCleanupResult( } @Override - public Optional getWorkflowExecutionResult() { + public Optional getWorkflowReconcileResult() { return Optional.ofNullable(workflowReconcileResult); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java index 3290a446a3..ff2f6bbfd1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java @@ -14,7 +14,7 @@ public interface ManagedDependentResourceContext { @SuppressWarnings("unused") T getMandatory(Object key, Class expectedType); - Optional getWorkflowExecutionResult(); + Optional getWorkflowReconcileResult(); Optional getWorkflowCleanupResult(); } 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 2de09bb2fb..d6803cd5b7 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 @@ -21,6 +21,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflow; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE; @@ -119,16 +120,23 @@ public String successTypeName(DeleteControl deleteControl) { @Override public DeleteControl execute() { initContextIfNeeded(resource, context); + WorkflowCleanupResult workflowCleanupResult = null; if (managedWorkflow.isCleaner()) { - var res = managedWorkflow.cleanup(resource, context); + workflowCleanupResult = managedWorkflow.cleanup(resource, context); ((DefaultManagedDependentResourceContext) context.managedDependentResourceContext()) - .setWorkflowCleanupResult(res); - res.throwAggregateExceptionIfErrorsPresent(); + .setWorkflowCleanupResult(workflowCleanupResult); + workflowCleanupResult.throwAggregateExceptionIfErrorsPresent(); } if (isCleaner) { - return ((Cleaner

) reconciler).cleanup(resource, context); + var cleanupResult = ((Cleaner

) reconciler).cleanup(resource, context); + if (!cleanupResult.isRemoveFinalizer()) { + return cleanupResult; + } else { + // this means there is no reschedule + return workflowCleanupResultToDefaultDelete(workflowCleanupResult); + } } else { - return DeleteControl.defaultDelete(); + return workflowCleanupResultToDefaultDelete(workflowCleanupResult); } } }); @@ -137,6 +145,16 @@ public DeleteControl execute() { } } + private DeleteControl workflowCleanupResultToDefaultDelete( + WorkflowCleanupResult workflowCleanupResult) { + if (workflowCleanupResult == null) { + return DeleteControl.defaultDelete(); + } else { + return workflowCleanupResult.allPostConditionsMet() ? DeleteControl.defaultDelete() + : DeleteControl.noFinalizerRemoval(); + } + } + private void initContextIfNeeded(P resource, Context

context) { if (contextInitializer) { ((ContextInitializer

) reconciler).initContext(resource, context); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java new file mode 100644 index 0000000000..f17e169c2f --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java @@ -0,0 +1,16 @@ +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; +import io.javaoperatorsdk.operator.processing.dependent.Creator; +import io.javaoperatorsdk.operator.processing.dependent.Updater; + +public class CRUDNoGCKubernetesDependentResource + extends + KubernetesDependentResource + implements Creator, Updater, Deleter

{ + + public CRUDNoGCKubernetesDependentResource(Class resourceType) { + super(resourceType); + } +} 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 e56a496cda..7ae75c10dd 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 @@ -78,7 +78,7 @@ private synchronized void handleCleanup(DependentResourceNode dependentResourceN workflow.getExecutorService().submit( new NodeExecutor(dependentResourceNode)); actualExecutions.put(dependentResourceNode, nodeFuture); - log.debug("Submitted to reconcile: {}", dependentResourceNode); + log.debug("Submitted for cleanup: {}", dependentResourceNode); } private class NodeExecutor implements Runnable { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java index dfa97981e7..f1e38ce6eb 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java @@ -16,7 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -class StandaloneDependentResourceIT { +public class StandaloneDependentResourceIT { public static final String DEPENDENT_TEST_NAME = "dependent-test1"; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java index 6e98cb4feb..43c248a7d7 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java @@ -1,8 +1,79 @@ package io.javaoperatorsdk.operator; +import java.time.Duration; +import java.util.Map; + +import org.junit.jupiter.api.Disabled; +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.apps.Deployment; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureCustomResource; +import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureReconciler; +import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureSpec; + +import static io.javaoperatorsdk.operator.sample.workflowallfeature.ConfigMapDependentResource.READY_TO_DELETE_ANNOTATION; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + public class WorkflowAllFeatureIT { - // todo test features - // todo test empty dependents (separate IT) + public static final String RESOURCE_NAME = "test"; + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder().withReconciler(WorkflowAllFeatureReconciler.class) + .build(); + + @Disabled + @Test + void configMapNotReconciledUntilDeploymentNotReady() { + operator.create(WorkflowAllFeatureCustomResource.class, customResource(true)); + await() + .pollInterval(Duration.ofMillis(20)) + .untilAsserted( + () -> { + assertThat( + operator + .getReconcilerOfType(WorkflowAllFeatureReconciler.class) + .getNumberOfReconciliationExecution()) + .isPositive(); + }); + assertThat(operator.get(Deployment.class, RESOURCE_NAME)).isNotNull(); + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); + + await().atMost(Duration.ofSeconds(230)).untilAsserted(() -> { + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); + }); + markConfigMapForDelete(); + } + + // @Test + void configMapNotReconciledIfReconcileConditionNotMet() { + + } + + // @Test + void configMapNotDeletedUntilNotMarked() { + + } + + private void markConfigMapForDelete() { + var cm = operator.get(ConfigMap.class, RESOURCE_NAME); + cm.getMetadata().setAnnotations(Map.of(READY_TO_DELETE_ANNOTATION, "true")); + operator.replace(ConfigMap.class, cm); + } + + private WorkflowAllFeatureCustomResource customResource(boolean createConfigMap) { + var res = new WorkflowAllFeatureCustomResource(); + res.setMetadata(new ObjectMetaBuilder() + .withName(RESOURCE_NAME) + .build()); + res.setSpec(new WorkflowAllFeatureSpec()); + res.getSpec().setCreateConfigMap(createConfigMap); + return res; + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java index 4853a22e9a..af6b2d7e25 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java @@ -7,6 +7,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.StandaloneDependentResourceIT; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler; @@ -96,7 +97,8 @@ public DeploymentDependentResource() { protected Deployment desired(StandaloneDependentTestCustomResource primary, Context context) { Deployment deployment = - ReconcilerUtils.loadYaml(Deployment.class, getClass(), "nginx-deployment.yaml"); + ReconcilerUtils.loadYaml(Deployment.class, StandaloneDependentResourceIT.class, + "nginx-deployment.yaml"); deployment.getMetadata().setName(primary.getMetadata().getName()); deployment.getSpec().setReplicas(primary.getSpec().getReplicaCount()); deployment.getMetadata().setNamespace(primary.getMetadata().getNamespace()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java new file mode 100644 index 0000000000..5feaeaaf94 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import io.fabric8.kubernetes.api.model.ConfigMap; +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 ConfigMapDeletePostCondition + implements Condition { + + @Override + public boolean isMet( + DependentResource dependentResource, + WorkflowAllFeatureCustomResource primary, Context context) { + return dependentResource.getSecondaryResource(primary).isPresent(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java new file mode 100644 index 0000000000..0e4a28e542 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java @@ -0,0 +1,52 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import java.util.Map; +import java.util.Optional; + +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.api.reconciler.dependent.Deleter; +import io.javaoperatorsdk.operator.processing.dependent.Creator; +import io.javaoperatorsdk.operator.processing.dependent.Updater; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; + +public class ConfigMapDependentResource + extends KubernetesDependentResource + implements Creator, + Updater, + Deleter { + + public static final String READY_TO_DELETE_ANNOTATION = "ready-to-delete"; + + public ConfigMapDependentResource() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(WorkflowAllFeatureCustomResource primary, + Context context) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + configMap.setData(Map.of("key", "data")); + return configMap; + } + + @Override + public void delete(WorkflowAllFeatureCustomResource primary, + Context context) { + Optional optionalConfigMap = context.getSecondaryResource(ConfigMap.class); + if (optionalConfigMap.isEmpty()) { + return; + } + optionalConfigMap.ifPresent((configMap -> { + if (configMap.getMetadata().getAnnotations() != null + && configMap.getMetadata().getAnnotations().get(READY_TO_DELETE_ANNOTATION) != null) { + client.resource(configMap).delete(); + } + })); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java new file mode 100644 index 0000000000..65409efc36 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import io.fabric8.kubernetes.api.model.ConfigMap; +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 ConfigMapReconcileCondition + implements Condition { + + @Override + public boolean isMet( + DependentResource dependentResource, + WorkflowAllFeatureCustomResource primary, Context context) { + return primary.getSpec().isCreateConfigMap(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentDependentResource.java new file mode 100644 index 0000000000..61cf18f57b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentDependentResource.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.WorkflowAllFeatureIT; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDNoGCKubernetesDependentResource; + +public class DeploymentDependentResource extends + CRUDNoGCKubernetesDependentResource { + + public DeploymentDependentResource() { + super(Deployment.class); + } + + @Override + protected Deployment desired(WorkflowAllFeatureCustomResource primary, + Context context) { + Deployment deployment = + ReconcilerUtils.loadYaml(Deployment.class, WorkflowAllFeatureIT.class, + "nginx-deployment.yaml"); + deployment.getMetadata().setName(primary.getMetadata().getName()); + deployment.getSpec().setReplicas(2); + deployment.getMetadata().setNamespace(primary.getMetadata().getNamespace()); + return deployment; + } +} + + diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java new file mode 100644 index 0000000000..c8646f6ea5 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator.sample.workflowallfeature; + +import io.fabric8.kubernetes.api.model.apps.Deployment; +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 DeploymentReadyCondition + implements Condition { + @Override + public boolean isMet( + DependentResource dependentResource, + WorkflowAllFeatureCustomResource primary, Context context) { + + var deployment = dependentResource.getSecondaryResource(primary).orElseThrow(); + var readyReplicas = deployment.getStatus().getReadyReplicas(); + + return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java deleted file mode 100644 index 680b6d90f0..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PrivateKeySecret.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.sample.workflowallfeature; - -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.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; - -public class PrivateKeySecret - extends CRUDKubernetesDependentResource { - - public PrivateKeySecret(Class resourceType) { - super(Secret.class); - } - - @Override - protected Secret desired(WorkflowAllFeatureCustomResource primary, - Context context) { - Secret secret = new Secret(); - secret.setMetadata(new ObjectMetaBuilder().build()); - - - return secret; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java deleted file mode 100644 index b93e6d0da2..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/PublicKeyConfigMap.java +++ /dev/null @@ -1,29 +0,0 @@ -package io.javaoperatorsdk.operator.sample.workflowallfeature; - -import java.util.Map; - -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.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; - -public class PublicKeyConfigMap - extends CRUDKubernetesDependentResource { - - public PublicKeyConfigMap() { - super(ConfigMap.class); - } - - @Override - protected ConfigMap desired(WorkflowAllFeatureCustomResource primary, - Context context) { - ConfigMap configMap = new ConfigMap(); - configMap.setMetadata(new ObjectMetaBuilder().withName(primary.getMetadata().getName()) - .withNamespace(primary.getMetadata().getNamespace()).build()); - var relatedSecret = context.getSecondaryResource(Secret.class); - configMap.setData(Map.of("publicKey", - "Public --- " + relatedSecret.orElseThrow().getData().get("createdKey") + " ---")); - return configMap; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java index b478381094..ee764f4681 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureCustomResource.java @@ -12,4 +12,7 @@ public class WorkflowAllFeatureCustomResource extends CustomResource implements Namespaced { + + + } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java index a130abf3b8..2daa37be30 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java @@ -3,26 +3,55 @@ import java.util.concurrent.atomic.AtomicInteger; import io.javaoperatorsdk.operator.api.reconciler.*; -import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; - -@ControllerConfiguration( - -) +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; + +import static io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureReconciler.DEPLOYMENT_NAME; + +@ControllerConfiguration(dependents = { + @Dependent(name = DEPLOYMENT_NAME, type = DeploymentDependentResource.class, + readyCondition = DeploymentReadyCondition.class), + @Dependent(type = ConfigMapDependentResource.class, + reconcileCondition = ConfigMapReconcileCondition.class, + deletePostCondition = ConfigMapDeletePostCondition.class, + dependsOn = DEPLOYMENT_NAME) +}) public class WorkflowAllFeatureReconciler - implements Reconciler, TestExecutionInfoProvider { + implements Reconciler, + Cleaner { - private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + public static final String DEPLOYMENT_NAME = "deployment"; + + private final AtomicInteger numberOfReconciliationExecution = new AtomicInteger(0); + private final AtomicInteger numberOfCleanupExecution = new AtomicInteger(0); @Override public UpdateControl reconcile( WorkflowAllFeatureCustomResource resource, Context context) { - numberOfExecutions.addAndGet(1); - return UpdateControl.noUpdate(); + numberOfReconciliationExecution.addAndGet(1); + if (resource.getStatus() == null) { + resource.setStatus(new WorkflowAllFeatureStatus()); + } + resource.getStatus() + .setReady( + context.managedDependentResourceContext() + .getWorkflowReconcileResult().orElseThrow() + .allDependentResourcesReady()); + return UpdateControl.patchStatus(resource); + } + + public int getNumberOfReconciliationExecution() { + return numberOfReconciliationExecution.get(); } - public int getNumberOfExecutions() { - return numberOfExecutions.get(); + public int getNumberOfCleanupExecution() { + return numberOfCleanupExecution.get(); } + @Override + public DeleteControl cleanup(WorkflowAllFeatureCustomResource resource, + Context context) { + numberOfCleanupExecution.addAndGet(1); + return DeleteControl.defaultDelete(); + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java index 9047df821a..6d5cfd7a5a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureSpec.java @@ -2,18 +2,14 @@ public class WorkflowAllFeatureSpec { - private String inputData; - private boolean secretApproved = false; private boolean createConfigMap = false; - - public String getInputData() { - return inputData; + public boolean isCreateConfigMap() { + return createConfigMap; } - public void setInputData(String inputData) { - this.inputData = inputData; + public WorkflowAllFeatureSpec setCreateConfigMap(boolean createConfigMap) { + this.createConfigMap = createConfigMap; + return this; } - - } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java index 5188ec8bf9..11d0798fca 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureStatus.java @@ -2,4 +2,14 @@ public class WorkflowAllFeatureStatus { + private Boolean ready; + + public Boolean getReady() { + return ready; + } + + public WorkflowAllFeatureStatus setReady(Boolean ready) { + this.ready = ready; + return this; + } } diff --git a/operator-framework/src/test/resources/io/javaoperatorsdk/operator/sample/standalonedependent/nginx-deployment.yaml b/operator-framework/src/test/resources/io/javaoperatorsdk/operator/nginx-deployment.yaml similarity index 100% rename from operator-framework/src/test/resources/io/javaoperatorsdk/operator/sample/standalonedependent/nginx-deployment.yaml rename to operator-framework/src/test/resources/io/javaoperatorsdk/operator/nginx-deployment.yaml From 29c0062cfda4c6efbf67c7b135a944ea7a483c69 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 6 Jun 2022 16:23:38 +0200 Subject: [PATCH 18/35] test, and fixis with the consig service provider in tests --- .../AnnotationControllerConfiguration.java | 1 - .../config/ConfigurationServiceProvider.java | 10 ++- .../source/informer/InformerEventSource.java | 12 ++- .../ConfigurationServiceProviderTest.java | 3 +- .../informer/InformerEventSourceTest.java | 2 + .../junit/AbstractOperatorExtension.java | 2 + .../operator/WorkflowAllFeatureIT.java | 86 ++++++++++++++----- .../ConfigMapDeletePostCondition.java | 9 +- .../ConfigMapDependentResource.java | 7 ++ 9 files changed, 102 insertions(+), 30 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 9088b80560..803253acf7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -176,7 +176,6 @@ public List getDependentResources() { } spec = new DependentResourceSpec(dependentType, config, name); spec.setDependsOn(Set.of(dependent.dependsOn())); - // todo tests addConditions(spec, dependent); specsMap.put(name, spec); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java index 50419f6a5d..0a8503ae05 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java @@ -7,10 +7,8 @@ * to the ConfigurationService is via the reconciliation context. */ public class ConfigurationServiceProvider { - static final ConfigurationService DEFAULT = - new BaseConfigurationService(Utils.loadFromProperties()); private static ConfigurationService instance; - private static ConfigurationService defaultConfigurationService = DEFAULT; + private static ConfigurationService defaultConfigurationService = createDefault(); private static boolean alreadyConfigured = false; private ConfigurationServiceProvider() {} @@ -64,8 +62,12 @@ synchronized static ConfigurationService getDefault() { } public synchronized static void reset() { - defaultConfigurationService = DEFAULT; + defaultConfigurationService = createDefault(); instance = null; alreadyConfigured = false; } + + static ConfigurationService createDefault() { + return new BaseConfigurationService(Utils.loadFromProperties()); + } } 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 47fe4abcf3..bdf2096465 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 @@ -93,7 +93,9 @@ public InformerEventSource(InformerConfiguration configuration, KubernetesCli @Override public void onAdd(R resource) { if (log.isDebugEnabled()) { - log.debug("On add event received for resource id: {}", ResourceID.fromResource(resource)); + log.debug("On add event received for resource id: {} type: {}", + ResourceID.fromResource(resource), + resourceType().getSimpleName()); } primaryToSecondaryIndex.onAddOrUpdate(resource); onAddOrUpdate("add", resource, () -> InformerEventSource.super.onAdd(resource)); @@ -102,7 +104,9 @@ public void onAdd(R resource) { @Override public void onUpdate(R oldObject, R newObject) { if (log.isDebugEnabled()) { - log.debug("On update event received for resource id: {}", ResourceID.fromResource(newObject)); + log.debug("On update event received for resource id: {} type: {}", + ResourceID.fromResource(newObject), + resourceType().getSimpleName()); } primaryToSecondaryIndex.onAddOrUpdate(newObject); onAddOrUpdate("update", newObject, @@ -112,7 +116,9 @@ public void onUpdate(R oldObject, R newObject) { @Override public void onDelete(R resource, boolean b) { if (log.isDebugEnabled()) { - log.debug("On delete event received for resource id: {}", ResourceID.fromResource(resource)); + log.debug("On delete event received for resource id: {} type: {}", + ResourceID.fromResource(resource), + resourceType().getSimpleName()); } primaryToSecondaryIndex.onDelete(resource); super.onDelete(resource, b); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java index f8c1a0f6aa..cd0038ab0b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java @@ -55,7 +55,8 @@ void resetShouldResetAllState() { shouldBePossibleToOverrideConfigOnce(); ConfigurationServiceProvider.reset(); - assertEquals(ConfigurationServiceProvider.DEFAULT, ConfigurationServiceProvider.getDefault()); + assertNotEquals(ConfigurationServiceProvider.getDefault(), + ConfigurationServiceProvider.createDefault()); shouldBePossibleToOverrideConfigOnce(); } 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 173379f802..b30dc32f8f 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 @@ -58,11 +58,13 @@ void setup() { .thenReturn(DEFAULT_NAMESPACES_SET); when(informerConfiguration.getSecondaryToPrimaryMapper()) .thenReturn(mock(SecondaryToPrimaryMapper.class)); + when(informerConfiguration.getResourceClass()).thenReturn(Deployment.class); informerEventSource = new InformerEventSource<>(informerConfiguration, clientMock); informerEventSource.setTemporalResourceCache(temporaryResourceCacheMock); informerEventSource.setEventHandler(eventHandlerMock); + SecondaryToPrimaryMapper secondaryToPrimaryMapper = mock(SecondaryToPrimaryMapper.class); when(informerConfiguration.getSecondaryToPrimaryMapper()) .thenReturn(secondaryToPrimaryMapper); diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java index 7164dd42af..1a2ff991c2 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java @@ -161,6 +161,8 @@ protected void afterAllImpl(ExtensionContext context) { } protected void afterEachImpl(ExtensionContext context) { + // resets the config service provider so the controller configs are reconstructed always + ConfigurationServiceProvider.reset(); if (!oneNamespacePerClass) { after(context); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java index 43c248a7d7..128f9dc4a5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java @@ -1,9 +1,8 @@ package io.javaoperatorsdk.operator; import java.time.Duration; -import java.util.Map; +import java.util.HashMap; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -22,47 +21,94 @@ public class WorkflowAllFeatureIT { public static final String RESOURCE_NAME = "test"; + private static final Duration ONE_MINUTE = Duration.ofMinutes(1); + @RegisterExtension LocallyRunOperatorExtension operator = LocallyRunOperatorExtension.builder().withReconciler(WorkflowAllFeatureReconciler.class) .build(); - @Disabled @Test void configMapNotReconciledUntilDeploymentNotReady() { operator.create(WorkflowAllFeatureCustomResource.class, customResource(true)); - await() - .pollInterval(Duration.ofMillis(20)) - .untilAsserted( - () -> { - assertThat( - operator - .getReconcilerOfType(WorkflowAllFeatureReconciler.class) - .getNumberOfReconciliationExecution()) - .isPositive(); - }); - assertThat(operator.get(Deployment.class, RESOURCE_NAME)).isNotNull(); - assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); - - await().atMost(Duration.ofSeconds(230)).untilAsserted(() -> { + await().untilAsserted( + () -> { + assertThat(operator + .getReconcilerOfType(WorkflowAllFeatureReconciler.class) + .getNumberOfReconciliationExecution()) + .isPositive(); + assertThat(operator.get(Deployment.class, RESOURCE_NAME)).isNotNull(); + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); + }); + + await().atMost(ONE_MINUTE).untilAsserted(() -> { + assertThat(operator + .getReconcilerOfType(WorkflowAllFeatureReconciler.class) + .getNumberOfReconciliationExecution()) + .isGreaterThan(1); assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) + .getStatus().getReady()).isTrue(); }); + markConfigMapForDelete(); } - // @Test + + @Test void configMapNotReconciledIfReconcileConditionNotMet() { + var resource = operator.create(WorkflowAllFeatureCustomResource.class, customResource(false)); + await().atMost(ONE_MINUTE).untilAsserted(() -> { + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) + .getStatus().getReady()).isTrue(); + }); + + resource.getSpec().setCreateConfigMap(true); + operator.replace(WorkflowAllFeatureCustomResource.class, resource); + + await().untilAsserted(() -> { + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) + .getStatus().getReady()).isTrue(); + }); } - // @Test + + @Test void configMapNotDeletedUntilNotMarked() { + var resource = operator.create(WorkflowAllFeatureCustomResource.class, customResource(true)); + + await().atMost(ONE_MINUTE).untilAsserted(() -> { + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME).getStatus()) + .isNotNull(); + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) + .getStatus().getReady()).isTrue(); + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); + }); + + operator.delete(WorkflowAllFeatureCustomResource.class, resource); + await().pollDelay(Duration.ofMillis(300)).untilAsserted(() -> { + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME)).isNotNull(); + }); + + markConfigMapForDelete(); + + await().atMost(ONE_MINUTE).untilAsserted(() -> { + assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); + assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME)).isNull(); + }); } private void markConfigMapForDelete() { var cm = operator.get(ConfigMap.class, RESOURCE_NAME); - cm.getMetadata().setAnnotations(Map.of(READY_TO_DELETE_ANNOTATION, "true")); + if (cm.getMetadata().getAnnotations() == null) { + cm.getMetadata().setAnnotations(new HashMap<>()); + } + cm.getMetadata().getAnnotations().put(READY_TO_DELETE_ANNOTATION, "true"); operator.replace(ConfigMap.class, cm); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java index 5feaeaaf94..c5c908dfe6 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java @@ -1,5 +1,8 @@ package io.javaoperatorsdk.operator.sample.workflowallfeature; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; @@ -8,10 +11,14 @@ public class ConfigMapDeletePostCondition implements Condition { + private static final Logger log = LoggerFactory.getLogger(ConfigMapDeletePostCondition.class); + @Override public boolean isMet( DependentResource dependentResource, WorkflowAllFeatureCustomResource primary, Context context) { - return dependentResource.getSecondaryResource(primary).isPresent(); + var configMapDeleted = dependentResource.getSecondaryResource(primary).isEmpty(); + log.debug("Config Map Deleted: {}", configMapDeleted); + return configMapDeleted; } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java index 0e4a28e542..0620d6a753 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDependentResource.java @@ -3,6 +3,9 @@ import java.util.Map; import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -10,6 +13,7 @@ import io.javaoperatorsdk.operator.processing.dependent.Creator; import io.javaoperatorsdk.operator.processing.dependent.Updater; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; +import io.javaoperatorsdk.operator.processing.event.ResourceID; public class ConfigMapDependentResource extends KubernetesDependentResource @@ -19,6 +23,8 @@ public class ConfigMapDependentResource public static final String READY_TO_DELETE_ANNOTATION = "ready-to-delete"; + private static final Logger log = LoggerFactory.getLogger(ConfigMapDependentResource.class); + public ConfigMapDependentResource() { super(ConfigMap.class); } @@ -40,6 +46,7 @@ public void delete(WorkflowAllFeatureCustomResource primary, Context context) { Optional optionalConfigMap = context.getSecondaryResource(ConfigMap.class); if (optionalConfigMap.isEmpty()) { + log.debug("Config Map not found for primary: {}", ResourceID.fromResource(primary)); return; } optionalConfigMap.ifPresent((configMap -> { From a3c203d64af4abdd79554bd70d71b3b0dc2af906 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 6 Jun 2022 17:01:38 +0200 Subject: [PATCH 19/35] renaming conditions --- .../AnnotationControllerConfiguration.java | 12 +++--- .../ControllerConfigurationOverrider.java | 4 +- .../dependent/DependentResourceSpec.java | 4 +- .../api/reconciler/dependent/Dependent.java | 6 +-- .../workflow/DependentResourceNode.java | 42 +++++++++--------- .../workflow/ManagedWorkflowSupport.java | 6 +-- .../workflow/WorkflowCleanupExecutor.java | 2 +- .../workflow/WorkflowReconcileExecutor.java | 6 +-- .../workflow/builder/DependentBuilder.java | 12 +++--- .../workflow/WorkflowCleanupExecutorTest.java | 6 +-- .../WorkflowReconcileExecutorTest.java | 43 +++++++++++-------- .../WorkflowAllFeatureReconciler.java | 6 +-- .../WebPageDependentsWorkflowReconciler.java | 2 +- .../WebPageManagedDependentsReconciler.java | 2 +- 14 files changed, 79 insertions(+), 74 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 803253acf7..ba0c5083be 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -187,14 +187,14 @@ public List getDependentResources() { @SuppressWarnings("unchecked") private void addConditions(DependentResourceSpec spec, Dependent dependent) { - if (dependent.deletePostCondition() != VoidCondition.class) { - spec.setDeletePostCondition(instantiateCondition(dependent.deletePostCondition())); + if (dependent.deletePostcondition() != VoidCondition.class) { + spec.setDeletePostCondition(instantiateCondition(dependent.deletePostcondition())); } - if (dependent.readyCondition() != VoidCondition.class) { - spec.setReadyCondition(instantiateCondition(dependent.readyCondition())); + if (dependent.readyPostcondition() != VoidCondition.class) { + spec.setReadyPostcondition(instantiateCondition(dependent.readyPostcondition())); } - if (dependent.reconcileCondition() != VoidCondition.class) { - spec.setReconcileCondition(instantiateCondition(dependent.reconcileCondition())); + if (dependent.reconcilePrecondition() != VoidCondition.class) { + spec.setReconcilePrecondition(instantiateCondition(dependent.reconcilePrecondition())); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index af5059e11b..88b511be31 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -167,8 +167,8 @@ public ControllerConfiguration build() { KubernetesDependentResourceConfig c) { var res = new DependentResourceSpec(spec.getDependentResourceClass(), c.setNamespaces(namespaces), name); - res.setReadyCondition(spec.getReadyCondition()); - res.setReconcileCondition(spec.getReconcileCondition()); + res.setReadyPostcondition(spec.getReadyCondition()); + res.setReconcilePrecondition(spec.getReconcileCondition()); res.setDeletePostCondition(spec.getDeletePostCondition()); res.setDependsOn(spec.getDependsOn()); return res; 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 6b0d985f8c..2798e79b1a 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 @@ -83,7 +83,7 @@ public DependentResourceSpec setDependsOn(Set dependsOn) { return readyCondition; } - public DependentResourceSpec setReadyCondition(Condition readyCondition) { + public DependentResourceSpec setReadyPostcondition(Condition readyCondition) { this.readyCondition = readyCondition; return this; } @@ -92,7 +92,7 @@ public DependentResourceSpec setReadyCondition(Condition readyCondit return reconcileCondition; } - public DependentResourceSpec setReconcileCondition(Condition reconcileCondition) { + public DependentResourceSpec setReconcilePrecondition(Condition reconcileCondition) { this.reconcileCondition = reconcileCondition; return this; } 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 9cefd7abcd..0eba3fb598 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 @@ -12,13 +12,13 @@ String name() default NO_VALUE_SET; @SuppressWarnings("rawtypes") - Class readyCondition() default VoidCondition.class; + Class readyPostcondition() default VoidCondition.class; @SuppressWarnings("rawtypes") - Class reconcileCondition() default VoidCondition.class; + Class reconcilePrecondition() default VoidCondition.class; @SuppressWarnings("rawtypes") - Class deletePostCondition() default VoidCondition.class; + Class deletePostcondition() default VoidCondition.class; String[] dependsOn() default {}; } 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 99d82a7885..d7af13bc43 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 @@ -11,9 +11,9 @@ public class DependentResourceNode { private final DependentResource dependentResource; - private Condition reconcileCondition; - private Condition deletePostCondition; - private Condition readyCondition; + private Condition reconcilePrecondition; + private Condition deletePostcondition; + private Condition readyPostcondition; private final List dependsOn = new LinkedList<>(); private final List parents = new LinkedList<>(); @@ -22,27 +22,27 @@ public DependentResourceNode(DependentResource dependentResource) { } public DependentResourceNode(DependentResource dependentResource, - Condition reconcileCondition) { - this(dependentResource, reconcileCondition, null); + Condition reconcilePrecondition) { + this(dependentResource, reconcilePrecondition, null); } public DependentResourceNode(DependentResource dependentResource, - Condition reconcileCondition, Condition deletePostCondition) { + Condition reconcilePrecondition, Condition deletePostcondition) { this.dependentResource = dependentResource; - this.reconcileCondition = reconcileCondition; - this.deletePostCondition = deletePostCondition; + this.reconcilePrecondition = reconcilePrecondition; + this.deletePostcondition = deletePostcondition; } public DependentResource getDependentResource() { return dependentResource; } - public Optional getReconcileCondition() { - return Optional.ofNullable(reconcileCondition); + public Optional getReconcilePrecondition() { + return Optional.ofNullable(reconcilePrecondition); } - public Optional getDeletePostCondition() { - return Optional.ofNullable(deletePostCondition); + public Optional getDeletePostcondition() { + return Optional.ofNullable(deletePostcondition); } public List getDependsOn() { @@ -62,23 +62,23 @@ public String toString() { '}'; } - public DependentResourceNode setReconcileCondition( - Condition reconcileCondition) { - this.reconcileCondition = reconcileCondition; + public DependentResourceNode setReconcilePrecondition( + Condition reconcilePrecondition) { + this.reconcilePrecondition = reconcilePrecondition; return this; } - public DependentResourceNode setDeletePostCondition(Condition cleanupCondition) { - this.deletePostCondition = cleanupCondition; + public DependentResourceNode setDeletePostcondition(Condition cleanupCondition) { + this.deletePostcondition = cleanupCondition; return this; } - public Optional> getReadyCondition() { - return Optional.ofNullable(readyCondition); + public Optional> getReadyPostcondition() { + return Optional.ofNullable(readyPostcondition); } - public DependentResourceNode setReadyCondition(Condition readyCondition) { - this.readyCondition = readyCondition; + public DependentResourceNode setReadyPostcondition(Condition readyPostcondition) { + this.readyPostcondition = readyPostcondition; return this; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index a0a083702d..09712a766e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -41,9 +41,9 @@ public Workflow

createWorkflow(List dependentResourceS w.addDependentResource(dependentResourceByName.get(spec.getName())).dependsOn( (Set) spec.getDependsOn() .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); - drBuilder.withDeletePostCondition(spec.getDeletePostCondition()) - .withReconcileCondition(spec.getReconcileCondition()) - .withReadyCondition(spec.getReadyCondition()); + drBuilder.withDeletePostcondition(spec.getDeletePostCondition()) + .withReconcilePrecondition(spec.getReconcileCondition()) + .withReadyPostcondition(spec.getReadyCondition()); }); return w.build(); } 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 7ae75c10dd..27ff266ad6 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 @@ -94,7 +94,7 @@ private NodeExecutor(DependentResourceNode dependentResourceNode) { public void run() { try { var dependentResource = dependentResourceNode.getDependentResource(); - var deletePostCondition = dependentResourceNode.getDeletePostCondition(); + var deletePostCondition = dependentResourceNode.getDeletePostcondition(); if (dependentResource instanceof Deleter && !(dependentResource instanceof GarbageCollected)) { 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 075993ebbe..531a05da0a 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 @@ -84,7 +84,7 @@ private synchronized void handleReconcile( return; } - boolean reconcileConditionMet = dependentResourceNode.getReconcileCondition().map( + boolean reconcileConditionMet = dependentResourceNode.getReconcilePrecondition().map( rc -> rc.isMet(dependentResourceNode.getDependentResource(), primary, context)) .orElse(true); @@ -170,7 +170,7 @@ public void run() { ReconcileResult reconcileResult = dependentResource.reconcile(primary, context); reconcileResults.put(dependentResource, reconcileResult); reconciled.add(dependentResourceNode); - boolean ready = dependentResourceNode.getReadyCondition() + boolean ready = dependentResourceNode.getReadyPostcondition() .map(rc -> rc.isMet(dependentResource, primary, context)) .orElse(true); @@ -202,7 +202,7 @@ private NodeDeleteExecutor(DependentResourceNode dependentResourceNode) { public void run() { try { DependentResource dependentResource = dependentResourceNode.getDependentResource(); - var deletePostCondition = dependentResourceNode.getDeletePostCondition(); + var deletePostCondition = dependentResourceNode.getDeletePostcondition(); if (dependentResource instanceof Deleter && !(dependentResource instanceof GarbageCollected)) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java index 07191fe961..9812cd6521 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java @@ -32,18 +32,18 @@ public DependentBuilder

dependsOn(DependentResource... dependentResources) { return dependsOn(new HashSet<>(Arrays.asList(dependentResources))); } - public DependentBuilder

withReconcileCondition(Condition reconcileCondition) { - node.setReconcileCondition(reconcileCondition); + public DependentBuilder

withReconcilePrecondition(Condition reconcilePrecondition) { + node.setReconcilePrecondition(reconcilePrecondition); return this; } - public DependentBuilder

withReadyCondition(Condition readyCondition) { - node.setReadyCondition(readyCondition); + public DependentBuilder

withReadyPostcondition(Condition readyPostcondition) { + node.setReadyPostcondition(readyPostcondition); return this; } - public DependentBuilder

withDeletePostCondition(Condition readyCondition) { - node.setDeletePostCondition(readyCondition); + public DependentBuilder

withDeletePostcondition(Condition deletePostcondition) { + node.setDeletePostcondition(deletePostcondition); return this; } 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 b819456a28..6540f00157 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 @@ -61,7 +61,7 @@ void dontDeleteIfDependentErrored() { void cleanupConditionTrivialCase() { var workflow = new WorkflowBuilder() .addDependentResource(dd1).build() - .addDependentResource(dd2).dependsOn(dd1).withDeletePostCondition(noMetDeletePostCondition) + .addDependentResource(dd2).dependsOn(dd1).withDeletePostcondition(noMetDeletePostCondition) .build() .build(); @@ -77,7 +77,7 @@ void cleanupConditionTrivialCase() { void cleanupConditionMet() { var workflow = new WorkflowBuilder() .addDependentResource(dd1).build() - .addDependentResource(dd2).dependsOn(dd1).withDeletePostCondition(metDeletePostCondition) + .addDependentResource(dd2).dependsOn(dd1).withDeletePostcondition(metDeletePostCondition) .build() .build(); @@ -97,7 +97,7 @@ void cleanupConditionDiamondWorkflow() { var workflow = new WorkflowBuilder() .addDependentResource(dd1).build() .addDependentResource(dd2).dependsOn(dd1).build() - .addDependentResource(dd3).dependsOn(dd1).withDeletePostCondition(noMetDeletePostCondition) + .addDependentResource(dd3).dependsOn(dd1).withDeletePostcondition(noMetDeletePostCondition) .build() .addDependentResource(dd4).dependsOn(dd2, dd3).build() .build(); 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 ecf562890d..2c81bcb50b 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 @@ -181,9 +181,10 @@ void onlyOneDependsOnErroredResourceNotReconciled() { @Test void simpleReconcileCondition() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReconcileCondition(not_met_reconcile_condition).build() - .addDependentResource(dr2).withReconcileCondition(met_reconcile_condition).build() - .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(dr1).withReconcilePrecondition(not_met_reconcile_condition).build() + .addDependentResource(dr2).withReconcilePrecondition(met_reconcile_condition).build() + .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -200,7 +201,7 @@ void triangleOnceConditionNotMet() { var workflow = new WorkflowBuilder() .addDependentResource(dr1).build() .addDependentResource(dr2).dependsOn(dr1).build() - .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) .dependsOn(dr1) .build() .build(); @@ -220,13 +221,13 @@ void reconcileConditionTransitiveDelete() { var workflow = new WorkflowBuilder() .addDependentResource(dr1).build() .addDependentResource(dr2).dependsOn(dr1) - .withReconcileCondition(not_met_reconcile_condition) + .withReconcilePrecondition(not_met_reconcile_condition) .build() .addDependentResource(drDeleter).dependsOn(dr2) - .withReconcileCondition(met_reconcile_condition) + .withReconcilePrecondition(met_reconcile_condition) .build() .addDependentResource(drDeleter2).dependsOn(drDeleter) - .withReconcileCondition(met_reconcile_condition).build() + .withReconcilePrecondition(met_reconcile_condition).build() .build(); var res = workflow.reconcile(new TestCustomResource(), null); @@ -247,9 +248,10 @@ void reconcileConditionAlsoErrorDependsOn() { var workflow = new WorkflowBuilder() .addDependentResource(drError).build() - .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .build() .addDependentResource(drDeleter2).dependsOn(drError, drDeleter) - .withReconcileCondition(met_reconcile_condition) + .withReconcilePrecondition(met_reconcile_condition) .build() .withThrowExceptionFurther(false) .build(); @@ -271,7 +273,7 @@ void reconcileConditionAlsoErrorDependsOn() { void oneDependsOnConditionNotMet() { var workflow = new WorkflowBuilder() .addDependentResource(dr1).build() - .addDependentResource(dr2).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(dr2).withReconcilePrecondition(not_met_reconcile_condition).build() .addDependentResource(drDeleter).dependsOn(dr1, dr2).build() .build(); @@ -291,7 +293,7 @@ void deletedIfReconcileConditionNotMet() { var workflow = new WorkflowBuilder() .addDependentResource(dr1).build() .addDependentResource(drDeleter).dependsOn(dr1) - .withReconcileCondition(not_met_reconcile_condition) + .withReconcilePrecondition(not_met_reconcile_condition) .build() .addDependentResource(drDeleter2).dependsOn(dr1, drDeleter).build() .build(); @@ -315,7 +317,7 @@ void deleteDoneInReverseOrder() { var workflow = new WorkflowBuilder() .addDependentResource(dr1).build() - .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition) + .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) .dependsOn(dr1) .build() .addDependentResource(drDeleter2).dependsOn(drDeleter).build() @@ -342,10 +344,11 @@ void diamondDeleteWithPostConditionInMiddle() { TestDeleterDependent drDeleter4 = new TestDeleterDependent("DR_DELETER_4"); var workflow = new WorkflowBuilder() - .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .build() .addDependentResource(drDeleter2).dependsOn(drDeleter).build() .addDependentResource(drDeleter3).dependsOn(drDeleter) - .withDeletePostCondition(noMetDeletePostCondition).build() + .withDeletePostcondition(noMetDeletePostCondition).build() .addDependentResource(drDeleter4).dependsOn(drDeleter3, drDeleter2).build() .build(); @@ -366,7 +369,8 @@ void diamondDeleteErrorInMiddle() { TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3"); var workflow = new WorkflowBuilder() - .addDependentResource(drDeleter).withReconcileCondition(not_met_reconcile_condition).build() + .addDependentResource(drDeleter).withReconcilePrecondition(not_met_reconcile_condition) + .build() .addDependentResource(drDeleter2).dependsOn(drDeleter).build() .addDependentResource(errorDD).dependsOn(drDeleter).build() .addDependentResource(drDeleter3).dependsOn(errorDD, drDeleter2).build() @@ -387,7 +391,7 @@ void diamondDeleteErrorInMiddle() { @Test void readyConditionTrivialCase() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReadyCondition(metReadyCondition).build() + .addDependentResource(dr1).withReadyPostcondition(metReadyCondition).build() .addDependentResource(dr2).dependsOn(dr1).build() .build(); @@ -403,7 +407,7 @@ void readyConditionTrivialCase() { @Test void readyConditionNotMetTrivialCase() { var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReadyCondition(notMetReadyCondition).build() + .addDependentResource(dr1).withReadyPostcondition(notMetReadyCondition).build() .addDependentResource(dr2).dependsOn(dr1).build() .build(); @@ -422,7 +426,7 @@ void readyConditionNotMetInOneParent() { TestDependent dr3 = new TestDependent("DR_3"); var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReadyCondition(notMetReadyCondition).build() + .addDependentResource(dr1).withReadyPostcondition(notMetReadyCondition).build() .addDependentResource(dr2).build() .addDependentResource(dr3).dependsOn(dr1, dr2).build() .build(); @@ -442,7 +446,8 @@ void diamondShareWithReadyCondition() { var workflow = new WorkflowBuilder() .addDependentResource(dr1).build() - .addDependentResource(dr2).dependsOn(dr1).withReadyCondition(notMetReadyCondition).build() + .addDependentResource(dr2).dependsOn(dr1).withReadyPostcondition(notMetReadyCondition) + .build() .addDependentResource(dr3).dependsOn(dr1).build() .addDependentResource(dr4).dependsOn(dr2, dr3).build() .build(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java index 2daa37be30..2c25d13924 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java @@ -9,10 +9,10 @@ @ControllerConfiguration(dependents = { @Dependent(name = DEPLOYMENT_NAME, type = DeploymentDependentResource.class, - readyCondition = DeploymentReadyCondition.class), + readyPostcondition = DeploymentReadyCondition.class), @Dependent(type = ConfigMapDependentResource.class, - reconcileCondition = ConfigMapReconcileCondition.class, - deletePostCondition = ConfigMapDeletePostCondition.class, + reconcilePrecondition = ConfigMapReconcileCondition.class, + deletePostcondition = ConfigMapDeletePostCondition.class, dependsOn = DEPLOYMENT_NAME) }) public class WorkflowAllFeatureReconciler diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java index b5765ed784..1a997d7a64 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java @@ -45,7 +45,7 @@ public WebPageDependentsWorkflowReconciler(KubernetesClient kubernetesClient) { .addDependentResource(configMapDR).build() .addDependentResource(deploymentDR).build() .addDependentResource(serviceDR).build() - .addDependentResource(ingressDR).withReconcileCondition(new ExposedIngressCondition()) + .addDependentResource(ingressDR).withReconcilePrecondition(new ExposedIngressCondition()) .build() .build(); } diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java index cf2e2e0025..b2e0963ef2 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java @@ -22,7 +22,7 @@ @Dependent(type = DeploymentDependentResource.class), @Dependent(type = ServiceDependentResource.class), @Dependent(type = IngressDependentResource.class, - reconcileCondition = ExposedIngressCondition.class) + reconcilePrecondition = ExposedIngressCondition.class) }) public class WebPageManagedDependentsReconciler implements Reconciler, ErrorStatusHandler { From 83aa6a59c2347158934ad22344cb70bfa453c67c Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 7 Jun 2022 09:55:41 +0200 Subject: [PATCH 20/35] sonar fixes --- .../api/config/dependent/DependentResourceSpec.java | 9 ++++++--- .../operator/junit/LocallyRunOperatorExtension.java | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) 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 2798e79b1a..2ef9e58de4 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 @@ -79,7 +79,8 @@ public DependentResourceSpec setDependsOn(Set dependsOn) { return this; } - public Condition getReadyCondition() { + @SuppressWarnings("rawtypes") + public Condition getReadyCondition() { return readyCondition; } @@ -88,7 +89,8 @@ public DependentResourceSpec setReadyPostcondition(Condition readyCo return this; } - public Condition getReconcileCondition() { + @SuppressWarnings("rawtypes") + public Condition getReconcileCondition() { return reconcileCondition; } @@ -97,7 +99,8 @@ public DependentResourceSpec setReconcilePrecondition(Condition reco return this; } - public Condition getDeletePostCondition() { + @SuppressWarnings("rawtypes") + public Condition getDeletePostCondition() { return deletePostCondition; } diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java index 321766d9af..a5266a74e5 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java @@ -95,6 +95,7 @@ public RegisteredController getRegisteredControllerForReconcile( } @SuppressWarnings("unchecked") + @Override protected void before(ExtensionContext context) { super.before(context); @@ -150,6 +151,7 @@ protected void before(ExtensionContext context) { this.operator.start(); } + @Override protected void after(ExtensionContext context) { super.after(context); From 36b2501a9278d1699801da8b320035ed148aa98f Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 7 Jun 2022 09:59:00 +0200 Subject: [PATCH 21/35] sonar issue --- .../operator/sample/WebPageDependentsWorkflowReconciler.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java index 1a997d7a64..e9c5218cf8 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageDependentsWorkflowReconciler.java @@ -3,9 +3,6 @@ import java.util.Arrays; import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.apps.Deployment; @@ -29,8 +26,6 @@ public class WebPageDependentsWorkflowReconciler implements Reconciler, ErrorStatusHandler, EventSourceInitializer { public static final String DEPENDENT_RESOURCE_LABEL_SELECTOR = "!low-level"; - private static final Logger log = - LoggerFactory.getLogger(WebPageDependentsWorkflowReconciler.class); private KubernetesDependentResource configMapDR; private KubernetesDependentResource deploymentDR; From 639c4145548b2447e1868dc86dccc129543cbfd0 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 16:12:36 +0200 Subject: [PATCH 22/35] refactor: avoid unneeded object creation --- .../dependent/workflow/ManagedWorkflow.java | 4 ++-- .../workflow/ManagedWorkflowSupport.java | 21 ++++++++++++++++--- .../workflow/ManagedWorkflowSupportTest.java | 3 +-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 3b045417cf..42e9f83af5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -22,12 +22,12 @@ public class ManagedWorkflow

{ public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { - this(client, dependentResourceSpecs, new ManagedWorkflowSupport<>()); + this(client, dependentResourceSpecs, ManagedWorkflowSupport.instance()); } ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs, - ManagedWorkflowSupport

managedWorkflowSupport) { + ManagedWorkflowSupport managedWorkflowSupport) { managedWorkflowSupport.checkForNameDuplication(dependentResourceSpecs); dependentResourceByName = dependentResourceSpecs .stream().collect(Collectors.toMap(DependentResourceSpec::getName, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index 09712a766e..38d4664d1b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -1,6 +1,12 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -14,7 +20,15 @@ import io.javaoperatorsdk.operator.processing.dependent.workflow.builder.WorkflowBuilder; @SuppressWarnings({"rawtypes", "unchecked"}) -class ManagedWorkflowSupport

{ +class ManagedWorkflowSupport { + + private final static ManagedWorkflowSupport instance = new ManagedWorkflowSupport(); + + static ManagedWorkflowSupport instance() { + return instance; + } + + private ManagedWorkflowSupport() {} public void checkForNameDuplication(List dependentResourceSpecs) { if (dependentResourceSpecs.size() <= 1) { @@ -31,7 +45,8 @@ public void checkForNameDuplication(List dependentResourc } @SuppressWarnings("unchecked") - public Workflow

createWorkflow(List dependentResourceSpecs, + public

Workflow

createWorkflow( + List dependentResourceSpecs, Map dependentResourceByName) { var orderedResourceSpecs = orderAndDetectCycles(dependentResourceSpecs); var w = new WorkflowBuilder

(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index 346c53b1d0..f53ad7bca3 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -17,7 +17,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -@SuppressWarnings("rawtypes") class ManagedWorkflowSupportTest { public static final String NAME_1 = "name1"; @@ -25,7 +24,7 @@ class ManagedWorkflowSupportTest { public static final String NAME_3 = "name3"; public static final String NAME_4 = "name4"; - ManagedWorkflowSupport managedWorkflowSupport = new ManagedWorkflowSupport(); + ManagedWorkflowSupport managedWorkflowSupport = ManagedWorkflowSupport.instance(); @Test void trivialCasesNameDuplicates() { From 682072cf37afdb4d648fb4f6fd030c318e6e4d9f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 16:19:58 +0200 Subject: [PATCH 23/35] refactor: improve readability --- .../workflow/ManagedWorkflowSupport.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index 38d4664d1b..4c3196e8b2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -49,18 +49,19 @@ public

Workflow

createWorkflow( List dependentResourceSpecs, Map dependentResourceByName) { var orderedResourceSpecs = orderAndDetectCycles(dependentResourceSpecs); - var w = new WorkflowBuilder

(); - w.withThrowExceptionFurther(false); + var workflowBuilder = new WorkflowBuilder

().withThrowExceptionFurther(false); orderedResourceSpecs.forEach(spec -> { - var drBuilder = - w.addDependentResource(dependentResourceByName.get(spec.getName())).dependsOn( - (Set) spec.getDependsOn() - .stream().map(dependentResourceByName::get).collect(Collectors.toSet())); - drBuilder.withDeletePostcondition(spec.getDeletePostCondition()) + final var dependentResource = dependentResourceByName.get(spec.getName()); + final var dependsOn = (Set) spec.getDependsOn() + .stream().map(dependentResourceByName::get).collect(Collectors.toSet()); + workflowBuilder + .addDependentResource(dependentResource) + .dependsOn(dependsOn) + .withDeletePostcondition(spec.getDeletePostCondition()) .withReconcilePrecondition(spec.getReconcileCondition()) .withReadyPostcondition(spec.getReadyCondition()); }); - return w.build(); + return workflowBuilder.build(); } @SuppressWarnings({"rawtypes", "unchecked"}) From a654a74778eda08a644a9c2604135c9e149c7e3f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 17:44:16 +0200 Subject: [PATCH 24/35] refactor: checkForNameDuplication can now report all duplicates Should also be more readable and slightly faster. --- .../workflow/ManagedWorkflowSupport.java | 22 +++++++++++++------ .../workflow/ManagedWorkflowSupportTest.java | 16 ++++++++++---- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index 4c3196e8b2..c40f75f204 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -31,16 +31,24 @@ static ManagedWorkflowSupport instance() { private ManagedWorkflowSupport() {} public void checkForNameDuplication(List dependentResourceSpecs) { - if (dependentResourceSpecs.size() <= 1) { + if (dependentResourceSpecs == null) { return; } - var nameList = - dependentResourceSpecs.stream().map(DependentResourceSpec::getName) - .sorted().collect(Collectors.toList()); - for (int i = 0; i < nameList.size() - 1; i++) { - if (nameList.get(i).equals(nameList.get(i + 1))) { - throw new OperatorException("Duplicate dependent resource name:" + nameList.get(i)); + final var size = dependentResourceSpecs.size(); + if (size == 0) { + return; + } + + final var uniqueNames = new HashSet<>(size); + final var duplicatedNames = new HashSet<>(size); + dependentResourceSpecs.forEach(spec -> { + final var name = spec.getName(); + if (!uniqueNames.add(name)) { + duplicatedNames.add(name); } + }); + if (!duplicatedNames.isEmpty()) { + throw new OperatorException("Duplicated dependent resource name(s): " + duplicatedNames); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index f53ad7bca3..ba5dacd12a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -28,6 +28,7 @@ class ManagedWorkflowSupportTest { @Test void trivialCasesNameDuplicates() { + managedWorkflowSupport.checkForNameDuplication(null); managedWorkflowSupport.checkForNameDuplication(Collections.emptyList()); managedWorkflowSupport.checkForNameDuplication(List.of(createDRS(NAME_1))); managedWorkflowSupport.checkForNameDuplication(List.of(createDRS(NAME_1), createDRS(NAME_2))); @@ -35,13 +36,20 @@ void trivialCasesNameDuplicates() { @Test void checkFindsDuplicates() { + final var drs2 = createDRS(NAME_2); + final var drs1 = createDRS(NAME_1); + Assertions.assertThrows(OperatorException.class, () -> managedWorkflowSupport - .checkForNameDuplication(List.of(createDRS(NAME_2), createDRS(NAME_2)))); + .checkForNameDuplication(List.of(drs2, drs2))); Assertions.assertThrows(OperatorException.class, - () -> managedWorkflowSupport.checkForNameDuplication(List.of(createDRS(NAME_1), - createDRS(NAME_2), - createDRS(NAME_2)))); + () -> managedWorkflowSupport.checkForNameDuplication( + List.of(drs1, drs2, drs2))); + + final var exception = Assertions.assertThrows(OperatorException.class, + () -> managedWorkflowSupport.checkForNameDuplication( + List.of(drs1, drs2, drs2, drs1))); + assertThat(exception.getMessage()).contains(NAME_1, NAME_2); } @Test From 4b411767347162d56abce8f6f878a06d4643b300 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 18:30:14 +0200 Subject: [PATCH 25/35] refactor: rename more appropriately --- .../operator/processing/Controller.java | 21 +++++++++++++++---- .../dependent/workflow/ManagedWorkflow.java | 10 ++++----- 2 files changed, 22 insertions(+), 9 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 d6803cd5b7..e37401bd68 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 @@ -1,6 +1,7 @@ package io.javaoperatorsdk.operator.processing; -import java.util.*; +import java.util.Optional; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -12,12 +13,24 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.*; +import io.javaoperatorsdk.operator.CustomResourceUtils; +import io.javaoperatorsdk.operator.MissingCRDException; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.RegisteredController; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Cleaner; +import io.javaoperatorsdk.operator.api.reconciler.Constants; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ContextInitializer; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; +import io.javaoperatorsdk.operator.api.reconciler.Ignore; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflow; @@ -163,7 +176,7 @@ private void initContextIfNeeded(P resource, Context

context) { public void initAndRegisterEventSources(EventSourceContext

context) { managedWorkflow - .getDependentResourceByName().entrySet().stream() + .getDependentResourcesByName().entrySet().stream() .filter(drEntry -> drEntry.getValue() instanceof EventSourceProvider) .forEach(drEntry -> { final var provider = (EventSourceProvider) drEntry.getValue(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 42e9f83af5..0bc05aa1d8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -18,7 +18,7 @@ public class ManagedWorkflow

{ private final Workflow

workflow; private final boolean isCleaner; private final boolean isEmptyWorkflow; - private final Map dependentResourceByName; + private final Map dependentResourcesByName; public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs) { @@ -29,13 +29,13 @@ public ManagedWorkflow(KubernetesClient client, List dependentResourceSpecs, ManagedWorkflowSupport managedWorkflowSupport) { managedWorkflowSupport.checkForNameDuplication(dependentResourceSpecs); - dependentResourceByName = dependentResourceSpecs + dependentResourcesByName = dependentResourceSpecs .stream().collect(Collectors.toMap(DependentResourceSpec::getName, spec -> managedWorkflowSupport.createAndConfigureFrom(spec, client))); isEmptyWorkflow = dependentResourceSpecs.isEmpty(); workflow = - managedWorkflowSupport.createWorkflow(dependentResourceSpecs, dependentResourceByName); + managedWorkflowSupport.createWorkflow(dependentResourceSpecs, dependentResourcesByName); isCleaner = checkIfCleaner(); } @@ -64,7 +64,7 @@ public boolean isEmptyWorkflow() { return isEmptyWorkflow; } - public Map getDependentResourceByName() { - return dependentResourceByName; + public Map getDependentResourcesByName() { + return dependentResourcesByName; } } From e3771f20651cf8c449a1139cca6e307a9b3e630a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 21:25:48 +0200 Subject: [PATCH 26/35] refactor: no need for mock, makes things faster & easier to debug --- .../workflow/ManagedWorkflowTestUtils.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) 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 cca36a2fe3..85d57db7a0 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 @@ -1,23 +1,19 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import java.util.Arrays; -import java.util.HashSet; +import java.util.Set; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.dependent.EmptyTestDependentResource; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - @SuppressWarnings("rawtypes") public class ManagedWorkflowTestUtils { + @SuppressWarnings("unchecked") public static DependentResourceSpec createDRS(String name, String... dependOns) { - var drcMock = mock(DependentResourceSpec.class); - when(drcMock.getDependentResourceClass()).thenReturn(EmptyTestDependentResource.class); - when(drcMock.getName()).thenReturn(name); - when(drcMock.getDependsOn()).thenReturn(new HashSet(Arrays.asList(dependOns))); - return drcMock; + final var spec = new DependentResourceSpec(EmptyTestDependentResource.class, + null, name); + spec.setDependsOn(Set.of(dependOns)); + return spec; } } From b3dbfbdc52dd1b902f84366901bba32b1772abce Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 21:26:12 +0200 Subject: [PATCH 27/35] chore(tests): add more tests --- .../workflow/ManagedWorkflowSupportTest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index ba5dacd12a..85f04f220f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -78,6 +78,34 @@ void orderingDiamondShape() { .contains(NAME_4, Index.atIndex(3)); } + + @Test + void orderingMultipleRoots() { + final var NAME_3 = "name3"; + final var NAME_4 = "name4"; + final var NAME_5 = "name5"; + final var NAME_6 = "name6"; + + var res = managedWorkflowSupport + .orderAndDetectCycles(List.of( + createDRS(NAME_2, NAME_1, NAME_5), + createDRS(NAME_1), + createDRS(NAME_3, NAME_1), + createDRS(NAME_4, NAME_2, NAME_3), + createDRS(NAME_5, NAME_1, NAME_6), + createDRS(NAME_6))) + .stream().map(DependentResourceSpec::getName).collect(Collectors.toList()); + + assertThat(res) + .containsExactlyInAnyOrder(NAME_1, NAME_5, NAME_6, NAME_2, NAME_3, NAME_4) + .contains(NAME_6, Index.atIndex(0)) + .contains(NAME_1, Index.atIndex(1)) + .contains(NAME_5, Index.atIndex(2)) + .contains(NAME_3, Index.atIndex(3)) + .contains(NAME_2, Index.atIndex(4)) + .contains(NAME_4, Index.atIndex(5)); + } + @Test void detectsCyclesTrivialCases() { String NAME_3 = "name3"; @@ -97,6 +125,13 @@ void detectsCycleOnSubTree() { createDRS(NAME_2, NAME_1), createDRS(NAME_3, NAME_1, NAME_4), createDRS(NAME_4, NAME_3)))); + + Assertions.assertThrows(OperatorException.class, + () -> managedWorkflowSupport.orderAndDetectCycles(List.of( + createDRS(NAME_1), + createDRS(NAME_2, NAME_1, NAME_4), + createDRS(NAME_3, NAME_2), + createDRS(NAME_4, NAME_3)))); } @Test From ff71f3b6c109a93d6a1e4a881c9a485bdef7b033 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 21:27:53 +0200 Subject: [PATCH 28/35] refactor: simplify and make orderAndDetectCycles more readable --- .../workflow/ManagedWorkflowSupport.java | 100 +++++++++++------- 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index c40f75f204..aad9475518 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -1,12 +1,12 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -90,57 +90,70 @@ public DependentResource createAndConfigureFrom(DependentResourceSpec spec, } /** - * Throws also exception if there is a cycle in the dependencies. * * @param dependentResourceSpecs list of specs * @return top-bottom ordered resources that can be added safely to workflow + * @throws OperatorException if there is a cycle in the dependencies * */ public List orderAndDetectCycles( List dependentResourceSpecs) { - List res = new ArrayList<>(dependentResourceSpecs.size()); - Set alreadySelected = new HashSet<>(); - Map> dependOnIndex = - createDependOnIndex(dependentResourceSpecs); - Map nameToDR = dependentResourceSpecs.stream() - .collect(Collectors.toMap(DependentResourceSpec::getName, s -> s)); - Set selectedLastIteration = - getTopDependentResources(dependentResourceSpecs); - - while (!selectedLastIteration.isEmpty()) { - res.addAll(selectedLastIteration); - alreadySelected.addAll(selectedLastIteration); - Set newAdds = new HashSet<>(); - selectedLastIteration.forEach(dr -> { - var dependsOn = dependOnIndex.get(dr.getName()); - if (dependsOn == null) - dependsOn = Collections.emptyList(); - dependsOn.forEach(ndr -> { - if (allDependsOnsAlreadySelected(ndr, alreadySelected, nameToDR, dr.getName())) { - newAdds.add(ndr); - } - }); + final var drInfosByName = createDRInfos(dependentResourceSpecs); + final var orderedSpecs = new ArrayList(dependentResourceSpecs.size()); + final var alreadyVisited = new HashSet(); + var toVisit = getTopDependentResources(dependentResourceSpecs); + + while (!toVisit.isEmpty()) { + final var toVisitNext = new HashSet(); + toVisit.forEach(dr -> { + final var name = dr.getName(); + var drInfo = drInfosByName.get(name); + if (drInfo != null) { + drInfo.waitingForCompletion.forEach(spec -> { + if (isReadyForVisit(spec, alreadyVisited, name)) { + toVisitNext.add(spec); + } + }); + orderedSpecs.add(dr); + } + alreadyVisited.add(name); }); - selectedLastIteration = newAdds; + + toVisit = toVisitNext; } - if (res.size() != dependentResourceSpecs.size()) { + if (orderedSpecs.size() != dependentResourceSpecs.size()) { // could provide improved message where the exact cycles are made visible - throw new OperatorException( - "Cycle(s) between dependent resources."); + throw new OperatorException("Cycle(s) between dependent resources."); + } + return orderedSpecs; + } + + private static class DRInfo { + private final DependentResourceSpec spec; + private final List waitingForCompletion; + + private DRInfo(DependentResourceSpec spec) { + this.spec = spec; + this.waitingForCompletion = new LinkedList<>(); + } + + void add(DependentResourceSpec spec) { + waitingForCompletion.add(spec); + } + + String name() { + return spec.getName(); } - return res; } - private boolean allDependsOnsAlreadySelected(DependentResourceSpec dr, - Set alreadySelected, - Map nameToDR, + private boolean isReadyForVisit(DependentResourceSpec dr, Set alreadyVisited, String alreadyPresentName) { for (var name : dr.getDependsOn()) { if (name.equals(alreadyPresentName)) continue; - if (!alreadySelected.contains(nameToDR.get(name))) { + if (!alreadyVisited.contains(name)) { return false; } } @@ -153,14 +166,19 @@ private Set getTopDependentResources( .collect(Collectors.toSet()); } - private Map> createDependOnIndex( - List dependentResourceSpecs) { - Map> dependsOnSpec = new HashMap<>(); - dependentResourceSpecs.forEach(dr -> dr.getDependsOn().forEach(name -> { - dependsOnSpec.computeIfAbsent((String) name, n -> new ArrayList<>()); - dependsOnSpec.get(name).add(dr); + private Map createDRInfos(List dependentResourceSpecs) { + // first create mappings + final var infos = dependentResourceSpecs.stream() + .map(DRInfo::new) + .collect(Collectors.toMap(DRInfo::name, Function.identity())); + + // then populate the reverse depends on information + dependentResourceSpecs.forEach(spec -> spec.getDependsOn().forEach(name -> { + final var drInfo = infos.get(name); + drInfo.add(spec); })); - return dependsOnSpec; + + return infos; } } From 18bdddfae817d517d793c1635fda46b52b1d989e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 22:27:31 +0200 Subject: [PATCH 29/35] refactor: avoid creating a ManagedWorkflow instance when not needed --- .../operator/processing/Controller.java | 2 +- .../workflow/DefaultManagedWorkflow.java | 65 ++++++++++++++ .../dependent/workflow/ManagedWorkflow.java | 84 +++++++++---------- .../workflow/ManagedWorkflowTest.java | 7 +- 4 files changed, 109 insertions(+), 49 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.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 e37401bd68..5c25787f1f 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 @@ -67,7 +67,7 @@ public Controller(Reconciler

reconciler, eventSourceManager = new EventSourceManager<>(this); isCleaner = reconciler instanceof Cleaner; managedWorkflow = - new ManagedWorkflow<>(kubernetesClient, configuration.getDependentResources()); + ManagedWorkflow.workflowFor(kubernetesClient, configuration.getDependentResources()); } @Override 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 new file mode 100644 index 0000000000..e1162ebcca --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java @@ -0,0 +1,65 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; + +@SuppressWarnings("rawtypes") +public class DefaultManagedWorkflow

implements ManagedWorkflow

{ + + private final Workflow

workflow; + private final boolean isCleaner; + private final boolean isEmptyWorkflow; + private final Map dependentResourcesByName; + + DefaultManagedWorkflow(KubernetesClient client, + List dependentResourceSpecs, + ManagedWorkflowSupport managedWorkflowSupport) { + managedWorkflowSupport.checkForNameDuplication(dependentResourceSpecs); + dependentResourcesByName = dependentResourceSpecs + .stream().collect(Collectors.toMap(DependentResourceSpec::getName, + spec -> managedWorkflowSupport.createAndConfigureFrom(spec, client))); + + isEmptyWorkflow = dependentResourceSpecs.isEmpty(); + workflow = + managedWorkflowSupport.createWorkflow(dependentResourceSpecs, dependentResourcesByName); + isCleaner = checkIfCleaner(); + } + + public WorkflowReconcileResult reconcile(P primary, Context

context) { + return workflow.reconcile(primary, context); + } + + public WorkflowCleanupResult cleanup(P primary, Context

context) { + return workflow.cleanup(primary, context); + } + + private boolean checkIfCleaner() { + for (var dr : workflow.getDependentResources()) { + if (dr instanceof Deleter && !(dr instanceof GarbageCollected)) { + return true; + } + } + return false; + } + + public boolean isCleaner() { + return isCleaner; + } + + public boolean isEmptyWorkflow() { + return isEmptyWorkflow; + } + + public Map getDependentResourcesByName() { + return dependentResourcesByName; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java index 0bc05aa1d8..08e2c497b0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflow.java @@ -1,70 +1,62 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; @SuppressWarnings("rawtypes") -public class ManagedWorkflow

{ +public interface ManagedWorkflow

{ - private final Workflow

workflow; - private final boolean isCleaner; - private final boolean isEmptyWorkflow; - private final Map dependentResourcesByName; - - public ManagedWorkflow(KubernetesClient client, - List dependentResourceSpecs) { - this(client, dependentResourceSpecs, ManagedWorkflowSupport.instance()); - } + ManagedWorkflow noOpWorkflow = new ManagedWorkflow() { + @Override + public WorkflowReconcileResult reconcile(HasMetadata primary, Context context) { + throw new IllegalStateException("Shouldn't be called"); + } - ManagedWorkflow(KubernetesClient client, - List dependentResourceSpecs, - ManagedWorkflowSupport managedWorkflowSupport) { - managedWorkflowSupport.checkForNameDuplication(dependentResourceSpecs); - dependentResourcesByName = dependentResourceSpecs - .stream().collect(Collectors.toMap(DependentResourceSpec::getName, - spec -> managedWorkflowSupport.createAndConfigureFrom(spec, client))); + @Override + public WorkflowCleanupResult cleanup(HasMetadata primary, Context context) { + throw new IllegalStateException("Shouldn't be called"); + } - isEmptyWorkflow = dependentResourceSpecs.isEmpty(); - workflow = - managedWorkflowSupport.createWorkflow(dependentResourceSpecs, dependentResourcesByName); - isCleaner = checkIfCleaner(); - } + @Override + public boolean isCleaner() { + return false; + } - public WorkflowReconcileResult reconcile(P primary, Context

context) { - return workflow.reconcile(primary, context); - } + @Override + public boolean isEmptyWorkflow() { + return true; + } - public WorkflowCleanupResult cleanup(P primary, Context

context) { - return workflow.cleanup(primary, context); - } + @Override + public Map getDependentResourcesByName() { + return Collections.emptyMap(); + } + }; - private boolean checkIfCleaner() { - for (var dr : workflow.getDependentResources()) { - if (dr instanceof Deleter && !(dr instanceof GarbageCollected)) { - return true; - } + @SuppressWarnings("unchecked") + static ManagedWorkflow workflowFor(KubernetesClient client, + List dependentResourceSpecs) { + if (dependentResourceSpecs == null || dependentResourceSpecs.isEmpty()) { + return noOpWorkflow; } - return false; + return new DefaultManagedWorkflow(client, dependentResourceSpecs, + ManagedWorkflowSupport.instance()); } - public boolean isCleaner() { - return isCleaner; - } + WorkflowReconcileResult reconcile(P primary, Context

context); - public boolean isEmptyWorkflow() { - return isEmptyWorkflow; - } + WorkflowCleanupResult cleanup(P primary, Context

context); - public Map getDependentResourcesByName() { - return dependentResourcesByName; - } + boolean isCleaner(); + + boolean isEmptyWorkflow(); + + Map getDependentResourcesByName(); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java index 2deb1dbeb8..df556885b1 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java @@ -14,7 +14,9 @@ import static io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowTestUtils.createDRS; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; @SuppressWarnings({"rawtypes", "unchecked"}) class ManagedWorkflowTest { @@ -56,7 +58,8 @@ void isCleanerIfAtLeastOneDRIsDeleterAndNoGC() { } ManagedWorkflow managedWorkflow(DependentResourceSpec... specs) { - return new ManagedWorkflow(kubernetesClientMock, List.of(specs), managedWorkflowSupportMock); + return new DefaultManagedWorkflow(kubernetesClientMock, List.of(specs), + managedWorkflowSupportMock); } } From 4b01cb88a43cc12ba97c235356521fc9609df1a2 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 22:52:42 +0200 Subject: [PATCH 30/35] fix: rename test to reflect use case --- .../java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java index 128f9dc4a5..2a8289101f 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java @@ -29,7 +29,7 @@ public class WorkflowAllFeatureIT { .build(); @Test - void configMapNotReconciledUntilDeploymentNotReady() { + void configMapNotReconciledUntilDeploymentReady() { operator.create(WorkflowAllFeatureCustomResource.class, customResource(true)); await().untilAsserted( () -> { From ad988b1e6ed05a5ea91d6911f59b6f69901c9134 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Jun 2022 22:52:59 +0200 Subject: [PATCH 31/35] chore: format --- .../workflow/WorkflowReconcileExecutor.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 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 531a05da0a..0412e1c912 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 @@ -91,12 +91,8 @@ private synchronized void handleReconcile( if (!reconcileConditionMet) { handleReconcileConditionNotMet(dependentResourceNode); } else { - Future nodeFuture = - workflow - .getExecutorService() - .submit( - new NodeReconcileExecutor( - dependentResourceNode)); + Future nodeFuture = workflow.getExecutorService() + .submit(new NodeReconcileExecutor(dependentResourceNode)); actualExecutions.put(dependentResourceNode, nodeFuture); log.debug("Submitted to reconcile: {}", dependentResourceNode); } @@ -113,9 +109,8 @@ private void handleDelete(DependentResourceNode dependentResourceNode) { return; } - Future nodeFuture = - workflow.getExecutorService() - .submit(new NodeDeleteExecutor(dependentResourceNode)); + Future nodeFuture = workflow.getExecutorService() + .submit(new NodeDeleteExecutor(dependentResourceNode)); actualExecutions.put(dependentResourceNode, nodeFuture); log.debug("Submitted to delete: {}", dependentResourceNode); } From 3f1236224b1e3d1dbcaca3e3da6886e46b1f5160 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 9 Jun 2022 08:54:05 +0200 Subject: [PATCH 32/35] =?UTF-8?q?=CB=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../kubernetes/CRUDNoGCKubernetesDependentResource.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java index f17e169c2f..e9b18c4734 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CRUDNoGCKubernetesDependentResource.java @@ -6,8 +6,7 @@ import io.javaoperatorsdk.operator.processing.dependent.Updater; public class CRUDNoGCKubernetesDependentResource - extends - KubernetesDependentResource + extends KubernetesDependentResource implements Creator, Updater, Deleter

{ public CRUDNoGCKubernetesDependentResource(Class resourceType) { From f8d3c16f7a53e2de10593e65f88e30dc47dec4c1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 9 Jun 2022 08:55:16 +0200 Subject: [PATCH 33/35] fix: possible NPE --- .../dependent/workflow/builder/DependentBuilder.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java index 9812cd6521..70991dc91d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/builder/DependentBuilder.java @@ -29,7 +29,10 @@ public DependentBuilder

dependsOn(Set dependentResources) } public DependentBuilder

dependsOn(DependentResource... dependentResources) { - return dependsOn(new HashSet<>(Arrays.asList(dependentResources))); + if (dependentResources != null) { + return dependsOn(new HashSet<>(Arrays.asList(dependentResources))); + } + return this; } public DependentBuilder

withReconcilePrecondition(Condition reconcilePrecondition) { From 915644150a34f8869fa72f36f5ba21e9a89854d1 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 9 Jun 2022 09:49:45 +0200 Subject: [PATCH 34/35] moved javadoc --- ...efaultManagedDependentResourceContext.java | 36 ------------------ .../ManagedDependentResourceContext.java | 37 +++++++++++++++++++ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java index 6ef3113306..5b1a21e5dd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java @@ -3,14 +3,9 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult; -/** - * Contextual information related to {@link DependentResource} either to retrieve the actual - * implementations to interact with them or to pass information between them and/or the reconciler - */ @SuppressWarnings("rawtypes") public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext { @@ -18,17 +13,6 @@ public class DefaultManagedDependentResourceContext implements ManagedDependentR private WorkflowCleanupResult workflowCleanupResult; private final ConcurrentHashMap attributes = new ConcurrentHashMap(); - /** - * Retrieve a contextual object, if it exists and is of the specified expected type, associated - * with the specified key. Contextual objects can be used to pass data between the reconciler and - * dependent resources and are scoped to the current reconciliation. - * - * @param key the key identifying which contextual object to retrieve - * @param expectedType the class representing the expected type of the contextual object - * @param the type of the expected contextual object - * @return an Optional containing the contextual object or {@link Optional#empty()} if no such - * object exists or doesn't match the expected type - */ @Override public Optional get(Object key, Class expectedType) { return Optional.ofNullable(attributes.get(key)) @@ -36,17 +20,6 @@ public Optional get(Object key, Class expectedType) { .map(expectedType::cast); } - /** - * Associates the specified contextual value to the specified key. If the value is {@code null}, - * the semantics of this operation is defined as removing the mapping associated with the - * specified key. - * - * @param key the key identifying which contextual object to add or remove from the context - * @param value the value to add to the context or {@code null} to remove an existing entry - * associated with the specified key - * @return an Optional containing the previous value associated with the key or - * {@link Optional#empty()} if none existed - */ @Override @SuppressWarnings("unchecked") public T put(Object key, T value) { @@ -56,15 +29,6 @@ public T put(Object key, T value) { return (T) Optional.ofNullable(attributes.put(key, value)); } - /** - * Retrieves the value associated with the key or fail with an exception if none exists. - * - * @param key the key identifying which contextual object to retrieve - * @param expectedType the expected type of the value to retrieve - * @param the type of the expected contextual object - * @return the contextual object value associated with the specified key - * @see #get(Object, Class) - */ @Override @SuppressWarnings("unused") public T getMandatory(Object key, Class expectedType) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java index ff2f6bbfd1..42741c3a97 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java @@ -2,15 +2,52 @@ import java.util.Optional; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult; +/** + * Contextual information related to {@link DependentResource} either to retrieve the actual + * implementations to interact with them or to pass information between them and/or the reconciler + */ public interface ManagedDependentResourceContext { + + /** + * Retrieve a contextual object, if it exists and is of the specified expected type, associated + * with the specified key. Contextual objects can be used to pass data between the reconciler and + * dependent resources and are scoped to the current reconciliation. + * + * @param key the key identifying which contextual object to retrieve + * @param expectedType the class representing the expected type of the contextual object + * @param the type of the expected contextual object + * @return an Optional containing the contextual object or {@link Optional#empty()} if no such + * object exists or doesn't match the expected type + */ Optional get(Object key, Class expectedType); + /** + * Associates the specified contextual value to the specified key. If the value is {@code null}, + * the semantics of this operation is defined as removing the mapping associated with the + * specified key. + * + * @param key the key identifying which contextual object to add or remove from the context + * @param value the value to add to the context or {@code null} to remove an existing entry + * associated with the specified key + * @return an Optional containing the previous value associated with the key or + * {@link Optional#empty()} if none existed + */ @SuppressWarnings("unchecked") T put(Object key, T value); + /** + * Retrieves the value associated with the key or fail with an exception if none exists. + * + * @param key the key identifying which contextual object to retrieve + * @param expectedType the expected type of the value to retrieve + * @param the type of the expected contextual object + * @return the contextual object value associated with the specified key + * @see #get(Object, Class) + */ @SuppressWarnings("unused") T getMandatory(Object key, Class expectedType); From e951058ace79602f308ce940ba9e4718ebcb1eeb Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 9 Jun 2022 10:00:24 +0200 Subject: [PATCH 35/35] comment in test --- .../operator/api/config/ConfigurationServiceProviderTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java index cd0038ab0b..fbe5a3542e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java @@ -55,6 +55,7 @@ void resetShouldResetAllState() { shouldBePossibleToOverrideConfigOnce(); ConfigurationServiceProvider.reset(); + // makes sure createDefault creates a new instance assertNotEquals(ConfigurationServiceProvider.getDefault(), ConfigurationServiceProvider.createDefault());