diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index d9c244749f..4f2c7392a7 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -33,8 +33,7 @@ const ( ClusterObjectSetReasonBlocked = "Blocked" ClusterObjectSetReasonProbeFailure = "ProbeFailure" ClusterObjectSetReasonProbesSucceeded = "ProbesSucceeded" - ClusterObjectSetReasonReconciling = "Reconciling" - ClusterObjectSetReasonRetrying = "Retrying" + ClusterObjectSetReasonRetryableError = "RetryableError" ) // ClusterObjectSetSpec defines the desired state of ClusterObjectSet. diff --git a/docs/draft/concepts/clusterobjectsets.md b/docs/draft/concepts/clusterobjectsets.md index bfd5f10b02..7b085ed432 100644 --- a/docs/draft/concepts/clusterobjectsets.md +++ b/docs/draft/concepts/clusterobjectsets.md @@ -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 | | Unknown | `Archived` | Objects torn down after archival | | Unknown | `Migrated` | Migrated from existing release; probes not yet observed | diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index 2959cdf005..08add6158c 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -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) } @@ -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) } @@ -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) @@ -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 } @@ -190,7 +190,7 @@ 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) } @@ -198,14 +198,14 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl // 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 } @@ -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 } } @@ -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 { @@ -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 @@ -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, @@ -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 { diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index 6ccf0bdcc3..47dd3274c7 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -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"), @@ -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"), @@ -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 { @@ -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")) }, }, @@ -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() @@ -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) { @@ -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() @@ -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 @@ -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 @@ -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 @@ -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) diff --git a/internal/operator-controller/controllers/common_controller.go b/internal/operator-controller/controllers/common_controller.go index 7f903725c2..1e20310c0d 100644 --- a/internal/operator-controller/controllers/common_controller.go +++ b/internal/operator-controller/controllers/common_controller.go @@ -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 // @@ -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 } @@ -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. diff --git a/internal/operator-controller/controllers/common_controller_test.go b/internal/operator-controller/controllers/common_controller_test.go index 83fc79ae75..180cb23641 100644 --- a/internal/operator-controller/controllers/common_controller_test.go +++ b/internal/operator-controller/controllers/common_controller_test.go @@ -317,7 +317,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, { - name: "rolling revision with error (Reconciling) - uses Failed", + name: "rolling revision with error (RetryableError) - uses Failed", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -327,7 +327,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T { Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionUnknown, - Reason: ocv1.ClusterObjectSetReasonReconciling, + Reason: ocv1.ClusterObjectSetReasonRetryableError, Message: "some error occurred", }, }, @@ -341,7 +341,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, { - name: "multiple rolling revisions with one Reconciling - uses Failed", + name: "multiple rolling revisions with one RetryableError - uses Failed", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -362,7 +362,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T { Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionUnknown, - Reason: ocv1.ClusterObjectSetReasonReconciling, + Reason: ocv1.ClusterObjectSetReasonRetryableError, Message: "validation error occurred", }, }, @@ -400,7 +400,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, { - name: "old revision with Reconciling superseded by latest healthy - uses Absent", + name: "old revision with RetryableError superseded by latest healthy - uses Absent", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -410,7 +410,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T { Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionUnknown, - Reason: ocv1.ClusterObjectSetReasonReconciling, + Reason: ocv1.ClusterObjectSetReasonRetryableError, Message: "old error that was superseded", }, }, @@ -466,8 +466,8 @@ func TestDetermineFailureReason(t *testing.T) { }{ {name: "no rolling revisions -> Failed", rolling: nil, expected: ocv1.ReasonFailed}, { - name: "latest reconciling -> Failed", - rolling: []*RevisionMetadata{{Conditions: availCond(metav1.ConditionUnknown, ocv1.ClusterObjectSetReasonReconciling)}}, + name: "latest retryable error -> Failed", + rolling: []*RevisionMetadata{{Conditions: availCond(metav1.ConditionUnknown, ocv1.ClusterObjectSetReasonRetryableError)}}, expected: ocv1.ReasonFailed, }, { @@ -483,7 +483,7 @@ func TestDetermineFailureReason(t *testing.T) { { name: "only the latest revision matters", rolling: []*RevisionMetadata{ - {Conditions: availCond(metav1.ConditionUnknown, ocv1.ClusterObjectSetReasonReconciling)}, + {Conditions: availCond(metav1.ConditionUnknown, ocv1.ClusterObjectSetReasonRetryableError)}, {Conditions: availCond(metav1.ConditionFalse, ocv1.ReasonRollingOut)}, }, expected: ocv1.ReasonAbsent, @@ -521,8 +521,8 @@ func TestProgressingFromReady(t *testing.T) { expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRollingOut, Message: "probe failed"}, }, { - name: "reconciling maps to Retrying", - ready: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionUnknown, Reason: ocv1.ClusterObjectSetReasonReconciling, Message: "boom"}, + name: "retryable error maps to Retrying", + ready: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeReady, Status: metav1.ConditionUnknown, Reason: ocv1.ClusterObjectSetReasonRetryableError, Message: "boom"}, completed: false, expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRetrying, Message: "boom"}, },