fix(controller): prevent crash-looping pods from resetting progress deadline. Fixes #3765

rollout/sync.go

@@ -655,17 +655,23 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt
 			}
 			condition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, reason, msg)
 
-			// Update the current Progressing condition or add a new one if it doesn't exist.
-			// If a Progressing condition with status=true already exists, we should update
-			// everything but lastTransitionTime. SetRolloutCondition already does that but
-			// it also is not updating conditions when the reason of the new condition is the
-			// same as the old. The Progressing condition is a special case because we want to
-			// update with the same reason and change just lastUpdateTime if we notice any
-			// progress. That's why we handle it here.
+			// Only reset the progress deadline (LastUpdateTime) when there is structural/scaling
+			// progress or when the condition doesn't exist yet. ReadyReplicas/AvailableReplicas
+			// oscillation (e.g., from crash-looping pods) should NOT reset the deadline, as this
+			// would prevent ProgressDeadlineExceeded from ever firing.
+			scalingProgress := conditions.RolloutScalingProgress(c.rollout, &newStatus)
+			readinessOnlyProgress := !scalingProgress && !becameUnhealthy
+
 			if currentCond != nil {
 				if currentCond.Status == corev1.ConditionTrue {
 					condition.LastTransitionTime = currentCond.LastTransitionTime
 				}
+				if readinessOnlyProgress && currentCond.Reason == conditions.ReplicaSetUpdatedReason {
+					// Readiness-only progress (ReadyReplicas/AvailableReplicas went up) should not
+					// reset the deadline timer when we're already tracking progress. This prevents
+					// crash-looping pods from indefinitely deferring the timeout.
+					condition.LastUpdateTime = currentCond.LastUpdateTime
+				}
 				conditions.RemoveRolloutCondition(&newStatus, v1alpha1.RolloutProgressing)
 			}
 			conditions.SetRolloutCondition(&newStatus, *condition)

utils/conditions/conditions.go

