✨ report superseded ClusterObjectSet as Ready=False/Superseded

api/v1/clusterobjectset_types.go

@@ -32,6 +32,7 @@ const (
 	ClusterObjectSetReasonArchived          = "Archived"
 	ClusterObjectSetReasonArchiving         = "Archiving"
 	ClusterObjectSetReasonObjectsNotReady   = "ObjectsNotReady"
+	ClusterObjectSetReasonSuperseded        = "Superseded"
 	ClusterObjectSetReasonObjectCollision   = "ObjectCollision"
 	ClusterObjectSetReasonRetryableError    = "RetryableError"
 	ClusterObjectSetReasonValidationFailure = "ValidationFailure"
@@ -501,6 +502,7 @@ type ClusterObjectSetStatus struct {
 	// the first reconciliation has evaluated the revision.
 	//   - True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
 	//   - False with reason ObjectsNotReady: one or more objects are still being applied or have not yet passed their readiness probes during rollout.
+	//   - False with reason Superseded: one or more objects have been adopted by a newer revision of the object set.
 	//   - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them.
 	//   - False with reason ValidationFailure: the revision failed a pre-apply check (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content) and requires manual intervention.
 	//   - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.

docs/draft/concepts/clusterobjectsets.md

@@ -218,6 +218,7 @@ Indicates whether all objects in the revision have been successfully rolled out
 | --- | --- | --- |
 | True | `AllObjectsReady` | All objects are at the desired state |
 | False | `ObjectsNotReady` | One or more objects are still being applied or have not yet passed their readiness probes during rollout |
+| False | `Superseded` | One or more objects have been adopted by a newer revision of the object set |
 | False | `ObjectCollision` | One or more objects are controlled by another owner, or are unowned and collision protection prevents adoption |
 | False | `ValidationFailure` | A pre-apply check failed (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content); requires manual intervention |
 | Unknown | `RetryableError` | Error prevented probe observation |

helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml

@@ -560,6 +560,7 @@ spec:
                   the first reconciliation has evaluated the revision.
                     - True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
                     - False with reason ObjectsNotReady: one or more objects are still being applied or have not yet passed their readiness probes during rollout.
+                    - False with reason Superseded: one or more objects have been adopted by a newer revision of the object set.
                     - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them.
                     - False with reason ValidationFailure: the revision failed a pre-apply check (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content) and requires manual intervention.
                     - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.

internal/object-controller/controllers/clusterobjectset_controller.go

@@ -48,10 +48,11 @@ const (
 	clusterObjectSetTeardownFinalizer = "olm.operatorframework.io/teardown"
 
 	// validationRetryInterval is how often the reconciler re-checks a revision that is
-	// Ready=False/ValidationFailure due to referenced-content problems (preflight validation,
-	// a mutable referenced Secret, a changed digest, or an invalid referenced manifest).
-	// Referenced Secrets are not watched and the phase/ref spec is immutable, so a periodic
-	// retry lets the revision recover once the Secret is replaced, without a controller restart.
+	// Ready=False/ValidationFailure. These failures stem from inputs that are not watched
+	// — a mutable or changed referenced Secret, an invalid referenced manifest, or a
+	// preflight dry-run rejected until RBAC or a dependency CRD propagates — and the ref
+	// spec is immutable, so a periodic retry lets the revision recover (once the Secret is
+	// replaced or the cluster state catches up) without a controller restart.
 	validationRetryInterval = 10 * time.Second
 )
 
@@ -230,8 +231,9 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err)
 	}
 
-	// Retry failing preflight checks with a flat 10s retry.
-	// TODO: report status, backoff?
+	// Report revision-level preflight validation failures and re-check on a flat timer:
+	// the dry-run can be rejected for reasons that clear without a spec change (e.g. RBAC
+	// or a dependency CRD still propagating), and those inputs are not watched.
 	if verr := rres.GetValidationError(); verr != nil {
 		l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s")
 		markAsNotReady(cos, ocv1.ClusterObjectSetReasonValidationFailure, fmt.Sprintf("revision validation error: %s", verr))
@@ -294,8 +296,15 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		}
 	}
 
+	// A revision whose objects have all been adopted by a newer sibling revision can
+	// report IsComplete() (progressed objects count as complete), but it is superseded,
+	// not successfully rolled out: skip the completion branch so it does not archive
+	// previous revisions, report AllObjectsReady, or record a first completion
+	// (completedAt) that would identify it as the installed revision.
+	superseded := hasSupersededObject(rres)
+
 	//nolint:nestif
