diff --git a/docs/content/en/docs/documentation/error-handling-retries.md b/docs/content/en/docs/documentation/error-handling-retries.md index 7bd4ad2e22..428c35b9ae 100644 --- a/docs/content/en/docs/documentation/error-handling-retries.md +++ b/docs/content/en/docs/documentation/error-handling-retries.md @@ -108,6 +108,22 @@ Retry can be skipped in cases of unrecoverable errors: ErrorStatusUpdateControl.patchStatus(customResource).withNoRetry(); ``` +When `updateErrorStatus` returns any `ErrorStatusUpdateControl` other than +`ErrorStatusUpdateControl.defaultErrorProcessing()`, the framework considers the error handled by +the reconciler and, while retry attempts remain, no longer logs the "Uncaught error during event +processing" warning; the error is logged on `DEBUG` level instead. This lets a reconciler keep +retrying an expected, recoverable condition without producing a continuous stream of `WARN` +messages, while it remains free to log the error at whatever level it deems appropriate inside +`updateErrorStatus`. + +This log downgrade applies only when retry is actually taking place, i.e. a retry is configured and +the returned control does not disable it. Controls that explicitly disable retry — `withNoRetry()` +and `rescheduleAfter()` — cancel the native retry and its `@GradualRetry` backoff, so nothing is +retried in those cases. Likewise, on the last retry attempt (or when no retry is configured) the +failure is final, so the framework keeps its higher-severity logging to avoid hiding a +non-recoverable error. Returning `ErrorStatusUpdateControl.defaultErrorProcessing()` preserves the +default behavior, including the warning. + ### Correctness and Automatic Retries While it is possible to deactivate automatic retries, this is not desirable unless there is a particular reason. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index c8322e47e5..ddc0f73a27 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -297,7 +297,9 @@ synchronized void eventProcessingFinished( && postExecutionControl.exceptionDuringExecution() && (!state.deleteEventPresent() || triggerOnAllEvents())) { handleRetryOnException( - executionScope, postExecutionControl.getRuntimeException().orElseThrow()); + executionScope, + postExecutionControl.getRuntimeException().orElseThrow(), + postExecutionControl.isErrorHandledByReconciler()); return; } cleanupOnSuccessfulExecution(executionScope); @@ -327,6 +329,9 @@ private boolean isTriggerOnAllEventAndDeleteEventPresent(ResourceState state) { private void logErrorIfNoRetryConfigured( ExecutionScope

executionScope, PostExecutionControl

postExecutionControl) { if (!isRetryConfigured() && postExecutionControl.exceptionDuringExecution()) { + // No retry is configured, so this failure is final. Even if the reconciler handled the error + // in updateErrorStatus, we keep the higher-severity log so a non-recoverable failure is not + // hidden from operators. log.error( "Error during event processing {}", executionScope, @@ -369,14 +374,16 @@ TimerEventSource

retryEventSource() { * events (received meanwhile retry is in place or already in buffer) instantly or always wait * according to the retry timing if there was an exception. */ - private void handleRetryOnException(ExecutionScope

executionScope, Exception exception) { + private void handleRetryOnException( + ExecutionScope

executionScope, Exception exception, boolean errorHandledByReconciler) { final var state = getOrInitRetryExecution(executionScope); var resourceID = state.getId(); boolean eventPresent = state.eventPresent() || (triggerOnAllEvents() && state.isAdditionalEventPresentAfterDeleteEvent()); state.markEventReceived(triggerOnAllEvents()); - retryAwareErrorLogging(state.getRetry(), eventPresent, exception, executionScope); + retryAwareErrorLogging( + state.getRetry(), eventPresent, errorHandledByReconciler, exception, executionScope); metrics.reconciliationFailed( executionScope.getResource(), state.getRetry(), exception, metricsMetadata); if (eventPresent) { @@ -408,17 +415,29 @@ private void handleRetryOnException(ExecutionScope

executionScope, Exception private void retryAwareErrorLogging( RetryExecution retry, boolean eventPresent, + boolean errorHandledByReconciler, Exception exception, ExecutionScope

executionScope) { if (!retry.isLastAttempt() && exception instanceof KubernetesClientException ex && ex.getCode() == HttpURLConnection.HTTP_CONFLICT) { + // The conflict branch is already low-noise (DEBUG + INFO) and provides actionable info, so it + // keeps precedence over the handled-error downgrade below. log.debug("Full client conflict error during event processing {}", executionScope, exception); log.info( "Resource Kubernetes Resource Creator/Update Conflict during reconciliation. Message:" + " {} Resource name: {}", ex.getMessage(), ex.getFullResourceName()); + } else if (errorHandledByReconciler && !retry.isLastAttempt()) { + // The reconciler already handled the error in updateErrorStatus (and had the chance to log it + // as needed), so while there are retries remaining the framework only logs it on debug level. + // On the last attempt we still fall through to the higher-severity logging below, so a final, + // non-recoverable failure is not hidden from operators. + log.debug( + "Error during event processing {}, but was handled by the reconciler", + executionScope, + exception); } else if (eventPresent || !retry.isLastAttempt()) { log.warn( "Uncaught error during event processing {} - but another reconciliation will be attempted" diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/PostExecutionControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/PostExecutionControl.java index 64d7a25166..ef09c74a08 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/PostExecutionControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/PostExecutionControl.java @@ -27,6 +27,7 @@ final class PostExecutionControl { private final Exception runtimeException; private Long reScheduleDelay = null; + private boolean errorHandledByReconciler = false; private PostExecutionControl( boolean finalizerRemoved, @@ -68,6 +69,22 @@ public static PostExecutionControl exceptionDuringExe return new PostExecutionControl<>(false, null, false, exception); } + /** + * Marks that the exception was handled by the reconciler's {@code updateErrorStatus} (i.e. the + * reconciler did not return {@link + * io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl#defaultErrorProcessing()}), + * but the error is still retried. In this case the framework logs the error on a lower level, + * since the reconciler already had the chance to handle and log it as needed. + */ + public PostExecutionControl withErrorHandledByReconciler() { + this.errorHandledByReconciler = true; + return this; + } + + public boolean isErrorHandledByReconciler() { + return errorHandledByReconciler; + } + public Optional getUpdatedCustomResource() { return Optional.ofNullable(updatedCustomResource); } 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 dfdf37d3e1..e65a611c19 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 @@ -266,7 +266,12 @@ public boolean isLastAttempt() { errorStatusUpdateControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule); return postExecutionControl; } - throw e; + // The reconciler handled the error via updateErrorStatus (it did not return + // defaultErrorProcessing()) but still wants the error to be retried. The retry (and its + // backoff) is kept intact, but since the reconciler already had the chance to handle and log + // the error, the framework logs it on a lower level instead of emitting an "uncaught error" + // warning. + return PostExecutionControl.

exceptionDuringExecution(e).withErrorHandledByReconciler(); } private PostExecutionControl

createPostExecutionControl( 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 e874731657..ac24375242 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 @@ -481,6 +481,40 @@ void callErrorStatusHandlerEvenOnFirstError() { assertThat(postExecControl.exceptionDuringExecution()).isTrue(); } + @Test + void errorHandledByReconcilerMarkedWhenErrorStatusHandledButRetried() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + reconciler.reconcile = + (r, c) -> { + throw new IllegalStateException("Error Status Test"); + }; + reconciler.errorHandler = () -> ErrorStatusUpdateControl.patchStatus(testCustomResource); + + var postExecControl = + reconciliationDispatcher.handleExecution( + new ExecutionScope(null, null, false, false).setResource(testCustomResource)); + + assertThat(postExecControl.exceptionDuringExecution()).isTrue(); + assertThat(postExecControl.isErrorHandledByReconciler()).isTrue(); + } + + @Test + void errorNotHandledByReconcilerOnDefaultErrorProcessing() { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + reconciler.reconcile = + (r, c) -> { + throw new IllegalStateException("Error Status Test"); + }; + reconciler.errorHandler = () -> ErrorStatusUpdateControl.defaultErrorProcessing(); + + var postExecControl = + reconciliationDispatcher.handleExecution( + new ExecutionScope(null, null, false, false).setResource(testCustomResource)); + + assertThat(postExecControl.exceptionDuringExecution()).isTrue(); + assertThat(postExecControl.isErrorHandledByReconciler()).isFalse(); + } + @Test void errorHandlerCanInstructNoRetryWithUpdate() { testCustomResource.addFinalizer(DEFAULT_FINALIZER);