Skip to content

⚠️ Rename ClusterObjectSet Ready transient-error reason to RetryableError #2981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Oct 6, 2026
Merged

⚠️ Rename ClusterObjectSet Ready transient-error reason to RetryableError #2981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions api/v1/clusterobjectset_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ const (
ClusterObjectSetReasonBlocked = "Blocked"
ClusterObjectSetReasonProbeFailure = "ProbeFailure"
ClusterObjectSetReasonProbesSucceeded = "ProbesSucceeded"
ClusterObjectSetReasonReconciling = "Reconciling"
ClusterObjectSetReasonRetrying = "Retrying"
ClusterObjectSetReasonRetryableError = "RetryableError"
)

// ClusterObjectSetSpec defines the desired state of ClusterObjectSet.
Expand Down
2 changes: 1 addition & 1 deletion docs/draft/concepts/clusterobjectsets.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Indicates whether all objects have been successfully rolled out and pass readine
| --- | --- | --- |
| True | `ProbesSucceeded` | All objects pass readiness probes |
| False | `ProbeFailure` | One or more probes failing |
| Unknown | `Reconciling` | Error prevented probe observation |
| Unknown | `RetryableError` | Error prevented probe observation |
Comment thread
perdasilva marked this conversation as resolved.
| Unknown | `Archived` | Objects torn down after archival |
| Unknown | `Migrated` | Migrated from existing release; probes not yet observed |

Expand Down
30 changes: 15 additions & 15 deletions internal/object-controller/controllers/clusterobjectset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl

phases, currentPhases, opts, err := c.buildBoxcutterPhases(ctx, cos)
if err != nil {
setRetryingConditions(cos, err.Error(), isDeadlineExceeded)
setRetryableErrorConditions(cos, err.Error(), isDeadlineExceeded)
return ctrl.Result{}, fmt.Errorf("converting to boxcutter revision: %v", err)
}

Expand All @@ -154,7 +154,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl

revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, cos)
if err != nil {
setRetryingConditions(cos, err.Error(), isDeadlineExceeded)
setRetryableErrorConditions(cos, err.Error(), isDeadlineExceeded)
return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err)
}

Expand All @@ -168,7 +168,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl

if cos.Spec.LifecycleState == ocv1.ClusterObjectSetLifecycleStateArchived {
if err := c.TrackingCache.Free(ctx, cos); err != nil {
markAsNotReady(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error())
markAsNotReady(cos, ocv1.ClusterObjectSetReasonRetryableError, err.Error())
return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err)
}
return c.archive(ctx, revisionEngine, cos, revision)
Expand All @@ -180,7 +180,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl

if err := c.establishWatch(ctx, cos, revision); err != nil {
werr := fmt.Errorf("establish watch: %v", err)
setRetryingConditions(cos, werr.Error(), isDeadlineExceeded)
setRetryableErrorConditions(cos, werr.Error(), isDeadlineExceeded)
return ctrl.Result{}, werr
}

Expand All @@ -190,22 +190,22 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl
// Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity.
l.V(1).Info("reconcile report", "report", rres.String())
}
setRetryingConditions(cos, err.Error(), isDeadlineExceeded)
setRetryableErrorConditions(cos, err.Error(), isDeadlineExceeded)
return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err)
}

// Retry failing preflight checks with a flat 10s retry.
// TODO: report status, backoff?
if verr := rres.GetValidationError(); verr != nil {
l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s")
setRetryingConditions(cos, fmt.Sprintf("revision validation error: %s", verr), isDeadlineExceeded)
setRetryableErrorConditions(cos, fmt.Sprintf("revision validation error: %s", verr), isDeadlineExceeded)
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}

for i, pres := range rres.GetPhases() {
if verr := pres.GetValidationError(); verr != nil {
l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", i)
setRetryingConditions(cos, fmt.Sprintf("phase %d validation error: %s", i, verr), isDeadlineExceeded)
setRetryableErrorConditions(cos, fmt.Sprintf("phase %d validation error: %s", i, verr), isDeadlineExceeded)
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}

Expand All @@ -218,7 +218,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl

if len(collidingObjs) > 0 {
l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs)
setRetryingConditions(cos, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), isDeadlineExceeded)
setRetryableErrorConditions(cos, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), isDeadlineExceeded)
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
}
Expand Down Expand Up @@ -307,7 +307,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl

func (c *ClusterObjectSetReconciler) delete(ctx context.Context, cos *ocv1.ClusterObjectSet) (ctrl.Result, error) {
if err := c.TrackingCache.Free(ctx, cos); err != nil {
markAsNotReady(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error())
markAsNotReady(cos, ocv1.ClusterObjectSetReasonRetryableError, err.Error())
return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err)
}
if err := c.removeFinalizer(ctx, cos, clusterObjectSetTeardownFinalizer); err != nil {
Expand All @@ -320,11 +320,11 @@ func (c *ClusterObjectSetReconciler) archive(ctx context.Context, revisionEngine
tdres, err := revisionEngine.Teardown(ctx, revision, machinerytypes.WithObserveAfterIncomplete{})
if err != nil {
err = fmt.Errorf("error archiving revision: %v", err)
setRetryingConditions(cos, err.Error(), false)
setRetryableErrorConditions(cos, err.Error(), false)
return ctrl.Result{}, err
}
if tdres != nil && !tdres.IsComplete() {
setRetryingConditions(cos, "removing revision resources that are not owned by another revision", false)
setRetryableErrorConditions(cos, "removing revision resources that are not owned by another revision", false)
return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
}
// Ensure conditions are set before removing the finalizer when archiving
Expand Down Expand Up @@ -657,8 +657,8 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing.

// setReadyWithDeadline sets the Ready condition to the supplied status/reason/message,
// unless the progress deadline has been exceeded — in which case it reports
// Ready=False/ProgressDeadlineExceeded instead. This centralises the deadline
// enforcement that previously lived in markAsProgressing.
// Ready=False/ProgressDeadlineExceeded instead. This centralises the progress
// deadline enforcement for the Ready condition.
func setReadyWithDeadline(cos *ocv1.ClusterObjectSet, status metav1.ConditionStatus, reason, message string, isDeadlineExceeded bool) { // nolint:unparam
if isDeadlineExceeded {
markAsNotReady(cos, ocv1.ReasonProgressDeadlineExceeded,
Expand All @@ -674,8 +674,8 @@ func setReadyWithDeadline(cos *ocv1.ClusterObjectSet, status metav1.ConditionSta
})
}

func setRetryingConditions(cos *ocv1.ClusterObjectSet, message string, isDeadlineExceeded bool) {
setReadyWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonReconciling, message, isDeadlineExceeded)
func setRetryableErrorConditions(cos *ocv1.ClusterObjectSet, message string, isDeadlineExceeded bool) {
setReadyWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonRetryableError, message, isDeadlineExceeded)
}

func markAsReady(cos *ocv1.ClusterObjectSet, reason, message string) bool {
Expand Down
28 changes: 14 additions & 14 deletions internal/object-controller/controllers/clusterobjectset_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
},
},
{
name: "Ready condition is set to False/Reconciling on error when not previously set",
name: "Ready condition is set to False/RetryableError on error when not previously set",
reconcilingRevisionName: clusterObjectSetName,
revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}),
revisionReconcileErr: errors.New("some error"),
Expand All @@ -86,13 +86,13 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Equal(t, "some error", cond.Message)
require.Equal(t, int64(1), cond.ObservedGeneration)
},
},
{
name: "Ready condition is set to False/Reconciling on error when previously set",
name: "Ready condition is set to False/RetryableError on error when previously set",
reconcilingRevisionName: clusterObjectSetName,
revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}),
revisionReconcileErr: errors.New("some error"),
Expand All @@ -117,15 +117,15 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Equal(t, "some error", cond.Message)
require.Equal(t, int64(1), cond.ObservedGeneration)
},
},
{
// A revision whose FIRST reconcile fails at engine creation (factory error)
// must set Ready=False/Reconciling even though Ready did not previously exist.
name: "Ready condition is set to False/Reconciling on factory error when not previously set",
// must set Ready=False/RetryableError even though Ready did not previously exist.
name: "Ready condition is set to False/RetryableError on factory error when not previously set",
reconcilingRevisionName: clusterObjectSetName,
factoryErr: errors.New("failed to create revision engine"),
existingObjs: func() []client.Object {
Expand All @@ -142,7 +142,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Nil(t, meta.FindStatusCondition(rev.Status.Conditions, "Progressing"))
},
},
Expand Down Expand Up @@ -729,7 +729,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
},
},
{
name: "set Ready:False:Reconciling when tracking cache free fails during deletion",
name: "set Ready:False:RetryableError when tracking cache free fails during deletion",
revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}),
existingObjs: func() []client.Object {
ext := newTestClusterExtension()
Expand All @@ -753,7 +753,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Contains(t, cond.Message, "tracking cache free failed")
},
revisionEngineTeardownFn: func(ctrl *gomock.Controller) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) {
Expand Down Expand Up @@ -794,7 +794,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
},
},
{
name: "set Ready:False:Reconciling and requeue when archived revision archival is incomplete",
name: "set Ready:False:RetryableError and requeue when archived revision archival is incomplete",
revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}),
existingObjs: func() []client.Object {
ext := newTestClusterExtension()
Expand Down Expand Up @@ -822,7 +822,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Equal(t, "removing revision resources that are not owned by another revision", cond.Message)

// Finalizer should still be present
Expand Down Expand Up @@ -856,7 +856,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Contains(t, cond.Message, "teardown failed: connection refused")

// Finalizer should still be present
Expand Down Expand Up @@ -889,7 +889,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Contains(t, cond.Message, "token getter failed")

// Finalizer should still be present
Expand Down Expand Up @@ -1626,7 +1626,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
require.Equal(t, ocv1.ClusterObjectSetReasonRetryableError, cond.Reason)
require.Contains(t, cond.Message, "revision object collisions")
} else {
require.Equal(t, ctrl.Result{}, result)
Expand Down
12 changes: 6 additions & 6 deletions internal/operator-controller/controllers/common_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,15 @@ func setProgressingFromReady(ext *ocv1.ClusterExtension, readyCond *metav1.Condi
//
// Returns Failed when:
// - No rolling revisions exist (nothing to install)
// - The latest rolling revision has Ready condition with Reason: Reconciling (indicates an error occurred)
// - The latest rolling revision has Ready condition with Reason: RetryableError (indicates an error occurred)
//
// Returns Absent when:
// - Rolling revisions exist with the latest not having Ready=Reconciling (healthy phased rollout in progress)
// - Rolling revisions exist with the latest not having Ready=RetryableError (healthy phased rollout in progress)
//
// Rationale:
// - Failed: Semantically indicates an error prevented installation
// - Absent: Semantically indicates "not there yet" (neutral state, e.g., during healthy rollout)
// - Reconciling reason on Ready indicates an error (config validation, apply failure, etc.)
// - RetryableError reason on Ready indicates an error (config validation, apply failure, etc.)
// - Other Ready reasons indicate healthy progress or terminal states handled elsewhere
// - Only the LATEST revision matters - old errors superseded by newer healthy revisions should not cause Failed
//
Expand All @@ -152,8 +152,8 @@ func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
// Latest revision is the last element (sorted ascending by Spec.Revision).
latestRevision := rollingRevisions[len(rollingRevisions)-1]
readyCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterObjectSetTypeReady)
// Reconciling is the new home of the old Retrying signal: it indicates an error occurred.
if readyCond != nil && readyCond.Reason == ocv1.ClusterObjectSetReasonReconciling {
// RetryableError on the Ready condition indicates a transient error occurred.
if readyCond != nil && readyCond.Reason == ocv1.ClusterObjectSetReasonRetryableError {
return ocv1.ReasonFailed
}

Expand Down Expand Up @@ -258,7 +258,7 @@ func progressingFromReady(ready *metav1.Condition, completed bool) metav1.Condit
case ocv1.ReasonProgressDeadlineExceeded:
cond.Status = metav1.ConditionFalse
cond.Reason = ocv1.ReasonProgressDeadlineExceeded
case ocv1.ClusterObjectSetReasonReconciling:
case ocv1.ClusterObjectSetReasonRetryableError:
cond.Reason = ocv1.ReasonRetrying
default:
// ProbeFailure, RollingOut, or ProbesSucceeded-but-not-yet-complete.
Expand Down
Loading
Loading