Skip to content

Commit 76be6c0

Browse files
committed
fix: report an event after a delete event as imminent reconciliation
Context.isNextReconciliationImminent() only checked ResourceState.eventPresent(), which is false for ADDITIONAL_EVENT_PRESENT_AFTER_DELETE_EVENT. In triggerReconcilerOnAllEvents mode that state does trigger a new reconciliation right after the current one, both on the success path (eventProcessingFinished) and on the failure path (handleRetryOnException), so the method contradicted its own contract and a reconciler could skip a status update that nothing else would produce. Extract the condition handleRetryOnException already computed into a shared helper and use it for isNextReconciliationImminent() too, so the two cannot drift apart again. The condition is the one that holds regardless of whether the current reconciliation succeeds or throws; documented on Context.
1 parent 4296fb2 commit 76be6c0

3 files changed

Lines changed: 21 additions & 7 deletions

File tree

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,10 @@ default ResourceEventRecorder eventRecorder() {
253253
* reconciliation is already scheduled, which would in turn trigger another status update, thus
254254
* rendering the current one moot.
255255
*
256+
* <p>This holds regardless of whether the current reconciliation succeeds or throws, so with
257+
* {@link ControllerConfiguration#triggerReconcilerOnAllEvents()} it also covers an event that
258+
* arrived after a delete event.
259+
*
256260
* @return {@code true} is another reconciliation is already scheduled, {@code false} otherwise
257261
*/
258262
boolean isNextReconciliationImminent();

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,7 @@ private void handleRetryOnException(
380380
ExecutionScope<P> executionScope, Exception exception, boolean errorHandledByReconciler) {
381381
final var state = getOrInitRetryExecution(executionScope);
382382
var resourceID = state.getId();
383-
boolean eventPresent =
384-
state.eventPresent()
385-
|| (triggerOnAllEvents() && state.isAdditionalEventPresentAfterDeleteEvent());
383+
boolean eventPresent = nextReconciliationImminent(state);
386384
state.markEventReceived();
387385
retryAwareErrorLogging(
388386
state.getRetry(), eventPresent, errorHandledByReconciler, exception, executionScope);
@@ -511,7 +509,19 @@ public synchronized void start() throws OperatorException {
511509
}
512510

513511
public boolean isNextReconciliationImminent(ResourceID resourceID) {
514-
return resourceStateManager.getOrCreate(resourceID).eventPresent();
512+
return nextReconciliationImminent(resourceStateManager.getOrCreate(resourceID));
513+
}
514+
515+
/**
516+
* An event that arrives after a delete event is tracked in a dedicated state, so {@link
517+
* ResourceState#eventPresent()} alone does not cover it. Such an event triggers a new
518+
* reconciliation right after the current one, both when it succeeds (see {@link
519+
* #eventProcessingFinished}) and when it fails (see {@link #handleRetryOnException}), so it has
520+
* to be reported as imminent too.
521+
*/
522+
private boolean nextReconciliationImminent(ResourceState state) {
523+
return state.eventPresent()
524+
|| (triggerOnAllEvents() && state.isAdditionalEventPresentAfterDeleteEvent());
515525
}
516526

517527
private void handleAlreadyMarkedEvents() {

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventReconciler.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ public UpdateControl<TriggerReconcilerOnAllEventCustomResource> reconcile(
9696
// update call returns, but the update event still has to travel back through the informer.
9797
// If this reconciliation failed before the event was registered, the framework would treat
9898
// the failure as a plain retry (consuming the last attempt) instead of instantly
99-
// re-triggering because of a superseding event. isNextReconciliationImminent() reflects
100-
// exactly the state (event marked as received) the framework checks after this
101-
// reconciliation fails, and it cannot be unset while this reconciliation is in progress.
99+
// re-triggering because of a superseding event. isNextReconciliationImminent() reports the
100+
// same condition the framework evaluates after this reconciliation fails, and it cannot be
101+
// unset while this reconciliation is in progress.
102102
var waitUntil = System.currentTimeMillis() + MAX_WAIT_FOR_SUPERSEDING_EVENT_MILLIS;
103103
while ((!continuerOnRetryWait || !context.isNextReconciliationImminent())
104104
&& System.currentTimeMillis() < waitUntil) {

0 commit comments

Comments
 (0)