removed context object from controller api

Author: csviri

operator-framework/src/main/java/com/github/containersolutions/operator/Operator.java

@@ -57,7 +57,7 @@ private <R extends CustomResource> void registerController(ResourceController<R>
             MixedOperation client = k8sClient.customResources(crd.get(), resClass, list, doneable);
 
             EventDispatcher<R> eventDispatcher =
-                    new EventDispatcher<>(controller, (CustomResourceOperationsImpl) client, client, k8sClient,
+                    new EventDispatcher<>(controller, (CustomResourceOperationsImpl) client,
                             ControllerUtils.getDefaultFinalizer(controller));
 
             eventScheduler = new EventScheduler(eventDispatcher);

operator-framework/src/main/java/com/github/containersolutions/operator/api/Context.java

@@ -12,21 +12,20 @@ public interface ResourceController<R extends CustomResource> {
      * 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);
+    boolean deleteResource(R resource);
 
     /**
      * The implementation of this operation is required to be idempotent.
      *
      * @return The resource is updated in api server if the return value is present
-     *  within Optional. This the common use cases. However in cases, for example the operator is restarted,
-     *  and we don't want to have an update call to k8s api to be made unnecessarily, by returning an empty Optional
-     *  this update can be skipped.
-     *  <b>However we will always call an update if there is no finalizer on object and its not marked for deletion.</b>
+     * within Optional. This the common use cases. However in cases, for example the operator is restarted,
+     * and we don't want to have an update call to k8s api to be made unnecessarily, by returning an empty Optional
+     * this update can be skipped.
+     * <b>However we will always call an update if there is no finalizer on object and its not marked for deletion.</b>
      */
-    Optional<R> createOrUpdateResource(R resource, Context<R> context);
+    Optional<R> createOrUpdateResource(R resource);
 
 }

operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceController.java

@@ -1,10 +1,10 @@
 package com.github.containersolutions.operator.processing;
 
-import com.github.containersolutions.operator.api.Context;
 import com.github.containersolutions.operator.api.ResourceController;
-import io.fabric8.kubernetes.client.*;
-import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
-import io.fabric8.kubernetes.client.dsl.Resource;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.client.CustomResourceDoneable;
+import io.fabric8.kubernetes.client.CustomResourceList;
+import io.fabric8.kubernetes.client.Watcher;
 import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -22,22 +22,14 @@ public class EventDispatcher<R extends CustomResource> {
     private final ResourceController<R> controller;
     private final CustomResourceOperationsImpl<R, CustomResourceList<R>, CustomResourceDoneable<R>> resourceOperation;
     private final String resourceDefaultFinalizer;
-    private final NonNamespaceOperation<R, CustomResourceList<R>, CustomResourceDoneable<R>,
-            Resource<R, CustomResourceDoneable<R>>> resourceClient;
-    private final KubernetesClient k8sClient;
 
     public EventDispatcher(ResourceController<R> controller,
                            CustomResourceOperationsImpl<R, CustomResourceList<R>, CustomResourceDoneable<R>> resourceOperation,
-                           NonNamespaceOperation<R, CustomResourceList<R>, CustomResourceDoneable<R>,
-                                   Resource<R, CustomResourceDoneable<R>>> resourceClient, KubernetesClient k8sClient,
                            String defaultFinalizer
-
     ) {
         this.controller = controller;
         this.resourceOperation = resourceOperation;
-        this.resourceClient = resourceClient;
         this.resourceDefaultFinalizer = defaultFinalizer;
-        this.k8sClient = k8sClient;
     }
 
     public void handleEvent(Watcher.Action action, R resource) {
@@ -49,13 +41,13 @@ public void handleEvent(Watcher.Action action, R resource) {
             // finalizer into the resource before marked for delete. So for now we will call delete every time, since delete
             // operation should be idempotent too, and this way we cover the corner case.
             if (markedForDeletion(resource)) {
-                boolean removeFinalizer = controller.deleteResource(resource, new Context(k8sClient, resourceClient));
+                boolean removeFinalizer = controller.deleteResource(resource);
                 if (removeFinalizer && hasDefaultFinalizer(resource)) {
                     log.debug("Removing finalizer on {}: {}", resource.getMetadata().getName(), resource.getMetadata());
                     removeDefaultFinalizer(resource);
                 }
             } else {
-                Optional<R> updateResult = controller.createOrUpdateResource(resource, new Context<>(k8sClient, resourceClient));
+                Optional<R> updateResult = controller.createOrUpdateResource(resource);
                 if (updateResult.isPresent()) {
                     log.debug("Updating resource: {} with version: {}", resource.getMetadata().getName(),
                             resource.getMetadata().getResourceVersion());

operator-framework/src/main/java/com/github/containersolutions/operator/api/ResourceControllerAdapter.java

@@ -16,14 +16,14 @@ class ControllerUtilsTest {
 
     @Test
     public void returnsValuesFromControllerAnnotationFinalizer() {
-        assertEquals(DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController()));
-        assertEquals(TEST_GROUP + "/" + DEFAULT_VERSION, ControllerUtils.getApiVersion(new TestCustomResourceController()));
-        assertEquals(DEFAULT_VERSION, ControllerUtils.getVersion(new TestCustomResourceController()));
-        assertEquals(TestCustomResourceController.KIND_NAME, ControllerUtils.getKind(new TestCustomResourceController()));
-        assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController()));
-        assertEquals(TestCustomResourceDoneable.class, ControllerUtils.getCustomResourceDonebaleClass(new TestCustomResourceController()));
-        assertEquals(TestCustomResourceList.class, ControllerUtils.getCustomResourceListClass(new TestCustomResourceController()));
-        assertEquals(CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController()).get());
+        assertEquals(DEFAULT_FINALIZER, ControllerUtils.getDefaultFinalizer(new TestCustomResourceController(null)));
+        assertEquals(TEST_GROUP + "/" + DEFAULT_VERSION, ControllerUtils.getApiVersion(new TestCustomResourceController(null)));
+        assertEquals(DEFAULT_VERSION, ControllerUtils.getVersion(new TestCustomResourceController(null)));
+        assertEquals(TestCustomResourceController.KIND_NAME, ControllerUtils.getKind(new TestCustomResourceController(null)));
+        assertEquals(TestCustomResource.class, ControllerUtils.getCustomResourceClass(new TestCustomResourceController(null)));
+        assertEquals(TestCustomResourceDoneable.class, ControllerUtils.getCustomResourceDonebaleClass(new TestCustomResourceController(null)));
+        assertEquals(TestCustomResourceList.class, ControllerUtils.getCustomResourceListClass(new TestCustomResourceController(null)));
+        assertEquals(CRD_NAME, ControllerUtils.getCrdName(new TestCustomResourceController(null)).get());
     }
 
 }

operator-framework/src/main/java/com/github/containersolutions/operator/processing/EventDispatcher.java

@@ -6,12 +6,12 @@
 import com.github.containersolutions.operator.processing.EventDispatcher;
 import com.github.containersolutions.operator.sample.TestCustomResource;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
-import io.fabric8.kubernetes.client.*;
-import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.client.CustomResourceDoneable;
+import io.fabric8.kubernetes.client.CustomResourceList;
+import io.fabric8.kubernetes.client.Watcher;
 import io.fabric8.kubernetes.client.dsl.Replaceable;
-import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
-import io.fabric8.kubernetes.client.dsl.internal.RawCustomResourceOperationsImpl;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentMatchers;
@@ -27,49 +27,39 @@ class EventDispatcherTest {
     private CustomResource testCustomResource;
     private EventDispatcher<CustomResource> eventDispatcher;
     private ResourceController<CustomResource> resourceController = mock(ResourceController.class);
-    private NonNamespaceOperation<CustomResource, CustomResourceList<CustomResource>,
-            CustomResourceDoneable<CustomResource>,
-            Resource<CustomResource, CustomResourceDoneable<CustomResource>>>
-            operation = mock(NonNamespaceOperation.class);
-    private KubernetesClient k8sClient = mock(KubernetesClient.class);
     private CustomResourceOperationsImpl<CustomResource, CustomResourceList<CustomResource>,
             CustomResourceDoneable<CustomResource>> resourceOperation = mock(CustomResourceOperationsImpl.class);
 
-    private RawCustomResourceOperationsImpl rawResourceOperations = mock(RawCustomResourceOperationsImpl.class);
     @BeforeEach
     void setup() {
-        eventDispatcher = new EventDispatcher(resourceController, resourceOperation, operation, k8sClient,
+        eventDispatcher = new EventDispatcher(resourceController, resourceOperation,
                 Controller.DEFAULT_FINALIZER);
 
         testCustomResource = getResource();
 
-        when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.of(testCustomResource));
-        when(resourceController.deleteResource(eq(testCustomResource), any())).thenReturn(true);
+        when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.of(testCustomResource));
+        when(resourceController.deleteResource(eq(testCustomResource))).thenReturn(true);
         when(resourceOperation.lockResourceVersion(any())).thenReturn(mock(Replaceable.class));
-
-        // K8s client mocking
-        when(k8sClient.customResource(any())).thenReturn(rawResourceOperations);
-        when(rawResourceOperations.get(any(), any())).thenReturn(getRawResource());
     }
 
     @Test
     void callCreateOrUpdateOnNewResource() {
         eventDispatcher.handleEvent(Watcher.Action.ADDED, testCustomResource);
-        verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any());
+        verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource));
     }
 
     @Test
     void callCreateOrUpdateOnModifiedResource() {
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
-        verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource), any());
+        verify(resourceController, times(1)).createOrUpdateResource(ArgumentMatchers.eq(testCustomResource));
     }
 
     @Test
     void adsDefaultFinalizerOnCreateIfNotThere() {
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
         verify(resourceController, times(1))
                 .createOrUpdateResource(argThat(testCustomResource ->
-                        testCustomResource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)), any());
+                        testCustomResource.getMetadata().getFinalizers().contains(Controller.DEFAULT_FINALIZER)));
     }
 
     @Test
