From 99464f8c7711457ec760c80551cbfa77acb8cc3d Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 16 Nov 2021 15:38:49 +0100 Subject: [PATCH 1/6] feat: Error Reporting Support in Status --- docs/documentation/features.md | 24 +++++ .../api/reconciler/ErrorStatusHandler.java | 27 ++++++ .../processing/ReconciliationDispatcher.java | 94 ++++++++++++++----- .../ReconciliationDispatcherTest.java | 87 ++++++++++------- .../operator/ErrorStatusHandlerIT.java | 58 ++++++++++++ .../operator/EventSourceIT.java | 1 - .../ErrorStatusHandlerTestCustomResource.java | 17 ++++ ...StatusHandlerTestCustomResourceStatus.java | 15 +++ .../ErrorStatusHandlerTestReconciler.java | 53 +++++++++++ 9 files changed, 318 insertions(+), 58 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResourceStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestReconciler.java diff --git a/docs/documentation/features.md b/docs/documentation/features.md index ddaee104c9..9c4f30ff84 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -127,6 +127,30 @@ won't be a result of a retry, but the new event. A successful execution resets the retry. +### Setting Error Status After Last Retry Attempt + +In order to facilitate error reporting in case a last retry attempt fails, Reconciler can implement the following +interface: + +```java +public interface ErrorStatusHandler> { + + T updateErrorStatus(T resource, RuntimeException e); + +} +``` + +The `updateErrorStatus` resource is called when it's the last retry attempt according the retry configuration and the +reconciler execution still resulted in a runtime exception. + +The result of the method call is used to make a status sub-resource update on the custom resource. This is always a +sub-resource update request, so no update on custom resource itself (like spec of metadata) happens. Note that this +update request will also produce an event, and will result in a reconciliation if the controller is not generation +aware. + +Note that the scope of this feature is only the `reconcile` method of the reconciler, since there should not be updates +on custom resource after it is marked for deletion. + ### Correctness and Automatic Retries There is a possibility to turn of the automatic retries. This is not desirable, unless there is a very specific reason. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java new file mode 100644 index 0000000000..43c52ce88a --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java @@ -0,0 +1,27 @@ +package io.javaoperatorsdk.operator.api.reconciler; + +import io.fabric8.kubernetes.client.CustomResource; + +public interface ErrorStatusHandler> { + + /** + *

+ * Reconcile can implement this interface in order to update the status sub-resource in the case when + * the last reconciliation retry attempt is failed on the Reconciler. In that case the updateErrorStatus is + * called automatically. + *

+ * The result of the method call is used to make a status sub-resource update on the custom resource. This is always a + * sub-resource update request, so no update on custom resource itself (like spec of metadata) happens. Note that this + * update request will also produce an event, and will result in a reconciliation if the controller is not generation + * aware. + *

+ * Note that the scope of this feature is only the reconcile method of the reconciler, since there should not be updates + * on custom resource after it is marked for deletion. + * + * @param resource to update the status on + * @param e exception thrown from the reconciler + * @return the updated resource + */ + T updateErrorStatus(T resource, RuntimeException e); + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java index fa01bce577..69cdbdc640 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java @@ -10,12 +10,7 @@ import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.api.ObservedGenerationAware; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.BaseControl; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; -import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.*; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; @@ -109,27 +104,76 @@ private PostExecutionControl handleCreateOrUpdate( updateCustomResourceWithFinalizer(resource); return PostExecutionControl.onlyFinalizerAdded(); } else { - log.debug( - "Executing createOrUpdate for resource {} with version: {} with execution scope: {}", - getName(resource), - getVersion(resource), - executionScope); + try { + var resourceForExecution = + cloneResourceIfErrorStatusHandlerIfCouldBeCalled(resource, context); + return createOrUpdateExecution(executionScope, resourceForExecution, context); + } catch (RuntimeException e) { + handleLastAttemptErrorStatusHandler(resource, context, e); + throw e; + } + } + } + + /** + * Resource make sense only to clone for the ErrorStatusHandler. Otherwise, this operation can be + * skipped since it can be memory and time-consuming. However, it needs to be cloned since it's + * common that the custom resource is changed during an execution, and it's much cleaner to have + * to original resource in place for status update. + */ + private R cloneResourceIfErrorStatusHandlerIfCouldBeCalled(R resource, Context context) { + if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { + return (R) this.controller.getConfiguration().getConfigurationService().getResourceCloner() + .clone(resource); + } else { + return resource; + } + } - UpdateControl updateControl = controller.reconcile(resource, context); - R updatedCustomResource = null; - if (updateControl.isUpdateCustomResourceAndStatusSubResource()) { - updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); - updateControl - .getCustomResource() - .getMetadata() - .setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); - updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); - } else if (updateControl.isUpdateStatusSubResource()) { - updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); - } else if (updateControl.isUpdateCustomResource()) { - updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); + private PostExecutionControl createOrUpdateExecution(ExecutionScope executionScope, + R resource, Context context) { + log.debug( + "Executing createOrUpdate for resource {} with version: {} with execution scope: {}", + getName(resource), + getVersion(resource), + executionScope); + + UpdateControl updateControl = controller.reconcile(resource, context); + R updatedCustomResource = null; + if (updateControl.isUpdateCustomResourceAndStatusSubResource()) { + updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); + updateControl + .getCustomResource() + .getMetadata() + .setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); + updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); + } else if (updateControl.isUpdateStatusSubResource()) { + updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); + } else if (updateControl.isUpdateCustomResource()) { + updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); + } + return createPostExecutionControl(updatedCustomResource, updateControl); + } + + private void handleLastAttemptErrorStatusHandler(R resource, Context context, + RuntimeException e) { + if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { + try { + var updatedResource = ((ErrorStatusHandler) controller.getReconciler()) + .updateErrorStatus(resource, e); + customResourceFacade.updateStatus(updatedResource); + } catch (RuntimeException ex) { + log.error("Error during error status handling.", ex); } - return createPostExecutionControl(updatedCustomResource, updateControl); + } + } + + private boolean isLastAttemptOfRetryAndErrorStatusHandlerPresent(Context context) { + if (context.getRetryInfo().isPresent()) { + return context.getRetryInfo().get().isLastAttempt() + && controller.getReconciler() instanceof ErrorStatusHandler; + } else { + return false; } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcherTest.java index 05b4b68ff4..781b48a53b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcherTest.java @@ -13,11 +13,7 @@ import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.processing.ReconciliationDispatcher.CustomResourceFacade; import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenCustomResource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -27,20 +23,16 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; class ReconciliationDispatcherTest { private static final String DEFAULT_FINALIZER = "javaoperatorsdk.io/finalizer"; + public static final String ERROR_MESSAGE = "ErrorMessage"; private TestCustomResource testCustomResource; private ReconciliationDispatcher reconciliationDispatcher; - private final Reconciler controller = mock(Reconciler.class); + private final Reconciler reconciler = mock(Reconciler.class, + withSettings().extraInterfaces(ErrorStatusHandler.class)); private final ControllerConfiguration configuration = mock(ControllerConfiguration.class); private final ConfigurationService configService = mock(ConfigurationService.class); @@ -51,7 +43,7 @@ class ReconciliationDispatcherTest { void setup() { testCustomResource = TestUtils.testCustomResource(); reconciliationDispatcher = - init(testCustomResource, controller, configuration, customResourceFacade); + init(testCustomResource, reconciler, configuration, customResourceFacade); } private > ReconciliationDispatcher init(R customResource, @@ -62,6 +54,7 @@ void setup() { when(configuration.getName()).thenReturn("EventDispatcherTestController"); when(configService.getMetrics()).thenReturn(Metrics.NOOP); when(configuration.getConfigurationService()).thenReturn(configService); + when(configService.getResourceCloner()).thenReturn(ConfigurationService.DEFAULT_CLONER); when(reconciler.reconcile(eq(customResource), any())) .thenReturn(UpdateControl.updateCustomResource(customResource)); when(reconciler.cleanup(eq(customResource), any())) @@ -77,7 +70,7 @@ void setup() { void addFinalizerOnNewResource() { assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller, never()) + verify(reconciler, never()) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) .replaceWithLock( @@ -89,7 +82,7 @@ void addFinalizerOnNewResource() { void callCreateOrUpdateOnNewResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller, times(1)) + verify(reconciler, times(1)) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); } @@ -97,7 +90,7 @@ void callCreateOrUpdateOnNewResourceIfFinalizerSet() { void updatesOnlyStatusSubResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(controller.reconcile(eq(testCustomResource), any())) + when(reconciler.reconcile(eq(testCustomResource), any())) .thenReturn(UpdateControl.updateStatusSubResource(testCustomResource)); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -110,7 +103,7 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() { void updatesBothResourceAndStatusIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(controller.reconcile(eq(testCustomResource), any())) + when(reconciler.reconcile(eq(testCustomResource), any())) .thenReturn(UpdateControl.updateCustomResourceAndStatus(testCustomResource)); when(customResourceFacade.replaceWithLock(testCustomResource)).thenReturn(testCustomResource); @@ -125,7 +118,7 @@ void callCreateOrUpdateOnModifiedResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller, times(1)) + verify(reconciler, times(1)) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); } @@ -137,7 +130,7 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller, times(1)).cleanup(eq(testCustomResource), any()); + verify(reconciler, times(1)).cleanup(eq(testCustomResource), any()); } /** @@ -150,7 +143,7 @@ void callDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller).cleanup(eq(testCustomResource), any()); + verify(reconciler).cleanup(eq(testCustomResource), any()); } @Test @@ -159,7 +152,7 @@ void doNotCallDeleteIfMarkedForDeletionWhenFinalizerHasAlreadyBeenRemoved() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller, never()).cleanup(eq(testCustomResource), any()); + verify(reconciler, never()).cleanup(eq(testCustomResource), any()); } private void configureToNotUseFinalizer() { @@ -170,7 +163,7 @@ private void configureToNotUseFinalizer() { when(configuration.getConfigurationService()).thenReturn(configService); when(configuration.useFinalizer()).thenReturn(false); reconciliationDispatcher = - new ReconciliationDispatcher(new Controller(controller, configuration, null), + new ReconciliationDispatcher(new Controller(reconciler, configuration, null), customResourceFacade); } @@ -198,7 +191,7 @@ void removesDefaultFinalizerOnDeleteIfSet() { void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(controller.cleanup(eq(testCustomResource), any())) + when(reconciler.cleanup(eq(testCustomResource), any())) .thenReturn(DeleteControl.noFinalizerRemoval()); markForDeletion(testCustomResource); @@ -212,7 +205,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(controller.reconcile(eq(testCustomResource), any())) + when(reconciler.reconcile(eq(testCustomResource), any())) .thenReturn(UpdateControl.noUpdate()); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -223,7 +216,7 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { @Test void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); - when(controller.reconcile(eq(testCustomResource), any())) + when(reconciler.reconcile(eq(testCustomResource), any())) .thenReturn(UpdateControl.noUpdate()); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -240,7 +233,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); verify(customResourceFacade, never()).replaceWithLock(any()); - verify(controller, never()).cleanup(eq(testCustomResource), any()); + verify(reconciler, never()).cleanup(eq(testCustomResource), any()); } @Test @@ -249,7 +242,7 @@ void executeControllerRegardlessGenerationInNonGenerationAwareModeIfFinalizerSet reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(controller, times(2)).reconcile(eq(testCustomResource), any()); + verify(reconciler, times(2)).reconcile(eq(testCustomResource), any()); } @Test @@ -273,8 +266,8 @@ public boolean isLastAttempt() { ArgumentCaptor contextArgumentCaptor = ArgumentCaptor.forClass(Context.class); - verify(controller, times(1)) - .reconcile(eq(testCustomResource), contextArgumentCaptor.capture()); + verify(reconciler, times(1)) + .reconcile(any(), contextArgumentCaptor.capture()); Context context = contextArgumentCaptor.getValue(); final var retryInfo = context.getRetryInfo().get(); assertThat(retryInfo.getAttemptCount()).isEqualTo(2); @@ -285,7 +278,7 @@ public boolean isLastAttempt() { void setReScheduleToPostExecutionControlFromUpdateControl() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(controller.reconcile(eq(testCustomResource), any())) + when(reconciler.reconcile(eq(testCustomResource), any())) .thenReturn( UpdateControl.updateStatusSubResource(testCustomResource).rescheduleAfter(1000L)); @@ -300,7 +293,7 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(controller.cleanup(eq(testCustomResource), any())) + when(reconciler.cleanup(eq(testCustomResource), any())) .thenReturn(DeleteControl.noFinalizerRemoval().rescheduleAfter(1000L)); PostExecutionControl control = @@ -331,6 +324,36 @@ void setObservedGenerationForStatusIfNeeded() { } @Test + void callErrorStatusHandlerIfImplemented() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + + when(reconciler.reconcile(any(), any())) + .thenThrow(new IllegalStateException("Error Status Test")); + when(((ErrorStatusHandler) reconciler).updateErrorStatus(any(), any())).then(a -> { + testCustomResource.getStatus().setConfigMapStatus(ERROR_MESSAGE); + return testCustomResource; + }); + + reconciliationDispatcher.handleExecution( + new ExecutionScope( + testCustomResource, + new RetryInfo() { + @Override + public int getAttemptCount() { + return 2; + } + + @Override + public boolean isLastAttempt() { + return true; + } + })); + + verify(customResourceFacade, times(1)).updateStatus(testCustomResource); + verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource), + any()); + } + private ObservedGenCustomResource createObservedGenCustomResource() { ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource(); observedGenCustomResource.setMetadata(new ObjectMeta()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java new file mode 100644 index 0000000000..5f49df5643 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java @@ -0,0 +1,58 @@ +package io.javaoperatorsdk.operator; + +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; +import io.javaoperatorsdk.operator.junit.OperatorExtension; +import io.javaoperatorsdk.operator.processing.retry.GenericRetry; +import io.javaoperatorsdk.operator.sample.errorstatushandler.ErrorStatusHandlerTestCustomResource; +import io.javaoperatorsdk.operator.sample.errorstatushandler.ErrorStatusHandlerTestReconciler; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class ErrorStatusHandlerIT { + + ErrorStatusHandlerTestReconciler reconciler = new ErrorStatusHandlerTestReconciler(); + + @RegisterExtension + OperatorExtension operator = + OperatorExtension.builder() + .withConfigurationService(DefaultConfigurationService.instance()) + .withReconciler(reconciler, new GenericRetry().setMaxAttempts(3).withLinearRetry()) + .build(); + + @Test + public void testErrorMessageSetEventually() { + ErrorStatusHandlerTestCustomResource resource = + operator.create(ErrorStatusHandlerTestCustomResource.class, createCustomResource()); + + await() + .atMost(10, TimeUnit.SECONDS) + .pollInterval(1L, TimeUnit.SECONDS) + .untilAsserted( + () -> { + ErrorStatusHandlerTestCustomResource res = + operator.get(ErrorStatusHandlerTestCustomResource.class, + resource.getMetadata().getName()); + assertThat(res.getStatus()).isNotNull(); + assertThat(res.getStatus().getMessage()) + .isEqualTo(ErrorStatusHandlerTestReconciler.ERROR_STATUS_MESSAGE); + }); + } + + public ErrorStatusHandlerTestCustomResource createCustomResource() { + ErrorStatusHandlerTestCustomResource resource = new ErrorStatusHandlerTestCustomResource(); + resource.setMetadata( + new ObjectMetaBuilder() + .withName("error-status-test") + .withNamespace(operator.getNamespace()) + .build()); + return resource; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java index 509cb74d42..033bbf2b8b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java @@ -47,7 +47,6 @@ public EventSourceTestCustomResource createTestCustomResource(String id) { .withNamespace(operator.getNamespace()) .withFinalizers(EventSourceTestCustomReconciler.FINALIZER_NAME) .build()); - resource.setKind("Eventsourcesample"); resource.setSpec(new EventSourceTestCustomResourceSpec()); resource.getSpec().setValue(id); return resource; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java new file mode 100644 index 0000000000..9fcda379ac --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java @@ -0,0 +1,17 @@ +package io.javaoperatorsdk.operator.sample.errorstatushandler; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Kind; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@Kind("DoubleUpdateSample") +@ShortNames("du") +public class ErrorStatusHandlerTestCustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResourceStatus.java new file mode 100644 index 0000000000..03fc517ecd --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResourceStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.errorstatushandler; + +public class ErrorStatusHandlerTestCustomResourceStatus { + + private String message; + + public String getMessage() { + return message; + } + + public ErrorStatusHandlerTestCustomResourceStatus setMessage(String message) { + this.message = message; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestReconciler.java new file mode 100644 index 0000000000..7ad8f20bae --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestReconciler.java @@ -0,0 +1,53 @@ +package io.javaoperatorsdk.operator.sample.errorstatushandler; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +@ControllerConfiguration +public class ErrorStatusHandlerTestReconciler + implements Reconciler, TestExecutionInfoProvider, + ErrorStatusHandler { + + private static final Logger log = LoggerFactory.getLogger(ErrorStatusHandlerTestReconciler.class); + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + public static final String ERROR_STATUS_MESSAGE = "Error Retries Exceeded"; + + @Override + public UpdateControl reconcile( + ErrorStatusHandlerTestCustomResource resource, Context context) { + var number = numberOfExecutions.addAndGet(1); + var retryAttempt = -1; + if (context.getRetryInfo().isPresent()) { + retryAttempt = context.getRetryInfo().get().getAttemptCount(); + } + log.info("Number of execution: {} retry attempt: {} , resource: {}", number, retryAttempt, + resource); + throw new IllegalStateException(); + } + + private void ensureStatusExists(ErrorStatusHandlerTestCustomResource resource) { + ErrorStatusHandlerTestCustomResourceStatus status = resource.getStatus(); + if (status == null) { + status = new ErrorStatusHandlerTestCustomResourceStatus(); + resource.setStatus(status); + } + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + @Override + public ErrorStatusHandlerTestCustomResource updateErrorStatus( + ErrorStatusHandlerTestCustomResource resource, RuntimeException e) { + log.info("Setting status."); + ensureStatusExists(resource); + resource.getStatus().setMessage(ERROR_STATUS_MESSAGE); + return resource; + } +} From e9cb07c652d21fced339d8166bb07a239410989e Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 16 Nov 2021 15:39:38 +0100 Subject: [PATCH 2/6] fix: formatting --- .../api/reconciler/ErrorStatusHandler.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java index 43c52ce88a..3ea7bb4862 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java @@ -6,17 +6,17 @@ public interface ErrorStatusHandler> { /** *

- * Reconcile can implement this interface in order to update the status sub-resource in the case when - * the last reconciliation retry attempt is failed on the Reconciler. In that case the updateErrorStatus is - * called automatically. + * Reconcile can implement this interface in order to update the status sub-resource in the case + * when the last reconciliation retry attempt is failed on the Reconciler. In that case the + * updateErrorStatus is called automatically. *

- * The result of the method call is used to make a status sub-resource update on the custom resource. This is always a - * sub-resource update request, so no update on custom resource itself (like spec of metadata) happens. Note that this - * update request will also produce an event, and will result in a reconciliation if the controller is not generation - * aware. + * The result of the method call is used to make a status sub-resource update on the custom + * resource. This is always a sub-resource update request, so no update on custom resource itself + * (like spec of metadata) happens. Note that this update request will also produce an event, and + * will result in a reconciliation if the controller is not generation aware. *

- * Note that the scope of this feature is only the reconcile method of the reconciler, since there should not be updates - * on custom resource after it is marked for deletion. + * Note that the scope of this feature is only the reconcile method of the reconciler, since there + * should not be updates on custom resource after it is marked for deletion. * * @param resource to update the status on * @param e exception thrown from the reconciler From d32a41f9fbe633893ac528e50b9f15efba1ae57f Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 16 Nov 2021 16:46:41 +0100 Subject: [PATCH 3/6] fix: cr in IT --- .../io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java | 2 +- .../ErrorStatusHandlerTestCustomResource.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java index 5f49df5643..01908757bc 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java @@ -32,7 +32,7 @@ public void testErrorMessageSetEventually() { operator.create(ErrorStatusHandlerTestCustomResource.class, createCustomResource()); await() - .atMost(10, TimeUnit.SECONDS) + .atMost(15, TimeUnit.SECONDS) .pollInterval(1L, TimeUnit.SECONDS) .untilAsserted( () -> { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java index 9fcda379ac..d6056ea486 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/errorstatushandler/ErrorStatusHandlerTestCustomResource.java @@ -9,8 +9,8 @@ @Group("sample.javaoperatorsdk") @Version("v1") -@Kind("DoubleUpdateSample") -@ShortNames("du") +@Kind("ErrorStatusHandlerTestCustomResource") +@ShortNames("esh") public class ErrorStatusHandlerTestCustomResource extends CustomResource implements Namespaced { From e99e184a15f70f7f5d7263c21778b44113356197 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 19 Nov 2021 14:08:52 +0100 Subject: [PATCH 4/6] refacor: generify Cloner --- .../io/javaoperatorsdk/operator/api/config/Cloner.java | 2 +- .../operator/api/config/ConfigurationService.java | 5 +++-- .../operator/processing/ReconciliationDispatcher.java | 10 ++++++++-- .../event/internal/CustomResourceEventSource.java | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Cloner.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Cloner.java index 92f569f36a..e50931a9b0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Cloner.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Cloner.java @@ -4,6 +4,6 @@ public interface Cloner { - CustomResource clone(CustomResource object); + > T clone(T object); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 24e0605955..35d77844b8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -19,9 +19,10 @@ public interface ConfigurationService { private final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); @Override - public CustomResource clone(CustomResource object) { + public > T clone(T object) { try { - return OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(object), object.getClass()); + return OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(object), + (Class) object.getClass()); } catch (JsonProcessingException e) { throw new IllegalStateException(e); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java index 69cdbdc640..8d92acfea8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java @@ -10,7 +10,13 @@ import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.api.ObservedGenerationAware; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.BaseControl; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; @@ -123,7 +129,7 @@ private PostExecutionControl handleCreateOrUpdate( */ private R cloneResourceIfErrorStatusHandlerIfCouldBeCalled(R resource, Context context) { if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { - return (R) this.controller.getConfiguration().getConfigurationService().getResourceCloner() + return controller.getConfiguration().getConfigurationService().getResourceCloner() .clone(resource); } else { return resource; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java index 67998c7584..a912072e5c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java @@ -172,7 +172,7 @@ public Optional getCustomResource(CustomResourceID resourceID) { if (resource == null) { return Optional.empty(); } else { - return Optional.of((T) (cloner.clone(resource))); + return Optional.of(cloner.clone(resource)); } } From e16e241b0ebf9ec90ec526f1c0da736595be50fd Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 19 Nov 2021 14:12:12 +0100 Subject: [PATCH 5/6] fix: method naming --- .../operator/processing/ReconciliationDispatcher.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java index 69cdbdc640..3526426fc7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java @@ -106,7 +106,7 @@ private PostExecutionControl handleCreateOrUpdate( } else { try { var resourceForExecution = - cloneResourceIfErrorStatusHandlerIfCouldBeCalled(resource, context); + cloneResourceIfErrorStatusHandlerIfNeeded(resource, context); return createOrUpdateExecution(executionScope, resourceForExecution, context); } catch (RuntimeException e) { handleLastAttemptErrorStatusHandler(resource, context, e); @@ -121,7 +121,7 @@ private PostExecutionControl handleCreateOrUpdate( * common that the custom resource is changed during an execution, and it's much cleaner to have * to original resource in place for status update. */ - private R cloneResourceIfErrorStatusHandlerIfCouldBeCalled(R resource, Context context) { + private R cloneResourceIfErrorStatusHandlerIfNeeded(R resource, Context context) { if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { return (R) this.controller.getConfiguration().getConfigurationService().getResourceCloner() .clone(resource); From fee947c32646f61047829d6b36f1d116236a1193 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 19 Nov 2021 15:07:37 +0100 Subject: [PATCH 6/6] refactor: rename method more appropriately --- .../operator/processing/ReconciliationDispatcher.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java index 5129783c6b..b38b880442 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java @@ -112,7 +112,7 @@ private PostExecutionControl handleCreateOrUpdate( } else { try { var resourceForExecution = - cloneResourceIfErrorStatusHandlerIfNeeded(resource, context); + cloneResourceForErrorStatusHandlerIfNeeded(resource, context); return createOrUpdateExecution(executionScope, resourceForExecution, context); } catch (RuntimeException e) { handleLastAttemptErrorStatusHandler(resource, context, e); @@ -127,7 +127,7 @@ private PostExecutionControl handleCreateOrUpdate( * common that the custom resource is changed during an execution, and it's much cleaner to have * to original resource in place for status update. */ - private R cloneResourceIfErrorStatusHandlerIfNeeded(R resource, Context context) { + private R cloneResourceForErrorStatusHandlerIfNeeded(R resource, Context context) { if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { return controller.getConfiguration().getConfigurationService().getResourceCloner() .clone(resource);