@@ -251,6 +251,13 @@ func filterOutCondition(conditions []v1alpha1.RolloutCondition, condType v1alpha
 // when new pods are scaled up, become ready or available, old pods are scaled down, or we modify the
 // services, then we consider the rollout is progressing.
 func RolloutProgressing(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool {
+	return RolloutScalingProgress(rollout, newStatus) || RolloutReadinessProgress(rollout, newStatus)
+}
+
+// RolloutScalingProgress returns true when there is structural progress: replicas being scaled
+// up/down, strategy-specific changes (step advancement, selector switches, stable RS changes).
+// This type of progress unconditionally resets the progress deadline.
+func RolloutScalingProgress(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool {
 	oldStatus := rollout.Status
 
 	strategySpecificProgress := false
@@ -276,11 +283,18 @@ func RolloutProgressing(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutSt
 
 	return (newStatus.UpdatedReplicas != oldStatus.UpdatedReplicas) ||
 		(newStatusOldReplicas < oldStatusOldReplicas) ||
-		newStatus.ReadyReplicas > rollout.Status.ReadyReplicas ||
-		newStatus.AvailableReplicas > rollout.Status.AvailableReplicas ||
 		strategySpecificProgress
 }
 
+// RolloutReadinessProgress returns true when pods have become ready or available compared to
+// the last observed status. This type of progress does NOT reset the progress deadline when
+// the Progressing condition already exists, to prevent crash-looping pods from indefinitely
+// resetting the deadline through ReadyReplicas/AvailableReplicas oscillation.
+func RolloutReadinessProgress(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool {
+	return newStatus.ReadyReplicas > rollout.Status.ReadyReplicas ||
+		newStatus.AvailableReplicas > rollout.Status.AvailableReplicas
+}
+
 // RolloutHealthy considers a rollout to be healthy once all of its desired replicas
 // are updated, available, and receiving traffic from the active service, and no old pods are running.
 func RolloutHealthy(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool {

utils/conditions/rollouts_test.go

@@ -350,6 +350,106 @@ func TestRolloutProgressing(t *testing.T) {
 
 }
 
+func TestRolloutProgressSplit(t *testing.T) {
+	canaryRollout := func(current, updated, ready, available int32, stableRS string, index int32, stepHash string) *v1alpha1.Rollout {
+		return &v1alpha1.Rollout{
+			Spec: v1alpha1.RolloutSpec{
+				Strategy: v1alpha1.RolloutStrategy{
+					Canary: &v1alpha1.CanaryStrategy{},
+				},
+			},
+			Status: v1alpha1.RolloutStatus{
+				Replicas:          current,
+				UpdatedReplicas:   updated,
+				ReadyReplicas:     ready,
+				AvailableReplicas: available,
+				StableRS:          stableRS,
+				CurrentStepIndex:  &index,
+				CurrentStepHash:   stepHash,
+			},
+		}
+	}
+
+	t.Run("ReadyReplicas oscillation is readiness-only progress", func(t *testing.T) {
+		// Simulate crash-loop: ReadyReplicas goes from 9 to 10 (pod restarted and briefly ready)
+		rollout := canaryRollout(10, 1, 9, 9, "stable", 1, "abc")
+		newStatus := v1alpha1.RolloutStatus{
+			Replicas:          10,
+			UpdatedReplicas:   1,
+			ReadyReplicas:     10,
+			AvailableReplicas: 9,
+			StableRS:          "stable",
+			CurrentStepIndex:  ptr.To[int32](1),
+			CurrentStepHash:   "abc",
+		}
+		assert.True(t, RolloutProgressing(rollout, &newStatus))
+		assert.False(t, RolloutScalingProgress(rollout, &newStatus))
+		assert.True(t, RolloutReadinessProgress(rollout, &newStatus))
+	})
+
+	t.Run("UpdatedReplicas change is scaling progress", func(t *testing.T) {
+		rollout := canaryRollout(10, 1, 10, 10, "stable", 1, "abc")
+		newStatus := v1alpha1.RolloutStatus{
+			Replicas:          10,
+			UpdatedReplicas:   2,
+			ReadyReplicas:     10,
+			AvailableReplicas: 10,
+			StableRS:          "stable",
+			CurrentStepIndex:  ptr.To[int32](1),
+			CurrentStepHash:   "abc",
+		}
+		assert.True(t, RolloutProgressing(rollout, &newStatus))
+		assert.True(t, RolloutScalingProgress(rollout, &newStatus))
+	})
+
+	t.Run("Step index change is scaling progress", func(t *testing.T) {
+		rollout := canaryRollout(10, 1, 10, 10, "stable", 1, "abc")
+		newStatus := v1alpha1.RolloutStatus{
+			Replicas:          10,
+			UpdatedReplicas:   1,
+			ReadyReplicas:     10,
+			AvailableReplicas: 10,
+			StableRS:          "stable",
+			CurrentStepIndex:  ptr.To[int32](2),
+			CurrentStepHash:   "abc",
+		}
+		assert.True(t, RolloutProgressing(rollout, &newStatus))
+		assert.True(t, RolloutScalingProgress(rollout, &newStatus))
+	})
+
+	t.Run("No change means no progress", func(t *testing.T) {
+		rollout := canaryRollout(10, 1, 10, 10, "stable", 1, "abc")
+		newStatus := v1alpha1.RolloutStatus{
+			Replicas:          10,
+			UpdatedReplicas:   1,
+			ReadyReplicas:     10,
+			AvailableReplicas: 10,
+			StableRS:          "stable",
+			CurrentStepIndex:  ptr.To[int32](1),
+			CurrentStepHash:   "abc",
+		}
+		assert.False(t, RolloutProgressing(rollout, &newStatus))
+		assert.False(t, RolloutScalingProgress(rollout, &newStatus))
+		assert.False(t, RolloutReadinessProgress(rollout, &newStatus))
+	})
+
+	t.Run("AvailableReplicas oscillation is readiness-only progress", func(t *testing.T) {
+		rollout := canaryRollout(10, 1, 9, 9, "stable", 1, "abc")
+		newStatus := v1alpha1.RolloutStatus{
+			Replicas:          10,
+			UpdatedReplicas:   1,
+			ReadyReplicas:     9,
+			AvailableReplicas: 10,
+			StableRS:          "stable",
+			CurrentStepIndex:  ptr.To[int32](1),
+			CurrentStepHash:   "abc",
+		}
+		assert.True(t, RolloutProgressing(rollout, &newStatus))
+		assert.False(t, RolloutScalingProgress(rollout, &newStatus))
+		assert.True(t, RolloutReadinessProgress(rollout, &newStatus))
+	})
+}
+
 func TestRolloutHealthy(t *testing.T) {
 	rollout := func(desired, current, updated, available int32, correctObservedGeneration bool) *v1alpha1.Rollout {
 		r := &v1alpha1.Rollout{