wip

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

Author: csviri

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,44 +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 <R> void markDependentsForDelete(
-      DependentResourceNode<R, P> dependentResourceNode, Set<DependentResourceNode> bottomNodes) {
-    boolean activationConditionMet =
-        isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode);
-    markDependentsForDelete(dependentResourceNode, bottomNodes, activationConditionMet);
-  }
-
   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
-      registerOrDeregisterEventSourceBasedOnActivation(true, dependentResourceNode);
-      createOrGetResultFor(dependentResourceNode).markForDelete();
-      if (dependents.isEmpty()) {
-        bottomNodes.add(dependentResourceNode);
-      } else {
-        dependents.forEach(d -> markDependentsForDelete(d, bottomNodes));
-      }
+    createOrGetResultFor(dependentResourceNode).markForDelete();
+    if (dependents.isEmpty()) {
+      bottomNodes.add(dependentResourceNode);
     } else {
-      if (dependents.isEmpty()) {
-        createOrGetResultFor(dependentResourceNode).markAsVisited();
-        handleNodeExecutionFinish(dependentResourceNode);
-      } else {
-        createOrGetResultFor(dependentResourceNode).markForDelete();
-        dependents.forEach(d -> markDependentsForDelete(d, bottomNodes));
-      }
+      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,41 @@ 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 resultFromReadyConditionShouldBeAvailableIfExisting() {
     final var result = Integer.valueOf(42);