✨ OPRUN-4791: Record first completion timestamps for ClusterObjectSet phases

api/v1/clusterobjectset_types.go

@@ -506,6 +506,7 @@ const (
 // update, which would otherwise allow the timestamp to be cleared and re-set.
 //
 // +kubebuilder:validation:XValidation:rule="!has(oldSelf.completedAt) || has(self.completedAt)",message="completedAt cannot be removed once set"
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.observedPhases) || oldSelf.observedPhases.all(p, !has(p.completedAt) || (has(self.observedPhases) && self.observedPhases.exists(q, q.name == p.name)))",message="completed phases cannot be removed from observedPhases"
 type ClusterObjectSetStatus struct {
 	// conditions is an optional list of status conditions describing the state of the
 	// ClusterObjectSet.
@@ -560,7 +561,9 @@ type ClusterObjectSetStatus struct {
 	CompletedAt metav1.Time `json:"completedAt,omitempty,omitzero"`
 }
 
-// ObservedPhase records the observed content digest of a resolved phase.
+// ObservedPhase records the content and rollout progress of a resolved phase.
+//
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.completedAt) || has(self.completedAt)",message="completedAt cannot be removed once set"
 type ObservedPhase struct {
 	// name is the phase name matching a phase in spec.phases.
 	//
@@ -585,6 +588,15 @@ type ObservedPhase struct {
 	// fields are reported (including zeros).
 	// +optional
 	ObjectCounts ObjectCounts `json:"objectCounts,omitzero"`
+
+	// completedAt is the timestamp at which this phase was first observed to have
+	// successfully reconciled and passed its probes after all preceding phases
+	// completed. It is set once and is immutable thereafter, even if the phase
+	// later becomes unhealthy. ClusterObjectDeployment uses it as a progress
+	// milestone for its inactivity deadline.
+	// +optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf == null",message="completedAt is immutable once set"
+	CompletedAt metav1.Time `json:"completedAt,omitempty,omitzero"`
 }
 
 // ObjectCounts reports the number of objects in a ClusterObjectSet or phase by state.

api/v1/validation_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -240,6 +241,72 @@ func TestClusterObjectSetCompletedAtCannotBeRemoved(t *testing.T) {
 	require.True(t, errors.IsInvalid(err), "expected removal to fail as invalid, but got: %v", err)
 }
 
+func TestClusterObjectSetPhaseCompletedAt(t *testing.T) {
+	c := newClient(t)
+	firstReady := metav1.NewTime(time.Date(2026, 10, 21, 12, 0, 0, 0, time.UTC))
+
+	for i, tc := range []struct {
+		name              string
+		oldObservedPhases []ObservedPhase
+		newObservedPhases []ObservedPhase
+		wantErr           bool
+	}{
+		{
+			name:              "set the first completion timestamp",
+			oldObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123"}},
+			newObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: firstReady}},
+		},
+		{
+			name:              "reapply the same completion timestamp",
+			oldObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: firstReady}},
+			newObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: firstReady}},
+		},
+		{
+			name:              "reject changing the completion timestamp",
+			oldObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: firstReady}},
+			newObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: metav1.NewTime(firstReady.Add(time.Minute))}},
+			wantErr:           true,
+		},
+		{
+			name:              "reject removing the completion timestamp",
+			oldObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: firstReady}},
+			newObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123"}},
+			wantErr:           true,
+		},
+		{
+			name:              "reject removing a completed phase",
+			oldObservedPhases: []ObservedPhase{{Name: "deploy", Digest: "sha256:abc123", CompletedAt: firstReady}},
+			newObservedPhases: nil,
+			wantErr:           true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cos := &ClusterObjectSet{
+				ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("cos-phase-completedat-%d", i)},
+				Spec: ClusterObjectSetSpec{
+					Group:               "test-group",
+					Revision:            1,
+					CollisionProtection: CollisionProtectionPrevent,
+					LifecycleState:      ClusterObjectSetLifecycleStateActive,
+				},
+			}
+			require.NoError(t, c.Create(t.Context(), cos))
+			cos.Status.ObservedPhases = tc.oldObservedPhases
+			require.NoError(t, c.Status().Update(t.Context(), cos))
+
+			cos.Status.ObservedPhases = tc.newObservedPhases
+			err := c.Status().Update(t.Context(), cos)
+			if tc.wantErr {
+				require.True(t, errors.IsInvalid(err), "expected update to fail as invalid, but got: %v", err)
+				return
+			}
+			require.NoError(t, err)
+			require.NoError(t, c.Get(t.Context(), client.ObjectKeyFromObject(cos), cos))
+			require.True(t, apiequality.Semantic.DeepEqual(tc.newObservedPhases, cos.Status.ObservedPhases), "phase status must survive storage")
+		})
+	}
+}
+
 func TestClusterObjectSetDigestImmutable(t *testing.T) {
 	c := newClient(t)
 

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

@@ -722,9 +722,21 @@ spec:
                   different content. Each entry covers all fully-resolved object
                   manifests within a phase, making it source-agnostic.
                 items:
-                  description: ObservedPhase records the observed content digest of
-                    a resolved phase.
+                  description: ObservedPhase records the content and rollout progress
+                    of a resolved phase.
                   properties:
+                    completedAt:
+                      description: |-
+                        completedAt is the timestamp at which this phase was first observed to have
+                        successfully reconciled and passed its probes after all preceding phases
+                        completed. It is set once and is immutable thereafter, even if the phase
+                        later becomes unhealthy. ClusterObjectDeployment uses it as a progress
+                        milestone for its inactivity deadline.
+                      format: date-time
+                      type: string
+                      x-kubernetes-validations:
+                      - message: completedAt is immutable once set
+                        rule: self == oldSelf || oldSelf == null
                     digest:
                       description: |-
                         digest is the digest of the phase's resolved object content
@@ -795,6 +807,9 @@ spec:
                   - digest
                   - name
                   type: object
+                  x-kubernetes-validations:
+                  - message: completedAt cannot be removed once set
+                    rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
                 maxItems: 20
                 type: array
                 x-kubernetes-list-map-keys:
@@ -804,6 +819,10 @@ spec:
             x-kubernetes-validations:
             - message: completedAt cannot be removed once set
               rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
+            - message: completed phases cannot be removed from observedPhases
+              rule: '!has(oldSelf.observedPhases) || oldSelf.observedPhases.all(p,
+                !has(p.completedAt) || (has(self.observedPhases) && self.observedPhases.exists(q,
+                q.name == p.name)))'
         required:
         - spec
         type: object

