From e9bafc7fb766ac2c8240e536d2a2775dd54cd1bb Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 8 Jul 2024 17:50:05 +0200 Subject: [PATCH] feat: allow returning additional information from conditions (#2426) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2424. --------- Signed-off-by: Attila Mészáros Signed-off-by: Chris Laprun Co-authored-by: Attila Mészáros Signed-off-by: Chris Laprun --- docs/content/en/docs/workflows/_index.md | 17 +- .../operator/api/reconciler/Workflow.java | 13 +- .../workflow/AbstractWorkflowExecutor.java | 70 ++++-- .../dependent/workflow/Condition.java | 4 + .../dependent/workflow/ConditionWithType.java | 31 +++ .../dependent/workflow/DefaultResult.java | 26 +++ .../dependent/workflow/DefaultWorkflow.java | 7 +- .../workflow/DependentResourceNode.java | 44 ++-- .../dependent/workflow/DetailedCondition.java | 94 ++++++++ .../dependent/workflow/NodeExecutor.java | 8 +- .../dependent/workflow/Workflow.java | 17 +- .../dependent/workflow/WorkflowBuilder.java | 4 + .../workflow/WorkflowCleanupExecutor.java | 91 ++++---- .../workflow/WorkflowCleanupResult.java | 20 +- .../workflow/WorkflowReconcileExecutor.java | 154 +++++++------ .../workflow/WorkflowReconcileResult.java | 28 +-- .../dependent/workflow/WorkflowResult.java | 211 +++++++++++++++++- .../processing/event/NamedEventSource.java | 0 .../dependent/workflow/ExecutionAssert.java | 25 ++- .../WorkflowReconcileExecutorTest.java | 99 +++++++- .../workflow/WorkflowResultTest.java | 11 +- .../dependent/workflow/WorkflowTest.java | 6 +- .../operator/WorkflowAllFeatureIT.java | 26 ++- .../config/BaseConfigurationServiceTest.java | 8 +- ...CreateUpdateEventFilterTestReconciler.java | 4 +- .../ConfigMapReconcileCondition.java | 17 +- .../WorkflowAllFeatureReconciler.java | 22 +- .../WorkflowAllFeatureStatus.java | 13 +- pom.xml | 10 +- 29 files changed, 797 insertions(+), 283 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ConditionWithType.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultResult.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DetailedCondition.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/NamedEventSource.java diff --git a/docs/content/en/docs/workflows/_index.md b/docs/content/en/docs/workflows/_index.md index cbb39840b0..4b61c6f1b0 100644 --- a/docs/content/en/docs/workflows/_index.md +++ b/docs/content/en/docs/workflows/_index.md @@ -49,11 +49,26 @@ reconciliation process. See related [integration test](https://github.com/operator-framework/java-operator-sdk/blob/ba5e33527bf9e3ea0bd33025ccb35e677f9d44b4/operator-framework/src/test/java/io/javaoperatorsdk/operator/CRDPresentActivationConditionIT.java). To have multiple resources of same type with an activation condition is a bit tricky, since you - don't want to have multiple `InformerEvetnSource` for the same type, you have to explicitly + don't want to have multiple `InformerEventSource` for the same type, you have to explicitly name the informer for the Dependent Resource (`@KubernetesDependent(informerConfig = @InformerConfig(name = "configMapInformer"))`) for all resource of same type with activation condition. This will make sure that only one is registered. See details at [low level api](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java#L20-L52). +### Result conditions + +While simple conditions are usually enough, it might happen you want to convey extra information as a result of the +evaluation of the conditions (e.g., to report error messages or because the result of the condition evaluation might be +interesting for other purposes). In this situation, you should implement `DetailedCondition` instead of `Condition` and +provide an implementation of the `detailedIsMet` method, which allows you to return a more detailed `Result` object via +which you can provide extra information. The `DetailedCondition.Result` interface provides factory method for your +convenience but you can also provide your own implementation if required. + +You can access the results for conditions from the `WorkflowResult` instance that is returned whenever a workflow is +evaluated. You can access that result from the `ManagedWorkflowAndDependentResourceContext` accessible from the +reconciliation `Context`. You can then access individual condition results using the ` +getDependentConditionResult` methods. You can see an example of this +in [this integration test](https://github.com/operator-framework/java-operator-sdk/blob/fd0e92c0de55c47d5df50658cf4e147ee5e6102d/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/WorkflowAllFeatureReconciler.java#L44-L49). + ## Defining Workflows Similarly to dependent resources, there are two ways to define workflows, in managed and standalone diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Workflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Workflow.java index a9497a9749..3fb6c2c830 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Workflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Workflow.java @@ -8,6 +8,8 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult; @Inherited @Retention(RetentionPolicy.RUNTIME) @@ -29,12 +31,11 @@ * execution as would normally be the case. Instead, it will proceed to its * {@link Reconciler#reconcile(HasMetadata, Context)} method as if no error occurred. It is then * up to the developer to decide how to proceed by retrieving the errored dependents (and their - * associated exception) via - * {@link io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowResult#erroredDependents}, - * the workflow result itself being accessed from - * {@link Context#managedWorkflowAndDependentResourceContext()}. If {@code false}, an exception - * will be automatically thrown at the end of the workflow execution, presenting an aggregated - * view of what happened. + * associated exception) via {@link WorkflowReconcileResult#getErroredDependents()} or + * {@link WorkflowCleanupResult#getErroredDependents()}, the workflow result itself being accessed + * from {@link Context#managedWorkflowAndDependentResourceContext()}. If {@code false}, an + * exception will be automatically thrown at the end of the workflow execution, presenting an + * aggregated view of what happened. */ boolean handleExceptionsInReconciler() default false; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index 319b1a9e56..32c0bddbc0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -1,12 +1,11 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import java.util.function.Function; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -20,25 +19,24 @@ @SuppressWarnings("rawtypes") abstract class AbstractWorkflowExecutor

{ - protected final Workflow

workflow; + protected final DefaultWorkflow

workflow; protected final P primary; protected final ResourceID primaryID; protected final Context

context; + protected final Map, WorkflowResult.DetailBuilder> results; /** * Covers both deleted and reconciled */ - private final Set alreadyVisited = ConcurrentHashMap.newKeySet(); private final Map> actualExecutions = new ConcurrentHashMap<>(); - private final Map exceptionsDuringExecution = - new ConcurrentHashMap<>(); private final ExecutorService executorService; - public AbstractWorkflowExecutor(Workflow

workflow, P primary, Context

context) { + protected AbstractWorkflowExecutor(DefaultWorkflow

workflow, P primary, Context

context) { this.workflow = workflow; this.primary = primary; this.context = context; this.primaryID = ResourceID.fromResource(primary); executorService = context.getWorkflowExecutorService(); + results = new ConcurrentHashMap<>(workflow.getDependentResourcesByName().size()); } protected abstract Logger logger(); @@ -75,11 +73,31 @@ protected boolean noMoreExecutionsScheduled() { } protected boolean alreadyVisited(DependentResourceNode dependentResourceNode) { - return alreadyVisited.contains(dependentResourceNode); + return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::isVisited); } - protected void markAsVisited(DependentResourceNode dependentResourceNode) { - alreadyVisited.add(dependentResourceNode); + protected boolean postDeleteConditionNotMet(DependentResourceNode drn) { + return getResultFlagFor(drn, WorkflowResult.DetailBuilder::hasPostDeleteConditionNotMet); + } + + protected boolean isMarkedForDelete(DependentResourceNode drn) { + return getResultFlagFor(drn, WorkflowResult.DetailBuilder::isMarkedForDelete); + } + + protected WorkflowResult.DetailBuilder createOrGetResultFor( + DependentResourceNode dependentResourceNode) { + return results.computeIfAbsent(dependentResourceNode, + unused -> new WorkflowResult.DetailBuilder()); + } + + protected Optional> getResultFor( + DependentResourceNode dependentResourceNode) { + return Optional.ofNullable(results.get(dependentResourceNode)); + } + + protected boolean getResultFlagFor(DependentResourceNode dependentResourceNode, + Function, Boolean> flag) { + return getResultFor(dependentResourceNode).map(flag).orElse(false); } protected boolean isExecutingNow(DependentResourceNode dependentResourceNode) { @@ -94,17 +112,15 @@ protected void markAsExecuting(DependentResourceNode dependentResourceNode protected synchronized void handleExceptionInExecutor( DependentResourceNode dependentResourceNode, RuntimeException e) { - exceptionsDuringExecution.put(dependentResourceNode, e); + createOrGetResultFor(dependentResourceNode).withError(e); } - protected boolean isInError(DependentResourceNode dependentResourceNode) { - return exceptionsDuringExecution.containsKey(dependentResourceNode); + protected boolean isNotReady(DependentResourceNode dependentResourceNode) { + return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::isNotReady); } - protected Map getErroredDependents() { - return exceptionsDuringExecution.entrySet().stream() - .collect( - Collectors.toMap(e -> e.getKey().getDependentResource(), Entry::getValue)); + protected boolean isInError(DependentResourceNode dependentResourceNode) { + return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::hasError); } protected synchronized void handleNodeExecutionFinish( @@ -116,9 +132,17 @@ protected synchronized void handleNodeExecutionFinish( } } - protected boolean isConditionMet(Optional> condition, - DependentResource dependentResource) { - return condition.map(c -> c.isMet(dependentResource, primary, context)).orElse(true); + @SuppressWarnings("unchecked") + protected boolean isConditionMet( + Optional> condition, + DependentResourceNode dependentResource) { + final var dr = dependentResource.getDependentResource(); + return condition.map(c -> { + final DetailedCondition.Result r = c.detailedIsMet(dr, primary, context); + results.computeIfAbsent(dependentResource, unused -> new WorkflowResult.DetailBuilder()) + .withResultForCondition(c, r); + return r; + }).orElse(DetailedCondition.Result.metWithoutResult).isSuccess(); } protected void submit(DependentResourceNode dependentResourceNode, @@ -145,4 +169,10 @@ protected void registerOrDeregisterEventSourceBasedOnActivation( } } } + + protected Map> asDetails() { + return results.entrySet().stream() + .collect( + Collectors.toMap(e -> e.getKey().getDependentResource(), e -> e.getValue().build())); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java index 87690ed69b..de96c99ca5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/Condition.java @@ -6,6 +6,10 @@ public interface Condition { + enum Type { + ACTIVATION, DELETE, READY, RECONCILE + } + /** * Checks whether a condition holds true for a given * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ConditionWithType.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ConditionWithType.java new file mode 100644 index 0000000000..de95830a92 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ConditionWithType.java @@ -0,0 +1,31 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; + +class ConditionWithType implements DetailedCondition { + private final Condition condition; + private final Type type; + + ConditionWithType(Condition condition, Type type) { + this.condition = condition; + this.type = type; + } + + public Type type() { + return type; + } + + @SuppressWarnings("unchecked") + @Override + public Result detailedIsMet(DependentResource dependentResource, P primary, + Context

context) { + if (condition instanceof DetailedCondition detailedCondition) { + return detailedCondition.detailedIsMet(dependentResource, primary, context); + } else { + return Result + .withoutResult(condition.isMet(dependentResource, primary, context)); + } + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultResult.java new file mode 100644 index 0000000000..cc63f637cf --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultResult.java @@ -0,0 +1,26 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +public class DefaultResult implements DetailedCondition.Result { + private final T result; + private final boolean success; + + public DefaultResult(boolean success, T result) { + this.result = result; + this.success = success; + } + + @Override + public T getDetail() { + return result; + } + + @Override + public boolean isSuccess() { + return success; + } + + @Override + public String toString() { + return asString(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java index 5ed5b5cb9e..e3f43f9e12 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java @@ -108,12 +108,10 @@ public WorkflowCleanupResult cleanup(P primary, Context

context) { return result; } - @Override public Set getTopLevelDependentResources() { return topLevelResources; } - @Override public Set getBottomLevelResource() { return bottomLevelResource; } @@ -140,6 +138,11 @@ public boolean isEmpty() { return dependentResourceNodes.isEmpty(); } + @Override + public int size() { + return dependentResourceNodes.size(); + } + @Override public Map getDependentResourcesByName() { final var resources = new HashMap(dependentResourceNodes.size()); 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 3e8a762c5e..aa8bb31c66 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 @@ -13,10 +13,10 @@ class DependentResourceNode { private final List dependsOn = new LinkedList<>(); private final List parents = new LinkedList<>(); - private Condition reconcilePrecondition; - private Condition deletePostcondition; - private Condition readyPostcondition; - private Condition activationCondition; + private ConditionWithType reconcilePrecondition; + private ConditionWithType deletePostcondition; + private ConditionWithType readyPostcondition; + private ConditionWithType activationCondition; private final DependentResource dependentResource; DependentResourceNode(DependentResource dependentResource) { @@ -26,10 +26,10 @@ class DependentResourceNode { public DependentResourceNode(Condition reconcilePrecondition, Condition deletePostcondition, Condition readyPostcondition, Condition activationCondition, DependentResource dependentResource) { - this.reconcilePrecondition = reconcilePrecondition; - this.deletePostcondition = deletePostcondition; - this.readyPostcondition = readyPostcondition; - this.activationCondition = activationCondition; + setReconcilePrecondition(reconcilePrecondition); + setDeletePostcondition(deletePostcondition); + setReadyPostcondition(readyPostcondition); + setActivationCondition(activationCondition); this.dependentResource = dependentResource; } @@ -50,36 +50,40 @@ public List getParents() { return parents; } - public Optional> getReconcilePrecondition() { + public Optional> getReconcilePrecondition() { return Optional.ofNullable(reconcilePrecondition); } - public Optional> getDeletePostcondition() { + public Optional> getDeletePostcondition() { return Optional.ofNullable(deletePostcondition); } - public Optional> getActivationCondition() { + public Optional> getActivationCondition() { return Optional.ofNullable(activationCondition); } - void setReconcilePrecondition(Condition reconcilePrecondition) { - this.reconcilePrecondition = reconcilePrecondition; + public Optional> getReadyPostcondition() { + return Optional.ofNullable(readyPostcondition); } - void setDeletePostcondition(Condition cleanupCondition) { - this.deletePostcondition = cleanupCondition; + void setReconcilePrecondition(Condition reconcilePrecondition) { + this.reconcilePrecondition = reconcilePrecondition == null ? null + : new ConditionWithType<>(reconcilePrecondition, Condition.Type.RECONCILE); } - void setActivationCondition(Condition activationCondition) { - this.activationCondition = activationCondition; + void setDeletePostcondition(Condition deletePostcondition) { + this.deletePostcondition = deletePostcondition == null ? null + : new ConditionWithType<>(deletePostcondition, Condition.Type.DELETE); } - public Optional> getReadyPostcondition() { - return Optional.ofNullable(readyPostcondition); + void setActivationCondition(Condition activationCondition) { + this.activationCondition = activationCondition == null ? null + : new ConditionWithType<>(activationCondition, Condition.Type.ACTIVATION); } void setReadyPostcondition(Condition readyPostcondition) { - this.readyPostcondition = readyPostcondition; + this.readyPostcondition = readyPostcondition == null ? null + : new ConditionWithType<>(readyPostcondition, Condition.Type.READY); } public DependentResource getDependentResource() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DetailedCondition.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DetailedCondition.java new file mode 100644 index 0000000000..200743cc0e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DetailedCondition.java @@ -0,0 +1,94 @@ +package io.javaoperatorsdk.operator.processing.dependent.workflow; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; + +/** + * A condition that can return extra information in addition of whether it is met or not. + * + * @param the resource type this condition applies to + * @param

the primary resource type associated with the dependent workflow this condition is + * part of + * @param the type of the extra information returned by the condition + */ +public interface DetailedCondition extends Condition { + + /** + * Checks whether a condition holds true for the specified {@link DependentResource}, returning + * additional information as needed. + * + * @param dependentResource the {@link DependentResource} for which we want to check the condition + * @param primary the primary resource being considered + * @param context the current reconciliation {@link Context} + * @return a {@link Result} instance containing the result of the evaluation of the condition as + * well as additional detail + * @see Condition#isMet(DependentResource, HasMetadata, Context) + */ + Result detailedIsMet(DependentResource dependentResource, P primary, Context

context); + + @Override + default boolean isMet(DependentResource dependentResource, P primary, Context

context) { + return detailedIsMet(dependentResource, primary, context).isSuccess(); + } + + /** + * Holds a more detailed {@link Condition} result. + * + * @param the type of the extra information provided in condition evaluation + */ + @SuppressWarnings({"rawtypes", "unchecked"}) + interface Result { + /** + * A result expressing a condition has been met without extra information + */ + Result metWithoutResult = new DefaultResult(true, null); + + /** + * A result expressing a condition has not been met without extra information + */ + Result unmetWithoutResult = new DefaultResult(false, null); + + /** + * Creates a {@link Result} without extra information + * + * @param success whether or not the condition has been met + * @return a {@link Result} without extra information + */ + static Result withoutResult(boolean success) { + return success ? metWithoutResult : unmetWithoutResult; + } + + /** + * Creates a {@link Result} with the specified condition evaluation result and extra information + * + * @param success whether or not the condition has been met + * @param detail the extra information that the condition provided during its evaluation + * @return a {@link Result} with the specified condition evaluation result and extra information + * @param the type of the extra information provided by the condition + */ + static Result withResult(boolean success, T detail) { + return new DefaultResult<>(success, detail); + } + + default String asString() { + return "Detail: " + getDetail() + " met: " + isSuccess(); + } + + /** + * The extra information provided by the associated {@link DetailedCondition} during its + * evaluation + * + * @return extra information provided by the associated {@link DetailedCondition} during its + * evaluation or {@code null} if none was provided + */ + T getDetail(); + + /** + * Whether the associated condition held true + * + * @return {@code true} if the associated condition was met, {@code false} otherwise + */ + boolean isSuccess(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/NodeExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/NodeExecutor.java index 0067c55321..2006486dc1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/NodeExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/NodeExecutor.java @@ -1,7 +1,6 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; abstract class NodeExecutor implements Runnable { @@ -17,9 +16,7 @@ protected NodeExecutor(DependentResourceNode dependentResourceNode, @Override public void run() { try { - var dependentResource = dependentResourceNode.getDependentResource(); - - doRun(dependentResourceNode, dependentResource); + doRun(dependentResourceNode); } catch (RuntimeException e) { workflowExecutor.handleExceptionInExecutor(dependentResourceNode, e); @@ -28,6 +25,5 @@ public void run() { } } - protected abstract void doRun(DependentResourceNode dependentResourceNode, - DependentResource dependentResource); + protected abstract void doRun(DependentResourceNode dependentResourceNode); } 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 7f30ba6c9e..02f7d9c89f 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 @@ -3,7 +3,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -21,22 +20,16 @@ default WorkflowCleanupResult cleanup(P primary, Context

context) { throw new UnsupportedOperationException("Implement this"); } - @SuppressWarnings("rawtypes") - default Set getTopLevelDependentResources() { - return Collections.emptySet(); - } - - @SuppressWarnings("rawtypes") - default Set getBottomLevelResource() { - return Collections.emptySet(); - } - default boolean hasCleaner() { return false; } default boolean isEmpty() { - return true; + return size() == 0; + } + + default int size() { + return getDependentResourcesByName().size(); } @SuppressWarnings("rawtypes") diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java index 90ece70d26..8e231e802a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowBuilder.java @@ -82,6 +82,10 @@ public WorkflowBuilder

withThrowExceptionFurther(boolean throwExceptionFurthe } public Workflow

build() { + return buildAsDefaultWorkflow(); + } + + DefaultWorkflow

buildAsDefaultWorkflow() { return new DefaultWorkflow(new HashSet<>(dependentResourceNodes.values()), throwExceptionAutomatically, isCleaner); } 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 dda4db6729..da518be6ce 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 @@ -1,9 +1,7 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.ArrayList; import java.util.List; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -11,19 +9,14 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; @SuppressWarnings("rawtypes") -public class WorkflowCleanupExecutor

extends AbstractWorkflowExecutor

{ +class WorkflowCleanupExecutor

extends AbstractWorkflowExecutor

{ private static final Logger log = LoggerFactory.getLogger(WorkflowCleanupExecutor.class); private static final String CLEANUP = "cleanup"; - private final Set postDeleteConditionNotMet = - ConcurrentHashMap.newKeySet(); - private final Set deleteCalled = ConcurrentHashMap.newKeySet(); - - public WorkflowCleanupExecutor(Workflow

workflow, P primary, Context

context) { + WorkflowCleanupExecutor(DefaultWorkflow

workflow, P primary, Context

context) { super(workflow, primary, context); } @@ -42,13 +35,30 @@ protected Logger logger() { @SuppressWarnings({"rawtypes", "unchecked"}) private synchronized void handleCleanup(DependentResourceNode dependentResourceNode) { - log.debug("Submitting for cleanup: {} primaryID: {}", dependentResourceNode, primaryID); - - if (alreadyVisited(dependentResourceNode) - || isExecutingNow(dependentResourceNode) - || !allDependentsCleaned(dependentResourceNode) - || hasErroredDependent(dependentResourceNode)) { - log.debug("Skipping submit of: {} primaryID: {}", dependentResourceNode, primaryID); + log.debug("Considering for cleanup: {} primaryID: {}", dependentResourceNode, primaryID); + + final var alreadyVisited = alreadyVisited(dependentResourceNode); + final var executingNow = isExecutingNow(dependentResourceNode); + final var waitingOnDependents = !allDependentsCleaned(dependentResourceNode); + final var hasErroredDependent = hasErroredDependent(dependentResourceNode); + if (waitingOnDependents || alreadyVisited || executingNow || hasErroredDependent) { + if (log.isDebugEnabled()) { + final var causes = new ArrayList(); + if (alreadyVisited) { + causes.add("already visited"); + } + if (executingNow) { + causes.add("executing now"); + } + if (waitingOnDependents) { + causes.add("waiting on dependents"); + } + if (hasErroredDependent) { + causes.add("errored dependent"); + } + log.debug("Skipping: {} primaryID: {} causes: {}", dependentResourceNode, + primaryID, String.join(", ", causes)); + } return; } @@ -64,33 +74,27 @@ private CleanupExecutor(DependentResourceNode drn) { @Override @SuppressWarnings("unchecked") - protected void doRun(DependentResourceNode dependentResourceNode, - DependentResource dependentResource) { - var deletePostCondition = dependentResourceNode.getDeletePostcondition(); - - var active = - isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource); + protected void doRun(DependentResourceNode dependentResourceNode) { + final var active = + isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode); registerOrDeregisterEventSourceBasedOnActivation(active, dependentResourceNode); - if (dependentResource.isDeletable() && active) { - ((Deleter

) dependentResource).delete(primary, context); - deleteCalled.add(dependentResourceNode); - } - - boolean deletePostConditionMet; + boolean deletePostConditionMet = true; if (active) { - deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); - } else { - deletePostConditionMet = true; + final var dependentResource = dependentResourceNode.getDependentResource(); + if (dependentResource.isDeletable()) { + ((Deleter

) dependentResource).delete(primary, context); + createOrGetResultFor(dependentResourceNode).markAsDeleted(); + } + + deletePostConditionMet = + isConditionMet(dependentResourceNode.getDeletePostcondition(), dependentResourceNode); } + + createOrGetResultFor(dependentResourceNode).markAsVisited(); + if (deletePostConditionMet) { - markAsVisited(dependentResourceNode); handleDependentCleaned(dependentResourceNode); - } else { - // updating alreadyVisited needs to be the last operation otherwise could lead to a race - // condition in handleCleanup condition checks - postDeleteConditionNotMet.add(dependentResourceNode); - markAsVisited(dependentResourceNode); } } } @@ -112,7 +116,7 @@ private boolean allDependentsCleaned(DependentResourceNode dependentResourceNode List parents = dependentResourceNode.getParents(); return parents.isEmpty() || parents.stream() - .allMatch(d -> alreadyVisited(d) && !postDeleteConditionNotMet.contains(d)); + .allMatch(d -> alreadyVisited(d) && !postDeleteConditionNotMet(d)); } @SuppressWarnings("unchecked") @@ -122,13 +126,6 @@ private boolean hasErroredDependent(DependentResourceNode dependentResourceNode) } private WorkflowCleanupResult createCleanupResult() { - final var erroredDependents = getErroredDependents(); - final var postConditionNotMet = postDeleteConditionNotMet.stream() - .map(DependentResourceNode::getDependentResource) - .collect(Collectors.toList()); - final var deleteCalled = this.deleteCalled.stream() - .map(DependentResourceNode::getDependentResource) - .collect(Collectors.toList()); - return new WorkflowCleanupResult(erroredDependents, postConditionNotMet, deleteCalled); + return new WorkflowCleanupResult(asDetails()); } } 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 860a553c39..b93741ef56 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 @@ -7,26 +7,24 @@ @SuppressWarnings("rawtypes") public class WorkflowCleanupResult extends WorkflowResult { + private Boolean allPostConditionsMet; - private final List deleteCalledOnDependents; - private final List postConditionNotMetDependents; - - WorkflowCleanupResult(Map erroredDependents, - List postConditionNotMet, List deleteCalled) { - super(erroredDependents); - this.deleteCalledOnDependents = deleteCalled; - this.postConditionNotMetDependents = postConditionNotMet; + WorkflowCleanupResult(Map> results) { + super(results); } public List getDeleteCalledOnDependents() { - return deleteCalledOnDependents; + return listFilteredBy(Detail::deleted); } public List getPostConditionNotMetDependents() { - return postConditionNotMetDependents; + return listFilteredBy(detail -> !detail.isConditionWithTypeMet(Condition.Type.DELETE)); } public boolean allPostConditionsMet() { - return postConditionNotMetDependents.isEmpty(); + if (allPostConditionsMet == null) { + allPostConditionsMet = getPostConditionNotMetDependents().isEmpty(); + } + return allPostConditionsMet; } } 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 2da0bd7525..b130e8fd5f 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 @@ -1,10 +1,8 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.ArrayList; import java.util.HashSet; -import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -12,28 +10,16 @@ import io.fabric8.kubernetes.api.model.HasMetadata; 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.ReconcileResult; @SuppressWarnings({"rawtypes", "unchecked"}) -public class WorkflowReconcileExecutor

extends AbstractWorkflowExecutor

{ +class WorkflowReconcileExecutor

extends AbstractWorkflowExecutor

{ private static final Logger log = LoggerFactory.getLogger(WorkflowReconcileExecutor.class); private static final String RECONCILE = "reconcile"; private static final String DELETE = "delete"; - - private final Set notReady = ConcurrentHashMap.newKeySet(); - - private final Set markedForDelete = ConcurrentHashMap.newKeySet(); - private final Set deletePostConditionNotMet = - ConcurrentHashMap.newKeySet(); - // used to remember reconciled (not deleted or errored) dependents - private final Set reconciled = ConcurrentHashMap.newKeySet(); - private final Map reconcileResults = - new ConcurrentHashMap<>(); - - public WorkflowReconcileExecutor(Workflow

workflow, P primary, Context

context) { + public WorkflowReconcileExecutor(DefaultWorkflow

workflow, P primary, Context

context) { super(workflow, primary, context); } @@ -51,25 +37,46 @@ protected Logger logger() { } private synchronized void handleReconcile(DependentResourceNode dependentResourceNode) { - log.debug("Submitting for reconcile: {} primaryID: {}", dependentResourceNode, primaryID); - - if (alreadyVisited(dependentResourceNode) - || isExecutingNow(dependentResourceNode) - || !allParentsReconciledAndReady(dependentResourceNode) - || markedForDelete.contains(dependentResourceNode) - || hasErroredParent(dependentResourceNode)) { - log.debug("Skipping submit of: {}, primaryID: {}", dependentResourceNode, primaryID); + log.debug("Considering for reconcile: {} primaryID: {}", dependentResourceNode, primaryID); + + final var alreadyVisited = alreadyVisited(dependentResourceNode); + final var executingNow = isExecutingNow(dependentResourceNode); + final var isWaitingOnParents = !allParentsReconciledAndReady(dependentResourceNode); + final var isMarkedForDelete = isMarkedForDelete(dependentResourceNode); + final var hasErroredParent = hasErroredParent(dependentResourceNode); + if (isWaitingOnParents || alreadyVisited || executingNow || isMarkedForDelete + || hasErroredParent) { + if (log.isDebugEnabled()) { + final var causes = new ArrayList(); + if (alreadyVisited) { + causes.add("already visited"); + } + if (executingNow) { + causes.add("executing now"); + } + if (isMarkedForDelete) { + causes.add("marked for delete"); + } + if (isWaitingOnParents) { + causes.add("waiting on parents"); + } + if (hasErroredParent) { + causes.add("errored parent"); + } + log.debug("Skipping: {} primaryID: {} causes: {}", dependentResourceNode, + primaryID, String.join(", ", causes)); + } return; } boolean activationConditionMet = isConditionMet(dependentResourceNode.getActivationCondition(), - dependentResourceNode.getDependentResource()); + dependentResourceNode); registerOrDeregisterEventSourceBasedOnActivation(activationConditionMet, dependentResourceNode); boolean reconcileConditionMet = true; if (activationConditionMet) { reconcileConditionMet = isConditionMet(dependentResourceNode.getReconcilePrecondition(), - dependentResourceNode.getDependentResource()); + dependentResourceNode); } if (!reconcileConditionMet || !activationConditionMet) { handleReconcileOrActivationConditionNotMet(dependentResourceNode, activationConditionMet); @@ -81,12 +88,29 @@ private synchronized void handleReconcile(DependentResourceNode depend private synchronized void handleDelete(DependentResourceNode dependentResourceNode) { log.debug("Submitting for delete: {}", dependentResourceNode); - if (alreadyVisited(dependentResourceNode) - || isExecutingNow(dependentResourceNode) - || !markedForDelete.contains(dependentResourceNode) - || !allDependentsDeletedAlready(dependentResourceNode)) { - log.debug("Skipping submit for delete of: {} primaryID: {} ", dependentResourceNode, - primaryID); + final var alreadyVisited = alreadyVisited(dependentResourceNode); + final var executingNow = isExecutingNow(dependentResourceNode); + final var isNotMarkedForDelete = !isMarkedForDelete(dependentResourceNode); + final var isWaitingOnDependents = !allDependentsDeletedAlready(dependentResourceNode); + if (isNotMarkedForDelete || alreadyVisited || executingNow || isWaitingOnDependents) { + if (log.isDebugEnabled()) { + final var causes = new ArrayList(); + if (alreadyVisited) { + causes.add("already visited"); + } + if (executingNow) { + causes.add("executing now"); + } + if (isNotMarkedForDelete) { + causes.add("not marked for delete"); + } + if (isWaitingOnDependents) { + causes.add("waiting on dependents"); + } + log.debug("Skipping submit for delete of: {} primaryID: {} causes: {}", + dependentResourceNode, + primaryID, String.join(", ", causes)); + } return; } @@ -96,16 +120,8 @@ private synchronized void handleDelete(DependentResourceNode dependentResourceNo private boolean allDependentsDeletedAlready(DependentResourceNode dependentResourceNode) { var dependents = dependentResourceNode.getParents(); - return dependents.stream().allMatch(d -> alreadyVisited(d) && !notReady.contains(d) - && !isInError(d) && !deletePostConditionNotMet.contains(d)); - } - - // needs to be in one step - private synchronized void setAlreadyReconciledButNotReady( - DependentResourceNode dependentResourceNode) { - log.debug("Setting already reconciled but not ready for: {}", dependentResourceNode); - markAsVisited(dependentResourceNode); - notReady.add(dependentResourceNode); + return dependents.stream().allMatch(d -> alreadyVisited(d) && !isNotReady(d) + && !isInError(d) && !postDeleteConditionNotMet(d)); } private class NodeReconcileExecutor extends NodeExecutor { @@ -115,24 +131,20 @@ private NodeReconcileExecutor(DependentResourceNode dependentResourceNode) } @Override - protected void doRun(DependentResourceNode dependentResourceNode, - DependentResource dependentResource) { - + protected void doRun(DependentResourceNode dependentResourceNode) { + final var dependentResource = dependentResourceNode.getDependentResource(); log.debug( "Reconciling for primary: {} node: {} ", primaryID, dependentResourceNode); ReconcileResult reconcileResult = dependentResource.reconcile(primary, context); - reconcileResults.put(dependentResource, reconcileResult); - reconciled.add(dependentResourceNode); + final var detailBuilder = createOrGetResultFor(dependentResourceNode); + detailBuilder.withReconcileResult(reconcileResult).markAsVisited(); - boolean ready = isConditionMet(dependentResourceNode.getReadyPostcondition(), - dependentResource); - if (ready) { + if (isConditionMet(dependentResourceNode.getReadyPostcondition(), dependentResourceNode)) { log.debug("Setting already reconciled for: {} primaryID: {}", dependentResourceNode, primaryID); - markAsVisited(dependentResourceNode); handleDependentsReconcile(dependentResourceNode); } else { - setAlreadyReconciledButNotReady(dependentResourceNode); + log.debug("Setting already reconciled but not ready for: {}", dependentResourceNode); } } } @@ -145,29 +157,23 @@ private NodeDeleteExecutor(DependentResourceNode dependentResourceNode) { @Override @SuppressWarnings("unchecked") - protected void doRun(DependentResourceNode dependentResourceNode, - DependentResource dependentResource) { - var deletePostCondition = dependentResourceNode.getDeletePostcondition(); - + protected void doRun(DependentResourceNode dependentResourceNode) { boolean deletePostConditionMet = true; - if (isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource)) { + if (isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode)) { // GarbageCollected status is irrelevant here, as this method is only called when a // precondition does not hold, // a deleter should be deleted even if it is otherwise garbage collected + final var dependentResource = dependentResourceNode.getDependentResource(); if (dependentResource instanceof Deleter) { ((Deleter

) dependentResource).delete(primary, context); } - deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource); + deletePostConditionMet = + isConditionMet(dependentResourceNode.getDeletePostcondition(), dependentResourceNode); } + createOrGetResultFor(dependentResourceNode).markAsVisited(); if (deletePostConditionMet) { - markAsVisited(dependentResourceNode); handleDependentDeleted(dependentResourceNode); - } else { - // updating alreadyVisited needs to be the last operation otherwise could lead to a race - // condition in handleDelete condition checks - deletePostConditionNotMet.add(dependentResourceNode); - markAsVisited(dependentResourceNode); } } } @@ -191,7 +197,6 @@ private synchronized void handleDependentsReconcile( }); } - private void handleReconcileOrActivationConditionNotMet( DependentResourceNode dependentResourceNode, boolean activationConditionMet) { @@ -206,7 +211,7 @@ private void markDependentsForDelete(DependentResourceNode dependentResour // so if the activation condition was false, this node is not meant to be deleted. var dependents = dependentResourceNode.getParents(); if (activationConditionMet) { - markedForDelete.add(dependentResourceNode); + createOrGetResultFor(dependentResourceNode).markForDelete(); if (dependents.isEmpty()) { bottomNodes.add(dependentResourceNode); } else { @@ -214,7 +219,7 @@ private void markDependentsForDelete(DependentResourceNode dependentResour } } else { // this is for an edge case when there is only one resource but that is not active - markAsVisited(dependentResourceNode); + createOrGetResultFor(dependentResourceNode).markAsVisited(); if (dependents.isEmpty()) { handleNodeExecutionFinish(dependentResourceNode); } else { @@ -226,7 +231,7 @@ private void markDependentsForDelete(DependentResourceNode dependentResour private boolean allParentsReconciledAndReady(DependentResourceNode dependentResourceNode) { return dependentResourceNode.getDependsOn().isEmpty() || dependentResourceNode.getDependsOn().stream() - .allMatch(d -> alreadyVisited(d) && !notReady.contains(d)); + .allMatch(d -> alreadyVisited(d) && !isNotReady(d)); } private boolean hasErroredParent(DependentResourceNode dependentResourceNode) { @@ -236,15 +241,6 @@ private boolean hasErroredParent(DependentResourceNode dependentResourceNo } private WorkflowReconcileResult createReconcileResult() { - return new WorkflowReconcileResult( - reconciled.stream() - .map(DependentResourceNode::getDependentResource) - .collect(Collectors.toList()), - notReady.stream() - .map(DependentResourceNode::getDependentResource) - .collect(Collectors.toList()), - getErroredDependents(), - reconcileResults); + return new WorkflowReconcileResult(asDetails()); } - } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java index 6fbd06486c..055fca3bfe 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java @@ -2,41 +2,31 @@ import java.util.List; import java.util.Map; +import java.util.Optional; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; @SuppressWarnings("rawtypes") public class WorkflowReconcileResult extends WorkflowResult { - private final List reconciledDependents; - private final List notReadyDependents; - private final Map reconcileResults; - - public WorkflowReconcileResult(List reconciledDependents, - List notReadyDependents, - Map erroredDependents, - Map reconcileResults) { - super(erroredDependents); - this.reconciledDependents = reconciledDependents; - this.notReadyDependents = notReadyDependents; - this.reconcileResults = reconcileResults; + WorkflowReconcileResult(Map> results) { + super(results); } public List getReconciledDependents() { - return reconciledDependents; + return listFilteredBy(detail -> detail.reconcileResult() != null); } public List getNotReadyDependents() { - return notReadyDependents; + return listFilteredBy(detail -> !detail.isConditionWithTypeMet(Condition.Type.READY)); } - @SuppressWarnings("unused") - public Map getReconcileResults() { - return reconcileResults; + public Optional getNotReadyDependentResult(DependentResource dependentResource, + Class expectedResultType) { + return getDependentConditionResult(dependentResource, Condition.Type.READY, expectedResultType); } public boolean allDependentResourcesReady() { - return notReadyDependents.isEmpty(); + return getNotReadyDependents().isEmpty(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java index 7014ccd5d4..900022444d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java @@ -1,36 +1,227 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; +import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import io.javaoperatorsdk.operator.AggregatedOperatorException; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; @SuppressWarnings("rawtypes") class WorkflowResult { + private final Map> results; + private Boolean hasErroredDependents; - private final Map erroredDependents; - - WorkflowResult(Map erroredDependents) { - this.erroredDependents = erroredDependents != null ? erroredDependents : Collections.emptyMap(); + WorkflowResult(Map> results) { + this.results = results; } public Map getErroredDependents() { - return erroredDependents; + return getErroredDependentsStream() + .collect(Collectors.toMap(Entry::getKey, entry -> entry.getValue().error)); + } + + private Stream>> getErroredDependentsStream() { + return results.entrySet().stream().filter(entry -> entry.getValue().error != null); + } + + protected Map> results() { + return results; + } + + /** + * Retrieves the {@link DependentResource} associated with the specified name if it exists, + * {@link Optional#empty()} otherwise. + * + * @param name the name of the {@link DependentResource} to retrieve + * @return the {@link DependentResource} associated with the specified name if it exists, + * {@link Optional#empty()} otherwise + */ + public Optional getDependentResourceByName(String name) { + if (name == null || name.isEmpty()) { + return Optional.empty(); + } + return results.keySet().stream().filter(dr -> dr.name().equals(name)).findFirst(); + } + + /** + * Retrieves the optional result of the condition with the specified type for the specified + * dependent resource. + * + * @param the expected result type of the condition + * @param dependentResourceName the dependent resource for which we want to retrieve a condition + * result + * @param conditionType the condition type which result we're interested in + * @param expectedResultType the expected result type of the condition + * @return the dependent condition result if it exists or {@link Optional#empty()} otherwise + * @throws IllegalArgumentException if a result exists but is not of the expected type + */ + public Optional getDependentConditionResult(String dependentResourceName, + Condition.Type conditionType, Class expectedResultType) { + return getDependentConditionResult( + getDependentResourceByName(dependentResourceName).orElse(null), conditionType, + expectedResultType); + } + + /** + * Retrieves the optional result of the condition with the specified type for the specified + * dependent resource. + * + * @param the expected result type of the condition + * @param dependentResource the dependent resource for which we want to retrieve a condition + * result + * @param conditionType the condition type which result we're interested in + * @param expectedResultType the expected result type of the condition + * @return the dependent condition result if it exists or {@link Optional#empty()} otherwise + * @throws IllegalArgumentException if a result exists but is not of the expected type + */ + public Optional getDependentConditionResult(DependentResource dependentResource, + Condition.Type conditionType, Class expectedResultType) { + if (dependentResource == null) { + return Optional.empty(); + } + + final var result = new Object[1]; + try { + return Optional.ofNullable(results().get(dependentResource)) + .flatMap(detail -> detail.getResultForConditionWithType(conditionType)) + .map(r -> result[0] = r.getDetail()) + .map(expectedResultType::cast); + } catch (Exception e) { + throw new IllegalArgumentException("Condition " + + "result " + result[0] + + " for Dependent " + dependentResource.name() + " doesn't match expected type " + + expectedResultType.getSimpleName(), e); + } + } + + protected List listFilteredBy( + Function filter) { + return results.entrySet().stream() + .filter(e -> filter.apply(e.getValue())) + .map(Map.Entry::getKey) + .toList(); } - @SuppressWarnings("unused") public boolean erroredDependentsExist() { - return !erroredDependents.isEmpty(); + if (hasErroredDependents == null) { + hasErroredDependents = !getErroredDependents().isEmpty(); + } + return hasErroredDependents; } public void throwAggregateExceptionIfErrorsPresent() { if (erroredDependentsExist()) { throw new AggregatedOperatorException("Exception(s) during workflow execution.", - erroredDependents.entrySet().stream() - .collect(Collectors.toMap(e -> e.getKey().name(), Entry::getValue))); + getErroredDependentsStream() + .collect(Collectors.toMap(e -> e.getKey().name(), e -> e.getValue().error))); + } + } + + @SuppressWarnings("UnusedReturnValue") + static class DetailBuilder { + private Exception error; + private ReconcileResult reconcileResult; + private DetailedCondition.Result activationConditionResult; + private DetailedCondition.Result deletePostconditionResult; + private DetailedCondition.Result readyPostconditionResult; + private DetailedCondition.Result reconcilePostconditionResult; + private boolean deleted; + private boolean visited; + private boolean markedForDelete; + + Detail build() { + return new Detail<>(error, reconcileResult, activationConditionResult, + deletePostconditionResult, readyPostconditionResult, reconcilePostconditionResult, + deleted, visited, markedForDelete); + } + + DetailBuilder withResultForCondition( + ConditionWithType conditionWithType, + DetailedCondition.Result conditionResult) { + switch (conditionWithType.type()) { + case ACTIVATION -> activationConditionResult = conditionResult; + case DELETE -> deletePostconditionResult = conditionResult; + case READY -> readyPostconditionResult = conditionResult; + case RECONCILE -> reconcilePostconditionResult = conditionResult; + default -> + throw new IllegalStateException("Unexpected condition type: " + conditionWithType); + } + return this; + } + + DetailBuilder withError(Exception error) { + this.error = error; + return this; + } + + DetailBuilder withReconcileResult(ReconcileResult reconcileResult) { + this.reconcileResult = reconcileResult; + return this; + } + + DetailBuilder markAsDeleted() { + this.deleted = true; + return this; + } + + public boolean hasError() { + return error != null; + } + + public boolean hasPostDeleteConditionNotMet() { + return deletePostconditionResult != null && !deletePostconditionResult.isSuccess(); + } + + public boolean isNotReady() { + return readyPostconditionResult != null && !readyPostconditionResult.isSuccess(); + } + + DetailBuilder markAsVisited() { + visited = true; + return this; + } + + public boolean isVisited() { + return visited; + } + + public boolean isMarkedForDelete() { + return markedForDelete; + } + + DetailBuilder markForDelete() { + markedForDelete = true; + return this; + } + } + + + record Detail(Exception error, ReconcileResult reconcileResult, + DetailedCondition.Result activationConditionResult, + DetailedCondition.Result deletePostconditionResult, + DetailedCondition.Result readyPostconditionResult, + DetailedCondition.Result reconcilePostconditionResult, + boolean deleted, boolean visited, boolean markedForDelete) { + + boolean isConditionWithTypeMet(Condition.Type conditionType) { + return getResultForConditionWithType(conditionType).map(DetailedCondition.Result::isSuccess) + .orElse(true); + } + + Optional> getResultForConditionWithType( + Condition.Type conditionType) { + return switch (conditionType) { + case ACTIVATION -> Optional.ofNullable(activationConditionResult); + case DELETE -> Optional.ofNullable(deletePostconditionResult); + case READY -> Optional.ofNullable(readyPostconditionResult); + case RECONCILE -> Optional.ofNullable(reconcilePostconditionResult); + }; } } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/NamedEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/NamedEventSource.java new file mode 100644 index 0000000000..e69de29bb2 diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ExecutionAssert.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ExecutionAssert.java index 095b090208..d857c0e9dd 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ExecutionAssert.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ExecutionAssert.java @@ -21,12 +21,13 @@ public static ExecutionAssert assertThat(List actual) { public ExecutionAssert reconciled(DependentResource... dependentResources) { for (int i = 0; i < dependentResources.length; i++) { - var rr = getReconcileRecordFor(dependentResources[i]); + final DependentResource dr = dependentResources[i]; + var rr = getReconcileRecordFor(dr); if (rr.isEmpty()) { - failWithMessage("Resource not reconciled: %s with index %d", dependentResources, i); + failWithMessage("Resource not reconciled: %s with index %d", dr, i); } else { if (rr.get().isDeleted()) { - failWithMessage("Resource deleted: %s with index %d", dependentResources, i); + failWithMessage("Resource deleted: %s with index %d", dr, i); } } } @@ -35,12 +36,13 @@ public ExecutionAssert reconciled(DependentResource... dependentResources) public ExecutionAssert deleted(DependentResource... dependentResources) { for (int i = 0; i < dependentResources.length; i++) { - var rr = getReconcileRecordFor(dependentResources[i]); + final DependentResource dr = dependentResources[i]; + var rr = getReconcileRecordFor(dr); if (rr.isEmpty()) { - failWithMessage("Resource not reconciled: %s with index %d", dependentResources, i); + failWithMessage("Resource not reconciled: %s with index %d", dr, i); } else { if (!rr.get().isDeleted()) { - failWithMessage("Resource not deleted: %s with index %d", dependentResources, i); + failWithMessage("Resource not deleted: %s with index %d", dr, i); } } } @@ -75,17 +77,18 @@ public ExecutionAssert reconciledInOrder(DependentResource... dependentRes public ExecutionAssert notReconciled(DependentResource... dependentResources) { for (int i = 0; i < dependentResources.length; i++) { - if (getActualDependentResources().contains(dependentResources[i])) { - failWithMessage("Resource was reconciled: %s with index %d", dependentResources, i); + final DependentResource dr = dependentResources[i]; + if (getActualDependentResources().contains(dr)) { + failWithMessage("Resource was reconciled: %s with index %d", dr, i); } } return this; } private void checkIfReconciled(int i, DependentResource[] dependentResources) { - if (!getActualDependentResources().contains(dependentResources[i])) { - failWithMessage("Dependent resource: %s, not reconciled on place %d", dependentResources[i], - i); + final DependentResource dr = dependentResources[i]; + if (!getActualDependentResources().contains(dr)) { + failWithMessage("Dependent resource: %s, not reconciled on place %d", dr, i); } } } 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 c66fcb02d6..dce39f2050 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 @@ -7,16 +7,18 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.AggregatedOperatorException; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; -@SuppressWarnings("rawtypes") class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { @SuppressWarnings("unchecked") @@ -27,6 +29,7 @@ class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { TestDependent dr4 = new TestDependent("DR_4"); @BeforeEach + @SuppressWarnings("unchecked") void setup() { when(mockContext.getWorkflowExecutorService()).thenReturn(executorService); when(mockContext.eventSourceRetriever()).thenReturn(mock(EventSourceRetriever.class)); @@ -182,8 +185,18 @@ void onlyOneDependsOnErroredResourceNotReconciled() { @Test void simpleReconcileCondition() { + final var result = "Some error message"; + final var unmetWithResult = new DetailedCondition() { + @Override + public Result detailedIsMet( + DependentResource dependentResource, + TestCustomResource primary, Context context) { + return Result.withResult(false, result); + } + }; + var workflow = new WorkflowBuilder() - .addDependentResource(dr1).withReconcilePrecondition(notMetCondition) + .addDependentResource(dr1).withReconcilePrecondition(unmetWithResult) .addDependentResource(dr2).withReconcilePrecondition(metCondition) .addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition) .build(); @@ -194,6 +207,8 @@ void simpleReconcileCondition() { Assertions.assertThat(res.getErroredDependents()).isEmpty(); Assertions.assertThat(res.getReconciledDependents()).containsExactlyInAnyOrder(dr2); Assertions.assertThat(res.getNotReadyDependents()).isEmpty(); + res.getDependentConditionResult(dr1, Condition.Type.RECONCILE, String.class) + .ifPresentOrElse(s -> assertEquals(result, s), org.junit.jupiter.api.Assertions::fail); } @@ -521,6 +536,7 @@ void readyConditionNotCheckedOnNonActiveDependent() { } @Test + @SuppressWarnings("unchecked") void reconcilePreconditionNotCheckedOnNonActiveDependent() { var precondition = mock(Condition.class); @@ -557,6 +573,7 @@ void deletesDependentsOfNonActiveDependentButNotTheNonActive() { } @Test + @SuppressWarnings("unchecked") void activationConditionOnlyCalledOnceOnDeleteDependents() { TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2"); var condition = mock(Condition.class); @@ -573,4 +590,80 @@ void activationConditionOnlyCalledOnceOnDeleteDependents() { verify(condition, times(1)).isMet(any(), any(), any()); } + + @Test + void resultFromReadyConditionShouldBeAvailableIfExisting() { + final var result = Integer.valueOf(42); + final var resultCondition = new DetailedCondition<>() { + @Override + public Result detailedIsMet(DependentResource dependentResource, + HasMetadata primary, Context context) { + return new Result<>() { + @Override + public Object getDetail() { + return result; + } + + @Override + public boolean isSuccess() { + return false; // force not ready + } + }; + } + }; + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withReadyPostcondition(resultCondition) + .build(); + + final var reconcileResult = workflow.reconcile(new TestCustomResource(), mockContext); + assertEquals(result, + reconcileResult.getNotReadyDependentResult(dr1, Integer.class).orElseThrow()); + } + + @Test + void shouldThrowIllegalArgumentExceptionIfTypesDoNotMatch() { + final var result = "FOO"; + final var resultCondition = new DetailedCondition<>() { + @Override + public Result detailedIsMet(DependentResource dependentResource, + HasMetadata primary, Context context) { + return new Result<>() { + @Override + public Object getDetail() { + return result; + } + + @Override + public boolean isSuccess() { + return false; // force not ready + } + }; + } + }; + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withReadyPostcondition(resultCondition) + .build(); + + final var reconcileResult = workflow.reconcile(new TestCustomResource(), mockContext); + final var expectedResultType = Integer.class; + final var e = assertThrows(IllegalArgumentException.class, + () -> reconcileResult.getNotReadyDependentResult(dr1, expectedResultType)); + final var message = e.getMessage(); + assertTrue(message.contains(dr1.name())); + assertTrue(message.contains(expectedResultType.getSimpleName())); + assertTrue(message.contains(result)); + } + + @Test + void shouldReturnEmptyIfNoConditionResultExists() { + var workflow = new WorkflowBuilder() + .addDependentResource(dr1) + .withReadyPostcondition(notMetCondition) + .build(); + + final var reconcileResult = workflow.reconcile(new TestCustomResource(), mockContext); + assertTrue(reconcileResult.getNotReadyDependentResult(dr1, Integer.class).isEmpty()); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java index c89ca53f07..ca0b883e99 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java @@ -13,11 +13,14 @@ import static org.assertj.core.api.Assertions.assertThat; class WorkflowResultTest { + private final static WorkflowResult.Detail detail = + new WorkflowResult.Detail<>(new RuntimeException(), null, null, null, null, null, false, + false, false); @Test void throwsExceptionWithoutNumberingIfAllDifferentClass() { - var res = new WorkflowResult(Map.of(new DependentA(), new RuntimeException(), - new DependentB(), new RuntimeException())); + var res = new WorkflowResult(Map.of(new DependentA(), detail, + new DependentB(), detail)); try { res.throwAggregateExceptionIfErrorsPresent(); } catch (AggregatedOperatorException e) { @@ -28,8 +31,8 @@ void throwsExceptionWithoutNumberingIfAllDifferentClass() { @Test void numbersDependentClassNamesIfMoreOfSameType() { - var res = new WorkflowResult(Map.of(new DependentA("name1"), new RuntimeException(), - new DependentA("name2"), new RuntimeException())); + var res = new WorkflowResult(Map.of(new DependentA("name1"), detail, + new DependentA("name2"), detail)); try { res.throwAggregateExceptionIfErrorsPresent(); } catch (AggregatedOperatorException e) { 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 ef48fccd6e..d6e14a1645 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 @@ -50,7 +50,7 @@ void calculatesTopLevelResources() { .addDependentResource(independentDR) .addDependentResource(dr1) .addDependentResource(dr2).dependsOn(dr1) - .build(); + .buildAsDefaultWorkflow(); Set topResources = workflow.getTopLevelDependentResources().stream() @@ -66,11 +66,11 @@ void calculatesBottomLevelResources() { var dr2 = mockDependent("dr2"); var independentDR = mockDependent("independentDR"); - Workflow workflow = new WorkflowBuilder() + final var workflow = new WorkflowBuilder() .addDependentResource(independentDR) .addDependentResource(dr1) .addDependentResource(dr2).dependsOn(dr1) - .build(); + .buildAsDefaultWorkflow(); Set bottomResources = workflow.getBottomLevelResource().stream() 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 f60f2b6fde..0473c50ddb 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowAllFeatureIT.java @@ -10,9 +10,11 @@ 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.ConfigMapReconcileCondition; import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureCustomResource; import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureReconciler; import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureSpec; +import io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureStatus; import static io.javaoperatorsdk.operator.sample.workflowallfeature.ConfigMapDependentResource.READY_TO_DELETE_ANNOTATION; import static org.assertj.core.api.Assertions.assertThat; @@ -39,6 +41,8 @@ void configMapNotReconciledUntilDeploymentReady() { .isPositive(); assertThat(operator.get(Deployment.class, RESOURCE_NAME)).isNotNull(); assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); + assertThat(getPrimaryStatus().getMsgFromCondition()) + .isEqualTo(ConfigMapReconcileCondition.NOT_RECONCILED_YET); }); await().atMost(ONE_MINUTE).untilAsserted(() -> { @@ -47,13 +51,20 @@ void configMapNotReconciledUntilDeploymentReady() { .getNumberOfReconciliationExecution()) .isGreaterThan(1); assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); - assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) - .getStatus().getReady()).isTrue(); + final var primaryStatus = getPrimaryStatus(); + assertThat(primaryStatus.getReady()).isTrue(); + assertThat(primaryStatus.getMsgFromCondition()) + .isEqualTo(ConfigMapReconcileCondition.CREATE_SET); }); markConfigMapForDelete(); } + private WorkflowAllFeatureStatus getPrimaryStatus() { + return operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) + .getStatus(); + } + @Test void configMapNotReconciledIfReconcileConditionNotMet() { @@ -61,8 +72,7 @@ void configMapNotReconciledIfReconcileConditionNotMet() { await().atMost(ONE_MINUTE).untilAsserted(() -> { assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNull(); - assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) - .getStatus().getReady()).isTrue(); + assertThat(getPrimaryStatus().getReady()).isTrue(); }); resource.getSpec().setCreateConfigMap(true); @@ -70,8 +80,7 @@ void configMapNotReconciledIfReconcileConditionNotMet() { await().untilAsserted(() -> { assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); - assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) - .getStatus().getReady()).isTrue(); + assertThat(getPrimaryStatus().getReady()).isTrue(); }); } @@ -81,10 +90,9 @@ void configMapNotDeletedUntilNotMarked() { var resource = operator.create(customResource(true)); await().atMost(ONE_MINUTE).untilAsserted(() -> { - assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME).getStatus()) + assertThat(getPrimaryStatus()) .isNotNull(); - assertThat(operator.get(WorkflowAllFeatureCustomResource.class, RESOURCE_NAME) - .getStatus().getReady()).isTrue(); + assertThat(getPrimaryStatus().getReady()).isTrue(); assertThat(operator.get(ConfigMap.class, RESOURCE_NAME)).isNotNull(); }); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index b69aae6fc7..127282daf2 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -19,7 +19,13 @@ import io.javaoperatorsdk.operator.api.config.dependent.ConfigurationConverter; import io.javaoperatorsdk.operator.api.config.dependent.Configured; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Constants; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.Workflow; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java index 8714b8c31f..5f8b20ffa8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java @@ -74,7 +74,9 @@ public List> prepareEv .withInformerConfiguration(c -> c .withLabelSelector("integrationtest = " + this.getClass().getSimpleName())) .build(); - final var informerEventSource = new InformerEventSource<>(informerConfiguration, context); + final var informerEventSource = + new InformerEventSource( + informerConfiguration, context); this.configMapDR.setEventSource(informerEventSource); return List.of(informerEventSource); 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 index b3d9d7a541..dfdc0ad8a4 100644 --- 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 @@ -3,17 +3,20 @@ 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; +import io.javaoperatorsdk.operator.processing.dependent.workflow.DetailedCondition; public class ConfigMapReconcileCondition - implements Condition { + implements DetailedCondition { + + public static final String CREATE_SET = "create set"; + public static final String CREATE_NOT_SET = "create not set"; + public static final String NOT_RECONCILED_YET = "Not reconciled yet"; @Override - public boolean isMet( + public Result detailedIsMet( DependentResource dependentResource, - WorkflowAllFeatureCustomResource primary, - Context context) { - - return primary.getSpec().isCreateConfigMap(); + WorkflowAllFeatureCustomResource primary, Context context) { + final var createConfigMap = primary.getSpec().isCreateConfigMap(); + return Result.withResult(createConfigMap, createConfigMap ? CREATE_SET : CREATE_NOT_SET); } } 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 f5c14d6f96..61b42e0c64 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 @@ -2,8 +2,16 @@ import java.util.concurrent.atomic.AtomicInteger; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Cleaner; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.Workflow; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import static io.javaoperatorsdk.operator.sample.workflowallfeature.WorkflowAllFeatureReconciler.DEPLOYMENT_NAME; @@ -33,11 +41,15 @@ public UpdateControl reconcile( if (resource.getStatus() == null) { resource.setStatus(new WorkflowAllFeatureStatus()); } + final var reconcileResult = context.managedWorkflowAndDependentResourceContext() + .getWorkflowReconcileResult(); + final var msgFromCondition = reconcileResult.getDependentConditionResult( + DependentResource.defaultNameFor(ConfigMapDependentResource.class), + Condition.Type.RECONCILE, String.class) + .orElse(ConfigMapReconcileCondition.NOT_RECONCILED_YET); resource.getStatus() - .setReady( - context.managedWorkflowAndDependentResourceContext() - .getWorkflowReconcileResult() - .allDependentResourcesReady()); + .withReady(reconcileResult.allDependentResourcesReady()) + .withMsgFromCondition(msgFromCondition); return UpdateControl.patchStatus(resource); } 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 11d0798fca..c53931b206 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 @@ -3,13 +3,24 @@ public class WorkflowAllFeatureStatus { private Boolean ready; + private String msgFromCondition; public Boolean getReady() { return ready; } - public WorkflowAllFeatureStatus setReady(Boolean ready) { + public String getMsgFromCondition() { + return msgFromCondition; + } + + public WorkflowAllFeatureStatus withReady(Boolean ready) { this.ready = ready; return this; } + + @SuppressWarnings("UnusedReturnValue") + public WorkflowAllFeatureStatus withMsgFromCondition(String msgFromCondition) { + this.msgFromCondition = msgFromCondition; + return this; + } } diff --git a/pom.xml b/pom.xml index edb93df309..c6c0f2fe09 100644 --- a/pom.xml +++ b/pom.xml @@ -61,14 +61,14 @@ jdk 5.10.1 - 6.13.1 + 6.13.1 2.0.12 2.23.1 5.12.0 3.14.0 0.21.0 1.13.0 - 3.26.3 + 3.26.3 4.2.0 2.7.3 1.13.2 @@ -80,12 +80,12 @@ 2.11 3.12.1 - 3.3.1 + 3.3.1 3.7.0 3.3.1 3.3.1 - 3.4.2 - 3.4.0 + 3.4.2 + 3.4.0 3.2.4 1.7.0 3.0.0