Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/v1/clusterobjectset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ const (
ClusterObjectSetReasonArchived = "Archived"
ClusterObjectSetReasonBlocked = "Blocked"
ClusterObjectSetReasonProbeFailure = "ProbeFailure"
ClusterObjectSetReasonProbesSucceeded = "ProbesSucceeded"
ClusterObjectSetReasonRetryableError = "RetryableError"
ClusterObjectSetReasonAllObjectsReady = "AllObjectsReady"
)

// ClusterObjectSetSpec defines the desired state of ClusterObjectSet.
Expand Down Expand Up @@ -497,7 +497,7 @@ type ClusterObjectSetStatus struct {
// True means all objects are at the desired state; False means one or more
// objects are not at the desired state; Unknown is the initial state, before
// the first reconciliation has evaluated the revision.
// - True with reason ProbesSucceeded: the revision has rolled out and all objects pass their readiness probes.
// - True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
// - False with reason ProbeFailure: one or more objects are failing their readiness probes during rollout.
// - False with reason RollingOut: the revision is actively rolling out and has not yet become ready.
// - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery.
Expand Down
3 changes: 3 additions & 0 deletions api/v1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const (
// Installed reasons
ReasonAbsent = "Absent"

// Available reasons
ReasonProbesSucceeded = "ProbesSucceeded"

// Progressing reasons
ReasonRollingOut = "RollingOut"
ReasonRetrying = "Retrying"
Expand Down
2 changes: 1 addition & 1 deletion applyconfigurations/api/v1/clusterobjectsetstatus.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions cmd/object-controller/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ func TestStandaloneController(t *testing.T) {
return
}
assert.False(collect, cos.Status.CompletedAt.IsZero(), "completedAt should be set after rollout; conditions: %v", cos.Status.Conditions)
available := meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
if assert.NotNil(collect, available) {
assert.Equal(collect, metav1.ConditionTrue, available.Status)
assert.Equal(collect, ocv1.ClusterObjectSetReasonProbesSucceeded, available.Reason)
ready := meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
if assert.NotNil(collect, ready) {
assert.Equal(collect, metav1.ConditionTrue, ready.Status)
assert.Equal(collect, ocv1.ClusterObjectSetReasonAllObjectsReady, ready.Reason)
}
}, time.Minute, 100*time.Millisecond)
cm := &corev1.ConfigMap{}
Expand Down
4 changes: 2 additions & 2 deletions docs/draft/concepts/clusterobjectsets.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,11 @@ Indicates whether the revision is actively rolling out.

### Ready

Indicates whether all objects have been successfully rolled out and pass readiness probes.
Indicates whether all objects in the revision have been successfully rolled out and reached their desired state.

| Status | Reason | Meaning |
| --- | --- | --- |
| True | `ProbesSucceeded` | All objects pass readiness probes |
| True | `AllObjectsReady` | All objects are at the desired state |
| False | `ProbeFailure` | One or more probes failing |
| Unknown | `RetryableError` | Error prevented probe observation |
| Unknown | `Archived` | Objects torn down after archival |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ spec:
True means all objects are at the desired state; False means one or more
objects are not at the desired state; Unknown is the initial state, before
the first reconciliation has evaluated the revision.
- True with reason ProbesSucceeded: the revision has rolled out and all objects pass their readiness probes.
- True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
- False with reason ProbeFailure: one or more objects are failing their readiness probes during rollout.
- False with reason RollingOut: the revision is actively rolling out and has not yet become ready.
- False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
}
}

markAsReady(cos, ocv1.ClusterObjectSetReasonProbesSucceeded, "Objects are available and pass all probes.")
markAsReady(cos, ocv1.ClusterObjectSetReasonAllObjectsReady, "All objects are at the desired state")

