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 32c0bddbc0..eb0f3f7e83 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,5 +1,6 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -36,7 +37,7 @@ protected AbstractWorkflowExecutor(DefaultWorkflow

workflow, P primary, Conte this.context = context; this.primaryID = ResourceID.fromResource(primary); executorService = context.getWorkflowExecutorService(); - results = new ConcurrentHashMap<>(workflow.getDependentResourcesByName().size()); + results = new HashMap<>(workflow.getDependentResourcesByName().size()); } protected abstract Logger logger(); @@ -84,13 +85,13 @@ protected boolean isMarkedForDelete(DependentResourceNode drn) { return getResultFlagFor(drn, WorkflowResult.DetailBuilder::isMarkedForDelete); } - protected WorkflowResult.DetailBuilder createOrGetResultFor( + protected synchronized WorkflowResult.DetailBuilder createOrGetResultFor( DependentResourceNode dependentResourceNode) { return results.computeIfAbsent(dependentResourceNode, unused -> new WorkflowResult.DetailBuilder()); } - protected Optional> getResultFor( + protected synchronized Optional> getResultFor( DependentResourceNode dependentResourceNode) { return Optional.ofNullable(results.get(dependentResourceNode)); } @@ -115,8 +116,8 @@ protected synchronized void handleExceptionInExecutor( createOrGetResultFor(dependentResourceNode).withError(e); } - protected boolean isNotReady(DependentResourceNode dependentResourceNode) { - return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::isNotReady); + protected boolean isReady(DependentResourceNode dependentResourceNode) { + return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::isReady); } protected boolean isInError(DependentResourceNode dependentResourceNode) { @@ -132,15 +133,17 @@ protected synchronized void handleNodeExecutionFinish( } } - @SuppressWarnings("unchecked") + @SuppressWarnings({"unchecked", "OptionalUsedAsFieldOrParameterType"}) 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); + synchronized (this) { + results.computeIfAbsent(dependentResource, unused -> new WorkflowResult.DetailBuilder()) + .withResultForCondition(c, r); + } return r; }).orElse(DetailedCondition.Result.metWithoutResult).isSuccess(); } @@ -170,7 +173,7 @@ protected void registerOrDeregisterEventSourceBasedOnActivation( } } - protected Map> asDetails() { + protected synchronized 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/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index b130e8fd5f..962d01c8e5 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 @@ -120,7 +120,7 @@ private synchronized void handleDelete(DependentResourceNode dependentResourceNo private boolean allDependentsDeletedAlready(DependentResourceNode dependentResourceNode) { var dependents = dependentResourceNode.getParents(); - return dependents.stream().allMatch(d -> alreadyVisited(d) && !isNotReady(d) + return dependents.stream().allMatch(d -> alreadyVisited(d) && isReady(d) && !isInError(d) && !postDeleteConditionNotMet(d)); } @@ -231,7 +231,7 @@ private void markDependentsForDelete(DependentResourceNode dependentResour private boolean allParentsReconciledAndReady(DependentResourceNode dependentResourceNode) { return dependentResourceNode.getDependsOn().isEmpty() || dependentResourceNode.getDependsOn().stream() - .allMatch(d -> alreadyVisited(d) && !isNotReady(d)); + .allMatch(d -> alreadyVisited(d) && isReady(d)); } private boolean hasErroredParent(DependentResourceNode dependentResourceNode) { 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 900022444d..1b278fed77 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 @@ -178,8 +178,8 @@ public boolean hasPostDeleteConditionNotMet() { return deletePostconditionResult != null && !deletePostconditionResult.isSuccess(); } - public boolean isNotReady() { - return readyPostconditionResult != null && !readyPostconditionResult.isSuccess(); + public boolean isReady() { + return readyPostconditionResult == null || readyPostconditionResult.isSuccess(); } DetailBuilder markAsVisited() { 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 aedfe44df9..f399655041 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 @@ -449,7 +449,8 @@ void readyConditionNotMetInOneParent() { void diamondShareWithReadyCondition() { var workflow = new WorkflowBuilder() .addDependentResource(dr1) - .addDependentResourceAndConfigure(dr2).toDependOn(dr1) + .addDependentResourceAndConfigure(dr2) + .toDependOn(dr1) .withReadyPostcondition(notMetCondition) .addDependentResourceAndConfigure(dr3).toDependOn(dr1) .addDependentResourceAndConfigure(dr4).toDependOn(dr2, dr3)