Merge branch 'master' into event-sources

csviri · csviri · commit f46ca07603d6 · 2020-11-19T12:19:48.000+01:00
# Conflicts:
#	operator-framework/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java
#	operator-framework/src/main/java/io/javaoperatorsdk/operator/Operator.java
#	operator-framework/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java
#	operator-framework/src/test/java/io/javaoperatorsdk/operator/EventDispatcherTest.java
#	operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSchedulerTest.java
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -29,7 +29,7 @@ jobs:
       - name: Set up Minikube
         uses: manusa/actions-setup-minikube@v2.0.1
         with:
-          minikube version: 'v1.15.0'
+          minikube version: 'v1.15.1'
           kubernetes version: ${{ matrix.kubernetes }}
           driver: 'docker'
       - name: Run integration tests
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java
@@ -16,23 +16,27 @@
 public class ControllerUtils {
 
     private final static double JAVA_VERSION = Double.parseDouble(System.getProperty("java.specification.version"));
-
-    private final static Logger log = LoggerFactory.getLogger(ControllerUtils.class);
+    private static final String FINALIZER_NAME_SUFFIX = "/finalizer";
 
     // this is just to support testing, this way we don't try to create class multiple times in memory with same name.
     // note that other solution is to add a random string to doneable class name
     private static Map<Class<? extends CustomResource>, Class<? extends CustomResourceDoneable<? extends CustomResource>>>
             doneableClassCache = new HashMap<>();
 
-    public static String getDefaultFinalizer(ResourceController controller) {
-        return getAnnotation(controller).finalizerName();
+    static String getFinalizer(ResourceController controller) {
+        final String annotationFinalizerName = getAnnotation(controller).finalizerName();
+        if (!Controller.NULL.equals(annotationFinalizerName)) {
+            return annotationFinalizerName;
+        }
+        final String crdName = getAnnotation(controller).crdName() + FINALIZER_NAME_SUFFIX;
+        return crdName;
     }
 
     public static boolean getGenerationEventProcessing(ResourceController controller) {
         return getAnnotation(controller).generationAwareEventProcessing();
     }
 
-    public static <R extends CustomResource> Class<R> getCustomResourceClass(ResourceController controller) {
+    static <R extends CustomResource> Class<R> getCustomResourceClass(ResourceController<R> controller) {
         return (Class<R>) getAnnotation(controller).customResourceClass();
     }
 
@@ -79,10 +83,7 @@ private static Controller getAnnotation(ResourceController controller) {
         return controller.getClass().getAnnotation(Controller.class);
     }
 
-    public static boolean hasDefaultFinalizer(CustomResource resource, String finalizer) {
-        if (resource.getMetadata().getFinalizers() != null) {
-            return resource.getMetadata().getFinalizers().contains(finalizer);
-        }
-        return false;
+    public static boolean hasGivenFinalizer(CustomResource resource, String finalizer) {
+        return resource.getMetadata().getFinalizers() != null && resource.getMetadata().getFinalizers().contains(finalizer);
     }
 }
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/Operator.java
@@ -51,8 +51,8 @@ private <R extends CustomResource> void registerController(ResourceController<R>
         Class<R> resClass = getCustomResourceClass(controller);
         CustomResourceDefinitionContext crd = getCustomResourceDefinitionForController(controller);
         KubernetesDeserializer.registerCustomKind(crd.getVersion(), crd.getKind(), resClass);
-        String finalizer = getDefaultFinalizer(controller);
-        MixedOperation client = k8sClient.customResources(crd, resClass, CustomResourceList.class, getCustomResourceDoneableClass(controller));
+        String finalizer = ControllerUtils.getFinalizer(controller);
+        MixedOperation client = k8sClient.customResources(crd, resClass, CustomResourceList.class, ControllerUtils.getCustomResourceDoneableClass(controller));
         EventDispatcher eventDispatcher = new EventDispatcher(controller,
                 finalizer, new EventDispatcher.CustomResourceFacade(client), ControllerUtils.getGenerationEventProcessing(controller));
 
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/api/Controller.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/api/Controller.java
@@ -10,14 +10,17 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE})
 public @interface Controller {
-
-    String DEFAULT_FINALIZER = "operator.default.finalizer";
+    String NULL = "";
 
     String crdName();
 
     Class<? extends CustomResource> customResourceClass();
 
-    String finalizerName() default DEFAULT_FINALIZER;
+    /**
+     * Optional finalizer name, if it is not,
+     * the crdName will be used as the name of the finalizer too.
+     */
+    String finalizerName() default NULL;
 
     /**
      * If true, will dispatch new event to the controller if generation increased since the last processing, otherwise will
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java
@@ -6,7 +6,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 its executed the default finalizer is automatically removed by the framework;
+     * is marked for deletion. After its executed the custom resource finalizer is automatically removed by the framework;
      * unless the return value is false - note that this is almost never the case.
      * Its important to have the implementation also idempotent, in the current implementation to cover all edge cases
      * actually will be executed mostly twice.
@@ -27,5 +27,4 @@ public interface ResourceController<R extends CustomResource> {
      * <b>However we will always call an update if there is no finalizer on object and its not marked for deletion.</b>
      */
     UpdateControl<R> createOrUpdateResource(R resource, Context<R> context);
-
 }
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java
@@ -21,7 +21,6 @@
 @TestInstance(TestInstance.Lifecycle.PER_METHOD)
 public class ControllerExecutionIT {
 
-    private final static Logger log = LoggerFactory.getLogger(ControllerExecutionIT.class);
     public static final String TEST_CUSTOM_RESOURCE_NAME = "test-custom-resource";
     private IntegrationTestSupport integrationTestSupport = new IntegrationTestSupport();
 
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerUtilsTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerUtilsTest.java
@@ -1,23 +1,46 @@
 package io.javaoperatorsdk.operator;
 
-import io.javaoperatorsdk.operator.sample.TestCustomResource;
-import io.javaoperatorsdk.operator.sample.TestCustomResourceController;
 import io.fabric8.kubernetes.client.CustomResourceDoneable;
+import io.javaoperatorsdk.operator.api.Context;
 import io.javaoperatorsdk.operator.api.Controller;
+import io.javaoperatorsdk.operator.api.ResourceController;
+import io.javaoperatorsdk.operator.api.UpdateControl;
+import io.javaoperatorsdk.operator.sample.TestCustomResource;
+import io.javaoperatorsdk.operator.sample.TestCustomResourceController;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 class ControllerUtilsTest {
 
+    public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer";
+
     @Test
     public void returnsValuesFromControllerAnnotationFinalizer() {
-        Assertions.assertEquals(Controller.DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController(null)));
+        Assertions.assertEquals(TestCustomResourceController.CRD_NAME + "/finalizer", ControllerUtils.getFinalizer(new TestCustomResourceController(null)));
         assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController(null)));
         Assertions.assertEquals(TestCustomResourceController.CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController(null)));
-        assertEquals(false, ControllerUtils.getGenerationEventProcessing(new TestCustomResourceController(null)));
+        assertFalse(ControllerUtils.getGenerationEventProcessing(new TestCustomResourceController(null)));
         assertTrue(CustomResourceDoneable.class.isAssignableFrom(ControllerUtils.getCustomResourceDoneableClass(new TestCustomResourceController(null))));
     }