// Record the timestamp of the first time the revision was observed to be
// ready. This is set once and never changes for subsequent reconciliations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectSetReasonProbesSucceeded,
Reason: ocv1.ClusterObjectSetReasonAllObjectsReady,
Message: "Revision 1 is rolled out.",
ObservedGeneration: 1,
})
Expand Down Expand Up @@ -371,7 +371,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
},
},
{
name: "set Ready:True:ProbesSucceeded condition and completedAt on successful revision rollout",
name: "set Ready:True:AllObjectsReady condition and completedAt on successful revision rollout",
revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{
isComplete: true,
}),
Expand All @@ -390,8 +390,8 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionTrue, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonProbesSucceeded, cond.Reason)
require.Equal(t, "Objects are available and pass all probes.", cond.Message)
require.Equal(t, ocv1.ClusterObjectSetReasonAllObjectsReady, cond.Reason)
require.Equal(t, "All objects are at the desired state", cond.Message)
require.Equal(t, int64(1), cond.ObservedGeneration)

require.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set on successful rollout")
Expand Down Expand Up @@ -1102,7 +1102,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
},
},
{
name: "recovery from ProgressDeadlineExceeded to ProbesSucceeded when revision completes",
name: "recovery from ProgressDeadlineExceeded to AllObjectsReady when revision completes",
existingObjs: func() []client.Object {
ext := newTestClusterExtension()
rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme)
Expand Down Expand Up @@ -1130,8 +1130,8 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cnd)
require.Equal(t, metav1.ConditionTrue, cnd.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonProbesSucceeded, cnd.Reason)
require.Equal(t, "Objects are available and pass all probes.", cnd.Message)
require.Equal(t, ocv1.ClusterObjectSetReasonAllObjectsReady, cnd.Reason)
require.Equal(t, "All objects are at the desired state", cnd.Message)
},
},
{
Expand All @@ -1144,7 +1144,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectSetReasonProbesSucceeded,
Reason: ocv1.ClusterObjectSetReasonAllObjectsReady,
ObservedGeneration: rev1.Generation,
})
rev1.Status.CompletedAt = metav1.Now()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@ var ConditionReasons = []string{
ocv1.ReasonAbsent,
ocv1.ReasonRollingOut,
ocv1.ReasonProgressDeadlineExceeded,
ocv1.ReasonProbesSucceeded,
}
31 changes: 22 additions & 9 deletions internal/operator-controller/controllers/common_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,25 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image))
}

// availableTrueMessage is the message surfaced on the ClusterExtension Available condition when the
// active revision is Ready. It is intentionally decoupled from the ClusterObjectSet Ready condition's
// own message.
const availableTrueMessage = "Objects are available and pass all probes."

// availableFromReady maps a ClusterObjectSet Ready condition onto a ClusterExtension Available condition.
// Non-ready states are surfaced as-is (only the Type is rewritten to Available). The ready (True) state is

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the long-term path? Or should ClusterExtension itself have its own contract of condition types and reasons, separate from the underlying COD/COS?

It seems like it should have a contract one way or the other, even if that contract is explicitly "matches COD/COS exactly as of 6 Oct 2026". That way we don't ever accidentally leak something into the CE API without an explicit decision about it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely of the camp that that CE should have its own set of exposed condition types and reasons. I think we should either address this in parallel or in tandem with the ClusterObjectDeployment integration.

// re-emitted with a ClusterExtension-owned reason and message.
func availableFromReady(ready metav1.Condition, generation int64) metav1.Condition {
a := ready
a.Type = ocv1.TypeAvailable
a.ObservedGeneration = generation
if a.Status == metav1.ConditionTrue {
a.Reason = ocv1.ReasonProbesSucceeded
a.Message = availableTrueMessage
}
return a
}

