From 27745876d881d889610e8d666a4c89630fa73731 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Thu, 24 Sep 2026 13:23:53 +0200 Subject: [PATCH 1/2] :sparkles: Add completedAt to ClusterObjectSet status Add a `.status.completedAt` field to ClusterObjectSet that records the timestamp of the first time the revision was observed to be ready (rolled out and passing all probes). The field is optional and immutable once set, enforced by a CEL transition rule and a write-once guard in the controller (using the reconciler's injectable Clock). Co-Authored-By: Claude Opus 4.8 Signed-off-by: Per G. da Silva --- api/v1/clusterobjectset_types.go | 14 +++ api/v1/validation_test.go | 57 ++++++++++++ api/v1/zz_generated.deepcopy.go | 1 + .../api/v1/clusterobjectsetstatus.go | 19 +++- applyconfigurations/internal/internal.go | 3 + ...peratorframework.io_clusterobjectsets.yaml | 15 ++- .../clusterobjectset_controller.go | 10 ++ .../clusterobjectset_controller_test.go | 92 +++++++++++++++++++ manifests/experimental-e2e.yaml | 15 ++- manifests/experimental.yaml | 15 ++- 10 files changed, 237 insertions(+), 4 deletions(-) diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index 56fe210bd9..96e52aedff 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -486,6 +486,12 @@ const ( ) // ClusterObjectSetStatus defines the observed state of a ClusterObjectSet. +// +// The completedAt removal guard lives here at the parent level because a +// field-level transition rule is skipped when the field is absent from an +// 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" type ClusterObjectSetStatus struct { // conditions is an optional list of status conditions describing the state of the // ClusterObjectSet. @@ -524,6 +530,14 @@ type ClusterObjectSetStatus struct { // +listMapKey=name // +optional ObservedPhases []ObservedPhase `json:"observedPhases,omitempty"` + + // completedAt is the timestamp at which the revision was first observed to be + // ready, meaning it had successfully rolled out and all of its objects passed + // their probes. It is set once and is immutable thereafter. + // + // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf == null",message="completedAt is immutable" + // +optional + CompletedAt metav1.Time `json:"completedAt,omitempty,omitzero"` } // ObservedPhase records the observed content digest of a resolved phase. diff --git a/api/v1/validation_test.go b/api/v1/validation_test.go index 10e8b6bc43..2c77c17663 100644 --- a/api/v1/validation_test.go +++ b/api/v1/validation_test.go @@ -3,7 +3,9 @@ package v1 import ( "fmt" "testing" + "time" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -179,3 +181,58 @@ func TestValidate(t *testing.T) { }) } } + +func TestClusterObjectSetCompletedAtImmutable(t *testing.T) { + c := newClient(t) + + cos := &ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: "cos-completedat-immutable"}, + Spec: ClusterObjectSetSpec{ + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + LifecycleState: ClusterObjectSetLifecycleStateActive, + }, + } + require.NoError(t, c.Create(t.Context(), cos)) + + firstReady := metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) + laterReady := metav1.NewTime(time.Date(2023, 6, 15, 12, 0, 0, 0, time.UTC)) + + // completedAt may be set once, from empty. + cos.Status.CompletedAt = firstReady + require.NoError(t, c.Status().Update(t.Context(), cos)) + + // Re-applying the same value must be allowed. + cos.Status.CompletedAt = firstReady + require.NoError(t, c.Status().Update(t.Context(), cos)) + + // Changing the value once set must be rejected. + cos.Status.CompletedAt = laterReady + err := c.Status().Update(t.Context(), cos) + require.True(t, errors.IsInvalid(err), "expected update to fail as invalid, but got: %v", err) +} + +func TestClusterObjectSetCompletedAtCannotBeRemoved(t *testing.T) { + c := newClient(t) + + cos := &ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: "cos-completedat-noremove"}, + Spec: ClusterObjectSetSpec{ + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + LifecycleState: ClusterObjectSetLifecycleStateActive, + }, + } + require.NoError(t, c.Create(t.Context(), cos)) + + // Set completedAt once. + cos.Status.CompletedAt = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) + require.NoError(t, c.Status().Update(t.Context(), cos)) + + // Removing completedAt once set must be rejected. The field-level transition + // rule is skipped when the field is absent from the update, so a parent-level + // rule must reject its removal. + cos.Status.CompletedAt = metav1.Time{} + err := c.Status().Update(t.Context(), cos) + require.True(t, errors.IsInvalid(err), "expected removal to fail as invalid, but got: %v", err) +} diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 6836216378..6b54d21ca4 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -565,6 +565,7 @@ func (in *ClusterObjectSetStatus) DeepCopyInto(out *ClusterObjectSetStatus) { *out = make([]ObservedPhase, len(*in)) copy(*out, *in) } + in.CompletedAt.DeepCopyInto(&out.CompletedAt) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterObjectSetStatus. diff --git a/applyconfigurations/api/v1/clusterobjectsetstatus.go b/applyconfigurations/api/v1/clusterobjectsetstatus.go index 6203563de7..380176545f 100644 --- a/applyconfigurations/api/v1/clusterobjectsetstatus.go +++ b/applyconfigurations/api/v1/clusterobjectsetstatus.go @@ -13,11 +13,12 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -// Code generated by controller-gen-v0.20. DO NOT EDIT. +// Code generated by controller-gen-v0.21. DO NOT EDIT. package v1 import ( + apismetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/client-go/applyconfigurations/meta/v1" ) @@ -25,6 +26,10 @@ import ( // with apply. // // ClusterObjectSetStatus defines the observed state of a ClusterObjectSet. +// +// The completedAt removal guard lives here at the parent level because a +// field-level transition rule is skipped when the field is absent from an +// update, which would otherwise allow the timestamp to be cleared and re-set. type ClusterObjectSetStatusApplyConfiguration struct { // conditions is an optional list of status conditions describing the state of the // ClusterObjectSet. @@ -52,6 +57,10 @@ type ClusterObjectSetStatusApplyConfiguration struct { // different content. Each entry covers all fully-resolved object // manifests within a phase, making it source-agnostic. ObservedPhases []ObservedPhaseApplyConfiguration `json:"observedPhases,omitempty"` + // completedAt is the timestamp at which the revision was first observed to be + // ready, meaning it had successfully rolled out and all of its objects passed + // their probes. It is set once and is immutable thereafter. + CompletedAt *apismetav1.Time `json:"completedAt,omitempty"` } // ClusterObjectSetStatusApplyConfiguration constructs a declarative configuration of the ClusterObjectSetStatus type for use with @@ -85,3 +94,11 @@ func (b *ClusterObjectSetStatusApplyConfiguration) WithObservedPhases(values ... } return b } + +// WithCompletedAt sets the CompletedAt field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the CompletedAt field is set to the value of the last call. +func (b *ClusterObjectSetStatusApplyConfiguration) WithCompletedAt(value apismetav1.Time) *ClusterObjectSetStatusApplyConfiguration { + b.CompletedAt = &value + return b +} diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index dde5aaf513..13a04520f8 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -327,6 +327,9 @@ var schemaYAML = typed.YAMLObject(`types: - name: com.github.operator-framework.operator-controller.api.v1.ClusterObjectSetStatus map: fields: + - name: completedAt + type: + namedType: io.k8s.apimachinery.pkg.apis.meta.v1.Time - name: conditions type: list: diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml index 0c94049181..bc49544831 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.21.0 olm.operatorframework.io/generator: experimental name: clusterobjectsets.olm.operatorframework.io spec: @@ -542,6 +542,16 @@ spec: description: status is optional and defines the observed state of the ClusterObjectSet. properties: + completedAt: + description: |- + completedAt is the timestamp at which the revision was first observed to be + ready, meaning it had successfully rolled out and all of its objects passed + their probes. It is set once and is immutable thereafter. + format: date-time + type: string + x-kubernetes-validations: + - message: completedAt is immutable + rule: self == oldSelf || oldSelf == null conditions: description: |- conditions is an optional list of status conditions describing the state of the @@ -665,6 +675,9 @@ spec: - message: observedPhases is immutable rule: self == oldSelf || oldSelf.size() == 0 type: object + x-kubernetes-validations: + - message: completedAt cannot be removed once set + rule: '!has(oldSelf.completedAt) || has(self.completedAt)' type: object served: true storage: true diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index e42e3c6144..b065d108ed 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -75,6 +75,10 @@ func (c *ClusterObjectSetReconciler) Reconcile(ctx context.Context, req ctrl.Req l := log.FromContext(ctx).WithName("cluster-extension-revision") ctx = log.IntoContext(ctx, l) + if c.Clock == nil { + c.Clock = clock.RealClock{} + } + existingRev := &ocv1.ClusterObjectSet{} if err := c.Client.Get(ctx, req.NamespacedName, existingRev); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) @@ -246,6 +250,12 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl markAsProgressing(l, cos, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion), isDeadlineExceeded) markAsAvailable(cos, ocv1.ClusterObjectSetReasonProbesSucceeded, "Objects are available and pass all probes.") + // Record the timestamp of the first time the revision was observed to be + // ready. This is set once and never changes for subsequent reconciliations. + if cos.Status.CompletedAt.IsZero() { + cos.Status.CompletedAt = metav1.NewTime(c.Clock.Now()) + } + // We'll probably only want to remove this once we are done updating the ClusterExtension conditions // as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now // that's fine. diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index e70fbb1c88..718289706d 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -511,6 +511,98 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing } } +func Test_ClusterObjectSetReconciler_Reconcile_CompletedAt(t *testing.T) { + testScheme := newScheme(t) + + firstReady := metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) + laterReady := metav1.NewTime(time.Date(2023, 6, 15, 12, 0, 0, 0, time.UTC)) + + for _, tc := range []struct { + name string + revisionResult machinery.RevisionResult + clock clock.Clock + existingObjs func() []client.Object + validate func(*testing.T, *ocv1.ClusterObjectSet) + }{ + { + name: "sets completedAt to the current time on first successful rollout", + revisionResult: newMockRevisionResult(gomock.NewController(t), revisionResultConfig{ + isComplete: true, + }), + clock: clocktesting.NewFakeClock(firstReady.Time), + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + return []client.Object{ext, rev} + }, + validate: func(t *testing.T, rev *ocv1.ClusterObjectSet) { + require.False(t, rev.Status.CompletedAt.IsZero()) + require.True(t, firstReady.Equal(&rev.Status.CompletedAt)) + }, + }, + { + name: "does not overwrite completedAt on a subsequent successful rollout", + revisionResult: newMockRevisionResult(gomock.NewController(t), revisionResultConfig{ + isComplete: true, + }), + clock: clocktesting.NewFakeClock(laterReady.Time), + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + rev.Status.CompletedAt = firstReady + return []client.Object{ext, rev} + }, + validate: func(t *testing.T, rev *ocv1.ClusterObjectSet) { + require.True(t, firstReady.Equal(&rev.Status.CompletedAt)) + }, + }, + { + name: "does not set completedAt while the revision is still rolling out", + revisionResult: newMockRevisionResult(gomock.NewController(t), revisionResultConfig{ + isComplete: false, + }), + clock: clocktesting.NewFakeClock(firstReady.Time), + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + return []client.Object{ext, rev} + }, + validate: func(t *testing.T, rev *ocv1.ClusterObjectSet) { + require.True(t, rev.Status.CompletedAt.IsZero()) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + + testClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithStatusSubresource(&ocv1.ClusterObjectSet{}). + WithObjects(tc.existingObjs()...). + Build() + + mockEngine := newMockRevisionEngineWithReconcile(mockCtrl, + func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return tc.revisionResult, nil + }, nil, + ) + _, err := (&controllers.ClusterObjectSetReconciler{ + Client: testClient, + RevisionEngineFactory: newMockRevisionEngineFactoryWithEngine(mockCtrl, mockEngine, nil), + TrackingCache: newMockTrackingCache(mockCtrl, testClient, nil), + Clock: tc.clock, + }).Reconcile(t.Context(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: clusterObjectSetName}, + }) + require.NoError(t, err) + + rev := &ocv1.ClusterObjectSet{} + require.NoError(t, testClient.Get(t.Context(), client.ObjectKey{Name: clusterObjectSetName}, rev)) + tc.validate(t, rev) + }) + } +} + func Test_ClusterObjectSetReconciler_Reconcile_ValidationError_Retries(t *testing.T) { const ( clusterExtensionName = "test-ext" diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 885d648b4a..dd73d232fd 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1359,7 +1359,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.21.0 olm.operatorframework.io/generator: experimental name: clusterobjectsets.olm.operatorframework.io spec: @@ -1898,6 +1898,16 @@ spec: description: status is optional and defines the observed state of the ClusterObjectSet. properties: + completedAt: + description: |- + completedAt is the timestamp at which the revision was first observed to be + ready, meaning it had successfully rolled out and all of its objects passed + their probes. It is set once and is immutable thereafter. + format: date-time + type: string + x-kubernetes-validations: + - message: completedAt is immutable + rule: self == oldSelf || oldSelf == null conditions: description: |- conditions is an optional list of status conditions describing the state of the @@ -2021,6 +2031,9 @@ spec: - message: observedPhases is immutable rule: self == oldSelf || oldSelf.size() == 0 type: object + x-kubernetes-validations: + - message: completedAt cannot be removed once set + rule: '!has(oldSelf.completedAt) || has(self.completedAt)' type: object served: true storage: true diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 2ceb70b4b5..1c9fef73a0 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1320,7 +1320,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.21.0 olm.operatorframework.io/generator: experimental name: clusterobjectsets.olm.operatorframework.io spec: @@ -1859,6 +1859,16 @@ spec: description: status is optional and defines the observed state of the ClusterObjectSet. properties: + completedAt: + description: |- + completedAt is the timestamp at which the revision was first observed to be + ready, meaning it had successfully rolled out and all of its objects passed + their probes. It is set once and is immutable thereafter. + format: date-time + type: string + x-kubernetes-validations: + - message: completedAt is immutable + rule: self == oldSelf || oldSelf == null conditions: description: |- conditions is an optional list of status conditions describing the state of the @@ -1982,6 +1992,9 @@ spec: - message: observedPhases is immutable rule: self == oldSelf || oldSelf.size() == 0 type: object + x-kubernetes-validations: + - message: completedAt cannot be removed once set + rule: '!has(oldSelf.completedAt) || has(self.completedAt)' type: object served: true storage: true From 0df77e07e5e3cd1645c1a7ff0767cfb343b44fac Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Thu, 24 Sep 2026 15:45:01 +0200 Subject: [PATCH 2/2] :warning: Remove Succeeded condition type from ClusterObjectSet Replace the ClusterObjectSet Succeeded condition type with the status.completedAt field as the signal that a revision has rolled out. The ClusterExtension status mapping and the progress deadline check now key off completedAt instead of the Succeeded condition, and the Helm-to-boxcutter migrator records completedAt on migrated revisions. Since ClusterObjectSet is experimental and does not guarantee upgrade safety, this is a clean cut with no backward-compatibility fallback. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Per G. da Silva --- api/v1/clusterextension_types.go | 2 +- api/v1/clusterobjectset_types.go | 4 - .../api/v1/clusterobjectsetstatus.go | 3 - applyconfigurations/api/v1/revisionstatus.go | 4 +- docs/api-reference/olmv1-api-reference.md | 2 +- ...peratorframework.io_clusterextensions.yaml | 2 +- ...peratorframework.io_clusterobjectsets.yaml | 3 - .../clusterobjectset_controller.go | 13 +--- .../clusterobjectset_controller_test.go | 16 +--- .../controllers/progress_deadline.go | 18 ++--- .../controllers/progress_deadline_test.go | 7 +- .../operator-controller/applier/boxcutter.go | 28 +++---- .../applier/boxcutter_test.go | 52 +++---------- .../controllers/boxcutter_reconcile_steps.go | 4 +- .../boxcutter_reconcile_steps_test.go | 73 +++++++++++++++++++ manifests/experimental-e2e.yaml | 5 +- manifests/experimental.yaml | 5 +- 17 files changed, 123 insertions(+), 118 deletions(-) create mode 100644 internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index f0fbfd6e08..244630b87d 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -520,7 +520,7 @@ type RevisionStatus struct { // name of the ClusterObjectSet resource Name string `json:"name"` // conditions optionally expose Progressing and Available condition of the revision, - // in case when it is not yet marked as successfully installed (condition Succeeded is not set to True). + // in case when it is not yet marked as successfully installed (completedAt is not set). // Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions // to get more insights about reasons for its current state. // diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index 96e52aedff..d37e6f9c49 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -28,7 +28,6 @@ const ( // Condition Types ClusterObjectSetTypeAvailable = "Available" ClusterObjectSetTypeProgressing = "Progressing" - ClusterObjectSetTypeSucceeded = "Succeeded" // Condition Reasons ClusterObjectSetReasonArchived = "Archived" @@ -510,9 +509,6 @@ type ClusterObjectSetStatus struct { // - When status is Unknown and reason is Archived, the ClusterObjectSet has been archived and its objects have been torn down. // - When status is Unknown and reason is Migrated, the ClusterObjectSet was migrated from an existing release and object status probe results have not yet been observed. // - // The Succeeded condition represents whether the revision has successfully completed its rollout: - // - When status is True and reason is Succeeded, the ClusterObjectSet has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. - // // +listType=map // +listMapKey=type // +optional diff --git a/applyconfigurations/api/v1/clusterobjectsetstatus.go b/applyconfigurations/api/v1/clusterobjectsetstatus.go index 380176545f..d5360f0192 100644 --- a/applyconfigurations/api/v1/clusterobjectsetstatus.go +++ b/applyconfigurations/api/v1/clusterobjectsetstatus.go @@ -47,9 +47,6 @@ type ClusterObjectSetStatusApplyConfiguration struct { // - When status is Unknown and reason is Reconciling, the ClusterObjectSet has encountered an error that prevented it from observing the probes. // - When status is Unknown and reason is Archived, the ClusterObjectSet has been archived and its objects have been torn down. // - When status is Unknown and reason is Migrated, the ClusterObjectSet was migrated from an existing release and object status probe results have not yet been observed. - // - // The Succeeded condition represents whether the revision has successfully completed its rollout: - // - When status is True and reason is Succeeded, the ClusterObjectSet has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. Conditions []metav1.ConditionApplyConfiguration `json:"conditions,omitempty"` // observedPhases records the content hashes of resolved phases // at first successful reconciliation. This is used to detect if diff --git a/applyconfigurations/api/v1/revisionstatus.go b/applyconfigurations/api/v1/revisionstatus.go index f5165c7679..049b7cfc18 100644 --- a/applyconfigurations/api/v1/revisionstatus.go +++ b/applyconfigurations/api/v1/revisionstatus.go @@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -// Code generated by controller-gen-v0.20. DO NOT EDIT. +// Code generated by controller-gen-v0.21. DO NOT EDIT. package v1 @@ -29,7 +29,7 @@ type RevisionStatusApplyConfiguration struct { // name of the ClusterObjectSet resource Name *string `json:"name,omitempty"` // conditions optionally expose Progressing and Available condition of the revision, - // in case when it is not yet marked as successfully installed (condition Succeeded is not set to True). + // in case when it is not yet marked as successfully installed (completedAt is not set). // Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions // to get more insights about reasons for its current state. Conditions []metav1.ConditionApplyConfiguration `json:"conditions,omitempty"` diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 4916ee54df..ed8d0657e4 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -561,7 +561,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `name` _string_ | name of the ClusterObjectSet resource | | | -| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions optionally expose Progressing and Available condition of the revision,
in case when it is not yet marked as successfully installed (condition Succeeded is not set to True).
Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions
to get more insights about reasons for its current state. | | Optional: \{\}
| +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions optionally expose Progressing and Available condition of the revision,
in case when it is not yet marked as successfully installed (completedAt is not set).
Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions
to get more insights about reasons for its current state. | | Optional: \{\}
| #### SelectorType diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 28d2ec68b0..d5d6e03b6a 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -518,7 +518,7 @@ spec: conditions: description: |- conditions optionally expose Progressing and Available condition of the revision, - in case when it is not yet marked as successfully installed (condition Succeeded is not set to True). + in case when it is not yet marked as successfully installed (completedAt is not set). Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions to get more insights about reasons for its current state. items: diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml index bc49544831..3ff30194b0 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml @@ -570,9 +570,6 @@ spec: - When status is Unknown and reason is Reconciling, the ClusterObjectSet has encountered an error that prevented it from observing the probes. - When status is Unknown and reason is Archived, the ClusterObjectSet has been archived and its objects have been torn down. - When status is Unknown and reason is Migrated, the ClusterObjectSet was migrated from an existing release and object status probe results have not yet been observed. - - The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is Succeeded, the ClusterObjectSet has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index b065d108ed..5862a24c91 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -252,20 +252,11 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl // Record the timestamp of the first time the revision was observed to be // ready. This is set once and never changes for subsequent reconciliations. + // It also serves as the signal that the revision has completed its rollout, + // which the ClusterExtension controller uses to determine the installed revision. if cos.Status.CompletedAt.IsZero() { cos.Status.CompletedAt = metav1.NewTime(c.Clock.Now()) } - - // We'll probably only want to remove this once we are done updating the ClusterExtension conditions - // as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now - // that's fine. - meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeSucceeded, - Status: metav1.ConditionTrue, - Reason: ocv1.ReasonSucceeded, - Message: "Revision succeeded rolling out.", - ObservedGeneration: cos.Generation, - }) } else { var probeFailureMsgs []string for _, pres := range rres.GetPhases() { diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index 718289706d..c1a4a3d5cc 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -398,7 +398,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing }, }, { - name: "set Available:True:ProbesSucceeded and Succeeded:True:Succeeded conditions on successful revision rollout", + name: "set Available:True:ProbesSucceeded condition and completedAt on successful revision rollout", revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ isComplete: true, }), @@ -428,12 +428,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing require.Equal(t, "Revision 1.0.0 has rolled out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) - cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ReasonSucceeded, cond.Reason) - require.Equal(t, "Revision succeeded rolling out.", cond.Message) - require.Equal(t, int64(1), cond.ObservedGeneration) + require.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set on successful rollout") }, }, { @@ -1137,12 +1132,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { Reason: ocv1.ReasonSucceeded, ObservedGeneration: rev1.Generation, }) - meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeSucceeded, - Status: metav1.ConditionTrue, - Reason: ocv1.ReasonSucceeded, - ObservedGeneration: rev1.Generation, - }) + rev1.Status.CompletedAt = metav1.Now() return []client.Object{rev1, ext} }, revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ diff --git a/internal/object-controller/controllers/progress_deadline.go b/internal/object-controller/controllers/progress_deadline.go index 5bce123070..cc4addfecc 100644 --- a/internal/object-controller/controllers/progress_deadline.go +++ b/internal/object-controller/controllers/progress_deadline.go @@ -7,7 +7,6 @@ import ( "sync" "time" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/client-go/util/workqueue" "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" @@ -78,17 +77,16 @@ func (r *deadlineAwareRateLimiter) NumRequeues(item ctrl.Request) int { // expires. A negative duration means the deadline has already passed. // // It derives the deadline from spec and metadata only, with one exception: -// it checks the Succeeded status condition so that a revision recovering -// from drift is not penalised by the original deadline. +// it checks status.completedAt so that a revision recovering from drift is not +// penalised by the original deadline. // -// Succeeded is a latch: there is no way to deduce from current cluster state -// alone that a COS succeeded in the past. If Succeeded is removed or set to -// False, this function will return a deadline and the reconciler will set -// ProgressDeadlineExceeded even though the revision previously succeeded. +// completedAt is a latch: there is no way to deduce from current cluster state +// alone that a COS became ready in the past. It is set once and never cleared, +// so once observed ready a revision is never subject to the deadline again. // // Returns (0, false) when there is no active deadline: // - progressDeadlineMinutes is 0 -// - the revision has already succeeded +// - the revision has already been observed ready (completedAt set) // - the revision is archived (deadline is irrelevant) // - the revision is being deleted func durationUntilDeadline(clk clock.Clock, cos *ocv1.ClusterObjectSet) (time.Duration, bool) { @@ -96,7 +94,9 @@ func durationUntilDeadline(clk clock.Clock, cos *ocv1.ClusterObjectSet) (time.Du if pd <= 0 { return 0, false } - if meta.IsStatusConditionTrue(cos.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { + // Once the revision has been observed ready (completedAt set), the deadline + // no longer applies. + if !cos.Status.CompletedAt.IsZero() { return 0, false } if cos.Spec.LifecycleState == ocv1.ClusterObjectSetLifecycleStateArchived { diff --git a/internal/object-controller/controllers/progress_deadline_test.go b/internal/object-controller/controllers/progress_deadline_test.go index 73102e9ab1..98fc10d59a 100644 --- a/internal/object-controller/controllers/progress_deadline_test.go +++ b/internal/object-controller/controllers/progress_deadline_test.go @@ -38,15 +38,12 @@ func TestDurationUntilDeadline(t *testing.T) { expectHasDeadline: false, }, { - name: "Succeeded is true — no deadline", + name: "completedAt is set — no deadline", cos: ocv1.ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, Status: ocv1.ClusterObjectSetStatus{ - Conditions: []metav1.Condition{{ - Type: ocv1.ClusterObjectSetTypeSucceeded, - Status: metav1.ConditionTrue, - }}, + CompletedAt: metav1.NewTime(creation), }, }, expectDuration: 0, diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 6b2a134d92..e0be36f2be 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -17,7 +17,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -376,8 +375,8 @@ func (m *BoxcutterStorageMigrator) ensureMigratedRevisionStatus(ctx context.Cont if revisions[i].Spec.Revision != 1 { continue } - // Skip if already succeeded - status is already set correctly. - if meta.IsStatusConditionTrue(revisions[i].Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { + // Skip if already completed - status is already set correctly. + if !revisions[i].Status.CompletedAt.IsZero() { return nil } // Ensure revision 1 status is set correctly, including for previously migrated @@ -416,8 +415,8 @@ func (m *BoxcutterStorageMigrator) findLatestDeployedRelease(ac helmclient.Actio return latestDeployed, nil } -// ensureRevisionStatus ensures the revision has the Succeeded status condition set. -// Returns nil if the status is already set or after successfully setting it. +// ensureRevisionStatus ensures the revision has completedAt set, marking it as +// installed. Returns nil if the status is already set or after successfully setting it. // Only sets status on revisions that were actually migrated from Helm (marked with MigratedFromHelmKey label). func (m *BoxcutterStorageMigrator) ensureRevisionStatus(ctx context.Context, name string) error { rev := &ocv1.ClusterObjectSet{} @@ -426,25 +425,22 @@ func (m *BoxcutterStorageMigrator) ensureRevisionStatus(ctx context.Context, nam } // Only set status if this revision was actually migrated from Helm. - // This prevents us from incorrectly marking normal Boxcutter revision 1 as succeeded + // This prevents us from incorrectly marking normal Boxcutter revision 1 as completed // when it's still in progress. if rev.Labels[labels.MigratedFromHelmKey] != "true" { return nil } - // Check if status is already set to Succeeded=True - if meta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { + // Check if completedAt is already set. + if !rev.Status.CompletedAt.IsZero() { return nil } - // Set the Succeeded status condition - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeSucceeded, - Status: metav1.ConditionTrue, - Reason: ocv1.ReasonSucceeded, - Message: "Revision succeeded - migrated from Helm release", - ObservedGeneration: rev.GetGeneration(), - }) + // Since we're migrating from a successfully deployed Helm release, the revision + // represents a working installation. Record completedAt so the ClusterExtension + // controller treats it as installed. The original ready time is not recoverable, + // so we use the migration time. + rev.Status.CompletedAt = metav1.Now() if err := m.Client.Status().Update(ctx, rev); err != nil { return fmt.Errorf("updating migrated revision status: %w", err) diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 4c435a19af..2971d402a0 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -17,7 +17,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -1196,18 +1195,13 @@ func TestBoxcutterStorageMigrator(t *testing.T) { err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) require.NoError(t, err) - // Verify the migrated revision has Succeeded=True status with Succeeded reason and a migration message + // Verify the migrated revision has completedAt set, marking it as installed require.NotNil(t, updatedObj, "Updated object should not be nil") rev, ok := updatedObj.(*ocv1.ClusterObjectSet) require.True(t, ok, "Updated object should be a ClusterObjectSet") - succeededCond := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) - require.NotNil(t, succeededCond, "Succeeded condition should be set") - assert.Equal(t, metav1.ConditionTrue, succeededCond.Status, "Succeeded condition should be True") - assert.Equal(t, ocv1.ReasonSucceeded, succeededCond.Reason, "Reason should be Succeeded") - assert.Equal(t, "Revision succeeded - migrated from Helm release", succeededCond.Message, "Message should indicate Helm migration") - assert.Equal(t, int64(1), succeededCond.ObservedGeneration, "ObservedGeneration should match revision generation") + assert.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set on migrated revision") }) t.Run("does not create revision when revisions exist", func(t *testing.T) { @@ -1245,13 +1239,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { Revision: 1, // Migration creates revision 1 }, Status: ocv1.ClusterObjectSetStatus{ - Conditions: []metav1.Condition{ - { - Type: ocv1.ClusterObjectSetTypeSucceeded, - Status: metav1.ConditionTrue, - Reason: ocv1.ReasonSucceeded, - }, - }, + CompletedAt: metav1.Now(), }, } @@ -1334,13 +1322,10 @@ func TestBoxcutterStorageMigrator(t *testing.T) { rev, ok := updatedObj.(*ocv1.ClusterObjectSet) require.True(t, ok, "Updated object should be a ClusterObjectSet") - succeededCond := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) - require.NotNil(t, succeededCond, "Succeeded condition should be set") - assert.Equal(t, metav1.ConditionTrue, succeededCond.Status, "Succeeded condition should be True") - assert.Equal(t, ocv1.ReasonSucceeded, succeededCond.Reason, "Reason should be Succeeded") + assert.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set") }) - t.Run("updates status from False to True for migrated revision", func(t *testing.T) { + t.Run("sets completedAt for migrated revision that has not completed", func(t *testing.T) { testScheme := runtime.NewScheme() require.NoError(t, ocv1.AddToScheme(testScheme)) @@ -1364,8 +1349,8 @@ func TestBoxcutterStorageMigrator(t *testing.T) { FieldOwner: "test-owner", } - // Migrated revision with Succeeded=False (e.g., from a previous failed status update attempt) - // This simulates a revision whose Succeeded condition should be corrected from False to True during migration. + // Migrated revision without completedAt (e.g., from a previous failed status update attempt). + // This simulates a revision whose completedAt should be set during migration. existingRev := ocv1.ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{ Name: "test-revision", @@ -1377,15 +1362,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { Spec: ocv1.ClusterObjectSetSpec{ Revision: 1, }, - Status: ocv1.ClusterObjectSetStatus{ - Conditions: []metav1.Condition{ - { - Type: ocv1.ClusterObjectSetTypeSucceeded, - Status: metav1.ConditionFalse, // Important: False, not missing - Reason: "InProgress", - }, - }, - }, + // completedAt is not set - simulating a revision that was migrated but never marked completed. } mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()). @@ -1412,16 +1389,13 @@ func TestBoxcutterStorageMigrator(t *testing.T) { err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) require.NoError(t, err) - // Verify the status was updated from False to True + // Verify completedAt was set require.NotNil(t, updatedObj, "Updated object should not be nil") rev, ok := updatedObj.(*ocv1.ClusterObjectSet) require.True(t, ok, "Updated object should be a ClusterObjectSet") - succeededCond := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) - require.NotNil(t, succeededCond, "Succeeded condition should be set") - assert.Equal(t, metav1.ConditionTrue, succeededCond.Status, "Succeeded condition should be updated to True") - assert.Equal(t, ocv1.ReasonSucceeded, succeededCond.Reason, "Reason should be Succeeded") + assert.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set during migration") }) t.Run("does not set status on non-migrated revision 1", func(t *testing.T) { @@ -1569,15 +1543,13 @@ func TestBoxcutterStorageMigrator(t *testing.T) { err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) require.NoError(t, err) - // Verify the migrated revision has Succeeded=True status + // Verify the migrated revision has completedAt set require.NotNil(t, updatedObj, "Updated object should not be nil") rev, ok := updatedObj.(*ocv1.ClusterObjectSet) require.True(t, ok, "Updated object should be a ClusterObjectSet") - succeededCond := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) - require.NotNil(t, succeededCond, "Succeeded condition should be set") - assert.Equal(t, metav1.ConditionTrue, succeededCond.Status, "Succeeded condition should be True") + assert.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set on migrated revision") }) t.Run("does not create revision when helm release is not deployed and no deployed history", func(t *testing.T) { diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index f340520fc7..ac2d426b27 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -76,7 +76,9 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e rm.Release = &releaseValue } - if apimeta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { + // A revision is considered installed once it has been observed ready at + // least once, recorded by status.completedAt. + if !rev.Status.CompletedAt.IsZero() { rs.Installed = rm } else { rs.RollingOut = append(rs.RollingOut, rm) diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go new file mode 100644 index 0000000000..876e914b55 --- /dev/null +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go @@ -0,0 +1,73 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/labels" +) + +func TestBoxcutterRevisionStatesGetter_ClassifiesByCompletedAt(t *testing.T) { + sch := apimachineryruntime.NewScheme() + require.NoError(t, ocv1.AddToScheme(sch)) + + const extName = "test-ext" + + newRevision := func(name string, revision int64, completed bool) *ocv1.ClusterObjectSet { + cos := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{labels.OwnerNameKey: extName}, + }, + Spec: ocv1.ClusterObjectSetSpec{Revision: revision}, + } + if completed { + cos.Status.CompletedAt = metav1.Now() + } + return cos + } + + completed := newRevision("test-ext-1", 1, true) + rollingOut := newRevision("test-ext-2", 2, false) + + cl := fake.NewClientBuilder(). + WithScheme(sch). + WithObjects(completed, rollingOut). + Build() + + getter := &BoxcutterRevisionStatesGetter{Reader: cl} + states, err := getter.GetRevisionStates(context.Background(), &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extName}, + }) + require.NoError(t, err) + + // A revision with completedAt set is Installed. + require.NotNil(t, states.Installed) + require.Equal(t, "test-ext-1", states.Installed.RevisionName) + + // A revision without completedAt is still RollingOut. + require.Len(t, states.RollingOut, 1) + require.Equal(t, "test-ext-2", states.RollingOut[0].RevisionName) +} diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index dd73d232fd..683d3969df 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1132,7 +1132,7 @@ spec: conditions: description: |- conditions optionally expose Progressing and Available condition of the revision, - in case when it is not yet marked as successfully installed (condition Succeeded is not set to True). + in case when it is not yet marked as successfully installed (completedAt is not set). Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions to get more insights about reasons for its current state. items: @@ -1926,9 +1926,6 @@ spec: - When status is Unknown and reason is Reconciling, the ClusterObjectSet has encountered an error that prevented it from observing the probes. - When status is Unknown and reason is Archived, the ClusterObjectSet has been archived and its objects have been torn down. - When status is Unknown and reason is Migrated, the ClusterObjectSet was migrated from an existing release and object status probe results have not yet been observed. - - The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is Succeeded, the ClusterObjectSet has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 1c9fef73a0..1d13fa0c87 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1093,7 +1093,7 @@ spec: conditions: description: |- conditions optionally expose Progressing and Available condition of the revision, - in case when it is not yet marked as successfully installed (condition Succeeded is not set to True). + in case when it is not yet marked as successfully installed (completedAt is not set). Given that a ClusterExtension should remain available during upgrades, an observer may use these conditions to get more insights about reasons for its current state. items: @@ -1887,9 +1887,6 @@ spec: - When status is Unknown and reason is Reconciling, the ClusterObjectSet has encountered an error that prevented it from observing the probes. - When status is Unknown and reason is Archived, the ClusterObjectSet has been archived and its objects have been torn down. - When status is Unknown and reason is Migrated, the ClusterObjectSet was migrated from an existing release and object status probe results have not yet been observed. - - The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is Succeeded, the ClusterObjectSet has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource.