diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index a5ea74de87..78f42e3bed 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -280,7 +280,6 @@ private void updatePostExecutionControlWithReschedule( baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule); } - private PostExecutionControl

handleCleanup(P resource, Context

context) { log.debug( @@ -295,12 +294,23 @@ private PostExecutionControl

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/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index f6f46daf19..9b113625df 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/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(); }