fix: cleanup of resources with activation condition (#2223)

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

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

@@ -128,4 +128,23 @@ protected <R> void submit(DependentResourceNode<R, P> dependentResourceNode,
     logger().debug("Submitted to {}: {} primaryID: {}", operation, dependentResourceNode,
         primaryID);
   }
+
+  protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
+      boolean activationConditionMet,
+      DependentResourceNode<R, P> dependentResourceNode) {
+    if (dependentResourceNode.getActivationCondition().isPresent()) {
+      if (activationConditionMet) {
+        var eventSource =
+            dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever()
+                .eventSourceContextForDynamicRegistration());
+        var es = eventSource.orElseThrow();
+        context.eventSourceRetriever()
+            .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
+
+      } else {
+        context.eventSourceRetriever()
+            .dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
+      }
+    }
+  }
 }

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

@@ -70,6 +70,7 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode,
 
       var active =
           isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource);
+      registerOrDeregisterEventSourceBasedOnActivation(active, dependentResourceNode);
 
       if (dependentResource.isDeletable() && active) {
         ((Deleter<P>) dependentResource).delete(primary, context);

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

@@ -78,24 +78,6 @@ private synchronized <R> void handleReconcile(DependentResourceNode<R, P> depend
     }
   }
 
-  private <R> void registerOrDeregisterEventSourceBasedOnActivation(boolean activationConditionMet,
-      DependentResourceNode<R, P> dependentResourceNode) {
-    if (dependentResourceNode.getActivationCondition().isPresent()) {
-      if (activationConditionMet) {
-        var eventSource =
-            dependentResourceNode.getDependentResource().eventSource(context.eventSourceRetriever()
-                .eventSourceContextForDynamicRegistration());
-        var es = eventSource.orElseThrow();
-        context.eventSourceRetriever()
-            .dynamicallyRegisterEventSource(dependentResourceNode.getName(), es);
-
-      } else {
-        context.eventSourceRetriever()
-            .dynamicallyDeRegisterEventSource(dependentResourceNode.getName());
-      }
-    }
-  }
-
   private synchronized void handleDelete(DependentResourceNode dependentResourceNode) {
     log.debug("Submitting for delete: {}", dependentResourceNode);
 

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

@@ -11,6 +11,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+import io.javaoperatorsdk.operator.processing.dependent.Creator;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
@@ -55,7 +56,8 @@ public String toString() {
     }
   }
 
