fix: Allow spec changes after ProgressDeadlineExceeded

Once ProgressDeadlineExceeded is set, the controller predicate was blocking ALL updates including spec changes like lifecycleState=Archived. This prevented timed-out ClusterObjectSets from being archived. Now allows generation/spec changes while still suppressing status-only churn.

Author: camilamacedo86

internal/operator-controller/controllers/clusterobjectset_controller.go

@@ -344,22 +344,44 @@ type Sourcoser interface {
 	Source(handler handler.EventHandler, predicates ...predicate.Predicate) source.Source
 }
 
+// skipProgressDeadlineExceededUpdateFunc is the update predicate function that
+// prevents reconciliation churn for ClusterObjectSets that have reached ProgressDeadlineExceeded.
+// It allows deletions and spec/generation changes (including lifecycleState transitions)
+// but suppresses status-only updates.
+func skipProgressDeadlineExceededUpdateFunc(e event.UpdateEvent) bool {
+	newRev, ok := e.ObjectNew.(*ocv1.ClusterObjectSet)
+	if !ok {
+		return true
+	}
+	oldRev, ok := e.ObjectOld.(*ocv1.ClusterObjectSet)
+	if !ok {
+		return true
+	}
+
+	// Allow deletions to happen
+	if !newRev.DeletionTimestamp.IsZero() {
+		return true
+	}
+
+	// Check if the revision has ProgressDeadlineExceeded condition
+	cnd := meta.FindStatusCondition(newRev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing)
+	if cnd == nil || cnd.Status != metav1.ConditionFalse || cnd.Reason != ocv1.ReasonProgressDeadlineExceeded {
+		return true
+	}
+
+	// Progress deadline exceeded - allow generation/spec changes but suppress status-only churn
+	// Allow if generation changed (indicates spec change, including lifecycleState transitions)
+	if oldRev.Generation != newRev.Generation {
+		return true
+	}
+
+	// Suppress status-only updates
+	return false
+}
+
 func (c *ClusterObjectSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	skipProgressDeadlineExceededPredicate := predicate.Funcs{
-		UpdateFunc: func(e event.UpdateEvent) bool {
-			rev, ok := e.ObjectNew.(*ocv1.ClusterObjectSet)
-			if !ok {
-				return true
-			}
-			// allow deletions to happen
-			if !rev.DeletionTimestamp.IsZero() {
-				return true
-			}
-			if cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing); cnd != nil && cnd.Status == metav1.ConditionFalse && cnd.Reason == ocv1.ReasonProgressDeadlineExceeded {
-				return false
-			}
-			return true
-		},
+		UpdateFunc: skipProgressDeadlineExceededUpdateFunc,
 	}
 	c.Clock = clock.RealClock{}
 	return ctrl.NewControllerManagedBy(mgr).

internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go

@@ -14,6 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/source"
@@ -235,3 +236,186 @@ func (m *mockTrackingCacheInternal) Watch(ctx context.Context, user client.Objec
 func (m *mockTrackingCacheInternal) Source(h handler.EventHandler, predicates ...predicate.Predicate) source.Source {
 	return nil
 }
+
+func Test_skipProgressDeadlineExceededUpdateFunc(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		oldRev   *ocv1.ClusterObjectSet
+		newRev   *ocv1.ClusterObjectSet
+		expected bool
+	}{
+		{
+			name: "allow deletion",
+			oldRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			newRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Generation:        1,
+					DeletionTimestamp: &metav1.Time{Time: time.Now()},
+				},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "allow when ProgressDeadlineExceeded condition not present",
+			oldRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			newRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			expected: true,
+		},
+		{
+			name: "allow when Progressing condition is true",
+			oldRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionTrue,
+							Reason: ocv1.ReasonRollingOut,
+						},
+					},
+				},
+			},
+			newRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionTrue,
+							Reason: ocv1.ReasonRollingOut,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "allow generation change when ProgressDeadlineExceeded",
+			oldRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			newRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 2}, // Generation changed
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "suppress status-only update when ProgressDeadlineExceeded",
+			oldRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			newRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1}, // Same generation
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+						{
+							Type:   ocv1.ClusterObjectSetTypeAvailable,
+							Status: metav1.ConditionFalse,
+							Reason: "SomeReason",
+						},
+					},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "allow spec change via generation bump when ProgressDeadlineExceeded (lifecycleState transition)",
+			oldRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+				Spec: ocv1.ClusterObjectSetSpec{
+					LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive,
+				},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			newRev: &ocv1.ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Generation: 2}, // Generation bumped due to spec change
+				Spec: ocv1.ClusterObjectSetSpec{
+					LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived,
+				},
+				Status: ocv1.ClusterObjectSetStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   ocv1.ClusterObjectSetTypeProgressing,
+							Status: metav1.ConditionFalse,
+							Reason: ocv1.ReasonProgressDeadlineExceeded,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			e := event.UpdateEvent{
+				ObjectOld: tc.oldRev,
+				ObjectNew: tc.newRev,
+			}
+			result := skipProgressDeadlineExceededUpdateFunc(e)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}