Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/content/en/docs/documentation/error-handling-retries.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -327,6 +329,9 @@ private boolean isTriggerOnAllEventAndDeleteEventPresent(ResourceState state) {
private void logErrorIfNoRetryConfigured(
ExecutionScope<P> executionScope, PostExecutionControl<P> 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,
Expand Down Expand Up @@ -369,14 +374,16 @@ TimerEventSource<P> 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<P> executionScope, Exception exception) {
private void handleRetryOnException(
ExecutionScope<P> 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) {
Expand Down Expand Up @@ -408,17 +415,29 @@ private void handleRetryOnException(ExecutionScope<P> executionScope, Exception
private void retryAwareErrorLogging(
RetryExecution retry,
boolean eventPresent,
boolean errorHandledByReconciler,
Exception exception,
ExecutionScope<P> 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.
Comment on lines +432 to +436
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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ final class PostExecutionControl<R extends HasMetadata> {
private final Exception runtimeException;

private Long reScheduleDelay = null;
private boolean errorHandledByReconciler = false;

private PostExecutionControl(
boolean finalizerRemoved,
Expand Down Expand Up @@ -68,6 +69,22 @@ public static <R extends HasMetadata> PostExecutionControl<R> 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<R> withErrorHandledByReconciler() {
this.errorHandledByReconciler = true;
return this;
}

public boolean isErrorHandledByReconciler() {
return errorHandledByReconciler;
}

public Optional<R> getUpdatedCustomResource() {
return Optional.ofNullable(updatedCustomResource);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.<P>exceptionDuringExecution(e).withErrorHandledByReconciler();
}

private PostExecutionControl<P> createPostExecutionControl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading