From 3b0170ca8737b78ad47e0905f25bbb9a6fd77240 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 19 May 2021 14:00:03 +0200 Subject: [PATCH] feat: make it possible to not automatically add finalizers Fixes #415 --- README.md | 150 +++++++++++------- .../operator/api/Controller.java | 6 +- .../operator/api/ResourceController.java | 14 +- .../api/config/ControllerConfiguration.java | 4 + .../processing/DefaultEventHandler.java | 2 +- .../operator/processing/EventDispatcher.java | 75 +++++---- .../processing/EventDispatcherTest.java | 62 +++++--- 7 files changed, 205 insertions(+), 108 deletions(-) diff --git a/README.md b/README.md index 691e27910e..fa4b791bb2 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,11 @@ -# ![java-operator-sdk](docs/assets/images/logo.png) +# ![java-operator-sdk](docs/assets/images/logo.png) + ![Java CI with Maven](https://github.com/java-operator-sdk/java-operator-sdk/workflows/Java%20CI%20with%20Maven/badge.svg) [![Discord](https://img.shields.io/discord/723455000604573736.svg?label=&logo=discord&logoColor=ffffff&color=7389D8&labelColor=6A7EC2)](https://discord.com/channels/723455000604573736) -Build Kubernetes Operators in Java without hassle. Inspired by [operator-sdk](https://github.com/operator-framework/operator-sdk). - +Build Kubernetes Operators in Java without hassle. Inspired +by [operator-sdk](https://github.com/operator-framework/operator-sdk). + Table of Contents ========== @@ -15,17 +17,24 @@ Table of Contents 1. [Usage](#Usage) ## Features + * Framework for handling Kubernetes API events * Automatic registration of Custom Resource watches * Retry action on failure * Smart event scheduling (only handle the latest event for the same resource) -Check out this [blog post](https://csviri.medium.com/deep-dive-building-a-kubernetes-operator-sdk-for-java-developers-5008218822cb) -about the non-trivial yet common problems needed to be solved for every operator. In case you are interested how to -handle more complex scenarios take a look on [event sources](https://csviri.medium.com/java-operator-sdk-introduction-to-event-sources-a1aab5af4b7b). +Check out +this [blog post](https://csviri.medium.com/deep-dive-building-a-kubernetes-operator-sdk-for-java-developers-5008218822cb) +about the non-trivial yet common problems needed to be solved for every operator. In case you are +interested how to handle more complex scenarios take a look +on [event sources](https://csviri.medium.com/java-operator-sdk-introduction-to-event-sources-a1aab5af4b7b) +. ## Why build your own Operator? -* Infrastructure automation using the power and flexibility of Java. See [blog post](https://blog.container-solutions.com/cloud-native-java-infrastructure-automation-with-kubernetes-operators). + +* Infrastructure automation using the power and flexibility of Java. + See [blog post](https://blog.container-solutions.com/cloud-native-java-infrastructure-automation-with-kubernetes-operators) + . * Provisioning of complex applications - avoiding Helm chart hell * Integration with Cloud services - e.g. Secret stores * Safer deployment of applications - only expose cluster to users by Custom Resources @@ -40,9 +49,11 @@ handle more complex scenarios take a look on [event sources](https://csviri.medi #### Overview of the 1.9.0 changes - The Spring Boot starters have been moved to their own repositories and are now found at: - - https://github.com/java-operator-sdk/operator-framework-spring-boot-starter - - https://github.com/java-operator-sdk/operator-framework-spring-boot-starter-test + - https://github.com/java-operator-sdk/operator-framework-spring-boot-starter + - https://github.com/java-operator-sdk/operator-framework-spring-boot-starter-test - Updated Fabric8 client to version 5.4.0 +- It is now possible to configure the controllers to not automatically add finalizers to resources. + See the `Controller` annotation documentation for more details. #### Overview of the 1.8.0 changes @@ -82,18 +93,23 @@ the `Namescaped` interface. ## Usage We have several sample Operators under the [samples](samples) directory: -* *pure-java*: Minimal Operator implementation which only parses the Custom Resource and prints to stdout. -Implemented with and without Spring Boot support. The two samples share the common module. + +* *pure-java*: Minimal Operator implementation which only parses the Custom Resource and prints to + stdout. Implemented with and without Spring Boot support. The two samples share the common module. * *spring-boot-plain*: Sample showing integration with Spring Boot. -There are also more samples in the standalone [samples repo](https://github.com/java-operator-sdk/samples): -* *webserver*: Simple example creating an NGINX webserver from a Custom Resource containing HTML code. +There are also more samples in the +standalone [samples repo](https://github.com/java-operator-sdk/samples): + +* *webserver*: Simple example creating an NGINX webserver from a Custom Resource containing HTML + code. * *mysql-schema*: Operator managing schemas in a MySQL database. * *tomcat*: Operator with two controllers, managing Tomcat instances and Webapps for these. Add [dependency](https://search.maven.org/search?q=a:operator-framework) to your project with Maven: ```xml + io.javaoperatorsdk operator-framework @@ -111,52 +127,59 @@ dependencies { } ``` -Once you've added the dependency, define a main method initializing the Operator and registering a controller. +Once you've added the dependency, define a main method initializing the Operator and registering a +controller. ```java public class Runner { - public static void main(String[] args) { - Operator operator = new Operator(new DefaultKubernetesClient(), - DefaultConfigurationService.instance()); - operator.register(new WebServerController()); - } + public static void main(String[] args) { + Operator operator = new Operator(new DefaultKubernetesClient(), + DefaultConfigurationService.instance()); + operator.register(new WebServerController()); + } } ``` The Controller implements the business logic and describes all the classes needed to handle the CRD. ```java + @Controller public class WebServerController implements ResourceController { - - // Return the changed resource, so it gets updated. See javadoc for details. - @Override - public UpdateControl createOrUpdateResource(CustomService resource, Context context) { - // ... your logic ... - return UpdateControl.updateStatusSubResource(resource); - } + + // Return the changed resource, so it gets updated. See javadoc for details. + @Override + public UpdateControl createOrUpdateResource(CustomService resource, + Context context) { + // ... your logic ... + return UpdateControl.updateStatusSubResource(resource); + } } ``` A sample custom resource POJO representation ```java + @Group("sample.javaoperatorsdk") @Version("v1") -public class WebServer extends CustomResource implements Namespaced {} +public class WebServer extends CustomResource implements + Namespaced { + +} public class WebServerSpec { - private String html; + private String html; - public String getHtml() { - return html; - } + public String getHtml() { + return html; + } - public void setHtml(String html) { - this.html = html; - } + public void setHtml(String html) { + this.html = html; + } } ``` @@ -175,6 +198,7 @@ To automatically generate CRD manifests from your annotated Custom Resource clas to add the following dependencies to your project: ```xml + io.fabric8 crd-generator-apt @@ -195,7 +219,8 @@ a `mycrs` plural form will result in 2 files: ### Quarkus -A [Quarkus](https://quarkus.io) extension is also provided to ease the development of Quarkus-based operators. +A [Quarkus](https://quarkus.io) extension is also provided to ease the development of Quarkus-based +operators. Add [this dependency](https://search.maven.org/search?q=a:quarkus-operator-sdk) to your project: @@ -205,17 +230,23 @@ to your project: io.quarkiverse.operatorsdk quarkus-operator-sdk - {see https://search.maven.org/search?q=a:quarkus-operator-sdk for latest version} + {see https://search.maven.org/search?q=a:quarkus-operator-sdk for latest version} + ``` -Create an Application, Quarkus will automatically create and inject a `KubernetesClient` (or `OpenShiftClient`), `Operator`, `ConfigurationService` and `ResourceController` instances that your application can use. Below, you can see the minimal code you need to write to get your operator and controllers up and running: +Create an Application, Quarkus will automatically create and inject a `KubernetesClient` ( +or `OpenShiftClient`), `Operator`, `ConfigurationService` and `ResourceController` instances that +your application can use. Below, you can see the minimal code you need to write to get your operator +and controllers up and running: ```java + @QuarkusMain public class QuarkusOperator implements QuarkusApplication { - @Inject Operator operator; + @Inject + Operator operator; public static void main(String... args) { Quarkus.run(QuarkusOperator.class, args); @@ -232,49 +263,62 @@ public class QuarkusOperator implements QuarkusApplication { ### Spring Boot -You can also let Spring Boot wire your application together and automatically register the controllers. +You can also let Spring Boot wire your application together and automatically register the +controllers. -Add [this dependency](https://search.maven.org/search?q=a:operator-framework-spring-boot-starter) to your project: +Add [this dependency](https://search.maven.org/search?q=a:operator-framework-spring-boot-starter) to +your project: ```xml + - io.javaoperatorsdk - operator-framework-spring-boot-starter - {see https://search.maven.org/search?q=a:operator-framework-spring-boot-starter for latest version} + io.javaoperatorsdk + operator-framework-spring-boot-starter + {see https://search.maven.org/search?q=a:operator-framework-spring-boot-starter for + latest version} + ``` Create an Application + ```java + @SpringBootApplication public class Application { - public static void main(String[] args) { - SpringApplication.run(Application.class, args); - } + + public static void main(String[] args) { + SpringApplication.run(Application.class, args); + } } ``` #### Spring Boot test support -Adding the following dependency would let you mock the operator for the -tests where loading the spring container is necessary, -but it doesn't need real access to a Kubernetes cluster. +Adding the following dependency would let you mock the operator for the tests where loading the +spring container is necessary, but it doesn't need real access to a Kubernetes cluster. ```xml + - io.javaoperatorsdk - operator-framework-spring-boot-starter-test - {see https://search.maven.org/search?q=a:operator-framework-spring-boot-starter for latest version} + io.javaoperatorsdk + operator-framework-spring-boot-starter-test + {see https://search.maven.org/search?q=a:operator-framework-spring-boot-starter for + latest version} + ``` Mock the operator: + ```java + @SpringBootTest @EnableMockOperator public class SpringBootStarterSampleApplicationTest { @Test - void contextLoads() {} + void contextLoads() { + } } ``` diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java index b8935a2718..399510b4e2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java @@ -11,12 +11,14 @@ String NULL = ""; String WATCH_CURRENT_NAMESPACE = "JOSDK_WATCH_CURRENT"; + String NO_FINALIZER = "JOSDK_NO_FINALIZER"; String name() default NULL; /** - * Optional finalizer name, if it is not, the crdName will be used as the name of the finalizer - * too. + * Optional finalizer name, if it is not provided, one will be automatically generated. If the + * provided value is the value specified by {@link #NO_FINALIZER}, then no finalizer will be added + * to custom resources. * * @return the finalizer name */ diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java index 0f6aed6cf4..a98cfd4a08 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java @@ -9,15 +9,21 @@ public interface ResourceController { * The implementation should delete the associated component(s). Note that this is method is * called when an object is marked for deletion. After it's executed the custom resource finalizer * is automatically removed by the framework; unless the return value is {@link - * DeleteControl#NO_FINALIZER_REMOVAL} - note that this is almost never the case. It's important - * to have the implementation also idempotent, in the current implementation to cover all edge - * cases actually will be executed mostly twice. + * DeleteControl#NO_FINALIZER_REMOVAL}, which indicates that the controller has determined that + * the resource should not be deleted yet, in which case it is up to the controller to restore the + * resource's status so that it's not marked for deletion anymore. + * + *

