Improvement/handle namespace deletion on finalizer gracefully (#1988)

operator-framework · Aug 1, 2023 · d4201fe · d4201fe
1 parent 03e06ce
commit d4201fe
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 10 deletions.
diff --git a/.../src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/.../src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java
@@ -280,7 +280,6 @@ private void updatePostExecutionControlWithReschedule(
     baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
   }
 
-
   private PostExecutionControl<P> handleCleanup(P resource,
       Context<P> context) {
     log.debug(
@@ -295,12 +294,23 @@ private PostExecutionControl<P> handleCleanup(P resource,
       // cleanup is finished, nothing left to done
       final var finalizerName = configuration().getFinalizerName();
       if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) {
-        P customResource = conflictRetryingUpdate(resource, r -> r.removeFinalizer(finalizerName));
+        P customResource = conflictRetryingUpdate(resource, r -> {
+          // the operator might not be allowed to retrieve the resource on a retry, e.g. when its
+          // permissions are removed by deleting the namespace concurrently
+          if (r == null) {
+            log.warn(
+                "Could not remove finalizer on null resource: {} with version: {}",
+                getUID(resource),
+                getVersion(resource));
+            return false;
+          }
+          return r.removeFinalizer(finalizerName);
+        });
         return PostExecutionControl.customResourceFinalizerRemoved(customResource);
       }
     }
     log.debug(
-        "Skipping finalizer remove for resource: {} with version: {}. delete control: {}, uses finalizer: {} ",
+        "Skipping finalizer remove for resource: {} with version: {}. delete control: {}, uses finalizer: {}",
         getUID(resource),
         getVersion(resource),
         deleteControl,
@@ -385,15 +395,14 @@ public R updateResource(R resource) {
           getName(resource),
           resource.getMetadata().getResourceVersion());
       return resource(resource).lockResourceVersion(resource.getMetadata().getResourceVersion())
-          .replace();
+          .update();
     }
 
-    @SuppressWarnings({"rawtypes", "unchecked"})
     public R updateStatus(R resource) {
       log.trace("Updating status for resource: {}", resource);
       return resource(resource)
           .lockResourceVersion()
-          .replaceStatus();
+          .updateStatus();
     }
 
     public R patchStatus(R resource, R originalResource) {

diff --git a/.../test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/.../test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java
@@ -240,6 +240,24 @@ void retriesFinalizerRemovalWithFreshResource() {
     verify(customResourceFacade, times(1)).getResource(any(), any());
   }
 
+  @Test
+  void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() {
+    // simulate the operator not able or not be allowed to get the custom resource during the retry
+    // of the finalizer removal
+    testCustomResource.addFinalizer(DEFAULT_FINALIZER);
+    markForDeletion(testCustomResource);
+    when(customResourceFacade.updateResource(any()))
+        .thenThrow(new KubernetesClientException(null, 409, null));
+    when(customResourceFacade.getResource(any(), any())).thenReturn(null);
+
+    var postExecControl =
+        reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
+
+    assertThat(postExecControl.isFinalizerRemoved()).isTrue();
+    verify(customResourceFacade, times(1)).updateResource(testCustomResource);
+    verify(customResourceFacade, times(1)).getResource(any(), any());
+  }
+
   @Test
   void throwsExceptionIfFinalizerRemovalRetryExceeded() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);
@@ -307,8 +325,6 @@ void doesNotAddFinalizerIfConfiguredNotTo() {
     assertEquals(0, testCustomResource.getMetadata().getFinalizers().size());
   }
 
-
-
   @Test
   void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);
@@ -590,7 +606,6 @@ void errorStatusHandlerCanPatchResource() {
     reconciler.errorHandler =
         (r, ri, e) -> ErrorStatusUpdateControl.patchStatus(testCustomResource);
 
-
     reconciliationDispatcher.handleExecution(
         new ExecutionScope(null).setResource(testCustomResource));
 
@@ -673,7 +688,6 @@ TestCustomResource createResourceWithFinalizer() {
     return resourceWithFinalizer;
   }
 
-
   private void removeFinalizers(CustomResource customResource) {
     customResource.getMetadata().getFinalizers().clear();
   }