Merge pull request operator-framework#62 from ContainerSolutions/resource_replace_namespace_handling

fix for issues, when we are having different default namespace then custom resource namespace

Author: csviri

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

@@ -62,9 +62,8 @@ private <R extends CustomResource> void registerController(ResourceController<R>
         Class<? extends CustomResourceList<R>> list = getCustomResourceListClass(controller);
         Class<? extends CustomResourceDoneable<R>> doneable = getCustomResourceDonebaleClass(controller);
         MixedOperation client = k8sClient.customResources(crd, resClass, list, doneable);
-
-        EventDispatcher eventDispatcher = new EventDispatcher(controller, (CustomResourceOperationsImpl) client,
-                getDefaultFinalizer(controller));
+        EventDispatcher eventDispatcher = new EventDispatcher(controller,
+                getDefaultFinalizer(controller), new EventDispatcher.CustomResourceReplaceFacade(client));
         EventScheduler eventScheduler = new EventScheduler(eventDispatcher, retry);
         registerWatches(controller, client, resClass, watchAllNamespaces, targetNamespaces, eventScheduler);
     }

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

@@ -3,7 +3,8 @@
 import com.github.containersolutions.operator.api.ResourceController;
 import io.fabric8.kubernetes.client.CustomResource;
 import io.fabric8.kubernetes.client.Watcher;
