From c9461b7534d949dab392b2811e9927b7049cd7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 26 Jan 2024 15:17:40 +0100 Subject: [PATCH 1/2] fix: cleanup of resources with activation condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../workflow/AbstractWorkflowExecutor.java | 19 +++++++++++++++++++ .../workflow/WorkflowCleanupExecutor.java | 1 + .../workflow/WorkflowReconcileExecutor.java | 18 ------------------ 3 files changed, 20 insertions(+), 18 deletions(-) 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 c546570e34..56e5e17d07 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 @@ -128,4 +128,23 @@ protected void submit(DependentResourceNode dependentResourceNode, logger().debug("Submitted to {}: {} primaryID: {}", operation, dependentResourceNode, primaryID); } + + protected void registerOrDeregisterEventSourceBasedOnActivation( + boolean activationConditionMet, + DependentResourceNode 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()); + } + } + } } 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 74ae47100e..dda4db6729 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 @@ -70,6 +70,7 @@ protected void doRun(DependentResourceNode dependentResourceNode, var active = isConditionMet(dependentResourceNode.getActivationCondition(), dependentResource); + registerOrDeregisterEventSourceBasedOnActivation(active, dependentResourceNode); if (dependentResource.isDeletable() && active) { ((Deleter

) dependentResource).delete(primary, context); 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 33aad99811..2da0bd7525 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 @@ -78,24 +78,6 @@ private synchronized void handleReconcile(DependentResourceNode depend } } - private void registerOrDeregisterEventSourceBasedOnActivation(boolean activationConditionMet, - DependentResourceNode 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); From de6005042b5bdfbebc72d4d34bf6358ddfe75cee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 29 Jan 2024 14:33:16 +0100 Subject: [PATCH 2/2] integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../AbstractWorkflowExecutorTest.java | 4 +- .../workflow/WorkflowCleanupExecutorTest.java | 18 +++++ .../operator/WorkflowActivationCleanupIT.java | 79 +++++++++++++++++++ .../ConfigMapDependentResource.java | 31 ++++++++ .../TestActivcationCondition.java | 18 +++++ ...rkflowActivationCleanupCustomResource.java | 17 ++++ .../WorkflowActivationCleanupReconciler.java | 27 +++++++ .../WorkflowActivationCleanupSpec.java | 14 ++++ 8 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationCleanupIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/ConfigMapDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/TestActivcationCondition.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupSpec.java diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java index 6cfdb3dc63..941c3fa434 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutorTest.java +++ b/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 { + public class TestDeleterDependent extends TestDependent + implements Creator, Deleter { public TestDeleterDependent(String name) { super(name); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java index 3b34dcf458..ed6ba5f80c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java +++ b/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 diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationCleanupIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowActivationCleanupIT.java new file mode 100644 index 0000000000..fe1b72cc5a --- /dev/null +++ b/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(); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/ConfigMapDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/ConfigMapDependentResource.java new file mode 100644 index 0000000000..241bff64ce --- /dev/null +++ b/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 { + + public static final String DATA_KEY = "data"; + + public ConfigMapDependentResource() { + super(ConfigMap.class); + } + + @Override + protected ConfigMap desired(WorkflowActivationCleanupCustomResource primary, + Context 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; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/TestActivcationCondition.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/TestActivcationCondition.java new file mode 100644 index 0000000000..ff215b7d72 --- /dev/null +++ b/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 { + + @Override + public boolean isMet( + DependentResource dependentResource, + WorkflowActivationCleanupCustomResource primary, + Context context) { + return true; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupCustomResource.java new file mode 100644 index 0000000000..f73b484d86 --- /dev/null +++ b/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 + implements Namespaced { + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupReconciler.java new file mode 100644 index 0000000000..3f2fba15c5 --- /dev/null +++ b/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, + Cleaner { + + @Override + public UpdateControl reconcile( + WorkflowActivationCleanupCustomResource resource, + Context context) { + + return UpdateControl.noUpdate(); + } + + @Override + public DeleteControl cleanup(WorkflowActivationCleanupCustomResource resource, + Context context) { + return DeleteControl.defaultDelete(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowactivationcleanup/WorkflowActivationCleanupSpec.java new file mode 100644 index 0000000000..23ea4cda3e --- /dev/null +++ b/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; + } +}