@@ -79,7 +69,7 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
 
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
-        verify(resourceController, times(1)).deleteResource(eq(testCustomResource), any());
+        verify(resourceController, times(1)).deleteResource(eq(testCustomResource));
     }
 
     /**
@@ -91,7 +81,7 @@ void callDeleteOnControllerIfMarkedForDeletionButThereIsNoDefaultFinalizer() {
 
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
-        verify(resourceController).deleteResource(eq(testCustomResource), any());
+        verify(resourceController).deleteResource(eq(testCustomResource));
     }
 
     @Test
@@ -107,7 +97,7 @@ void removesDefaultFinalizerOnDelete() {
 
     @Test
     void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
-        when(resourceController.deleteResource(eq(testCustomResource), any())).thenReturn(false);
+        when(resourceController.deleteResource(eq(testCustomResource))).thenReturn(false);
         markForDeletion(testCustomResource);
         testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
 
@@ -120,7 +110,7 @@ void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
     @Test
     void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
         testCustomResource.getMetadata().getFinalizers().add(Controller.DEFAULT_FINALIZER);
-        when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.empty());
+        when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
 
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
@@ -129,7 +119,7 @@ void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
 
     @Test
     void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
-        when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.empty());
+        when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
 
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
@@ -140,26 +130,14 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
     @Test
     void doesNotAddFinalizerIfOptionalIsReturnedButMarkedForDeletion() {
         markForDeletion(testCustomResource);
-        when(resourceController.createOrUpdateResource(eq(testCustomResource), any())).thenReturn(Optional.empty());
+        when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
 
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
         assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
         verify(resourceOperation, never()).lockResourceVersion(any());
     }
 
-    /**
-     * When receiving a DELETE event the resource is already gone and we did the processing already on a MODIFIED
-     * event received when then finalizer is added. The correct action on a DELETE event is a noop.
-     */
-    @Test
-    void doesntDoAnythingOnDeleteEvent() {
-        eventDispatcher.handleEvent(Watcher.Action.DELETED, testCustomResource);
-
-        verify(k8sClient, never()).customResource(any());
-    }
-
-
     private void markForDeletion(CustomResource customResource) {
         customResource.getMetadata().setDeletionTimestamp("2019-8-10");
     }

operator-framework/src/test/java/com/github/containersolutions/operator/ControllerUtilsTest.java

@@ -44,7 +44,7 @@ public void initialize() {
                     .withMetadata(new ObjectMetaBuilder().withName(TEST_NAMESPACE).build()).build());
         }
         operator = new Operator(k8sClient);
-        operator.registerController(new TestCustomResourceController(), TEST_NAMESPACE);
+        operator.registerController(new TestCustomResourceController(k8sClient), TEST_NAMESPACE);
     }
 
     public void cleanup() {