Skip to content

Commit 59de710

Browse files
fix: Allow Retrying status while preventing RollingOut loop
Final fix for ProgressDeadlineExceeded archival issue: - markAsProgressing: Only blocks RollingOut, allows Retrying/Succeeded - Progress deadline check: Skips enforcement for Archived revisions - This allows archival teardown to show Retrying state without triggering deadline reset - Prevents original RollingOut reconcile loop for Active revisions Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 451e43a commit 59de710

2 files changed

Lines changed: 16 additions & 16 deletions

File tree

internal/operator-controller/controllers/clusterobjectset_controller.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -672,27 +672,27 @@ func setRetryingConditions(cos *ocv1.ClusterObjectSet, message string) {
672672
// markAsProgressing sets the Progressing condition to True with the given reason.
673673
//
674674
// Once ProgressDeadlineExceeded has been set for the current generation, this function
675-
// keeps that condition sticky for non-terminal progressing reasons that would otherwise
676-
// cause the reconciler to oscillate between Progressing=True and
677-
// ProgressDeadlineExceeded=False for the same generation.
675+
// blocks only the RollingOut reason to prevent a reconcile loop for Active revisions.
676+
// The loop would occur when reconcile() sees in-transition objects and sets
677+
// Progressing=True/RollingOut, then the progress deadline check immediately resets it
678+
// to ProgressDeadlineExceeded, causing a status-only update that triggers another reconcile.
678679
//
679-
// In particular, both RollingOut and Retrying are blocked for Active revisions.
680-
// Allowing those reasons to overwrite ProgressDeadlineExceeded can trigger unnecessary
681-
// status-only reconciles and interfere with retry/requeue behavior due to the progress
682-
// deadline check resetting the condition.
680+
// Archived revisions are exempt because they skip progress deadline enforcement entirely,
681+
// so their status updates won't be overridden.
683682
//
684-
// Archived revisions are exempt from this guard because the progress deadline check
685-
// does not apply to them, allowing archival teardown retries to show Retrying status.
683+
// Other reasons like Succeeded and Retrying are always allowed because:
684+
// - Succeeded represents terminal success (rollout eventually completed)
685+
// - Retrying represents transient errors that need to be visible in status
686686
//
687-
// If the generation changed (spec update) since ProgressDeadlineExceeded was recorded,
688-
// the guard allows the update through so the condition reflects the new generation.
687+
// If the generation changed since ProgressDeadlineExceeded was recorded, the guard
688+
// allows all updates through so the condition reflects the new generation.
689689
func markAsProgressing(cos *ocv1.ClusterObjectSet, reason, message string) {
690690
if cnd := meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing); cnd != nil &&
691691
cnd.Status == metav1.ConditionFalse && cnd.Reason == ocv1.ReasonProgressDeadlineExceeded &&
692692
cnd.ObservedGeneration == cos.Generation &&
693-
(reason == ocv1.ReasonRollingOut || reason == ocv1.ClusterObjectSetReasonRetrying) &&
693+
reason == ocv1.ReasonRollingOut &&
694694
cos.Spec.LifecycleState != ocv1.ClusterObjectSetLifecycleStateArchived {
695-
log.Log.V(1).Info("skipping markAsProgressing: ProgressDeadlineExceeded is sticky for non-terminal progressing reasons",
695+
log.Log.V(1).Info("skipping markAsProgressing: ProgressDeadlineExceeded is sticky for RollingOut on Active revisions",
696696
"requestedReason", reason, "revision", cos.Name, "lifecycleState", cos.Spec.LifecycleState)
697697
return
698698
}

internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func Test_markAsProgressing_doesNotOverwriteProgressDeadlineExceeded(t *testing.
300300
expectReason: ocv1.ReasonSucceeded,
301301
},
302302
{
303-
name: "does not overwrite ProgressDeadlineExceeded with Retrying (same generation)",
303+
name: "allows Retrying to overwrite ProgressDeadlineExceeded (same generation)",
304304
generation: 1,
305305
existingConditions: []metav1.Condition{
306306
{
@@ -311,8 +311,8 @@ func Test_markAsProgressing_doesNotOverwriteProgressDeadlineExceeded(t *testing.
311311
},
312312
},
313313
reason: ocv1.ClusterObjectSetReasonRetrying,
314-
expectProgressing: metav1.ConditionFalse,
315-
expectReason: ocv1.ReasonProgressDeadlineExceeded,
314+
expectProgressing: metav1.ConditionTrue,
315+
expectReason: ocv1.ClusterObjectSetReasonRetrying,
316316
},
317317
{
318318
name: "allows RollingOut when generation changed since ProgressDeadlineExceeded",

0 commit comments

Comments
 (0)