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 56fe210bd9..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"
@@ -486,6 +485,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.
@@ -504,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
@@ -524,6 +526,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..d5360f0192 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.
@@ -42,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
@@ -52,6 +54,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 +91,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/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/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/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 0c94049181..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
@@ -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
@@ -560,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.
@@ -665,6 +672,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..5862a24c91 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,16 +250,13 @@ 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.")
- // 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,
- })
+ // 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())
+ }
} 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 e70fbb1c88..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")
},
},
{
@@ -511,6 +506,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"
@@ -1045,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 885d648b4a..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:
@@ -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
@@ -1916,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.
@@ -2021,6 +2028,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..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:
@@ -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
@@ -1877,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.
@@ -1982,6 +1989,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