Merge pull request #2 from java-operator-sdk/master

update master branch

Author: s-soroosh

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
       - name: Set up Minikube
         uses: manusa/[email protected]
         with:
-          minikube version: 'v1.15.0'
+          minikube version: 'v1.15.1'
           kubernetes version: ${{ matrix.kubernetes }}
           driver: 'docker'
       - name: Run integration tests

README.md

@@ -20,7 +20,7 @@ Build Kubernetes Operators in Java without hassle. Inspired by [operator-sdk](ht
 * Smart event scheduling (only handle latest event for the same resource)
 
 Check out this [blog post](https://blog.container-solutions.com/a-deep-dive-into-the-java-operator-sdk) 
-about the non-trivial yet common problems needs to be solved for every operator. 
+about the non-trivial yet common problems needed to be solved for every operator. 
 
 #### 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).
@@ -31,7 +31,7 @@ about the non-trivial yet common problems needs to be solved for every operator.
 #### Roadmap
 * Testing of the framework and all samples while running on a real cluster.
 * Generate a project skeleton
-* Generate Java classes from CRD definion (and/or the other way around)
+* Generate Java classes from CRD defintion (and/or the other way around)
 * Integrate with Quarkus (including native image build)
 * Integrate with OLM (Operator Lifecycle Manager)
 

docs/DOCS.md

@@ -56,16 +56,16 @@ TODO: explain running operator locally against a cluster
 
 ### Run Single Instance
 
-There should be always just one instance of an operator running at a time (think process). If there there would be 
+There should be always just one instance of an operator running at a time (think process). If there would be 
 two ore more, in general it could lead to concurrency issues. Note that we are also doing optimistic locking when we update a resource.
-In this way the operator is not highly available. However for operators this not necessary an issue, 
+In this way the operator is not highly available. However for operators this is not necessarily an issue, 
 if the operator just gets restarted after it went down. 
 
 ### At Least Once
 
 To implement controller logic, we have to override two methods: `createOrUpdateResource` and `deleteResource`. 
-These methods are called if a resource is create/changed or marked for deletion. In most cases these methods will be
-called just once, but in some rare cases can happen that are called more then once. In practice this means that the 
+These methods are called if a resource is created/changed or marked for deletion. In most cases these methods will be
+called just once, but in some rare cases, it can happen that they are called more then once. In practice this means that the 
 implementation needs to be **idempotent**.    
 
 ### Smart Scheduling
@@ -77,11 +77,11 @@ a customizable retry mechanism to deal with temporal errors.
 
 When an operator is started we got events for every resource (of a type we listen to) already on the cluster. Even if the resource is not changed 
 (We use `kubectl get ... --watch` in the background). This can be a huge amount of resources depending on your use case.
-So it could be a good case just have a status field on the resource which is checked, if there anything needs to be done.
+So it could be a good case to just have a status field on the resource which is checked, if there is anything needed to be done.
 
 ### Deleting a Resource
 
 During deletion process we use [Kubernetes finalizers](https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#finalizers 
 "Kubernetes docs") finalizers. This is required, since it can happen that the operator is not running while the delete 
 of resource is executed (think `oc delete`). In this case we would not catch the delete event. So we automatically add a
-finalizer first time we update the resource if its not there. 
+finalizer first time we update the resource if it's not there. 

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<>();
 
-    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;
     }
 
     static boolean getGenerationEventProcessing(ResourceController controller) {
         return getAnnotation(controller).generationAwareEventProcessing();
     }
 
-    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);
     }
 }

operator-framework/src/main/java/io/javaoperatorsdk/operator/Operator.java

@@ -55,7 +55,7 @@ private <R extends CustomResource> void registerController(ResourceController<R>
         Class<R> resClass = ControllerUtils.getCustomResourceClass(controller);
         CustomResourceDefinitionContext crd = getCustomResourceDefinitionForController(controller);
         KubernetesDeserializer.registerCustomKind(crd.getVersion(), crd.getKind(), resClass);
-        String finalizer = ControllerUtils.getDefaultFinalizer(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));

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

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);
-
 }

operator-framework/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java