// setActiveRevisionsFromRevisionStates derives the active revisions for the ClusterExtension status
func setActiveRevisionsFromRevisionStates(ext *ocv1.ClusterExtension, revisionStates *RevisionStates) {
ext.Status.ActiveRevisions = make([]ocv1.RevisionStatus, 0, 1+len(revisionStates.RollingOut))
Expand All @@ -79,10 +98,7 @@ func setActiveRevisionsFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
rs := ocv1.RevisionStatus{Name: r.RevisionName}
ready := apimeta.FindStatusCondition(r.Conditions, ocv1.ClusterObjectSetTypeReady)
if ready != nil {
a := *ready
a.Type = ocv1.TypeAvailable
a.ObservedGeneration = ext.GetGeneration()
apimeta.SetStatusCondition(&rs.Conditions, a)
apimeta.SetStatusCondition(&rs.Conditions, availableFromReady(*ready, ext.GetGeneration()))
}
ext.Status.ActiveRevisions = append(ext.Status.ActiveRevisions, rs)
}
Expand All @@ -93,10 +109,7 @@ func setAvailableFromRevisionStates(ext *ocv1.ClusterExtension, revisionStates *
if i := revisionStates.Installed; i != nil {
ready := apimeta.FindStatusCondition(i.Conditions, ocv1.ClusterObjectSetTypeReady)
if ready != nil {
a := *ready
a.Type = ocv1.TypeAvailable
a.ObservedGeneration = ext.GetGeneration()
apimeta.SetStatusCondition(&ext.Status.Conditions, a)
apimeta.SetStatusCondition(&ext.Status.Conditions, availableFromReady(*ready, ext.GetGeneration()))
}
}
}
Expand Down Expand Up @@ -261,7 +274,7 @@ func progressingFromReady(ready *metav1.Condition, completed bool) metav1.Condit
case ocv1.ClusterObjectSetReasonRetryableError:
cond.Reason = ocv1.ReasonRetrying
default:
// ProbeFailure, RollingOut, or ProbesSucceeded-but-not-yet-complete.
// ProbeFailure, RollingOut, or AllObjectsReady-but-not-yet-complete.
cond.Reason = ocv1.ReasonRollingOut
}
return cond
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ func TestProgressingFromReady(t *testing.T) {
}{
{
name: "completed revision maps to Succeeded",
ready: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionTrue, Reason: ocv1.ClusterObjectSetReasonProbesSucceeded, Message: "ok"},
ready: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionTrue, Reason: ocv1.ClusterObjectSetReasonAllObjectsReady, Message: "ok"},
completed: true,
expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonSucceeded, Message: "Desired state reached"},
},
Expand Down Expand Up @@ -558,22 +558,24 @@ func TestSetAvailableFromRevisionStates(t *testing.T) {
expectAvailable *metav1.Condition
}{
{
name: "installed revision with Ready=True is remapped to Available=True",
name: "installed revision with Ready=True is remapped to Available=True with a decoupled reason and message",
revisionStates: &RevisionStates{
Installed: &RevisionMetadata{
RevisionName: "rev-1",
Conditions: []metav1.Condition{{
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectSetReasonProbesSucceeded,
Message: "Revision 1 is rolled out.",
Reason: ocv1.ClusterObjectSetReasonAllObjectsReady,
Message: "All objects are at the desired state",
}},
},
},
// The ClusterExtension Available condition is deliberately decoupled from the
// ClusterObjectSet Ready condition's reason/message for the ready (True) state.
expectAvailable: &metav1.Condition{
Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectSetReasonProbesSucceeded,
Message: "Revision 1 is rolled out.",
Reason: ocv1.ReasonProbesSucceeded,
Message: "Objects are available and pass all probes.",
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion manifests/experimental-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1914,7 +1914,7 @@ spec:
True means all objects are at the desired state; False means one or more
objects are not at the desired state; Unknown is the initial state, before
the first reconciliation has evaluated the revision.
- True with reason ProbesSucceeded: the revision has rolled out and all objects pass their readiness probes.
- True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
- False with reason ProbeFailure: one or more objects are failing their readiness probes during rollout.
- False with reason RollingOut: the revision is actively rolling out and has not yet become ready.
- False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery.
Expand Down
2 changes: 1 addition & 1 deletion manifests/experimental.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1875,7 +1875,7 @@ spec:
True means all objects are at the desired state; False means one or more
objects are not at the desired state; Unknown is the initial state, before
the first reconciliation has evaluated the revision.
- True with reason ProbesSucceeded: the revision has rolled out and all objects pass their readiness probes.
- True with reason AllObjectsReady: the revision has rolled out and all objects are at the desired state.
- False with reason ProbeFailure: one or more objects are failing their readiness probes during rollout.
- False with reason RollingOut: the revision is actively rolling out and has not yet become ready.
- False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery.
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/features/revision.feature
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ Feature: Install ClusterObjectSet
revision: 1
"""

Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason ProbesSucceeded
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady
And resource "persistentvolume/test-pv" is installed
And resource "persistentvolumeclaim/test-pvc" is installed
And resource "configmap/test-configmap" is installed
Expand Down Expand Up @@ -322,7 +322,7 @@ Feature: Install ClusterObjectSet
And resource "serviceaccount/test-serviceaccount" is installed
And resource "pod/test-pod" is installed
And resource "configmap/test-configmap-3" is installed
And ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason ProbesSucceeded
And ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady

Scenario: User can install a ClusterObjectSet with objects stored in Secrets
Given namespace "${TEST_NAMESPACE}" is available
Expand Down Expand Up @@ -418,7 +418,7 @@ Feature: Install ClusterObjectSet
key: deployment
revision: 1
"""
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason ProbesSucceeded
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady
And resource "configmap/test-configmap-ref" is installed
And resource "deployment/test-httpd" is installed
And ClusterObjectSet "${COS_NAME}" has observed phase "resources" with a non-empty digest
Expand Down Expand Up @@ -513,7 +513,7 @@ Feature: Install ClusterObjectSet
key: configmap
revision: 1
"""
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason ProbesSucceeded
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady
And ClusterObjectSet "${COS_NAME}" has observed phase "resources" with a non-empty digest
# Delete the immutable Secret and recreate with different content
When resource "secret/${COS_NAME}-change-secret" is removed
Expand Down Expand Up @@ -571,7 +571,7 @@ Feature: Install ClusterObjectSet
}
"""
And ClusterObjectSet "${COS_NAME}" reconciliation is triggered
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason ProbesSucceeded
Then ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady


@ProgressDeadline
Expand Down Expand Up @@ -703,4 +703,4 @@ Feature: Install ClusterObjectSet
revision: 1
"""
Then ClusterObjectSet "${COS_NAME}" reports Ready as False with Reason ProgressDeadlineExceeded
And ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason ProbesSucceeded
And ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady
2 changes: 1 addition & 1 deletion test/e2e/features/status.feature
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ Feature: Report status of the managed ClusterExtension workload
And ClusterObjectSet "${NAME}-1" reports Ready as False with Reason ProbeFailure
When deployment "test-operator-${SCENARIO_ID}" reports as ready
Then ClusterExtension is available
And ClusterObjectSet "${NAME}-1" reports Ready as True with Reason ProbesSucceeded
And ClusterObjectSet "${NAME}-1" reports Ready as True with Reason AllObjectsReady
2 changes: 1 addition & 1 deletion test/e2e/features/update.feature
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ Feature: Update ClusterExtension
And ClusterExtension is rolled out
And ClusterExtension is available
And ClusterExtension reports "${NAME}-2" as active revision
And ClusterObjectSet "${NAME}-2" reports Ready as True with Reason ProbesSucceeded
And ClusterObjectSet "${NAME}-2" reports Ready as True with Reason AllObjectsReady
And ClusterObjectSet "${NAME}-1" is archived
And ClusterObjectSet "${NAME}-1" phase objects are not found or not owned by the revision

Expand Down
Loading