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..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,8 +194,9 @@ public DeleteControl execute() throws Exception { WorkflowCleanupResult workflowCleanupResult = null; // The cleanup is called also when explicit invocation is true, but the cleaner is not - // implemented - if (managedWorkflow.hasCleaner() || !explicitWorkflowInvocation) { + // implemented, also in case when explicit invocation is false, but there is cleaner + // implemented. + if (managedWorkflow.hasCleaner() && (!explicitWorkflowInvocation || !isCleaner)) { workflowCleanupResult = managedWorkflow.cleanup(resource, context); } 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..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 @@ -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,59 @@ 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; + if (reconcilerIsCleaner) { + reconciler = mock(Reconciler.class, withSettings().extraInterfaces(Cleaner.class)); + } else { + reconciler = mock(Reconciler.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; + } }