-	if rres.IsComplete() {
+	if rres.IsComplete() && !superseded {
 		// Archive previous revisions
 		previous, err := c.listPreviousRevisions(ctx, cos)
 		if err != nil {
@@ -357,9 +366,13 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 		if len(notReadyMsgs) > 0 {
 			l.V(1).Info("objects not yet ready", "count", len(notReadyMsgs), "details", notReadyMsgs)
 		}
-		if isDeadlineExceeded {
+		// Precedence: ProgressDeadlineExceeded > Superseded > ObjectsNotReady.
+		switch {
+		case isDeadlineExceeded:
 			setReadyWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonObjectsNotReady, genericNotReadyMsg, true)
-		} else {
+		case superseded:
+			markAsNotReady(cos, ocv1.ClusterObjectSetReasonSuperseded, "Object(s) have been adopted by a newer revision of the object set.")
+		default:
 			notReadyMsg := genericNotReadyMsg
 			if len(notReadyMsgs) > 0 {
 				notReadyMsg = fmt.Sprintf("%d object(s) are not yet ready.", len(notReadyMsgs))
@@ -780,6 +793,21 @@ func markAsNotReady(cos *ocv1.ClusterObjectSet, reason, message string) bool {
 	})
 }
 
+// hasSupersededObject reports whether any object in the revision has been adopted
+// by a newer sibling revision (machinery.ActionProgressed). boxcutter only emits
+// ActionProgressed for a newer sibling (a foreign owner is a collision), so no
+// revision comparison is needed here.
+func hasSupersededObject(rres machinery.RevisionResult) bool {
+	for _, pres := range rres.GetPhases() {
+		for _, ores := range pres.GetObjects() {
+			if ores.Action() == machinery.ActionProgressed {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func markAsArchiving(cos *ocv1.ClusterObjectSet) {
 	const msg = "The object set is being archived; removing owned objects."
 	markAsNotReady(cos, ocv1.ClusterObjectSetReasonArchiving, msg)

internal/object-controller/controllers/clusterobjectset_controller_test.go

@@ -213,6 +213,97 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
 				require.Equal(t, ocv1.ClusterObjectSetReasonObjectsNotReady, cond.Reason)
 			},
 		},
+		{
+			// A fully handed-off revision can report rres.IsComplete()==true because
+			// progressed objects count as complete (boxcutter). It must still report
+			// Superseded, not AllObjectsReady, and must NOT stamp completedAt — otherwise
+			// the superseded revision could be mis-identified as the installed one.
+			name:                    "complete revision with a progressed object reports Superseded, not AllObjectsReady",
+			reconcilingRevisionName: clusterObjectSetName,
+			revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{
+				isComplete: true,
+				phases: []machinery.PhaseResult{
+					newMockPhaseResult(mockCtrl, phaseResultConfig{
+						name:       "somephase",
+						isComplete: true,
+						objects: []machinery.ObjectResult{
+							newMockObjectResult(mockCtrl, objectResultConfig{
+								action:   machinery.ActionProgressed,
+								complete: true,
+								object: func() client.Object {
+									obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "ns"}}
+									obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap"))
+									return obj
+								}(),
+							}),
+						},
+					}),
+				},
+			}),
+			existingObjs: func() []client.Object {
+				ext := newTestClusterExtension()
+				return []client.Object{ext, newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme)}
+			},
+			validate: func(t *testing.T, c client.Client) {
+				rev := &ocv1.ClusterObjectSet{}
+				require.NoError(t, c.Get(t.Context(), client.ObjectKey{Name: clusterObjectSetName}, rev))
+				cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
+				require.NotNil(t, cond)
+				require.Equal(t, metav1.ConditionFalse, cond.Status)
+				require.Equal(t, ocv1.ClusterObjectSetReasonSuperseded, cond.Reason)
+				require.True(t, rev.Status.CompletedAt.IsZero(), "a superseded revision must not stamp completedAt")
+			},
+		},
+		{
+			// When one or more objects have been adopted by a newer sibling revision
+			// (machinery.ActionProgressed), the revision reports Superseded, which takes
+			// precedence over ObjectsNotReady even when other objects are still not ready.
+			name:                    "set Ready:False:Superseded when an object has progressed to a newer revision",
+			reconcilingRevisionName: clusterObjectSetName,
+			revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{
+				inTransition: true,
+				isComplete:   false,
+				phases: []machinery.PhaseResult{
+					newMockPhaseResult(mockCtrl, phaseResultConfig{
+						name:       "somephase",
+						isComplete: false,
+						objects: []machinery.ObjectResult{
+							newMockObjectResult(mockCtrl, objectResultConfig{
+								action: machinery.ActionProgressed,
+								object: func() client.Object {
+									obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "ns"}}
+									obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap"))
+									return obj
+								}(),
+							}),
+							newMockObjectResult(mockCtrl, objectResultConfig{
+								object: func() client.Object {
+									obj := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc", Namespace: "ns"}}
+									obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service"))
+									return obj
+								}(),
+								probes: machinerytypes.ProbeResultContainer{
+									boxcutter.ProgressProbeType: {Status: machinerytypes.ProbeStatusFalse, Messages: []string{"not ready"}},
+								},
+							}),
+						},
+					}),
+				},
+			}),
+			existingObjs: func() []client.Object {
+				ext := newTestClusterExtension()
+				return []client.Object{ext, newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme)}
+			},
+			validate: func(t *testing.T, c client.Client) {
+				rev := &ocv1.ClusterObjectSet{}
+				require.NoError(t, c.Get(t.Context(), client.ObjectKey{Name: clusterObjectSetName}, rev))
+				cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
+				require.NotNil(t, cond)
+				require.Equal(t, metav1.ConditionFalse, cond.Status)
+				require.Equal(t, ocv1.ClusterObjectSetReasonSuperseded, cond.Reason)
+				require.Equal(t, "Object(s) have been adopted by a newer revision of the object set.", cond.Message)
+			},
+		},
 		{
 			name:                    "set Ready:False:ObjectsNotReady condition when probe failures are detected and revision is in transition",
 			reconcilingRevisionName: clusterObjectSetName,
@@ -1438,6 +1529,47 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
 				require.Equal(t, "Revision has not rolled out for 1 minute(s). Last status: Objects are not yet ready.", cnd.Message)
 			},
 		},
