diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java new file mode 100644 index 0000000000..327bc34791 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/BaseControl.java @@ -0,0 +1,22 @@ +package io.javaoperatorsdk.operator.api; + +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +public abstract class BaseControl { + + private Long scheduleDelay = null; + + public T rescheduleAfter(long delay) { + this.scheduleDelay = delay; + return (T) this; + } + + public T rescheduleAfter(long delay, TimeUnit timeUnit) { + return rescheduleAfter(timeUnit.toMillis(delay)); + } + + public Optional getScheduleDelay() { + return Optional.ofNullable(scheduleDelay); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java index 2c721442cd..b1ddac3df3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/DeleteControl.java @@ -1,5 +1,30 @@ package io.javaoperatorsdk.operator.api; -public enum DeleteControl { - DEFAULT_DELETE, NO_FINALIZER_REMOVAL +public class DeleteControl extends BaseControl { + + private final boolean removeFinalizer; + + private DeleteControl(boolean removeFinalizer) { + this.removeFinalizer = removeFinalizer; + } + + public static DeleteControl defaultDelete() { + return new DeleteControl(true); + } + + public static DeleteControl noFinalizerRemoval() { + return new DeleteControl(false); + } + + public boolean isRemoveFinalizer() { + return removeFinalizer; + } + + @Override + public DeleteControl rescheduleAfter(long delay) { + if (removeFinalizer == true) { + throw new IllegalStateException("Cannot reschedule deleteResource if removing finalizer"); + } + return super.rescheduleAfter(delay); + } } 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 6d1adb84b7..edad82be27 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,7 +9,7 @@ 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}, which indicates that the controller has determined + * {@link DeleteControl#noFinalizerRemoval()}, 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. * @@ -21,13 +21,13 @@ public interface ResourceController { * * @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 + * @return {@link DeleteControl#defaultDelete()} - so the finalizer is automatically removed after + * the call. {@link DeleteControl#noFinalizerRemoval()} if you don't want to remove the * 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; + return DeleteControl.defaultDelete(); } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java index d9e14a6c5a..18cc2ca4ee 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/UpdateControl.java @@ -1,16 +1,12 @@ package io.javaoperatorsdk.operator.api; -import java.util.Optional; -import java.util.concurrent.TimeUnit; - import io.fabric8.kubernetes.client.CustomResource; -public class UpdateControl { +public class UpdateControl extends BaseControl> { private final T customResource; private final boolean updateStatusSubResource; private final boolean updateCustomResource; - private Long reScheduleDelay = null; private UpdateControl( T customResource, boolean updateStatusSubResource, boolean updateCustomResource) { @@ -47,19 +43,6 @@ public static UpdateControl noUpdate() { return new UpdateControl<>(null, false, false); } - public UpdateControl withReSchedule(long delay, TimeUnit timeUnit) { - return withReSchedule(timeUnit.toMillis(delay)); - } - - public UpdateControl withReSchedule(long delay) { - this.reScheduleDelay = delay; - return this; - } - - public Optional getReScheduleDelay() { - return Optional.ofNullable(reScheduleDelay); - } - public T getCustomResource() { return customResource; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java index 120a05bb65..3ce90ebb67 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java @@ -52,15 +52,8 @@ public String controllerName() { } @Override - public String successTypeName(DeleteControl result) { - switch (result) { - case DEFAULT_DELETE: - return "delete"; - case NO_FINALIZER_REMOVAL: - return "finalizerNotRemoved"; - default: - return "unknown"; - } + public String successTypeName(DeleteControl deleteControl) { + return deleteControl.isRemoveFinalizer() ? "delete" : "finalizerNotRemoved"; } @Override 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 92347d5460..0e385dc553 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 @@ -8,11 +8,7 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.api.Context; -import io.javaoperatorsdk.operator.api.DefaultContext; -import io.javaoperatorsdk.operator.api.DeleteControl; -import io.javaoperatorsdk.operator.api.ResourceController; -import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.*; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventList; @@ -152,10 +148,16 @@ private PostExecutionControl createPostExecutionControl(R updatedCustomResour } else { postExecutionControl = PostExecutionControl.defaultDispatch(); } - updateControl.getReScheduleDelay().ifPresent(postExecutionControl::withReSchedule); + updatePostExecutionControlWithReschedule(postExecutionControl, updateControl); return postExecutionControl; } + private void updatePostExecutionControlWithReschedule( + PostExecutionControl postExecutionControl, + BaseControl baseControl) { + baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule); + } + private PostExecutionControl handleDelete(R resource, Context context) { log.debug( "Executing delete for resource: {} with version: {}", @@ -165,7 +167,9 @@ private PostExecutionControl handleDelete(R resource, Context context) { DeleteControl deleteControl = controller.deleteResource(resource, context); final var useFinalizer = configuration().useFinalizer(); if (useFinalizer) { - if (deleteControl == DeleteControl.DEFAULT_DELETE + // note that we don't reschedule here even if instructed. Removing finalizer means that + // cleanup is finished, nothing left to done + if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(configuration().getFinalizer())) { R customResource = removeFinalizer(resource); return PostExecutionControl.customResourceUpdated(customResource); @@ -177,7 +181,9 @@ private PostExecutionControl handleDelete(R resource, Context context) { getVersion(resource), deleteControl, useFinalizer); - return PostExecutionControl.defaultDispatch(); + PostExecutionControl postExecutionControl = PostExecutionControl.defaultDispatch(); + updatePostExecutionControlWithReschedule(postExecutionControl, deleteControl); + return postExecutionControl; } private void updateCustomResourceWithFinalizer(R resource) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java new file mode 100644 index 0000000000..645c997285 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/DeleteControlTest.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.api; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class DeleteControlTest { + + @Test + void cannotReScheduleForDefaultDelete() { + Assertions.assertThrows(IllegalStateException.class, () -> { + DeleteControl.defaultDelete().rescheduleAfter(1000L); + }); + } + +} 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 2a8ad4b0b9..00693e2f34 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 @@ -66,7 +66,7 @@ void setup() { when(controller.createOrUpdateResource(eq(testCustomResource), any())) .thenReturn(UpdateControl.updateCustomResource(testCustomResource)); when(controller.deleteResource(eq(testCustomResource), any())) - .thenReturn(DeleteControl.DEFAULT_DELETE); + .thenReturn(DeleteControl.defaultDelete()); when(customResourceFacade.replaceWithLock(any())).thenReturn(null); } @@ -202,7 +202,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); when(controller.deleteResource(eq(testCustomResource), any())) - .thenReturn(DeleteControl.NO_FINALIZER_REMOVAL); + .thenReturn(DeleteControl.noFinalizerRemoval()); markForDeletion(testCustomResource); eventDispatcher.handleExecution( @@ -297,7 +297,7 @@ void setReScheduleToPostExecutionControlFromUpdateControl() { when(controller.createOrUpdateResource(eq(testCustomResource), any())) .thenReturn( - UpdateControl.updateStatusSubResource(testCustomResource).withReSchedule(1000L)); + UpdateControl.updateStatusSubResource(testCustomResource).rescheduleAfter(1000L)); PostExecutionControl control = eventDispatcher.handleExecution( executionScopeWithCREvent(ADDED, testCustomResource)); @@ -305,6 +305,21 @@ void setReScheduleToPostExecutionControlFromUpdateControl() { assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); } + @Test + void reScheduleOnDeleteWithoutFinalizerRemoval() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + markForDeletion(testCustomResource); + + when(controller.deleteResource(eq(testCustomResource), any())) + .thenReturn( + DeleteControl.noFinalizerRemoval().rescheduleAfter(1000L)); + + PostExecutionControl control = eventDispatcher.handleExecution( + executionScopeWithCREvent(UPDATED, testCustomResource)); + + assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); + } + private void markForDeletion(CustomResource customResource) { customResource.getMetadata().setDeletionTimestamp("2019-8-10"); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java index 0af533d0c3..2cef04a09a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java @@ -57,7 +57,7 @@ public DeleteControl deleteResource( resource.getSpec().getConfigMapName(), resource.getMetadata().getName()); } - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } @Override diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java index 4726a0fdfa..b6aabd1e67 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceController.java @@ -81,7 +81,7 @@ public DeleteControl deleteResource( resource.getSpec().getConfigMapName(), resource.getMetadata().getName()); } - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } @Override diff --git a/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java b/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java index 406d314091..e56943fa30 100644 --- a/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java +++ b/operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java @@ -21,6 +21,6 @@ public UpdateControl createOrUpdateResource(MyCustomResource c @Override public DeleteControl deleteResource(MyCustomResource customResource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } } diff --git a/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java b/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java index f048fb25f0..4f5619b4a4 100644 --- a/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java +++ b/operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java @@ -18,6 +18,6 @@ public UpdateControl createOrUpdateResource public DeleteControl deleteResource(AbstractController.MyCustomResource customResource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } } diff --git a/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java b/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java index 8c1b6a7ffb..d941a16d9d 100644 --- a/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java +++ b/operator-framework/src/test/resources/compile-fixtures/MultilevelController.java @@ -22,7 +22,7 @@ public UpdateControl createOrUpdateResour public DeleteControl deleteResource(MultilevelController.MyCustomResource customResource, Context context) { - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } } diff --git a/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java b/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java index aacff140c9..67fb81dbd7 100644 --- a/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java +++ b/samples/common/src/main/java/io/javaoperatorsdk/operator/sample/CustomServiceController.java @@ -36,7 +36,7 @@ public CustomServiceController(KubernetesClient kubernetesClient) { @Override public DeleteControl deleteResource(CustomService resource, Context context) { log.info("Execution deleteResource for: {}", resource.getMetadata().getName()); - return DeleteControl.DEFAULT_DELETE; + return DeleteControl.defaultDelete(); } @Override