fix: handling delete non-active dependent when parent reconcile condition false

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java

@@ -157,6 +157,13 @@ protected <R> boolean isConditionMet(
     return condition
         .map(
             c -> {
+              synchronized (this) {
+                Optional<DetailedCondition.Result> existingResult =
+                    createOrGetResultFor(dependentResource).getConditionResult(c.type());
+                if (existingResult.isPresent()) {
+                  return existingResult.get();
+                }
+              }
               final DetailedCondition.Result<?> r = c.detailedIsMet(dr, primary, context);
               synchronized (this) {
                 results

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/BaseWorkflowResult.java

@@ -172,6 +172,15 @@ public boolean hasPostDeleteConditionNotMet() {
       return deletePostconditionResult != null && !deletePostconditionResult.isSuccess();
     }
 
+    public Optional<DetailedCondition.Result> getConditionResult(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);
+      };
+    }
+
     public boolean isReady() {
       return readyPostconditionResult == null || readyPostconditionResult.isSuccess();
     }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java

@@ -100,7 +100,7 @@ private synchronized <R> void handleReconcile(DependentResourceNode<R, P> depend
           isConditionMet(dependentResourceNode.getReconcilePrecondition(), dependentResourceNode);
     }
     if (!reconcileConditionMet || !activationConditionMet) {
-      handleReconcileOrActivationConditionNotMet(dependentResourceNode, activationConditionMet);
+      handleReconcileOrActivationConditionNotMet(dependentResourceNode);
     } else {
       submit(dependentResourceNode, new NodeReconcileExecutor<>(dependentResourceNode), RECONCILE);
     }
@@ -183,7 +183,10 @@ private NodeDeleteExecutor(DependentResourceNode<R, P> dependentResourceNode) {
     @SuppressWarnings("unchecked")
     protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
       boolean deletePostConditionMet = true;
-      if (isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode)) {
+      var active =
+          isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode);
+      registerOrDeregisterEventSourceBasedOnActivation(active, dependentResourceNode);
+      if (active) {
         // 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
@@ -194,7 +197,6 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
         deletePostConditionMet =
             isConditionMet(dependentResourceNode.getDeletePostcondition(), dependentResourceNode);
       }
-
       createOrGetResultFor(dependentResourceNode).markAsVisited();
       if (deletePostConditionMet) {
         handleDependentDeleted(dependentResourceNode);
@@ -232,38 +234,20 @@ private synchronized void handleDependentsReconcile(
   }
 
   private void handleReconcileOrActivationConditionNotMet(
-      DependentResourceNode<?, P> dependentResourceNode, boolean activationConditionMet) {
+      DependentResourceNode<?, P> dependentResourceNode) {
     Set<DependentResourceNode> bottomNodes = new HashSet<>();
-    markDependentsForDelete(dependentResourceNode, bottomNodes, activationConditionMet);
+    markDependentsForDelete(dependentResourceNode, bottomNodes);
     bottomNodes.forEach(this::handleDelete);
   }
 
   private void markDependentsForDelete(
-      DependentResourceNode<?, P> dependentResourceNode,
-      Set<DependentResourceNode> bottomNodes,
-      boolean activationConditionMet) {
-    // this is a check so the activation condition is not evaluated twice,
-    // so if the activation condition was false, this node is not meant to be deleted.
+      DependentResourceNode<?, P> dependentResourceNode, Set<DependentResourceNode> bottomNodes) {
     var dependents = dependentResourceNode.getParents();
-    if (activationConditionMet) {
-      // make sure we register the dependent's event source if it hasn't been added already
-      // this might be needed in corner cases such as
-      // https://github.com/operator-framework/java-operator-sdk/issues/3249
-      registerOrDeregisterEventSourceBasedOnActivation(true, dependentResourceNode);
-      createOrGetResultFor(dependentResourceNode).markForDelete();
-      if (dependents.isEmpty()) {
-        bottomNodes.add(dependentResourceNode);
-      } else {
-        dependents.forEach(d -> markDependentsForDelete(d, bottomNodes, true));
-      }
+    createOrGetResultFor(dependentResourceNode).markForDelete();
+    if (dependents.isEmpty()) {
+      bottomNodes.add(dependentResourceNode);
     } else {
-      // this is for an edge case when there is only one resource but that is not active
-      createOrGetResultFor(dependentResourceNode).markAsVisited();
-      if (dependents.isEmpty()) {
-        handleNodeExecutionFinish(dependentResourceNode);
-      } else {
-        dependents.forEach(d -> markDependentsForDelete(d, bottomNodes, true));
-      }
+      dependents.forEach(d -> markDependentsForDelete(d, bottomNodes));
     }
   }
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

@@ -139,6 +139,9 @@ protected void handleEvent(
 
   @Override
   public synchronized void start() {
+    if (isRunning()) {
+      return;
+    }
     super.start();
     // this makes sure that on first reconciliation all resources are
     // present on the index

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java

@@ -45,6 +45,7 @@ class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest {
   Context<TestCustomResource> mockContext = spy(Context.class);
 
   ExecutorService executorService = Executors.newCachedThreadPool();
+  EventSourceRetriever eventSourceRetriever = mock(EventSourceRetriever.class);
 
   TestDependent dr3 = new TestDependent("DR_3");
   TestDependent dr4 = new TestDependent("DR_4");
@@ -56,7 +57,7 @@ void setup(TestInfo testInfo) {
     when(mockContext.managedWorkflowAndDependentResourceContext())
         .thenReturn(mock(ManagedWorkflowAndDependentResourceContext.class));
     when(mockContext.getWorkflowExecutorService()).thenReturn(executorService);
-    when(mockContext.eventSourceRetriever()).thenReturn(mock(EventSourceRetriever.class));
+    when(mockContext.eventSourceRetriever()).thenReturn(eventSourceRetriever);
   }
 
   @Test
@@ -693,6 +694,52 @@ void activationConditionOnlyCalledOnceOnDeleteDependents() {
     verify(condition, times(1)).isMet(any(), any(), any());
   }
 
+  @Test
+  @SuppressWarnings("unchecked")
+  void activationConditionEventSourceRegistrationWithParentWithFalsePrecondition() {
+    var workflow =
+        new WorkflowBuilder<TestCustomResource>()
+            .addDependentResourceAndConfigure(dr1)
+            .withReconcilePrecondition(notMetCondition)
+            .addDependentResourceAndConfigure(drDeleter)
+            .withActivationCondition(notMetCondition)
+            .dependsOn(dr1)
+            .build();
+
+    workflow.reconcile(new TestCustomResource(), mockContext);
+    assertThat(executionHistory).notReconciled(dr1, drDeleter);
+    verify(eventSourceRetriever, never()).dynamicallyRegisterEventSource(any());
+    verify(eventSourceRetriever, times(1)).dynamicallyDeRegisterEventSource(any());
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  void activationConditionEventSourceRegistration() {
+    var workflow =
+        new WorkflowBuilder<TestCustomResource>()
+            .addDependentResourceAndConfigure(dr1)
+            .addDependentResourceAndConfigure(drDeleter)
+            .withActivationCondition(notMetCondition)
+            .dependsOn(dr1)
+            .build();
+
+    workflow.reconcile(new TestCustomResource(), mockContext);
+    assertThat(executionHistory).reconciled(dr1).notReconciled(drDeleter);
+    verify(eventSourceRetriever, never()).dynamicallyRegisterEventSource(any());
+    verify(eventSourceRetriever, atLeast(1)).dynamicallyDeRegisterEventSource(any());
+  }
+
+  @Test
+  void oneDependentWithActivationCondition() {
+    var workflow =
+        new WorkflowBuilder<TestCustomResource>()
+            .addDependentResourceAndConfigure(dr1)
+            .withActivationCondition(notMetCondition)
+            .build();
+    workflow.reconcile(new TestCustomResource(), mockContext);
+    assertThat(executionHistory).notReconciled(dr1);
+  }
+
   @Test
   void resultFromReadyConditionShouldBeAvailableIfExisting() {
     final var result = Integer.valueOf(42);