@@ -21,17 +21,17 @@ public class EventDispatcher {
     private final static Logger log = LoggerFactory.getLogger(EventDispatcher.class);
 
     private final ResourceController controller;
-    private final String resourceDefaultFinalizer;
+    private final String resourceFinalizer;
     private final CustomResourceFacade customResourceFacade;
     private final boolean generationAware;
     private final Map<String, Long> lastGenerationProcessedSuccessfully = new ConcurrentHashMap<>();
 
     public EventDispatcher(ResourceController controller,
-                           String defaultFinalizer,
+                           String finalizer,
                            CustomResourceFacade customResourceFacade, boolean generationAware) {
         this.controller = controller;
         this.customResourceFacade = customResourceFacade;
-        this.resourceDefaultFinalizer = defaultFinalizer;
+        this.resourceFinalizer = finalizer;
         this.generationAware = generationAware;
     }
 
@@ -43,24 +43,24 @@ public void handleEvent(CustomResourceEvent event) {
             log.error("Received error for resource: {}", resource.getMetadata().getName());
             return;
         }
-        if (markedForDeletion(resource) && !ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer)) {
-            log.debug("Skipping event dispatching since its marked for deletion but has no default finalizer: {}", event);
+        if (markedForDeletion(resource) && !ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer)) {
+            log.debug("Skipping event dispatching since its marked for deletion but does not have finalizer: {}", event);
             return;
         }
         Context context = new DefaultContext(new RetryInfo(event.getRetryCount(), event.getRetryExecution().isLastExecution()));
         if (markedForDeletion(resource)) {
             boolean removeFinalizer = controller.deleteResource(resource, context);
-            boolean hasDefaultFinalizer = ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer);
-            if (removeFinalizer && hasDefaultFinalizer) {
-                removeDefaultFinalizer(resource);
+            boolean hasFinalizer = ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer);
+            if (removeFinalizer && hasFinalizer) {
+                removeFinalizer(resource);
             } else {
-                log.debug("Skipping finalizer remove. removeFinalizer: {}, hasDefaultFinalizer: {} ",
-                        removeFinalizer, hasDefaultFinalizer);
+                log.debug("Skipping finalizer remove. removeFinalizer: {}, hasFinalizer: {} ",
+                        removeFinalizer, hasFinalizer);
             }
             cleanup(resource);
         } else {
-            if (!ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer) && !markedForDeletion(resource)) {
-                /*  We always add the default finalizer if missing and not marked for deletion.
+            if (!ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer) && !markedForDeletion(resource)) {
+                /*  We always add a finalizer if missing and not marked for deletion.
                     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.
@@ -118,9 +118,9 @@ private void updateCustomResource(CustomResource updatedResource) {
     }
 
 
-    private void removeDefaultFinalizer(CustomResource resource) {
+    private void removeFinalizer(CustomResource resource) {
         log.debug("Removing finalizer on resource {}:", resource);
-        resource.getMetadata().getFinalizers().remove(resourceDefaultFinalizer);
+        resource.getMetadata().getFinalizers().remove(resourceFinalizer);
         customResourceFacade.replaceWithLock(resource);
     }
 
@@ -130,12 +130,12 @@ private void replace(CustomResource resource) {
     }
 
     private void addFinalizerIfNotPresent(CustomResource resource) {
-        if (!ControllerUtils.hasDefaultFinalizer(resource, resourceDefaultFinalizer) && !markedForDeletion(resource)) {
-            log.info("Adding default finalizer to {}", resource.getMetadata());
+        if (!ControllerUtils.hasGivenFinalizer(resource, resourceFinalizer) && !markedForDeletion(resource)) {
+            log.info("Adding finalizer {} to {}", resourceFinalizer, resource.getMetadata());
             if (resource.getMetadata().getFinalizers() == null) {
                 resource.getMetadata().setFinalizers(new ArrayList<>(1));
             }
-            resource.getMetadata().getFinalizers().add(resourceDefaultFinalizer);
+            resource.getMetadata().getFinalizers().add(resourceFinalizer);
         }
     }
 

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();
 

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()));
+    }
 }