It's important that this method be idempotent, as it could be called several times, + * depending on the conditions and the controller's configuration (for example, if the controller + * is configured to not use a finalizer but the resource does have finalizers, it might be be + * "offered" again for deletion several times until the finalizers are all removed. * * @param resource the resource that is marked for deletion * @param context the context with which the operation is executed * @return {@link DeleteControl#DEFAULT_DELETE} - so the finalizer is automatically removed after * the call. {@link DeleteControl#NO_FINALIZER_REMOVAL} if you don't want to remove the - * finalizer. Note that this is ALMOST NEVER the case. + * finalizer to indicate that the resource should not be deleted after all, in which case the + * controller should restore the resource's state appropriately. */ default DeleteControl deleteResource(R resource, Context context) { return DeleteControl.DEFAULT_DELETE; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index d67e6d2a0e..b487f2af5f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -61,4 +61,8 @@ default RetryConfiguration getRetryConfiguration() { ConfigurationService getConfigurationService(); void setConfigurationService(ConfigurationService service); + + default boolean useFinalizer() { + return !Controller.NO_FINALIZER.equals(getFinalizer()); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index a082e770b0..41316505d3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -48,7 +48,7 @@ public class DefaultEventHandler implements EventHandler { public DefaultEventHandler( ResourceController controller, ControllerConfiguration configuration, MixedOperation client) { this( - new EventDispatcher(controller, configuration.getFinalizer(), client), + new EventDispatcher(controller, configuration, client), configuration.getName(), GenericRetry.fromConfiguration(configuration.getRetryConfiguration()), configuration.getConfigurationService().concurrentReconciliationThreads()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java index e4f1b088f5..8bf71117a0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java @@ -14,6 +14,7 @@ import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,23 +27,21 @@ class EventDispatcher { private static final Logger log = LoggerFactory.getLogger(EventDispatcher.class); private final ResourceController controller; - private final String resourceFinalizer; + private final ControllerConfiguration configuration; private final CustomResourceFacade customResourceFacade; EventDispatcher( ResourceController controller, - String finalizer, + ControllerConfiguration configuration, CustomResourceFacade customResourceFacade) { this.controller = controller; this.customResourceFacade = customResourceFacade; - this.resourceFinalizer = finalizer; + this.configuration = configuration; } public EventDispatcher( - ResourceController controller, - String finalizer, - MixedOperation, Resource> client) { - this(controller, finalizer, new CustomResourceFacade<>(client)); + ResourceController controller, ControllerConfiguration configuration, MixedOperation client) { + this(controller, configuration, new CustomResourceFacade<>(client)); } public PostExecutionControl handleExecution(ExecutionScope executionScope) { @@ -73,26 +72,44 @@ private PostExecutionControl handleDispatch(ExecutionScope executionScope) { getVersion(resource)); return PostExecutionControl.defaultDispatch(); } - if ((resource.isMarkedForDeletion() && !resource.hasFinalizer(resourceFinalizer))) { + + final var markedForDeletion = resource.isMarkedForDeletion(); + if (markedForDeletion && shouldNotDispatchToDelete(resource)) { log.debug( - "Skipping event dispatching since its marked for deletion but has no finalizer: {}", - executionScope); + "Skipping delete of resource {} because finalizer(s) {} don't allow processing yet", + resource.getMetadata().getName(), + resource.getMetadata().getFinalizers()); return PostExecutionControl.defaultDispatch(); } + Context context = new DefaultContext<>( new EventList(executionScope.getEvents()), executionScope.getRetryInfo()); - if (resource.isMarkedForDeletion()) { + if (markedForDeletion) { return handleDelete(resource, context); } else { return handleCreateOrUpdate(executionScope, resource, context); } } + /** + * Determines whether the given resource should be dispatched to the controller's {@link + * ResourceController#deleteResource(CustomResource, Context)} method + * + * @param resource the resource to be potentially deleted + * @return {@code true} if the resource should be handed to the controller's {@code + * deleteResource} method, {@code false} otherwise + */ + private boolean shouldNotDispatchToDelete(R resource) { + // we don't dispatch to delete if the controller is configured to use a finalizer but that + // finalizer is not present (which means it's already been removed) + return configuration.useFinalizer() && !resource.hasFinalizer(configuration.getFinalizer()); + } + private PostExecutionControl handleCreateOrUpdate( ExecutionScope executionScope, R resource, Context context) { - if (!resource.hasFinalizer(resourceFinalizer)) { - /* We always add the finalizer if missing. + if (configuration.useFinalizer() && !resource.hasFinalizer(configuration.getFinalizer())) { + /* We always add the finalizer if missing and the controller is configured to use a finalizer. We execute the controller processing only for processing the event sent as a results of the finalizer add. This will make sure that the resources are not created before there is a finalizer. @@ -135,26 +152,30 @@ private PostExecutionControl handleDelete(R resource, Context context) { "Executing delete for resource: {} with version: {}", getUID(resource), getVersion(resource)); + // todo: this is be executed in a try-catch statement, in case this fails DeleteControl deleteControl = controller.deleteResource(resource, context); - boolean hasFinalizer = resource.hasFinalizer(resourceFinalizer); - if (deleteControl == DeleteControl.DEFAULT_DELETE && hasFinalizer) { - R customResource = removeFinalizer(resource); - return PostExecutionControl.customResourceUpdated(customResource); - } else { - log.debug( - "Skipping finalizer remove for resource: {} with version: {}. delete control: {}, hasFinalizer: {} ", - getUID(resource), - getVersion(resource), - deleteControl, - hasFinalizer); - return PostExecutionControl.defaultDispatch(); + final var useFinalizer = configuration.useFinalizer(); + if (useFinalizer) { + if (deleteControl == DeleteControl.DEFAULT_DELETE + && resource.hasFinalizer(configuration.getFinalizer())) { + R customResource = removeFinalizer(resource); + // todo: should we patch the resource to remove the finalizer instead of updating it + return PostExecutionControl.customResourceUpdated(customResource); + } } + log.debug( + "Skipping finalizer remove for resource: {} with version: {}. delete control: {}, uses finalizer: {} ", + getUID(resource), + getVersion(resource), + deleteControl, + useFinalizer); + return PostExecutionControl.defaultDispatch(); } private void updateCustomResourceWithFinalizer(R resource) { log.debug( "Adding finalizer for resource: {} version: {}", getUID(resource), getVersion(resource)); - resource.addFinalizer(resourceFinalizer); + resource.addFinalizer(configuration.getFinalizer()); replace(resource); } @@ -169,7 +190,7 @@ private R removeFinalizer(R resource) { "Removing finalizer on resource: {} with version: {}", getUID(resource), getVersion(resource)); - resource.removeFinalizer(resourceFinalizer); + resource.removeFinalizer(configuration.getFinalizer()); return customResourceFacade.replaceWithLock(resource); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java index 93a6561439..1d00ebfcac 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java @@ -4,8 +4,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.any; -import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -21,13 +21,13 @@ import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.RetryInfo; import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; @@ -37,16 +37,20 @@ class EventDispatcherTest { private static final String DEFAULT_FINALIZER = "javaoperatorsdk.io/finalizer"; private CustomResource testCustomResource; private EventDispatcher eventDispatcher; - private ResourceController controller = mock(ResourceController.class); - private EventDispatcher.CustomResourceFacade customResourceFacade = + private final ResourceController controller = mock(ResourceController.class); + private ControllerConfiguration configuration = + mock(ControllerConfiguration.class); + private final EventDispatcher.CustomResourceFacade customResourceFacade = mock(EventDispatcher.CustomResourceFacade.class); @BeforeEach void setup() { - eventDispatcher = new EventDispatcher(controller, DEFAULT_FINALIZER, customResourceFacade); + eventDispatcher = new EventDispatcher(controller, configuration, customResourceFacade); testCustomResource = TestUtils.testCustomResource(); + when(configuration.getFinalizer()).thenReturn(DEFAULT_FINALIZER); + when(configuration.useFinalizer()).thenCallRealMethod(); when(controller.createOrUpdateResource(eq(testCustomResource), any())) .thenReturn(UpdateControl.updateCustomResource(testCustomResource)); when(controller.deleteResource(eq(testCustomResource), any())) @@ -61,7 +65,9 @@ void addFinalizerOnNewResource() { executionScopeWithCREvent(Watcher.Action.ADDED, testCustomResource)); verify(controller, never()) .createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any()); - verify(customResourceFacade, times(1)).replaceWithLock(testCustomResource); + verify(customResourceFacade, times(1)) + .replaceWithLock( + argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER))); assertTrue(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); } @@ -113,18 +119,6 @@ void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() { .createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any()); } - @Test - @Disabled( - "This test is wrong, if the finalizer is not present, it is added, bypassing calling the controller") - void addsDefaultFinalizerOnCreateIfNotThere() { - eventDispatcher.handleExecution( - executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); - verify(controller, times(1)) - .createOrUpdateResource( - argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)), - any()); - } - @Test void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() { // we need to add the finalizer before marking it for deletion, as otherwise it won't get added @@ -139,9 +133,9 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() { /** Note that there could be more finalizers. Out of our control. */ @Test - @Disabled( - "This test is wrong, it only passes if the finalizer is set despite what its name implies") - void callDeleteOnControllerIfMarkedForDeletionButThereIsNoDefaultFinalizer() { + void callDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() { + configureToNotUseFinalizer(); + markForDeletion(testCustomResource); eventDispatcher.handleExecution( @@ -150,6 +144,32 @@ void callDeleteOnControllerIfMarkedForDeletionButThereIsNoDefaultFinalizer() { verify(controller).deleteResource(eq(testCustomResource), any()); } + @Test + void doNotCallDeleteIfMarkedForDeletionWhenFinalizerHasAlreadyBeenRemoved() { + markForDeletion(testCustomResource); + + eventDispatcher.handleExecution( + executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + + verify(controller, never()).deleteResource(eq(testCustomResource), any()); + } + + private void configureToNotUseFinalizer() { + ControllerConfiguration configuration = mock(ControllerConfiguration.class); + when(configuration.useFinalizer()).thenReturn(false); + eventDispatcher = new EventDispatcher(controller, configuration, customResourceFacade); + } + + @Test + void doesNotAddFinalizerIfConfiguredNotTo() { + configureToNotUseFinalizer(); + + eventDispatcher.handleExecution( + executionScopeWithCREvent(Watcher.Action.MODIFIED, testCustomResource)); + + assertEquals(0, testCustomResource.getMetadata().getFinalizers().size()); + } + @Test void removesDefaultFinalizerOnDeleteIfSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER);