From 48e1dee0d68b7345cf1949cd2543ee679638df88 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 7 Dec 2021 15:38:42 +0100 Subject: [PATCH 1/6] feature: base impl --- .../api/reconciler/ErrorStatusHandler.java | 5 ++- .../event/ReconciliationDispatcher.java | 40 +++++++++---------- .../event/ReconciliationDispatcherTest.java | 17 +++++--- .../ErrorStatusHandlerTestReconciler.java | 7 ++-- .../operator/sample/WebPageReconciler.java | 6 ++- 5 files changed, 43 insertions(+), 32 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 2d5592053b..60e9e671b2 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 @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator.api.reconciler; +import java.util.Optional; + import io.fabric8.kubernetes.api.model.HasMetadata; public interface ErrorStatusHandler { @@ -19,9 +21,10 @@ public interface ErrorStatusHandler { * should not be updates on custom resource after it is marked for deletion. * * @param resource to update the status on + * @param retryInfo * @param e exception thrown from the reconciler * @return the updated resource */ - T updateErrorStatus(T resource, RuntimeException e); + Optional updateErrorStatus(T resource, RetryInfo retryInfo, RuntimeException e); } 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 01f6de14ac..1b02ed9d5d 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 @@ -11,13 +11,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.ErrorStatusHandler; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.processing.Controller; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; @@ -114,7 +108,7 @@ private PostExecutionControl handleReconcile( cloneResourceForErrorStatusHandlerIfNeeded(originalResource, context); return reconcileExecution(executionScope, resourceForExecution, originalResource, context); } catch (RuntimeException e) { - handleLastAttemptErrorStatusHandler(originalResource, context, e); + handleErrorStatusHandler(originalResource, context, e); throw e; } } @@ -128,7 +122,7 @@ private PostExecutionControl handleReconcile( * place for status update. */ private R cloneResourceForErrorStatusHandlerIfNeeded(R resource, Context context) { - if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context) || + if (isErrorStatusHandlerPresent() || shouldUpdateObservedGenerationAutomatically(resource)) { return configuration().getConfigurationService().getResourceCloner().clone(resource); } else { @@ -164,26 +158,32 @@ && shouldUpdateObservedGenerationAutomatically(resourceForExecution)) { return createPostExecutionControl(updatedCustomResource, updateControl); } - private void handleLastAttemptErrorStatusHandler(R resource, Context context, + private void handleErrorStatusHandler(R resource, Context context, RuntimeException e) { - if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { + if (isErrorStatusHandlerPresent()) { try { + var retryInfo = context.getRetryInfo().orElse(new RetryInfo() { + @Override + public int getAttemptCount() { + return 0; + } + + @Override + public boolean isLastAttempt() { + return false; + } + }); var updatedResource = ((ErrorStatusHandler) controller.getReconciler()) - .updateErrorStatus(resource, e); - customResourceFacade.updateStatus(updatedResource); + .updateErrorStatus(resource, retryInfo, e); + updatedResource.ifPresent(customResourceFacade::updateStatus); } catch (RuntimeException ex) { log.error("Error during error status handling.", ex); } } } - private boolean isLastAttemptOfRetryAndErrorStatusHandlerPresent(Context context) { - if (context.getRetryInfo().isPresent()) { - return context.getRetryInfo().get().isLastAttempt() - && controller.getReconciler() instanceof ErrorStatusHandler; - } else { - return false; - } + private boolean isErrorStatusHandlerPresent() { + return controller.getReconciler() instanceof ErrorStatusHandler; } private R updateStatusGenerationAware(R 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 db4736697a..095d4d2ebb 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 @@ -1,6 +1,7 @@ package io.javaoperatorsdk.operator.processing.event; import java.util.ArrayList; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -11,6 +12,7 @@ import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; @@ -56,9 +58,12 @@ private ReconciliationDispatcher init(R customResourc 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.updateResource(customResource)); + when(configService.getResourceCloner()).thenReturn(new Cloner() { + @Override + public R clone(R object) { + return object; + } + }); when(reconciler.cleanup(eq(customResource), any())) .thenReturn(DeleteControl.defaultDelete()); when(customResourceFacade.replaceWithLock(any())).thenReturn(null); @@ -351,9 +356,9 @@ void callErrorStatusHandlerIfImplemented() { when(reconciler.reconcile(any(), any())) .thenThrow(new IllegalStateException("Error Status Test")); - when(((ErrorStatusHandler) reconciler).updateErrorStatus(any(), any())).then(a -> { + when(((ErrorStatusHandler) reconciler).updateErrorStatus(any(), any(), any())).then(a -> { testCustomResource.getStatus().setConfigMapStatus(ERROR_MESSAGE); - return testCustomResource; + return Optional.of(testCustomResource); }); reconciliationDispatcher.handleExecution( @@ -373,7 +378,7 @@ public boolean isLastAttempt() { verify(customResourceFacade, times(1)).updateStatus(testCustomResource); verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource), - any()); + any(), any()); } private ObservedGenCustomResource createObservedGenCustomResource() { 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 index 3a922ce092..e8564f9938 100644 --- 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 @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.sample.errorstatushandler; +import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; import org.slf4j.Logger; @@ -45,11 +46,11 @@ public int getNumberOfExecutions() { } @Override - public ErrorStatusHandlerTestCustomResource updateErrorStatus( - ErrorStatusHandlerTestCustomResource resource, RuntimeException e) { + public Optional updateErrorStatus( + ErrorStatusHandlerTestCustomResource resource, RetryInfo retryInfo, RuntimeException e) { log.info("Setting status."); ensureStatusExists(resource); resource.getStatus().setMessage(ERROR_STATUS_MESSAGE); - return resource; + return Optional.of(resource); } } diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 37caf1503c..ddb353ac05 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -4,6 +4,7 @@ import java.io.InputStream; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -173,8 +174,9 @@ private T loadYaml(Class clazz, String yaml) { } @Override - public WebPage updateErrorStatus(WebPage resource, RuntimeException e) { + public Optional updateErrorStatus(WebPage resource, RetryInfo retryInfo, + RuntimeException e) { resource.getStatus().setErrorMessage("Error: " + e.getMessage()); - return resource; + return Optional.of(resource); } } From 5405a2beb44fe24f0fe9c08f7df31cc9b1b8fcef Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 8 Dec 2021 10:56:33 +0100 Subject: [PATCH 2/6] fix: additional intergation test --- .../event/ReconciliationDispatcherTest.java | 18 ++++++++ .../io/javaoperatorsdk/operator/RetryIT.java | 13 +++--- .../operator/RetryMaxAttemptIT.java | 42 +++++++++++++++++++ .../retry/RetryTestCustomReconciler.java | 10 +++-- 4 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java 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 095d4d2ebb..dd9d5e6fda 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 @@ -381,6 +381,24 @@ public boolean isLastAttempt() { any(), any()); } + @Test + void callErrorStatusHandlerEvenOnFirstError() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + + when(reconciler.reconcile(any(), any())) + .thenThrow(new IllegalStateException("Error Status Test")); + when(((ErrorStatusHandler) reconciler).updateErrorStatus(any(), any(), any())).then(a -> { + testCustomResource.getStatus().setConfigMapStatus(ERROR_MESSAGE); + return Optional.of(testCustomResource); + }); + reconciliationDispatcher.handleExecution( + new ExecutionScope( + testCustomResource, null)); + verify(customResourceFacade, times(1)).updateStatus(testCustomResource); + verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource), + any(), any()); + } + private ObservedGenCustomResource createObservedGenCustomResource() { ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource(); observedGenCustomResource.setMetadata(new ObjectMeta()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java index cb7109f8cf..4ab63c7513 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java @@ -20,15 +20,18 @@ public class RetryIT { public static final int RETRY_INTERVAL = 150; + public static final int MAX_RETRY_ATTEMPTS = 5; + + public static final int NUMBER_FAILED_EXECUTIONS = 3; @RegisterExtension OperatorExtension operator = OperatorExtension.builder() .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler( - new RetryTestCustomReconciler(), + new RetryTestCustomReconciler(NUMBER_FAILED_EXECUTIONS), new GenericRetry().setInitialInterval(RETRY_INTERVAL).withLinearRetry() - .setMaxAttempts(5)) + .setMaxAttempts(MAX_RETRY_ATTEMPTS)) .build(); @@ -40,7 +43,7 @@ public void retryFailedExecution() { await("cr status updated") .pollDelay( - RETRY_INTERVAL * (RetryTestCustomReconciler.NUMBER_FAILED_EXECUTIONS + 2), + RETRY_INTERVAL * (NUMBER_FAILED_EXECUTIONS + 2), TimeUnit.MILLISECONDS) .pollInterval( RETRY_INTERVAL, @@ -49,7 +52,7 @@ public void retryFailedExecution() { .untilAsserted(() -> { assertThat( TestUtils.getNumberOfExecutions(operator)) - .isEqualTo(RetryTestCustomReconciler.NUMBER_FAILED_EXECUTIONS + 1); + .isEqualTo(NUMBER_FAILED_EXECUTIONS + 1); RetryTestCustomResource finalResource = operator.get(RetryTestCustomResource.class, @@ -59,7 +62,7 @@ public void retryFailedExecution() { }); } - public RetryTestCustomResource createTestCustomResource(String id) { + public static RetryTestCustomResource createTestCustomResource(String id) { RetryTestCustomResource resource = new RetryTestCustomResource(); resource.setMetadata( new ObjectMetaBuilder() diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java new file mode 100644 index 0000000000..e855b016fd --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java @@ -0,0 +1,42 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +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.retry.RetryTestCustomReconciler; +import io.javaoperatorsdk.operator.sample.retry.RetryTestCustomResource; + +import static io.javaoperatorsdk.operator.RetryIT.createTestCustomResource; +import static org.assertj.core.api.Assertions.assertThat; + +public class RetryMaxAttemptIT { + + public static final int MAX_RETRY_ATTEMPTS = 3; + public static final int RETRY_INTERVAL = 100; + public static final int ALL_EXECUTION_TO_FAIL = 99; + + RetryTestCustomReconciler reconciler = new RetryTestCustomReconciler(ALL_EXECUTION_TO_FAIL); + + @RegisterExtension + OperatorExtension operator = + OperatorExtension.builder() + .withConfigurationService(DefaultConfigurationService.instance()) + .withReconciler(reconciler, + new GenericRetry().setInitialInterval(RETRY_INTERVAL).withLinearRetry() + .setMaxAttempts(MAX_RETRY_ATTEMPTS)) + .build(); + + + @Test + public void retryFailedExecution() throws InterruptedException { + RetryTestCustomResource resource = createTestCustomResource("max-retry"); + + operator.create(RetryTestCustomResource.class, resource); + + Thread.sleep((MAX_RETRY_ATTEMPTS + 2) * RETRY_INTERVAL); + assertThat(reconciler.getNumberOfExecutions()).isEqualTo(4); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/retry/RetryTestCustomReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/retry/RetryTestCustomReconciler.java index b2a7ca0e17..21e44631fb 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/retry/RetryTestCustomReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/retry/RetryTestCustomReconciler.java @@ -17,8 +17,6 @@ public class RetryTestCustomReconciler implements Reconciler, TestExecutionInfoProvider { - public static final int NUMBER_FAILED_EXECUTIONS = 2; - public static final String FINALIZER_NAME = ControllerUtils.getDefaultFinalizerName( CustomResource.getCRDName(RetryTestCustomResource.class)); @@ -26,6 +24,12 @@ public class RetryTestCustomReconciler LoggerFactory.getLogger(RetryTestCustomReconciler.class); private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + private int numberOfExecutionFails; + + public RetryTestCustomReconciler(int numberOfExecutionFails) { + this.numberOfExecutionFails = numberOfExecutionFails; + } + @Override public UpdateControl reconcile( RetryTestCustomResource resource, Context context) { @@ -36,7 +40,7 @@ public UpdateControl reconcile( } log.info("Value: " + resource.getSpec().getValue()); - if (numberOfExecutions.get() < NUMBER_FAILED_EXECUTIONS + 1) { + if (numberOfExecutions.get() < numberOfExecutionFails + 1) { throw new RuntimeException("Testing Retry"); } if (context.getRetryInfo().isEmpty() || context.getRetryInfo().get().isLastAttempt()) { From bbe9c51cd248d5b014c903ceb37a479a557ac4ee Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 8 Dec 2021 11:26:54 +0100 Subject: [PATCH 3/6] fix: imrpoved intergation tests --- .../operator/ErrorStatusHandlerIT.java | 14 +++++++++----- ...rorStatusHandlerTestCustomResourceStatus.java | 16 +++++++++++----- .../ErrorStatusHandlerTestReconciler.java | 2 +- 3 files changed, 21 insertions(+), 11 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 01908757bc..4a1b3293e7 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java @@ -17,13 +17,15 @@ public class ErrorStatusHandlerIT { + public static final int MAX_RETRY_ATTEMPTS = 3; ErrorStatusHandlerTestReconciler reconciler = new ErrorStatusHandlerTestReconciler(); @RegisterExtension OperatorExtension operator = OperatorExtension.builder() .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(reconciler, new GenericRetry().setMaxAttempts(3).withLinearRetry()) + .withReconciler(reconciler, + new GenericRetry().setMaxAttempts(MAX_RETRY_ATTEMPTS).withLinearRetry()) .build(); @Test @@ -32,16 +34,18 @@ public void testErrorMessageSetEventually() { operator.create(ErrorStatusHandlerTestCustomResource.class, createCustomResource()); await() - .atMost(15, TimeUnit.SECONDS) - .pollInterval(1L, TimeUnit.SECONDS) + .atMost(10, TimeUnit.SECONDS) + .pollInterval(250, TimeUnit.MICROSECONDS) .untilAsserted( () -> { ErrorStatusHandlerTestCustomResource res = operator.get(ErrorStatusHandlerTestCustomResource.class, resource.getMetadata().getName()); assertThat(res.getStatus()).isNotNull(); - assertThat(res.getStatus().getMessage()) - .isEqualTo(ErrorStatusHandlerTestReconciler.ERROR_STATUS_MESSAGE); + for (int i = 0; i < MAX_RETRY_ATTEMPTS + 1; i++) { + assertThat(res.getStatus().getMessages()) + .contains(ErrorStatusHandlerTestReconciler.ERROR_STATUS_MESSAGE + i); + } }); } 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 index 03fc517ecd..4e54c877a5 100644 --- 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 @@ -1,15 +1,21 @@ package io.javaoperatorsdk.operator.sample.errorstatushandler; +import java.util.ArrayList; +import java.util.List; + public class ErrorStatusHandlerTestCustomResourceStatus { - private String message; + private List messages; - public String getMessage() { - return message; + public List getMessages() { + if (messages == null) { + messages = new ArrayList<>(); + } + return messages; } - public ErrorStatusHandlerTestCustomResourceStatus setMessage(String message) { - this.message = message; + public ErrorStatusHandlerTestCustomResourceStatus setMessages(List messages) { + this.messages = messages; 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 index e8564f9938..8f20c44353 100644 --- 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 @@ -50,7 +50,7 @@ public Optional updateErrorStatus( ErrorStatusHandlerTestCustomResource resource, RetryInfo retryInfo, RuntimeException e) { log.info("Setting status."); ensureStatusExists(resource); - resource.getStatus().setMessage(ERROR_STATUS_MESSAGE); + resource.getStatus().getMessages().add(ERROR_STATUS_MESSAGE + retryInfo.getAttemptCount()); return Optional.of(resource); } } From 77fdc1e1eddecf0123ba1e417a07b51abc8d9160 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 8 Dec 2021 12:12:29 +0100 Subject: [PATCH 4/6] docs: updated --- docs/documentation/features.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 194effb36e..f981693ae2 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -166,19 +166,18 @@ 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 +In order to facilitate error reporting Reconciler can implement the following [interface](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusHandler.java): ```java -public interface ErrorStatusHandler> { +public interface ErrorStatusHandler { - T updateErrorStatus(T resource, RuntimeException e); + Optional updateErrorStatus(T resource, RetryInfo retryInfo, 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 `updateErrorStatus` resource is called in case an exception is thrown from the reconciler. The result of the method call is used to make a status 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 From 5bc820d3809a5dd13867faf118c52808ab699433 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 8 Dec 2021 12:20:13 +0100 Subject: [PATCH 5/6] docs: improved --- docs/documentation/features.md | 5 ++++- .../operator/processing/event/ReconciliationDispatcher.java | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index f981693ae2..da55ff2e48 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -177,7 +177,10 @@ public interface ErrorStatusHandler { } ``` -The `updateErrorStatus` resource is called in case an exception is thrown from the reconciler. +The `updateErrorStatus` is called in case an exception is thrown from the reconciler. It is also called when +there is no retry configured, just after the reconciler execution. +In the first call the `RetryInfo.getAttemptCount()` is always zero, since it is not a result of a retry +(regardless if retry is configured or not). The result of the method call is used to make a status 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 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 1b02ed9d5d..f29865d338 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 @@ -170,7 +170,7 @@ public int getAttemptCount() { @Override public boolean isLastAttempt() { - return false; + return controller.getConfiguration().getRetryConfiguration() == null; } }); var updatedResource = ((ErrorStatusHandler) controller.getReconciler()) From 8b9feb3372978a95d21e0dfcc3b9c36c6e78cb3f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 8 Dec 2021 12:35:26 +0100 Subject: [PATCH 6/6] fix: missing word --- docs/documentation/features.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index da55ff2e48..b710eeb3e3 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -177,7 +177,7 @@ public interface ErrorStatusHandler { } ``` -The `updateErrorStatus` is called in case an exception is thrown from the reconciler. It is also called when +The `updateErrorStatus` method is called in case an exception is thrown from the reconciler. It is also called when there is no retry configured, just after the reconciler execution. In the first call the `RetryInfo.getAttemptCount()` is always zero, since it is not a result of a retry (regardless if retry is configured or not).