diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java index eb44735a89..43da799688 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java @@ -7,15 +7,17 @@ public class UpdateControl extends BaseControl UpdateControl updateResource(T customResource) { - return new UpdateControl<>(customResource, false, true); + return new UpdateControl<>(customResource, false, true, false); } public static UpdateControl updateStatus(T customResource) { - return new UpdateControl<>(customResource, true, false); + return new UpdateControl<>(customResource, true, false, false); + } + + public static UpdateControl patchResource(T customResource) { + return new UpdateControl<>(customResource, false, true, true); + } + + public static UpdateControl patchStatus(T customResource) { + return new UpdateControl<>(customResource, true, false, true); } /** @@ -45,11 +55,17 @@ public static UpdateControl updateStatus(T customReso */ public static UpdateControl updateResourceAndStatus( T customResource) { - return new UpdateControl<>(customResource, true, true); + return new UpdateControl<>(customResource, true, true, false); + } + + public static UpdateControl patchResourceAndStatus( + T customResource) { + return new UpdateControl<>(customResource, true, true, true); } + public static UpdateControl noUpdate() { - return new UpdateControl<>(null, false, false); + return new UpdateControl<>(null, false, false, false); } public T getResource() { @@ -64,6 +80,10 @@ public boolean isUpdateResource() { return updateResource; } + public boolean isPatch() { + return patch; + } + public boolean isNoUpdate() { return !updateResource && !updateStatus; } 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 2a0f0997f0..ffe4018ccd 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 @@ -133,22 +133,28 @@ private PostExecutionControl reconcileExecution(ExecutionScope executionSc UpdateControl updateControl = controller.reconcile(resourceForExecution, context); R updatedCustomResource = null; if (updateControl.isUpdateResourceAndStatus()) { - updatedCustomResource = updateCustomResource(updateControl.getResource()); + updatedCustomResource = + updateCustomResource(updateControl.getResource(), updateControl.isPatch()); updateControl .getResource() .getMetadata() .setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); - updatedCustomResource = updateStatusGenerationAware(updateControl.getResource()); + updatedCustomResource = + updateStatusGenerationAware(updateControl.getResource(), updateControl.isPatch()); } else if (updateControl.isUpdateStatus()) { - updatedCustomResource = updateStatusGenerationAware(updateControl.getResource()); + updatedCustomResource = + updateStatusGenerationAware(updateControl.getResource(), updateControl.isPatch()); } else if (updateControl.isUpdateResource()) { - updatedCustomResource = updateCustomResource(updateControl.getResource()); + updatedCustomResource = + updateCustomResource(updateControl.getResource(), updateControl.isPatch()); if (shouldUpdateObservedGenerationAutomatically(updatedCustomResource)) { - updatedCustomResource = updateStatusGenerationAware(originalResource); + updatedCustomResource = + updateStatusGenerationAware(originalResource, updateControl.isPatch()); } } else if (updateControl.isNoUpdate() && shouldUpdateObservedGenerationAutomatically(resourceForExecution)) { - updatedCustomResource = updateStatusGenerationAware(originalResource); + updatedCustomResource = + updateStatusGenerationAware(originalResource, updateControl.isPatch()); } return createPostExecutionControl(updatedCustomResource, updateControl); } @@ -197,9 +203,13 @@ private boolean isErrorStatusHandlerPresent() { return controller.getReconciler() instanceof ErrorStatusHandler; } - private R updateStatusGenerationAware(R resource) { + private R updateStatusGenerationAware(R resource, boolean patch) { updateStatusObservedGenerationIfRequired(resource); - return customResourceFacade.updateStatus(resource); + if (patch) { + return customResourceFacade.patchStatus(resource); + } else { + return customResourceFacade.updateStatus(resource); + } } private boolean shouldUpdateObservedGenerationAutomatically(R resource) { @@ -281,13 +291,17 @@ private void updateCustomResourceWithFinalizer(R resource) { log.debug( "Adding finalizer for resource: {} version: {}", getUID(resource), getVersion(resource)); resource.addFinalizer(configuration().getFinalizerName()); - replace(resource); + customResourceFacade.replaceResourceWithLock(resource); } - private R updateCustomResource(R resource) { + private R updateCustomResource(R resource, boolean patch) { log.debug("Updating resource: {} with version: {}", getUID(resource), getVersion(resource)); log.trace("Resource before update: {}", resource); - return replace(resource); + if (patch) { + return customResourceFacade.patchResource(resource); + } else { + return customResourceFacade.replaceResourceWithLock(resource); + } } private R removeFinalizer(R resource) { @@ -296,16 +310,9 @@ private R removeFinalizer(R resource) { getUID(resource), getVersion(resource)); resource.removeFinalizer(configuration().getFinalizerName()); - return customResourceFacade.replaceWithLock(resource); + return customResourceFacade.replaceResourceWithLock(resource); } - private R replace(R resource) { - log.debug( - "Trying to replace resource {}, version: {}", - getName(resource), - resource.getMetadata().getResourceVersion()); - return customResourceFacade.replaceWithLock(resource); - } ControllerConfiguration configuration() { return controller.getConfiguration(); @@ -321,6 +328,25 @@ public CustomResourceFacade( this.resourceOperation = resourceOperation; } + public R replaceResourceWithLock(R resource) { + log.debug( + "Trying to replace resource {}, version: {}", + getName(resource), + resource.getMetadata().getResourceVersion()); + return resourceOperation + .inNamespace(resource.getMetadata().getNamespace()) + .withName(getName(resource)) + .lockResourceVersion(resource.getMetadata().getResourceVersion()) + .replace(resource); + } + + public R patchResource(R resource) { + return resourceOperation + .inNamespace(resource.getMetadata().getNamespace()) + .withName(getName(resource)) + .patch(resource); + } + public R updateStatus(R resource) { log.trace("Updating status for resource: {}", resource); return resourceOperation @@ -329,12 +355,12 @@ public R updateStatus(R resource) { .replaceStatus(resource); } - public R replaceWithLock(R resource) { + public R patchStatus(R resource) { + log.trace("Updating status for resource: {}", resource); return resourceOperation .inNamespace(resource.getMetadata().getNamespace()) .withName(getName(resource)) - .lockResourceVersion(resource.getMetadata().getResourceVersion()) - .replace(resource); + .patchStatus(resource); } } } 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 ccf218d3d0..d1b6883075 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 @@ -120,7 +120,7 @@ void addFinalizerOnNewResource() throws Exception { verify(reconciler, never()) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) - .replaceWithLock( + .replaceResourceWithLock( argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER))); assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue(); } @@ -142,7 +142,7 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() throws Exception { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); verify(customResourceFacade, times(1)).updateStatus(testCustomResource); - verify(customResourceFacade, never()).replaceWithLock(any()); + verify(customResourceFacade, never()).replaceResourceWithLock(any()); } @Test @@ -150,14 +150,42 @@ void updatesBothResourceAndStatusIfFinalizerSet() throws Exception { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.updateResourceAndStatus(testCustomResource); - when(customResourceFacade.replaceWithLock(testCustomResource)).thenReturn(testCustomResource); + when(customResourceFacade.replaceResourceWithLock(testCustomResource)) + .thenReturn(testCustomResource); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(1)).replaceWithLock(testCustomResource); + verify(customResourceFacade, times(1)).replaceResourceWithLock(testCustomResource); verify(customResourceFacade, times(1)).updateStatus(testCustomResource); } + + @Test + void patchesResource() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + + reconciler.reconcile = (r, c) -> UpdateControl.patchResource(testCustomResource); + + reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + + verify(customResourceFacade, times(1)).patchResource(testCustomResource); + verify(customResourceFacade, never()).updateStatus(any()); + verify(customResourceFacade, never()).replaceResourceWithLock(any()); + } + + @Test + void patchesStatus() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + + reconciler.reconcile = (r, c) -> UpdateControl.patchStatus(testCustomResource); + + reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + + verify(customResourceFacade, times(1)).patchStatus(testCustomResource); + verify(customResourceFacade, never()).updateStatus(any()); + verify(customResourceFacade, never()).replaceResourceWithLock(any()); + } + @Test void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() throws Exception { testCustomResource.addFinalizer(DEFAULT_FINALIZER); @@ -216,7 +244,7 @@ void removesDefaultFinalizerOnDeleteIfSet() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(0, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, times(1)).replaceWithLock(any()); + verify(customResourceFacade, times(1)).replaceResourceWithLock(any()); } @Test @@ -229,7 +257,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, never()).replaceWithLock(any()); + verify(customResourceFacade, never()).replaceResourceWithLock(any()); } @Test @@ -239,7 +267,7 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() throws Exce reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, never()).replaceWithLock(any()); + verify(customResourceFacade, never()).replaceResourceWithLock(any()); verify(customResourceFacade, never()).updateStatus(testCustomResource); } @@ -252,7 +280,7 @@ void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() throws reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, times(1)).replaceWithLock(any()); + verify(customResourceFacade, times(1)).replaceResourceWithLock(any()); } @Test @@ -262,7 +290,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, never()).replaceWithLock(any()); + verify(customResourceFacade, never()).replaceResourceWithLock(any()); verify(reconciler, never()).cleanup(eq(testCustomResource), any()); } @@ -388,7 +416,7 @@ void updateObservedGenerationOnCustomResourceUpdate() throws Exception { when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())) .thenReturn(UpdateControl.updateResource(observedGenResource)); - when(facade.replaceWithLock(any())).thenReturn(observedGenResource); + when(facade.replaceResourceWithLock(any())).thenReturn(observedGenResource); when(facade.updateStatus(observedGenResource)).thenReturn(observedGenResource); var dispatcher = init(observedGenResource, reconciler, config, facade, true); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java index 3688b55d3c..67bc908482 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java @@ -16,6 +16,7 @@ import static org.awaitility.Awaitility.await; class ControllerExecutionIT { + @RegisterExtension OperatorExtension operator = OperatorExtension.builder().withReconciler(new TestReconciler(true)).build(); @@ -32,6 +33,17 @@ void configMapGetsCreatedForTestCustomResource() { assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); } + @Test + void patchesStatusForTestCustomResource() { + operator.getControllerOfType(TestReconciler.class).setPatchStatus(true); + operator.getControllerOfType(TestReconciler.class).setUpdateStatus(true); + + TestCustomResource resource = TestUtils.testCustomResource(); + operator.create(TestCustomResource.class, resource); + + awaitStatusUpdated(); + } + @Test void eventIsSkippedChangedOnMetadataOnlyUpdate() { operator.getControllerOfType(TestReconciler.class).setUpdateStatus(false); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestReconciler.java index 78db3b16f7..ebf809799e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestReconciler.java @@ -31,10 +31,21 @@ public class TestReconciler private final AtomicInteger numberOfExecutions = new AtomicInteger(0); private final AtomicInteger numberOfCleanupExecutions = new AtomicInteger(0); private KubernetesClient kubernetesClient; - private boolean updateStatus; + private volatile boolean updateStatus; + private volatile boolean patchStatus; public TestReconciler(boolean updateStatus) { + this(updateStatus, false); + } + + public TestReconciler(boolean updateStatus, boolean patchStatus) { this.updateStatus = updateStatus; + this.patchStatus = patchStatus; + } + + public TestReconciler setPatchStatus(boolean patchStatus) { + this.patchStatus = patchStatus; + return this; } public void setUpdateStatus(boolean updateStatus) { @@ -122,7 +133,11 @@ public UpdateControl reconcile( } resource.getStatus().setConfigMapStatus("ConfigMap Ready"); } - return UpdateControl.updateStatus(resource); + if (patchStatus) { + return UpdateControl.patchStatus(resource); + } else { + return UpdateControl.updateStatus(resource); + } } private Map configMapData(TestCustomResource resource) {