From 5887fbc3152053cf8b9f90857fde5f714a4d5129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 4 Feb 2025 15:14:25 +0100 Subject: [PATCH 1/5] fix: explicit workflow invovation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../java/io/javaoperatorsdk/operator/processing/Controller.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index da9843ec40..5b3781fa44 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -195,7 +195,7 @@ public DeleteControl execute() throws Exception { // The cleanup is called also when explicit invocation is true, but the cleaner is not // implemented - if (managedWorkflow.hasCleaner() || !explicitWorkflowInvocation) { + if (managedWorkflow.hasCleaner() && !explicitWorkflowInvocation) { workflowCleanupResult = managedWorkflow.cleanup(resource, context); } From 51f7d97dcdf0e977f9ead7c0f59286343902b2f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 4 Feb 2025 15:24:06 +0100 Subject: [PATCH 2/5] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../java/io/javaoperatorsdk/operator/processing/Controller.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 5b3781fa44..de55a1a5b4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -195,7 +195,7 @@ public DeleteControl execute() throws Exception { // The cleanup is called also when explicit invocation is true, but the cleaner is not // implemented - if (managedWorkflow.hasCleaner() && !explicitWorkflowInvocation) { + if (managedWorkflow.hasCleaner() && (!explicitWorkflowInvocation || !isCleaner)) { workflowCleanupResult = managedWorkflow.cleanup(resource, context); } From c2d039f9a52933265b9dfb419aa18c1fb8e278c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 4 Feb 2025 15:29:08 +0100 Subject: [PATCH 3/5] update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../io/javaoperatorsdk/operator/processing/Controller.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index de55a1a5b4..491ac7fb01 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -194,7 +194,8 @@ public DeleteControl execute() throws Exception { WorkflowCleanupResult workflowCleanupResult = null; // The cleanup is called also when explicit invocation is true, but the cleaner is not - // implemented + // implemented, also in case when explicit invocation is false, but there is cleaner + // implemented. if (managedWorkflow.hasCleaner() && (!explicitWorkflowInvocation || !isCleaner)) { workflowCleanupResult = managedWorkflow.cleanup(resource, context); } From 9616c7432a28625f19ffb0e63c3402b4bb1061aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 5 Feb 2025 12:16:20 +0100 Subject: [PATCH 4/5] unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/ControllerTest.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 36fdc2dee8..13e02278d4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -1,19 +1,33 @@ package io.javaoperatorsdk.operator.processing; +import java.util.Optional; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import io.fabric8.kubernetes.api.model.Secret; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; +import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflow; +import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow; +import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; +import static io.javaoperatorsdk.operator.api.monitoring.Metrics.NOOP; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; @@ -61,4 +75,56 @@ void usesFinalizerIfThereIfReconcilerImplementsCleaner() { assertThat(controller.useFinalizer()).isTrue(); } + + @ParameterizedTest + @CsvSource({ + "true, true, true, false", + "true, true, false, true", + "false, true, true, true", + "false, true, false, true", + "true, false, true, false", + }) + void callsCleanupOnWorkflowWhenHasCleanerAndReconcilerIsNotCleaner(boolean reconcilerIsCleaner, + boolean workflowIsCleaner, + boolean isExplicitWorkflowInvocation, + boolean workflowCleanerExecuted) throws Exception { + Reconciler reconciler = mock(Reconciler.class); + if (reconcilerIsCleaner) { + reconciler = mock(Reconciler.class, withSettings().extraInterfaces(Cleaner.class)); + } + + final var configuration = MockControllerConfiguration.forResource(Secret.class); + + if (reconciler instanceof Cleaner cleaner) { + when(cleaner.cleanup(any(), any())).thenReturn(DeleteControl.noFinalizerRemoval()); + } + + var configurationService = mock(ConfigurationService.class); + var mockWorkflowFactory = mock(ManagedWorkflowFactory.class); + var mockManagedWorkflow = mock(ManagedWorkflow.class); + + when(configuration.getConfigurationService()).thenReturn(configurationService); + var workflowSpec = mock(WorkflowSpec.class); + when(workflowSpec.isExplicitInvocation()).thenReturn(isExplicitWorkflowInvocation); + when(configuration.getWorkflowSpec()).thenReturn(Optional.of(workflowSpec)); + when(configurationService.getMetrics()).thenReturn(NOOP); + when(configurationService.getWorkflowFactory()).thenReturn(mockWorkflowFactory); + when(mockWorkflowFactory.workflowFor(any())).thenReturn(mockManagedWorkflow); + var managedWorkflowMock = workflow(workflowIsCleaner); + when(mockManagedWorkflow.resolve(any(), any())).thenReturn(managedWorkflowMock); + + final var controller = new Controller(reconciler, configuration, + MockKubernetesClient.client(Secret.class)); + + controller.cleanup(new Secret(), new DefaultContext<>(null, controller, new Secret())); + + verify(managedWorkflowMock, times(workflowCleanerExecuted ? 1 : 0)).cleanup(any(), any()); + } + + private Workflow workflow(boolean hasCleaner) { + var workflow = mock(Workflow.class); + when(workflow.cleanup(any(), any())).thenReturn(mock(WorkflowCleanupResult.class)); + when(workflow.hasCleaner()).thenReturn(hasCleaner); + return workflow; + } } From 8681ddc9f43e8f8b05cf85fcbf3a8d187feda091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 5 Feb 2025 12:19:19 +0100 Subject: [PATCH 5/5] improve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../javaoperatorsdk/operator/processing/ControllerTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 13e02278d4..a81f524326 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -88,9 +88,12 @@ void callsCleanupOnWorkflowWhenHasCleanerAndReconcilerIsNotCleaner(boolean recon boolean workflowIsCleaner, boolean isExplicitWorkflowInvocation, boolean workflowCleanerExecuted) throws Exception { - Reconciler reconciler = mock(Reconciler.class); + + Reconciler reconciler; if (reconcilerIsCleaner) { reconciler = mock(Reconciler.class, withSettings().extraInterfaces(Cleaner.class)); + } else { + reconciler = mock(Reconciler.class); } final var configuration = MockControllerConfiguration.forResource(Secret.class);