-import io.fabric8.kubernetes.client.dsl.internal.CustomResourceOperationsImpl;
+import io.fabric8.kubernetes.client.dsl.MixedOperation;
+import io.fabric8.kubernetes.client.dsl.Resource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -18,15 +19,14 @@ public class EventDispatcher {
     private final static Logger log = LoggerFactory.getLogger(EventDispatcher.class);
 
     private final ResourceController controller;
-    private final CustomResourceOperationsImpl resourceOperation;
     private final String resourceDefaultFinalizer;
+    private final CustomResourceReplaceFacade customResourceReplaceFacade;
 
     public EventDispatcher(ResourceController controller,
-                           CustomResourceOperationsImpl resourceOperation,
-                           String defaultFinalizer
-    ) {
+                           String defaultFinalizer,
+                           CustomResourceReplaceFacade customResourceReplaceFacade) {
         this.controller = controller;
-        this.resourceOperation = resourceOperation;
+        this.customResourceReplaceFacade = customResourceReplaceFacade;
         this.resourceDefaultFinalizer = defaultFinalizer;
     }
 
@@ -77,12 +77,12 @@ private boolean hasDefaultFinalizer(CustomResource resource) {
     private void removeDefaultFinalizer(CustomResource resource) {
         resource.getMetadata().getFinalizers().remove(resourceDefaultFinalizer);
         log.debug("Removed finalizer. Trying to replace resource {}, version: {}", resource.getMetadata().getName(), resource.getMetadata().getResourceVersion());
-        resourceOperation.lockResourceVersion(resource.getMetadata().getResourceVersion()).replace(resource);
+        customResourceReplaceFacade.replaceWithLock(resource);
     }
 
     private void replace(CustomResource resource) {
         log.debug("Trying to replace resource {}, version: {}", resource.getMetadata().getName(), resource.getMetadata().getResourceVersion());
-        resourceOperation.lockResourceVersion(resource.getMetadata().getResourceVersion()).replace(resource);
+        customResourceReplaceFacade.replaceWithLock(resource);
     }
 
     private void addFinalizerIfNotPresent(CustomResource resource) {
@@ -98,4 +98,21 @@ private void addFinalizerIfNotPresent(CustomResource resource) {
     private boolean markedForDeletion(CustomResource resource) {
         return resource.getMetadata().getDeletionTimestamp() != null && !resource.getMetadata().getDeletionTimestamp().isEmpty();
     }
+
+    // created to support unit testing
+    public static class CustomResourceReplaceFacade {
+
+        private final MixedOperation<?, ?, ?, Resource<CustomResource, ?>> resourceOperation;
+
+        public CustomResourceReplaceFacade(MixedOperation<?, ?, ?, Resource<CustomResource, ?>> resourceOperation) {
+            this.resourceOperation = resourceOperation;
+        }
+
+        public CustomResource replaceWithLock(CustomResource resource) {
+            return resourceOperation.inNamespace(resource.getMetadata().getNamespace())
+                    .withName(resource.getMetadata().getName())
+                    .lockResourceVersion(resource.getMetadata().getResourceVersion())
+                    .replace(resource);
+        }
+    }
 }

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

@@ -7,11 +7,7 @@
 import com.github.containersolutions.operator.sample.TestCustomResource;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 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.internal.CustomResourceOperationsImpl;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentMatchers;
@@ -27,19 +23,19 @@ class EventDispatcherTest {
     private CustomResource testCustomResource;
     private EventDispatcher eventDispatcher;
     private ResourceController<CustomResource> resourceController = mock(ResourceController.class);
-    private CustomResourceOperationsImpl<CustomResource, CustomResourceList<CustomResource>,
-            CustomResourceDoneable<CustomResource>> resourceOperation = mock(CustomResourceOperationsImpl.class);
+    private EventDispatcher.CustomResourceReplaceFacade customResourceReplaceFacade = mock(EventDispatcher.CustomResourceReplaceFacade.class);
+
 
     @BeforeEach
     void setup() {
-        eventDispatcher = new EventDispatcher(resourceController, resourceOperation,
-                Controller.DEFAULT_FINALIZER);
+        eventDispatcher = new EventDispatcher(resourceController,
+                Controller.DEFAULT_FINALIZER, customResourceReplaceFacade);
 
         testCustomResource = getResource();
 
         when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.of(testCustomResource));
         when(resourceController.deleteResource(eq(testCustomResource))).thenReturn(true);
-        when(resourceOperation.lockResourceVersion(any())).thenReturn(mock(Replaceable.class));
+        when(customResourceReplaceFacade.replaceWithLock(any())).thenReturn(null);
     }
 
     @Test
@@ -92,7 +88,7 @@ void removesDefaultFinalizerOnDelete() {
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
         assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
-        verify(resourceOperation, times(1)).lockResourceVersion(any());
+        verify(customResourceReplaceFacade, times(1)).replaceWithLock(any());
     }
 
     @Test
@@ -104,7 +100,7 @@ void doesNotRemovesTheFinalizerIfTheDeleteMethodRemovesFalse() {
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
         assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
-        verify(resourceOperation, never()).lockResourceVersion(any());
+        verify(customResourceReplaceFacade, never()).replaceWithLock(any());
     }
 
     @Test
@@ -113,8 +109,7 @@ void doesNotUpdateTheResourceIfEmptyOptionalReturned() {
         when(resourceController.createOrUpdateResource(eq(testCustomResource))).thenReturn(Optional.empty());
 
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
-
-        verify(resourceOperation, never()).lockResourceVersion(any());
+        verify(customResourceReplaceFacade, never()).replaceWithLock(any());
     }
 
     @Test
@@ -124,7 +119,7 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() {
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
         assertEquals(1, testCustomResource.getMetadata().getFinalizers().size());
-        verify(resourceOperation, times(1)).lockResourceVersion(any());
+        verify(customResourceReplaceFacade, times(1)).replaceWithLock(any());
     }
 
     @Test
@@ -135,7 +130,7 @@ void doesNotAddFinalizerIfOptionalIsReturnedButMarkedForDeletion() {
         eventDispatcher.handleEvent(Watcher.Action.MODIFIED, testCustomResource);
 
         assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
-        verify(resourceOperation, never()).lockResourceVersion(any());
+        verify(customResourceReplaceFacade, never()).replaceWithLock(any());
     }
 
     private void markForDeletion(CustomResource customResource) {