+
+    @Controller(crdName = "test.crd", customResourceClass = TestCustomResource.class, finalizerName = CUSTOM_FINALIZER_NAME)
+    static class TestCustomFinalizerController implements ResourceController<TestCustomResource> {
+
+        @Override
+        public boolean deleteResource(TestCustomResource resource, Context<TestCustomResource> context) {
+            return false;
+        }
+
+        @Override
+        public UpdateControl<TestCustomResource> createOrUpdateResource(TestCustomResource resource, Context<TestCustomResource> context) {
+            return null;
+        }
+    }
+
+    @Test
+    public void returnCustomerFinalizerNameIfSet() {
+        assertEquals(CUSTOM_FINALIZER_NAME, ControllerUtils.getFinalizer(new TestCustomFinalizerController()));
+    }
 }
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/IntegrationTestSupport.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/IntegrationTestSupport.java
@@ -53,7 +53,7 @@ public void initialize(KubernetesClient k8sClient, ResourceController controller
         }
         operator = new Operator(k8sClient);
         operator.registerController(controller, TEST_NAMESPACE);
-        log.info("Operator is running with TestCustomeResourceController");
+        log.info("Operator is running with {}", controller.getClass().getCanonicalName());
     }
 
     public CustomResourceDefinition loadCRDAndApplyToCluster(String classPathYaml) {
@@ -107,7 +107,7 @@ public void teardownIfSuccess(TestRun test) {
             if (namespace.getStatus().getPhase().equals("Active")) {
                 k8sClient.namespaces().withName(TEST_NAMESPACE).delete();
             }
-            await("namespace deleted").atMost(30, TimeUnit.SECONDS)
+            await("namespace deleted").atMost(45, TimeUnit.SECONDS)
                     .until(() -> k8sClient.namespaces().withName(TEST_NAMESPACE).get() == null);
         } catch (Exception e) {
             throw new IllegalStateException(e);
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java
@@ -68,7 +68,7 @@ public void ifNoFinalizerPresentFirstAddsTheFinalizerThenExecutesControllerAgain
      * If this would not happen (no optimistic locking on status sub-resource)  we could receive and store an event
      * while processing the controller method. But this event would always fail since its resource version is outdated
      * already.
-     * */
+     */
     @Test
     public void updateCustomResourceAfterSubResourceChange() {
         integrationTestSupport.teardownIfSuccess(() -> {
@@ -104,7 +104,7 @@ public SubResourceTestCustomResource createTestCustomResource(String id) {
         resource.setMetadata(new ObjectMetaBuilder()
                 .withName("subresource-" + id)
                 .withNamespace(TEST_NAMESPACE)
-                .withFinalizers(Controller.DEFAULT_FINALIZER)
+                .withFinalizers(SubResourceTestCustomResourceController.FINALIZER_NAME)
                 .build());
         resource.setKind("SubresourceSample");
         resource.setSpec(new SubResourceTestCustomResourceSpec());
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/TestCustomResourceController.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/TestCustomResourceController.java
@@ -22,6 +22,7 @@ public class TestCustomResourceController implements ResourceController<TestCust
     private static final Logger log = LoggerFactory.getLogger(TestCustomResourceController.class);
 
     public static final String CRD_NAME = "customservices.sample.javaoperatorsdk";
+    public static final String FINALIZER_NAME = CRD_NAME + "/finalizer";
 
     private final KubernetesClient kubernetesClient;
     private final boolean updateStatus;
@@ -52,7 +53,7 @@ public DeleteControl deleteResource(TestCustomResource resource, Context<TestCus
     public UpdateControl<TestCustomResource> createOrUpdateResource(TestCustomResource resource,
                                                                     Context<TestCustomResource> context) {
         numberOfExecutions.addAndGet(1);
-        if (!resource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)) {
+        if (!resource.getMetadata().getFinalizers().contains(FINALIZER_NAME)) {
             throw new IllegalStateException("Finalizer is not present.");
         }
 
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/subresource/SubResourceTestCustomResourceController.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/subresource/SubResourceTestCustomResourceController.java
@@ -15,8 +15,9 @@ public class SubResourceTestCustomResourceController implements ResourceControll
         TestExecutionInfoProvider {
 
     public static final String CRD_NAME = "subresourcesample.sample.javaoperatorsdk";
+    public static final String FINALIZER_NAME = CRD_NAME + "/finalizer";
     private static final Logger log = LoggerFactory.getLogger(SubResourceTestCustomResourceController.class);
-    private AtomicInteger numberOfExecutions = new AtomicInteger(0);
+    private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
 
     @Override
     public DeleteControl deleteResource(SubResourceTestCustomResource resource, Context<SubResourceTestCustomResource> context) {
@@ -27,7 +28,7 @@ public DeleteControl deleteResource(SubResourceTestCustomResource resource, Cont
     public UpdateControl<SubResourceTestCustomResource> createOrUpdateResource(SubResourceTestCustomResource resource,
                                                                                Context<SubResourceTestCustomResource> context) {
         numberOfExecutions.addAndGet(1);
-        if (!resource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)) {
+        if (!resource.getMetadata().getFinalizers().contains(FINALIZER_NAME)) {
             throw new IllegalStateException("Finalizer is not present.");
         }
         log.info("Value: " + resource.getSpec().getValue());

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ public void initialize(KubernetesClient k8sClient, ResourceController controller`
`53`	`53`	`}`
`54`	`54`	`operator = new Operator(k8sClient);`
`55`	`55`	`operator.registerController(controller, TEST_NAMESPACE);`
`56`		`- log.info("Operator is running with TestCustomeResourceController");`
	`56`	`+ log.info("Operator is running with {}", controller.getClass().getCanonicalName());`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`public CustomResourceDefinition loadCRDAndApplyToCluster(String classPathYaml) {`
`@@ -107,7 +107,7 @@ public void teardownIfSuccess(TestRun test) {`
`107`	`107`	`if (namespace.getStatus().getPhase().equals("Active")) {`
`108`	`108`	`k8sClient.namespaces().withName(TEST_NAMESPACE).delete();`
`109`	`109`	`}`
`110`		`- await("namespace deleted").atMost(30, TimeUnit.SECONDS)`
	`110`	`+ await("namespace deleted").atMost(45, TimeUnit.SECONDS)`
`111`	`111`	`.until(() -> k8sClient.namespaces().withName(TEST_NAMESPACE).get() == null);`
`112`	`112`	`} catch (Exception e) {`
`113`	`113`	`throw new IllegalStateException(e);`