-  public class TestDeleterDependent extends TestDependent implements Deleter<TestCustomResource> {
+  public class TestDeleterDependent extends TestDependent
+      implements Creator<ConfigMap, TestCustomResource>, Deleter<TestCustomResource> {
 
     public TestDeleterDependent(String name) {
       super(name);

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

@@ -7,8 +7,14 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.javaoperatorsdk.operator.AggregatedOperatorException;
+import io.javaoperatorsdk.operator.MockKubernetesClient;
+import io.javaoperatorsdk.operator.api.config.ConfigurationService;
+import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
+import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
 import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat;
@@ -29,7 +35,19 @@ class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest {
 
   @BeforeEach
   void setup() {
+    var eventSourceContextMock = mock(EventSourceContext.class);
+    var eventSourceRetrieverMock = mock(EventSourceRetriever.class);
+    var mockControllerConfig = mock(ControllerConfiguration.class);
+    when(eventSourceRetrieverMock.eventSourceContextForDynamicRegistration())
+        .thenReturn(eventSourceContextMock);
+    var client = MockKubernetesClient.client(ConfigMap.class);
+    when(eventSourceContextMock.getClient()).thenReturn(client);
+    when(eventSourceContextMock.getControllerConfiguration()).thenReturn(mockControllerConfig);
+    when(mockControllerConfig.getConfigurationService())
+        .thenReturn(mock(ConfigurationService.class));
+
     when(mockContext.getWorkflowExecutorService()).thenReturn(executorService);
+    when(mockContext.eventSourceRetriever()).thenReturn(eventSourceRetrieverMock);
   }
 
   @Test

operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationCleanupIT.java

@@ -0,0 +1,79 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.jupiter.api.*;
+
+import io.fabric8.kubernetes.api.model.Namespace;
+import io.fabric8.kubernetes.api.model.NamespaceBuilder;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.KubernetesClientBuilder;
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.workflowactivationcleanup.WorkflowActivationCleanupCustomResource;
+import io.javaoperatorsdk.operator.sample.workflowactivationcleanup.WorkflowActivationCleanupReconciler;
+import io.javaoperatorsdk.operator.sample.workflowactivationcleanup.WorkflowActivationCleanupSpec;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class WorkflowActivationCleanupIT {
+
+  private final KubernetesClient client = new KubernetesClientBuilder().build();
+  private Operator operator;
+
+  private String testNamespace;
+
+  @BeforeEach
+  void beforeEach(TestInfo testInfo) {
+    LocallyRunOperatorExtension.applyCrd(WorkflowActivationCleanupCustomResource.class,
+        client);
+
+    testInfo.getTestMethod()
+        .ifPresent(method -> testNamespace = KubernetesResourceUtil.sanitizeName(method.getName()));
+    client.namespaces().resource(testNamespace(testNamespace)).create();
+    operator = new Operator(o -> o.withCloseClientOnStop(false));
+    operator.register(new WorkflowActivationCleanupReconciler(),
+        o -> o.settingNamespaces(testNamespace));
+  }
+
+  @AfterEach
+  void stopOperator() {
+    client.namespaces().withName(testNamespace).delete();
+    await().untilAsserted(() -> {
+      var ns = client.namespaces().withName(testNamespace).get();
+      assertThat(ns).isNull();
+    });
+    operator.stop();
+  }
+
+  @Test
+  void testCleanupOnMarkedResourceOnOperatorStartup() {
+    var resource = client.resource(testResourceWithFinalizer()).create();
+    client.resource(resource).delete();
+    operator.start();
+
+    await().untilAsserted(() -> {
+      var res = client.resource(resource).get();
+      assertThat(res).isNull();
+    });
+  }
+
+  private WorkflowActivationCleanupCustomResource testResourceWithFinalizer() {
+    var resource = new WorkflowActivationCleanupCustomResource();
+    resource.setMetadata(new ObjectMetaBuilder()
+        .withName("test1")
+        .withFinalizers("workflowactivationcleanupcustomresources.sample.javaoperatorsdk/finalizer")
+        .withNamespace(testNamespace)
+        .build());
+    resource.setSpec(new WorkflowActivationCleanupSpec());
+    resource.getSpec().setValue("val1");
+    return resource;
+  }
+
+  private Namespace testNamespace(String name) {
+    return new NamespaceBuilder().withMetadata(new ObjectMetaBuilder()
+        .withName(name)
+        .build()).build();
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/ConfigMapDependentResource.java

@@ -0,0 +1,31 @@
+package io.javaoperatorsdk.operator.sample.workflowactivationcleanup;
+
+import java.util.Map;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDNoGCKubernetesDependentResource;
+
+public class ConfigMapDependentResource
+    extends
+    CRUDNoGCKubernetesDependentResource<ConfigMap, WorkflowActivationCleanupCustomResource> {
+
+  public static final String DATA_KEY = "data";
+
+  public ConfigMapDependentResource() {
+    super(ConfigMap.class);
+  }
+
+  @Override
+  protected ConfigMap desired(WorkflowActivationCleanupCustomResource primary,
+      Context<WorkflowActivationCleanupCustomResource> context) {
+    ConfigMap configMap = new ConfigMap();
+    configMap.setMetadata(new ObjectMetaBuilder()
+        .withName(primary.getMetadata().getName())
+        .withNamespace(primary.getMetadata().getNamespace())
+        .build());
+    configMap.setData(Map.of(DATA_KEY, primary.getSpec().getValue()));
+    return configMap;
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/TestActivcationCondition.java

@@ -0,0 +1,18 @@
+package io.javaoperatorsdk.operator.sample.workflowactivationcleanup;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
+
+public class TestActivcationCondition
+    implements Condition<ConfigMap, WorkflowActivationCleanupCustomResource> {
+
+  @Override
+  public boolean isMet(
+      DependentResource<ConfigMap, WorkflowActivationCleanupCustomResource> dependentResource,
+      WorkflowActivationCleanupCustomResource primary,
+      Context<WorkflowActivationCleanupCustomResource> context) {
+    return true;
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupCustomResource.java

@@ -0,0 +1,17 @@
+package io.javaoperatorsdk.operator.sample.workflowactivationcleanup;
+
+import io.fabric8.kubernetes.api.model.Namespaced;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.model.annotation.Group;
+import io.fabric8.kubernetes.model.annotation.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@ShortNames("wacc")
+public class WorkflowActivationCleanupCustomResource
+    extends CustomResource<WorkflowActivationCleanupSpec, Void>
+    implements Namespaced {
+
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupReconciler.java

@@ -0,0 +1,27 @@
+package io.javaoperatorsdk.operator.sample.workflowactivationcleanup;
+
+import io.javaoperatorsdk.operator.api.reconciler.*;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
+
+@ControllerConfiguration(dependents = {
+    @Dependent(type = ConfigMapDependentResource.class,
+        activationCondition = TestActivcationCondition.class),
+})
+public class WorkflowActivationCleanupReconciler
+    implements Reconciler<WorkflowActivationCleanupCustomResource>,
+    Cleaner<WorkflowActivationCleanupCustomResource> {
+
+  @Override
+  public UpdateControl<WorkflowActivationCleanupCustomResource> reconcile(
+      WorkflowActivationCleanupCustomResource resource,
+      Context<WorkflowActivationCleanupCustomResource> context) {
+
+    return UpdateControl.noUpdate();
+  }
+
+  @Override
+  public DeleteControl cleanup(WorkflowActivationCleanupCustomResource resource,
+      Context<WorkflowActivationCleanupCustomResource> context) {
+    return DeleteControl.defaultDelete();
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupSpec.java

@@ -0,0 +1,14 @@
+package io.javaoperatorsdk.operator.sample.workflowactivationcleanup;
+
+public class WorkflowActivationCleanupSpec {
+
+  private String value;
+
+  public String getValue() {
+    return value;
+  }
+
+  public void setValue(String value) {
+    this.value = value;
+  }
+}