now possible to not remove finalizer after delete, added simple adapter for interface

csviri · csviri · commit f3d06111affe · 2019-07-03T14:40:51.000+02:00
diff --git a/README.md b/README.md
@@ -60,8 +60,9 @@ public class CustomServiceController implements ResourceController<CustomService
     public static final String GROUP = "sample.javaoperatorsdk";
 
     @Override
-    public void deleteResource(CustomService resource, Context<CustomService> context) {
+    public boolean deleteResource(CustomService resource, Context<CustomService> context) {
         // ... your logic ...
+        return true;
     }
     
     // Return the changed resource, so it gets updated. See javadoc for details.
diff --git a/operator-framework/src/main/java/com/github/containersolutions/operator/EventDispatcher.java b/operator-framework/src/main/java/com/github/containersolutions/operator/EventDispatcher.java
@@ -53,8 +53,10 @@ private void handleEvent(Action action, R resource) {
             // we don't want to call delete resource if it not contains our finalizer,
             // since the resource still can be updates when marked for deletion and contains other finalizers
             if (markedForDeletion(resource) && hasDefaultFinalizer(resource)) {
-                controller.deleteResource(resource, new Context(k8sClient, resourceClient));
-                removeDefaultFinalizer(resource);
+                boolean removeFinalizer = controller.deleteResource(resource, new Context(k8sClient, resourceClient));
+                if (removeFinalizer) {
+                    removeDefaultFinalizer(resource);
+                }
             } else {
                 Optional<R> updateResult = controller.createOrUpdateResource(resource, new Context<>(k8sClient, resourceClient));
                 if (updateResult.isPresent()) {
diff --git a/operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceController.java b/operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceController.java
@@ -7,7 +7,17 @@
 
 public interface ResourceController<R extends CustomResource> {
 
-    void deleteResource(R resource, Context<R> context);
+    /**
+     * The implementation should delete the associated component(s). Note that this is method is called when an object
+     * is marked for deletion. After its executed the default finalizer is automatically removed by the framework;
+     * unless the return value is false - note that this is almost never the case.
+     *
+     * @param resource
+     * @param context
+     * @return true - so the finalizer is automatically removed after the call.
+     * false if you don't want to remove the finalizer. Note that this is ALMOST NEVER the case.
+     */
+    boolean deleteResource(R resource, Context<R> context);
 
     /**
      * The implementation of this operation is required to be idempotent.
diff --git a/operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceControllerAdapter.java b/operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceControllerAdapter.java
@@ -0,0 +1,31 @@
+package com.github.containersolutions.operator.api;
+
+import com.github.containersolutions.operator.Context;
+import io.fabric8.kubernetes.client.CustomResource;
+
+import java.util.Optional;
+
+/**
+ * Provides a more clear interface for the most common use case.
+ *
+ * @param <R>
+ */
+public abstract class ResourceControllerAdapter<R extends CustomResource> implements ResourceController<R> {
+
+
+    @Override
+    public boolean deleteResource(R resource, Context<R> context) {
+        delete(resource, context);
+        return true;
+    }
+
+    @Override
+    public Optional<R> createOrUpdateResource(R resource, Context<R> context) {
+        createOrUpdate(resource, context);
+        return Optional.of(resource);
+    }
+
+    public abstract void delete(R resource, Context<R> context);
+
+    public abstract void createOrUpdate(R resource, Context<R> context);
+}
diff --git a/operator-framework/src/test/java/com/github/containersolutions/operator/EventDispatcherTest.java b/operator-framework/src/test/java/com/github/containersolutions/operator/EventDispatcherTest.java
@@ -42,6 +42,7 @@ public void setup() {
         testCustomResource.getMetadata().setFinalizers(new ArrayList<>());
 
         when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.of(testCustomResource));
+        when(resourceController.deleteResource(eq(testCustomResource), any())).thenReturn(true);
     }
 
     @Test
@@ -100,6 +101,18 @@ public void removesDefaultFinalizerOnDelete() {
         verify(resourceOperation, times(1)).lockResourceVersion(any());
     }
 
+    @Test
+    public void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
+        when(resourceController.deleteResource(eq(testCustomResource), any())).thenReturn(false);
+        markForDeletion(testCustomResource);
+        testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
+
+        eventDispatcher.eventReceived(Watcher.Action.MODIFIED, testCustomResource);
+
+        assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
+        verify(resourceOperation, never()).lockResourceVersion(any());
+    }
+
     @Test
     public void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
         testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
diff --git a/operator-framework/src/test/java/com/github/containersolutions/operator/sample/TestCustomResourceController.java b/operator-framework/src/test/java/com/github/containersolutions/operator/sample/TestCustomResourceController.java
@@ -18,7 +18,8 @@ public class TestCustomResourceController implements ResourceController<TestCust
     public static final String TEST_GROUP = "test.group";
 
     @Override
-    public void deleteResource(TestCustomResource resource, Context<TestCustomResource> context) {
+    public boolean deleteResource(TestCustomResource resource, Context<TestCustomResource> context) {
+        return true;
     }
 
     @Override
diff --git a/samples/common/src/main/java/com/github/containersolutions/operator/sample/CustomServiceController.java b/samples/common/src/main/java/com/github/containersolutions/operator/sample/CustomServiceController.java
@@ -26,10 +26,11 @@ public class CustomServiceController implements ResourceController<CustomService
     public static final String GROUP = "sample.javaoperatorsdk";
 
     @Override
-    public void deleteResource(CustomService resource, Context<CustomService> context) {
+    public boolean deleteResource(CustomService resource, Context<CustomService> context) {
         log.info("Execution deleteResource for: {}", resource.getMetadata().getName());
         context.getK8sClient().services().inNamespace(resource.getMetadata().getNamespace())
                 .withName(resource.getMetadata().getName()).delete();
+        return true;
     }
 
     @Override
diff --git a/spring-boot-starter/src/test/java/com/github/containersolutions/operator/spingboot/starter/TestController.java b/spring-boot-starter/src/test/java/com/github/containersolutions/operator/spingboot/starter/TestController.java
@@ -22,8 +22,8 @@ public class TestController implements ResourceController {
     public static final String KIND = "testKind";
 
     @Override
-    public void deleteResource(CustomResource resource, Context context) {
-
+    public boolean deleteResource(CustomResource resource, Context context) {
+        return true;
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -60,8 +60,9 @@ public class CustomServiceController implements ResourceController<CustomService`
`60`	`60`	`public static final String GROUP = "sample.javaoperatorsdk";`
`61`	`61`
`62`	`62`	`@Override`
`63`		`- public void deleteResource(CustomService resource, Context<CustomService> context) {`
	`63`	`+ public boolean deleteResource(CustomService resource, Context<CustomService> context) {`
`64`	`64`	`// ... your logic ...`
	`65`	`+ return true;`
`65`	`66`	`}`
`66`	`67`
`67`	`68`	`// Return the changed resource, so it gets updated. See javadoc for details.`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,8 @@ public class TestCustomResourceController implements ResourceController<TestCust`
`18`	`18`	`public static final String TEST_GROUP = "test.group";`
`19`	`19`
`20`	`20`	`@Override`
`21`		`- public void deleteResource(TestCustomResource resource, Context<TestCustomResource> context) {`
	`21`	`+ public boolean deleteResource(TestCustomResource resource, Context<TestCustomResource> context) {`
	`22`	`+ return true;`
`22`	`23`	`}`
`23`	`24`
`24`	`25`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -22,8 +22,8 @@ public class TestController implements ResourceController {`
`22`	`22`	`public static final String KIND = "testKind";`
`23`	`23`
`24`	`24`	`@Override`
`25`		`- public void deleteResource(CustomResource resource, Context context) {`
`26`		`-`
	`25`	`+ public boolean deleteResource(CustomResource resource, Context context) {`
	`26`	`+ return true;`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`@Override`