internal/object-controller/controllers/clusterobjectset_controller.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"slices"
 	"strings"
 	"time"
 
@@ -245,6 +246,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
 
 	// Set object counts
 	status.FromReconcile(cos, rres)
+	c.recordPhaseCompletions(cos, rres)
 
 	// 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
@@ -795,6 +797,26 @@ func markAsNotReady(cos *ocv1.ClusterObjectSet, reason, message string) bool {
 	})
 }
 
+// recordPhaseCompletions records each phase's first successful rollout. Read-only
+// downstream phases and phases superseded by a newer revision are not milestones.
+// Existing timestamps survive retries, drift, and controller restarts.
+func (c *ClusterObjectSetReconciler) recordPhaseCompletions(cos *ocv1.ClusterObjectSet, result machinery.RevisionResult) {
+	now := metav1.NewTime(c.Clock.Now())
+	for _, phase := range result.GetPhases() {
+		if !phase.IsComplete() || slices.ContainsFunc(phase.GetObjects(), func(obj machinery.ObjectResult) bool {
+			return obj.Action() == machinery.ActionProgressed
+		}) {
+			break
+		}
+		for i := range cos.Status.ObservedPhases {
+			observed := &cos.Status.ObservedPhases[i]
+			if observed.Name == phase.GetName() && observed.CompletedAt.IsZero() {
+				observed.CompletedAt = now
+			}
+		}
+	}
+}
+
 // 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

internal/object-controller/controllers/clusterobjectset_controller_test.go

@@ -763,6 +763,80 @@ func Test_ClusterObjectSetReconciler_Reconcile_CompletedAt(t *testing.T) {
 	}
 }
 
