Reschedule delete (#600)

* chore: renaming vars named k8sClient to kubernetsClient

* chore(deps): bump jandex-maven-plugin from 1.1.1 to 1.2.1 (#592)

Bumps [jandex-maven-plugin](https://github.com/wildfly/jandex-maven-plugin) from 1.1.1 to 1.2.1.
- [Release notes](https://github.com/wildfly/jandex-maven-plugin/releases)
- [Commits](wildfly/jandex-maven-plugin@1.1.1...1.2.1)

---
updated-dependencies:
- dependency-name: org.jboss.jandex:jandex-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump mockito-core from 3.12.4 to 4.0.0 (#591)

Bumps [mockito-core](https://github.com/mockito/mockito) from 3.12.4 to 4.0.0.
- [Release notes](https://github.com/mockito/mockito/releases)
- [Commits](mockito/mockito@v3.12.4...v4.0.0)

---
updated-dependencies:
- dependency-name: org.mockito:mockito-core
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feature: Build PR on v2

* chore(ci): use Java 17

* chore(ci): use only Temurin distribution

* chore: add generics to PostExecutionControl to reduce IDEs noise (#594)

* chore: polish the junit5 extension (#593)

* feat: Use informers as CustomResourceEventSource backbone and cache

This is a major change and backbone of v2. We change also how the Resources are addressed both internally and from event source with CustomResourceID. Additional improvements also added.

* feat: delete reschedule

* fix: naming after review

Co-authored-by: Ioannis Canellos <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Chris Laprun <[email protected]>
Co-authored-by: Luca Burgazzoli <[email protected]>

Author: 5 people

Diff for: 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<T extends 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<Long> getScheduleDelay() {
+    return Optional.ofNullable(scheduleDelay);
+  }
+}

Diff for: 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<DeleteControl> {
+
+  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 for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java

@@ -9,7 +9,7 @@ public interface ResourceController<R extends CustomResource> {
    * 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<R extends CustomResource> {
    *
    * @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<R> context) {
-    return DeleteControl.DEFAULT_DELETE;
+    return DeleteControl.defaultDelete();
   }
 
   /**

Diff for: 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<T extends CustomResource> {
+public class UpdateControl<T extends CustomResource> extends BaseControl<UpdateControl<T>> {
 
   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 <T extends CustomResource> UpdateControl<T> noUpdate() {
     return new UpdateControl<>(null, false, false);
   }
 
-  public UpdateControl<T> withReSchedule(long delay, TimeUnit timeUnit) {
-    return withReSchedule(timeUnit.toMillis(delay));
-  }
-
-  public UpdateControl<T> withReSchedule(long delay) {
-    this.reScheduleDelay = delay;
-    return this;
-  }
-
-  public Optional<Long> getReScheduleDelay() {
-    return Optional.ofNullable(reScheduleDelay);
-  }
-
   public T getCustomResource() {
     return customResource;
   }

Diff for: 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 for: 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<R> createPostExecutionControl(R updatedCustomResour
     } else {
       postExecutionControl = PostExecutionControl.defaultDispatch();
     }
-    updateControl.getReScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
+    updatePostExecutionControlWithReschedule(postExecutionControl, updateControl);
     return postExecutionControl;
   }
 
+  private void updatePostExecutionControlWithReschedule(
+      PostExecutionControl<R> postExecutionControl,
+      BaseControl<?> baseControl) {
+    baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
+  }
+
   private PostExecutionControl<R> handleDelete(R resource, Context<R> context) {
     log.debug(
         "Executing delete for resource: {} with version: {}",
@@ -165,7 +167,9 @@ private PostExecutionControl<R> handleDelete(R resource, Context<R> 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<R> handleDelete(R resource, Context<R> context) {
         getVersion(resource),
         deleteControl,
         useFinalizer);
-    return PostExecutionControl.defaultDispatch();
+    PostExecutionControl<R> postExecutionControl = PostExecutionControl.defaultDispatch();
+    updatePostExecutionControlWithReschedule(postExecutionControl, deleteControl);
+    return postExecutionControl;
   }
 
   private void updateCustomResourceWithFinalizer(R resource) {

Diff for: 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 for: 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,14 +297,29 @@ 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));
 
     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 for: 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 for: 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 for: operator-framework/src/test/resources/compile-fixtures/ControllerImplemented2Interfaces.java

@@ -21,6 +21,6 @@ public UpdateControl<MyCustomResource> createOrUpdateResource(MyCustomResource c
 
     @Override
     public DeleteControl deleteResource(MyCustomResource customResource, Context<MyCustomResource> context) {
-        return DeleteControl.DEFAULT_DELETE;
+        return DeleteControl.defaultDelete();
     }
 }

Diff for: operator-framework/src/test/resources/compile-fixtures/ControllerImplementedIntermediateAbstractClass.java

@@ -18,6 +18,6 @@ public UpdateControl<AbstractController.MyCustomResource> createOrUpdateResource
 
   public DeleteControl deleteResource(AbstractController.MyCustomResource customResource,
       Context<AbstractController.MyCustomResource> context) {
-    return DeleteControl.DEFAULT_DELETE;
+    return DeleteControl.defaultDelete();
   }
 }

Diff for: operator-framework/src/test/resources/compile-fixtures/MultilevelController.java

@@ -22,7 +22,7 @@ public UpdateControl<MultilevelController.MyCustomResource> createOrUpdateResour
 
   public DeleteControl deleteResource(MultilevelController.MyCustomResource customResource,
       Context<MultilevelController.MyCustomResource> context) {
-    return DeleteControl.DEFAULT_DELETE;
+    return DeleteControl.defaultDelete();
   }
 
 }

Diff for: 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<CustomService> context) {
     log.info("Execution deleteResource for: {}", resource.getMetadata().getName());
-    return DeleteControl.DEFAULT_DELETE;
+    return DeleteControl.defaultDelete();
   }
 
   @Override