+		{
+			// A progressed (superseded) object AND an exceeded progress deadline must
+			// set Ready=False/ProgressDeadlineExceeded (deadline wins over Superseded).
+			name: "Ready set to false/ProgressDeadlineExceeded when deadline is exceeded with a superseded object (deadline wins over Superseded)",
+			existingObjs: func() []client.Object {
+				ext := newTestClusterExtension()
+				rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme)
+				rev1.Spec.ProgressDeadlineMinutes = 1
+				rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC))
+				return []client.Object{rev1, ext}
+			},
+			clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 1, 1, 0, time.UTC)),
+			revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{
+				inTransition: true,
+				isComplete:   false,
+				phases: []machinery.PhaseResult{
+					newMockPhaseResult(mockCtrl, phaseResultConfig{
+						name:       "somephase",
+						isComplete: false,
+						objects: []machinery.ObjectResult{
+							newMockObjectResult(mockCtrl, objectResultConfig{
+								action: machinery.ActionProgressed,
+								object: func() client.Object {
+									obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "ns"}}
+									obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap"))
+									return obj
+								}(),
+							}),
+						},
+					}),
+				},
+			}),
+			validate: func(t *testing.T, c client.Client) {
+				rev := &ocv1.ClusterObjectSet{}
+				require.NoError(t, c.Get(t.Context(), client.ObjectKey{Name: clusterObjectSetName}, rev))
+				cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
+				require.NotNil(t, cnd)
+				require.Equal(t, metav1.ConditionFalse, cnd.Status)
+				require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason)
+			},
+		},
 		{
 			name: "requeue after progressDeadline time for final progression deadline check",
 			existingObjs: func() []client.Object {

internal/operator-controller/controllers/common_controller_test.go

@@ -480,6 +480,11 @@ func TestDetermineFailureReason(t *testing.T) {
 			rolling:  []*RevisionMetadata{{Conditions: availCond(metav1.ConditionFalse, ocv1.ClusterObjectSetReasonValidationFailure)}},
 			expected: ocv1.ReasonFailed,
 		},
+		{
+			name:     "latest superseded -> Absent",
+			rolling:  []*RevisionMetadata{{Conditions: availCond(metav1.ConditionFalse, ocv1.ClusterObjectSetReasonSuperseded)}},
+			expected: ocv1.ReasonAbsent,
+		},
 		{
 			name: "only the latest revision matters",
 			rolling: []*RevisionMetadata{
@@ -520,6 +525,12 @@ func TestProgressingFromReady(t *testing.T) {
 			completed: false,
 			expected:  metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRollingOut, Message: "Objects are not yet ready."},
 		},
+		{
+			name:      "superseded maps to RollingOut",
+			ready:     &metav1.Condition{Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectSetReasonSuperseded, Message: "Object(s) have been adopted by a newer revision of the object set."},
+			completed: false,
+			expected:  metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRollingOut, Message: "Object(s) have been adopted by a newer revision of the object set."},
+		},
 		{
 			name:      "archiving maps to RollingOut",
 			ready:     &metav1.Condition{Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectSetReasonArchiving, Message: "The object set is being archived; removing owned objects."},

manifests/experimental-e2e.yaml

@@ -1971,6 +1971,7 @@ spec:
                   the first reconciliation has evaluated the revision.
                     - True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
                     - False with reason ObjectsNotReady: one or more objects are still being applied or have not yet passed their readiness probes during rollout.
+                    - False with reason Superseded: one or more objects have been adopted by a newer revision of the object set.
                     - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them.
                     - False with reason ValidationFailure: the revision failed a pre-apply check (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content) and requires manual intervention.
                     - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.

manifests/experimental.yaml

@@ -1932,6 +1932,7 @@ spec:
                   the first reconciliation has evaluated the revision.
                     - True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
                     - False with reason ObjectsNotReady: one or more objects are still being applied or have not yet passed their readiness probes during rollout.
+                    - False with reason Superseded: one or more objects have been adopted by a newer revision of the object set.
                     - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them.
                     - False with reason ValidationFailure: the revision failed a pre-apply check (an invalid revision/phase object or probe, preflight validation, a mutable referenced Secret, or changed referenced content) and requires manual intervention.
                     - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.