+func Test_ClusterObjectSetReconciler_Reconcile_PhaseCompletedAt(t *testing.T) {
+	firstReady := metav1.NewTime(time.Date(2026, 10, 21, 12, 0, 0, 0, time.UTC))
+	now := metav1.NewTime(firstReady.Add(time.Minute))
+	for _, tc := range []struct {
+		name           string
+		firstComplete  bool
+		secondComplete bool
+		superseded     bool
+		existing       metav1.Time
+		wantFirst      metav1.Time
+		wantSecond     metav1.Time
+	}{
+		{name: "records progress before the revision completes", firstComplete: true, wantFirst: now},
+		{name: "does not count read-only downstream phases", secondComplete: true},
+		{name: "preserves an earlier milestone when the next phase completes", firstComplete: true, secondComplete: true, existing: firstReady, wantFirst: firstReady, wantSecond: now},
+		{name: "preserves completion after drift", existing: firstReady, wantFirst: firstReady},
+		{name: "a partially superseded phase is not progress", firstComplete: true, secondComplete: true, superseded: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			mockCtrl := gomock.NewController(t)
+			testScheme := newScheme(t)
+			ext := newTestClusterExtension()
+			rev := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme)
+			rev.Finalizers = []string{"olm.operatorframework.io/teardown"}
+			second := rev.Spec.Phases[0]
+			second.Name = "later"
+			rev.Spec.Phases = append(rev.Spec.Phases, second)
+			c := testutil.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes).
+				WithStatusSubresource(&ocv1.ClusterObjectSet{}).
+				WithObjects(ext, rev).
+				Build()
+			var objects []machinery.ObjectResult
+			if tc.superseded {
+				objects = []machinery.ObjectResult{
+					newMockObjectResult(mockCtrl, objectResultConfig{action: machinery.ActionProgressed, complete: true}),
+					newMockObjectResult(mockCtrl, objectResultConfig{action: machinery.ActionIdle, complete: true}),
+				}
+			}
+			phases := []machinery.PhaseResult{
+				newMockPhaseResult(mockCtrl, phaseResultConfig{name: "everything", isComplete: tc.firstComplete, objects: objects}),
+				newMockPhaseResult(mockCtrl, phaseResultConfig{name: "later", isComplete: tc.secondComplete}),
+			}
+			result := newMockRevisionResult(mockCtrl, revisionResultConfig{phases: phases})
+			nextResult := newMockRevisionResult(mockCtrl, revisionResultConfig{phases: []machinery.PhaseResult{
+				newMockPhaseResult(mockCtrl, phaseResultConfig{name: "everything", isComplete: !tc.existing.IsZero()}),
+				newMockPhaseResult(mockCtrl, phaseResultConfig{name: "later"}),
+			}})
+			engine := newMockRevisionEngineWithReconcile(mockCtrl,
+				func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
+					current := nextResult
+					nextResult = result
+					return current, nil
+				}, nil)
+			clk := clocktesting.NewFakeClock(firstReady.Time)
+			reconciler := &controllers.ClusterObjectSetReconciler{
+				Client: c, Clock: clk,
+				RevisionEngineFactory: newMockRevisionEngineFactoryWithEngine(mockCtrl, engine, nil),
+				TrackingCache:         newMockTrackingCache(mockCtrl, c, nil),
+			}
+			req := ctrl.Request{NamespacedName: client.ObjectKeyFromObject(rev)}
+			_, err := reconciler.Reconcile(t.Context(), req)
+			require.NoError(t, err)
+			require.NoError(t, c.Get(t.Context(), req.NamespacedName, rev))
+			clk.SetTime(now.Time)
+			_, err = reconciler.Reconcile(t.Context(), req)
+			require.NoError(t, err)
+			require.NoError(t, c.Get(t.Context(), req.NamespacedName, rev))
+			require.Len(t, rev.Status.ObservedPhases, 2)
+			require.True(t, tc.wantFirst.Equal(&rev.Status.ObservedPhases[0].CompletedAt), "first phase: %v", rev.Status.ObservedPhases[0].CompletedAt)
+			require.True(t, tc.wantSecond.Equal(&rev.Status.ObservedPhases[1].CompletedAt), "second phase: %v", rev.Status.ObservedPhases[1].CompletedAt)
+		})
+	}
+}
+
 func Test_ClusterObjectSetReconciler_Reconcile_ValidationError_Retries(t *testing.T) {
 	const (
 		clusterExtensionName = "test-ext"

manifests/experimental-e2e.yaml

@@ -2693,9 +2693,21 @@ spec:
                   different content. Each entry covers all fully-resolved object
                   manifests within a phase, making it source-agnostic.
                 items:
-                  description: ObservedPhase records the observed content digest of
-                    a resolved phase.
+                  description: ObservedPhase records the content and rollout progress
+                    of a resolved phase.
                   properties:
+                    completedAt:
+                      description: |-
+                        completedAt is the timestamp at which this phase was first observed to have
+                        successfully reconciled and passed its probes after all preceding phases
+                        completed. It is set once and is immutable thereafter, even if the phase
+                        later becomes unhealthy. ClusterObjectDeployment uses it as a progress
+                        milestone for its inactivity deadline.
+                      format: date-time
+                      type: string
+                      x-kubernetes-validations:
+                      - message: completedAt is immutable once set
+                        rule: self == oldSelf || oldSelf == null
                     digest:
                       description: |-
                         digest is the digest of the phase's resolved object content
@@ -2766,6 +2778,9 @@ spec:
                   - digest
                   - name
                   type: object
+                  x-kubernetes-validations:
+                  - message: completedAt cannot be removed once set
+                    rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
                 maxItems: 20
                 type: array
                 x-kubernetes-list-map-keys:
@@ -2775,6 +2790,10 @@ spec:
             x-kubernetes-validations:
             - message: completedAt cannot be removed once set
               rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
+            - message: completed phases cannot be removed from observedPhases
+              rule: '!has(oldSelf.observedPhases) || oldSelf.observedPhases.all(p,
+                !has(p.completedAt) || (has(self.observedPhases) && self.observedPhases.exists(q,
+                q.name == p.name)))'
         required:
         - spec
         type: object