diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index d37e6f9c49..fe6ad76fd0 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -26,8 +26,7 @@ const ( ClusterObjectSetKind = "ClusterObjectSet" // Condition Types - ClusterObjectSetTypeAvailable = "Available" - ClusterObjectSetTypeProgressing = "Progressing" + ClusterObjectSetTypeAvailable = "Available" // Condition Reasons ClusterObjectSetReasonArchived = "Archived" @@ -495,19 +494,19 @@ type ClusterObjectSetStatus struct { // conditions is an optional list of status conditions describing the state of the // ClusterObjectSet. // - // The Progressing condition represents whether the revision is actively rolling out: - // - When status is True and reason is RollingOut, the ClusterObjectSet rollout is actively making progress and is in transition. - // - When status is True and reason is Retrying, the ClusterObjectSet has encountered an error that could be resolved on subsequent reconciliation attempts. - // - When status is True and reason is Succeeded, the ClusterObjectSet has reached the desired state. - // - When status is False and reason is Blocked, the ClusterObjectSet has encountered an error that requires manual intervention for recovery. - // - When status is False and reason is Archived, the ClusterObjectSet is archived and not being actively reconciled. + // The Available condition represents the state of the revision. + // 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. + // - 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 available. + // - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery. + // - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. + // - False with reason Reconciling: the revision encountered an error that prevented it from observing the probes. + // - False with reason Archived: the revision has been archived and its objects have been torn down. // - // The Available condition represents whether the revision has been successfully rolled out and is available: - // - When status is True and reason is ProbesSucceeded, the ClusterObjectSet has been successfully rolled out and all objects pass their readiness probes. - // - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - // - 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. + // Rollout completion is recorded separately by status.completedAt. // // +listType=map // +listMapKey=type @@ -562,7 +561,6 @@ type ObservedPhase struct { // +kubebuilder:resource:scope=Cluster // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=='Available')].status` -// +kubebuilder:printcolumn:name="Progressing",type=string,JSONPath=`.status.conditions[?(@.type=='Progressing')].status` // +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp` // ClusterObjectSet represents an immutable snapshot of Kubernetes objects diff --git a/applyconfigurations/api/v1/clusterobjectsetstatus.go b/applyconfigurations/api/v1/clusterobjectsetstatus.go index d5360f0192..51a13c04ab 100644 --- a/applyconfigurations/api/v1/clusterobjectsetstatus.go +++ b/applyconfigurations/api/v1/clusterobjectsetstatus.go @@ -34,19 +34,19 @@ type ClusterObjectSetStatusApplyConfiguration struct { // conditions is an optional list of status conditions describing the state of the // ClusterObjectSet. // - // The Progressing condition represents whether the revision is actively rolling out: - // - When status is True and reason is RollingOut, the ClusterObjectSet rollout is actively making progress and is in transition. - // - When status is True and reason is Retrying, the ClusterObjectSet has encountered an error that could be resolved on subsequent reconciliation attempts. - // - When status is True and reason is Succeeded, the ClusterObjectSet has reached the desired state. - // - When status is False and reason is Blocked, the ClusterObjectSet has encountered an error that requires manual intervention for recovery. - // - When status is False and reason is Archived, the ClusterObjectSet is archived and not being actively reconciled. + // The Available condition represents the state of the revision. + // 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. + // - 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 available. + // - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery. + // - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. + // - False with reason Reconciling: the revision encountered an error that prevented it from observing the probes. + // - False with reason Archived: the revision has been archived and its objects have been torn down. // - // The Available condition represents whether the revision has been successfully rolled out and is available: - // - When status is True and reason is ProbesSucceeded, the ClusterObjectSet has been successfully rolled out and all objects pass their readiness probes. - // - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - // - 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. + // Rollout completion is recorded separately by status.completedAt. Conditions []metav1.ConditionApplyConfiguration `json:"conditions,omitempty"` // observedPhases records the content hashes of resolved phases // at first successful reconciliation. This is used to detect if diff --git a/cmd/object-controller/main_test.go b/cmd/object-controller/main_test.go index c5d8bb6b3c..1466ff6c50 100644 --- a/cmd/object-controller/main_test.go +++ b/cmd/object-controller/main_test.go @@ -132,10 +132,6 @@ func TestStandaloneController(t *testing.T) { assert.Equal(collect, metav1.ConditionTrue, available.Status) assert.Equal(collect, ocv1.ClusterObjectSetReasonProbesSucceeded, available.Reason) } - progressing := meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) - if assert.NotNil(collect, progressing) { - assert.Equal(collect, ocv1.ReasonSucceeded, progressing.Reason) - } }, time.Minute, 100*time.Millisecond) cm := &corev1.ConfigMap{} require.NoError(t, cl.Get(ctx, client.ObjectKey{Name: name, Namespace: ns.Name}, cm)) 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 3ff30194b0..58eae5b035 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 @@ -19,9 +19,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Available')].status name: Available type: string - - jsonPath: .status.conditions[?(@.type=='Progressing')].status - name: Progressing - type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -557,19 +554,19 @@ spec: conditions is an optional list of status conditions describing the state of the ClusterObjectSet. - The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is RollingOut, the ClusterObjectSet rollout is actively making progress and is in transition. - - When status is True and reason is Retrying, the ClusterObjectSet has encountered an error that could be resolved on subsequent reconciliation attempts. - - When status is True and reason is Succeeded, the ClusterObjectSet has reached the desired state. - - When status is False and reason is Blocked, the ClusterObjectSet has encountered an error that requires manual intervention for recovery. - - When status is False and reason is Archived, the ClusterObjectSet is archived and not being actively reconciled. - - The Available condition represents whether the revision has been successfully rolled out and is available: - - When status is True and reason is ProbesSucceeded, the ClusterObjectSet has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - - 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 Available condition represents the state of the revision. + 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. + - 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 available. + - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery. + - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. + - False with reason Reconciling: the revision encountered an error that prevented it from observing the probes. + - False with reason Archived: the revision has been archived and its objects have been torn down. + + Rollout completion is recorded separately by status.completedAt. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index 9e6d4b15a8..a632e4aa9c 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -14,7 +14,6 @@ import ( "strings" "time" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -135,13 +134,13 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl // Blocked takes precedence over ProgressDeadlineExceeded: it is more actionable for the user. if err := c.verifyReferencedSecretsImmutable(ctx, cos); err != nil { l.Error(err, "referenced Secret verification failed, blocking reconciliation") - markAsNotProgressing(cos, ocv1.ClusterObjectSetReasonBlocked, err.Error()) + markAsUnavailable(cos, ocv1.ClusterObjectSetReasonBlocked, err.Error()) return ctrl.Result{}, nil } phases, currentPhases, opts, err := c.buildBoxcutterPhases(ctx, cos) if err != nil { - setRetryingConditions(l, cos, err.Error(), isDeadlineExceeded) + setRetryingConditions(cos, err.Error(), isDeadlineExceeded) return ctrl.Result{}, fmt.Errorf("converting to boxcutter revision: %v", err) } @@ -149,13 +148,13 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl cos.Status.ObservedPhases = currentPhases } else if err := verifyObservedPhases(cos.Status.ObservedPhases, currentPhases); err != nil { l.Error(err, "resolved phases content changed, blocking reconciliation") - markAsNotProgressing(cos, ocv1.ClusterObjectSetReasonBlocked, err.Error()) + markAsUnavailable(cos, ocv1.ClusterObjectSetReasonBlocked, err.Error()) return ctrl.Result{}, nil } revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, cos) if err != nil { - setRetryingConditions(l, cos, err.Error(), isDeadlineExceeded) + setRetryingConditions(cos, err.Error(), isDeadlineExceeded) return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err) } @@ -169,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 { - markAsAvailableUnknown(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error()) + markAsUnavailable(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error()) return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err) } return c.archive(ctx, revisionEngine, cos, revision) @@ -181,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(l, cos, werr.Error(), isDeadlineExceeded) + setRetryingConditions(cos, werr.Error(), isDeadlineExceeded) return ctrl.Result{}, werr } @@ -191,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(l, cos, err.Error(), isDeadlineExceeded) + setRetryingConditions(cos, err.Error(), isDeadlineExceeded) return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err) } @@ -199,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(l, cos, fmt.Sprintf("revision validation error: %s", verr), isDeadlineExceeded) + setRetryingConditions(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(l, cos, fmt.Sprintf("phase %d validation error: %s", i, verr), isDeadlineExceeded) + setRetryingConditions(cos, fmt.Sprintf("phase %d validation error: %s", i, verr), isDeadlineExceeded) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } @@ -219,14 +218,14 @@ 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(l, cos, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), isDeadlineExceeded) + setRetryingConditions(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 } } revisionNumber := cos.Spec.Revision if rres.InTransition() { - markAsProgressing(l, cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %d is rolling out.", revisionNumber), isDeadlineExceeded) + setAvailableWithDeadline(cos, metav1.ConditionFalse, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %d is rolling out.", revisionNumber), isDeadlineExceeded) } //nolint:nestif @@ -246,7 +245,6 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl } } - markAsProgressing(l, cos, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %d has rolled out.", revisionNumber), isDeadlineExceeded) markAsAvailable(cos, ocv1.ClusterObjectSetReasonProbesSucceeded, "Objects are available and pass all probes.") // Record the timestamp of the first time the revision was observed to be @@ -283,12 +281,19 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl } } - if len(probeFailureMsgs) > 0 { - markAsUnavailable(cos, ocv1.ClusterObjectSetReasonProbeFailure, strings.Join(probeFailureMsgs, "\n")) - } else { - markAsUnavailable(cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %d is rolling out.", revisionNumber)) + rollingOutMsg := fmt.Sprintf("Revision %d is rolling out.", revisionNumber) + switch { + case isDeadlineExceeded: + // Deadline wins over ProbeFailure. Use the generic rolling-out summary as + // the "last status", not the per-object probe detail, so the reconstructed + // ClusterExtension Progressing/ProgressDeadlineExceeded message matches the + // message the removed COS Progressing condition historically carried. + setAvailableWithDeadline(cos, metav1.ConditionFalse, ocv1.ReasonRollingOut, rollingOutMsg, true) + case len(probeFailureMsgs) > 0: + setAvailableWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonProbeFailure, strings.Join(probeFailureMsgs, "\n"), false) + default: + setAvailableWithDeadline(cos, metav1.ConditionFalse, ocv1.ReasonRollingOut, rollingOutMsg, false) } - markAsProgressing(l, cos, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %d is rolling out.", revisionNumber), isDeadlineExceeded) if hasDeadline && !isDeadlineExceeded { return ctrl.Result{RequeueAfter: remaining}, nil } @@ -299,7 +304,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 { - markAsAvailableUnknown(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error()) + markAsUnavailable(cos, ocv1.ClusterObjectSetReasonReconciling, err.Error()) return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err) } if err := c.removeFinalizer(ctx, cos, clusterObjectSetTeardownFinalizer); err != nil { @@ -309,15 +314,14 @@ func (c *ClusterObjectSetReconciler) delete(ctx context.Context, cos *ocv1.Clust } func (c *ClusterObjectSetReconciler) archive(ctx context.Context, revisionEngine RevisionEngine, cos *ocv1.ClusterObjectSet, revision boxcutter.RevisionBuilder) (ctrl.Result, error) { - l := log.FromContext(ctx) tdres, err := revisionEngine.Teardown(ctx, revision) if err != nil { err = fmt.Errorf("error archiving revision: %v", err) - setRetryingConditions(l, cos, err.Error(), false) + setRetryingConditions(cos, err.Error(), false) return ctrl.Result{}, err } if tdres != nil && !tdres.IsComplete() { - setRetryingConditions(l, cos, "removing revision resources that are not owned by another revision", false) + setRetryingConditions(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 @@ -647,49 +651,27 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing. return userProbes, nil } -func setRetryingConditions(l logr.Logger, cos *ocv1.ClusterObjectSet, message string, isDeadlineExceeded bool) { - markAsProgressing(l, cos, ocv1.ClusterObjectSetReasonRetrying, message, isDeadlineExceeded) - if meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) != nil { - markAsAvailableUnknown(cos, ocv1.ClusterObjectSetReasonReconciling, message) - } -} - -var nonTerminalProgressingReasons = map[string]struct{}{ - ocv1.ReasonRollingOut: {}, - ocv1.ClusterObjectSetReasonRetrying: {}, -} - -func markAsProgressing(l logr.Logger, cos *ocv1.ClusterObjectSet, reason, message string, isDeadlineExceeded bool) { - switch reason { - case ocv1.ReasonSucceeded: - // Terminal — always apply. - default: - if _, known := nonTerminalProgressingReasons[reason]; !known { - l.Error(fmt.Errorf("unregistered progressing reason: %q", reason), "treating as non-terminal for deadline enforcement") - } - if isDeadlineExceeded { - markAsNotProgressing(cos, ocv1.ReasonProgressDeadlineExceeded, - fmt.Sprintf("Revision has not rolled out for %d minute(s). Last status: %s", cos.Spec.ProgressDeadlineMinutes, message)) - return - } +// setAvailableWithDeadline sets the Available condition to the supplied status/reason/message, +// unless the progress deadline has been exceeded — in which case it reports +// Available=False/ProgressDeadlineExceeded instead. This centralises the deadline +// enforcement that previously lived in markAsProgressing. +func setAvailableWithDeadline(cos *ocv1.ClusterObjectSet, status metav1.ConditionStatus, reason, message string, isDeadlineExceeded bool) { // nolint:unparam + if isDeadlineExceeded { + markAsUnavailable(cos, ocv1.ReasonProgressDeadlineExceeded, + fmt.Sprintf("Revision has not rolled out for %d minute(s). Last status: %s", cos.Spec.ProgressDeadlineMinutes, message)) + return } meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: status, Reason: reason, Message: message, ObservedGeneration: cos.Generation, }) } -func markAsNotProgressing(cos *ocv1.ClusterObjectSet, reason, message string) bool { - return meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionFalse, - Reason: reason, - Message: message, - ObservedGeneration: cos.Generation, - }) +func setRetryingConditions(cos *ocv1.ClusterObjectSet, message string, isDeadlineExceeded bool) { + setAvailableWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonReconciling, message, isDeadlineExceeded) } func markAsAvailable(cos *ocv1.ClusterObjectSet, reason, message string) bool { @@ -702,20 +684,10 @@ func markAsAvailable(cos *ocv1.ClusterObjectSet, reason, message string) bool { }) } -func markAsUnavailable(cos *ocv1.ClusterObjectSet, reason, message string) { - meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeAvailable, - Status: metav1.ConditionFalse, - Reason: reason, - Message: message, - ObservedGeneration: cos.Generation, - }) -} - -func markAsAvailableUnknown(cos *ocv1.ClusterObjectSet, reason, message string) bool { +func markAsUnavailable(cos *ocv1.ClusterObjectSet, reason, message string) bool { return meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ Type: ocv1.ClusterObjectSetTypeAvailable, - Status: metav1.ConditionUnknown, + Status: metav1.ConditionFalse, Reason: reason, Message: message, ObservedGeneration: cos.Generation, @@ -724,8 +696,7 @@ func markAsAvailableUnknown(cos *ocv1.ClusterObjectSet, reason, message string) func markAsArchived(cos *ocv1.ClusterObjectSet) bool { const msg = "revision is archived" - updated := markAsNotProgressing(cos, ocv1.ClusterObjectSetReasonArchived, msg) - return markAsAvailableUnknown(cos, ocv1.ClusterObjectSetReasonArchived, msg) || updated + return markAsUnavailable(cos, ocv1.ClusterObjectSetReasonArchived, msg) } // computePhaseDigest computes a deterministic SHA-256 digest of a phase's diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index 808762c62f..c11be9df4f 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -46,6 +46,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing existingObjs func() []client.Object revisionResult machinery.RevisionResult revisionReconcileErr error + factoryErr error validate func(*testing.T, client.Client) }{ { @@ -67,7 +68,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing }, }, { - name: "Available condition is not updated on error if its not already set", + name: "Available condition is set to False/Reconciling on error when not previously set", reconcilingRevisionName: clusterObjectSetName, revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}), revisionReconcileErr: errors.New("some error"), @@ -83,11 +84,15 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing }, rev) require.NoError(t, err) cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) - require.Nil(t, cond) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) + require.Equal(t, "some error", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "Available condition is updated to Unknown on error if its been already set", + name: "Available condition is set to False/Reconciling on error when previously set", reconcilingRevisionName: clusterObjectSetName, revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}), revisionReconcileErr: errors.New("some error"), @@ -111,12 +116,36 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing require.NoError(t, err) cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) + require.Equal(t, metav1.ConditionFalse, cond.Status) require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, 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 Available=False/Reconciling even though Available did not previously exist. + name: "Available condition is set to False/Reconciling on factory error when not previously set", + reconcilingRevisionName: clusterObjectSetName, + factoryErr: errors.New("failed to create revision engine"), + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterObjectSet{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterObjectSetName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) + require.Nil(t, meta.FindStatusCondition(rev.Status.Conditions, "Progressing")) + }, + }, { name: "set Available:False:RollingOut status condition during rollout when no probe failures are detected", reconcilingRevisionName: clusterObjectSetName, @@ -317,30 +346,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing }, }, { - name: "set Progressing:True:Retrying when there's an error reconciling the revision", - revisionReconcileErr: errors.New("some error"), - reconcilingRevisionName: clusterObjectSetName, - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - return []client.Object{ext, rev1} - }, - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonRetrying, cond.Reason) - require.Equal(t, "some error", cond.Message) - require.Equal(t, int64(1), cond.ObservedGeneration) - }, - }, - { - name: "set Progressing:True:RollingOut condition while revision is transitioning", + name: "set Available:False:RollingOut condition while revision is transitioning", revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ inTransition: true, }), @@ -356,47 +362,14 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, metav1.ConditionFalse, cond.Status) require.Equal(t, ocv1.ReasonRollingOut, cond.Reason) require.Equal(t, "Revision 1 is rolling out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, }, - { - name: "set Progressing:True:Succeeded once transition rollout is finished", - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - inTransition: false, - isComplete: true, - }), - reconcilingRevisionName: clusterObjectSetName, - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.TypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ReasonRollingOut, - Message: "Revision 1 is rolling out.", - ObservedGeneration: 1, - }) - return []client.Object{ext, rev1} - }, - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ReasonSucceeded, cond.Reason) - require.Equal(t, "Revision 1 has rolled out.", cond.Message) - require.Equal(t, int64(1), cond.ObservedGeneration) - }, - }, { name: "set Available:True:ProbesSucceeded condition and completedAt on successful revision rollout", revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ @@ -421,13 +394,6 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing require.Equal(t, "Objects are available and pass all probes.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) - cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ReasonSucceeded, cond.Reason) - require.Equal(t, "Revision 1 has rolled out.", cond.Message) - require.Equal(t, int64(1), cond.ObservedGeneration) - require.False(t, rev.Status.CompletedAt.IsZero(), "completedAt should be set on successful rollout") }, }, @@ -484,7 +450,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing ) result, err := (&controllers.ClusterObjectSetReconciler{ Client: testClient, - RevisionEngineFactory: newMockRevisionEngineFactoryWithEngine(mockCtrl, mockEngine, nil), + RevisionEngineFactory: newMockRevisionEngineFactoryWithEngine(mockCtrl, mockEngine, tc.factoryErr), TrackingCache: newMockTrackingCache(mockCtrl, testClient, nil), }).Reconcile(t.Context(), ctrl.Request{ NamespacedName: types.NamespacedName{ @@ -494,10 +460,14 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing // reconcile cluster extension revision require.Equal(t, ctrl.Result{}, result) - if tc.revisionReconcileErr == nil { + wantErr := tc.revisionReconcileErr + if tc.factoryErr != nil { + wantErr = tc.factoryErr + } + if wantErr == nil { require.NoError(t, err) } else { - require.Contains(t, err.Error(), tc.revisionReconcileErr.Error()) + require.Contains(t, err.Error(), wantErr.Error()) } // validate test case @@ -759,7 +729,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) }, }, { - name: "set Available:Unknown:Reconciling when tracking cache free fails during deletion", + name: "set Available:False:Reconciling when tracking cache free fails during deletion", revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}), existingObjs: func() []client.Object { ext := newTestClusterExtension() @@ -782,7 +752,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) require.NoError(t, err) cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) + require.Equal(t, metav1.ConditionFalse, cond.Status) require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) require.Contains(t, cond.Message, "tracking cache free failed") }, @@ -791,7 +761,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) }, }, { - name: "set Available:Archived:Unknown and Progressing:False:Archived conditions when a revision is archived", + name: "set Available:False:Archived condition when a revision is archived", revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}), existingObjs: func() []client.Object { ext := newTestClusterExtension() @@ -817,13 +787,6 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) require.NoError(t, err) cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonArchived, cond.Reason) - require.Equal(t, "revision is archived", cond.Message) - require.Equal(t, int64(1), cond.ObservedGeneration) - - cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) - require.NotNil(t, cond) require.Equal(t, metav1.ConditionFalse, cond.Status) require.Equal(t, ocv1.ClusterObjectSetReasonArchived, cond.Reason) require.Equal(t, "revision is archived", cond.Message) @@ -831,7 +794,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) }, }, { - name: "set Progressing:True:Retrying and requeue when archived revision archival is incomplete", + name: "set Available:False:Reconciling and requeue when archived revision archival is incomplete", revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{}), existingObjs: func() []client.Object { ext := newTestClusterExtension() @@ -856,10 +819,10 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonRetrying, cond.Reason) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) require.Equal(t, "removing revision resources that are not owned by another revision", cond.Message) // Finalizer should still be present @@ -890,10 +853,10 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonRetrying, cond.Reason) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) require.Contains(t, cond.Message, "teardown failed: connection refused") // Finalizer should still be present @@ -923,10 +886,10 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonRetrying, cond.Reason) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) require.Contains(t, cond.Message, "token getter failed") // Finalizer should still be present @@ -945,13 +908,6 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) rev1.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateArchived meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ Type: ocv1.ClusterObjectSetTypeAvailable, - Status: metav1.ConditionUnknown, - Reason: ocv1.ClusterObjectSetReasonArchived, - Message: "revision is archived", - ObservedGeneration: rev1.Generation, - }) - meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeProgressing, Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectSetReasonArchived, Message: "revision is archived", @@ -1037,7 +993,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { clock clock.Clock }{ { - name: "progressing set to false when progress deadline is exceeded", + name: "available set to false/ProgressDeadlineExceeded when progress deadline is exceeded", existingObjs: func() []client.Object { ext := newTestClusterExtension() rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) @@ -1056,11 +1012,70 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.Equal(t, metav1.ConditionFalse, cnd.Status) require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason) }, }, + { + // A rolling revision with failing probes AND an exceeded progress deadline must + // set Available=False/ProgressDeadlineExceeded (deadline wins over ProbeFailure). + name: "available set to false/ProgressDeadlineExceeded when deadline is exceeded during probe failure (deadline wins over ProbeFailure)", + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + rev1.Spec.ProgressDeadlineMinutes = 1 + rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) + return []client.Object{rev1, ext} + }, + // 61sec elapsed since the creation of the revision + clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 1, 1, 0, time.UTC)), + revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ + inTransition: false, + isComplete: false, + phases: []machinery.PhaseResult{ + newMockPhaseResult(mockCtrl, phaseResultConfig{ + name: "somephase", + isComplete: false, + objects: []machinery.ObjectResult{ + newMockObjectResult(mockCtrl, objectResultConfig{ + object: func() client.Object { + obj := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + Namespace: "my-namespace", + }, + } + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) + return obj + }(), + probes: machinerytypes.ProbeResultContainer{ + boxcutter.ProgressProbeType: { + Status: machinerytypes.ProbeStatusFalse, + Messages: []string{"probe failed"}, + }, + }, + }), + }, + }), + }, + }), + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterObjectSet{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterObjectSetName, + }, rev) + require.NoError(t, err) + cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) + require.NotNil(t, cnd) + require.Equal(t, metav1.ConditionFalse, cnd.Status) + require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason) + // The deadline "last status" must be the generic rolling-out summary, not the + // per-object probe detail, so the reconstructed ClusterExtension Progressing + // message matches what the removed COS Progressing condition historically carried. + require.Equal(t, "Revision has not rolled out for 1 minute(s). Last status: Revision 1 is rolling out.", cnd.Message) + }, + }, { name: "requeue after progressDeadline time for final progression deadline check", existingObjs: func() []client.Object { @@ -1081,20 +1096,20 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) - require.Equal(t, metav1.ConditionTrue, cnd.Status) + cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) + require.Equal(t, metav1.ConditionFalse, cnd.Status) require.Equal(t, ocv1.ReasonRollingOut, cnd.Reason) }, }, { - name: "recovery from ProgressDeadlineExceeded to Succeeded when revision completes", + name: "recovery from ProgressDeadlineExceeded to ProbesSucceeded when revision completes", existingObjs: func() []client.Object { ext := newTestClusterExtension() rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) rev1.Spec.ProgressDeadlineMinutes = 1 rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeProgressing, + Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionFalse, Reason: ocv1.ReasonProgressDeadlineExceeded, Message: "Revision has not rolled out for 1 minute(s). Last status: Revision 1 is rolling out.", @@ -1112,11 +1127,11 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cnd) require.Equal(t, metav1.ConditionTrue, cnd.Status) - require.Equal(t, ocv1.ReasonSucceeded, cnd.Reason) - require.Equal(t, "Revision 1 has rolled out.", cnd.Message) + require.Equal(t, ocv1.ClusterObjectSetReasonProbesSucceeded, cnd.Reason) + require.Equal(t, "Objects are available and pass all probes.", cnd.Message) }, }, { @@ -1127,9 +1142,9 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { rev1.Spec.ProgressDeadlineMinutes = 1 rev1.CreationTimestamp = metav1.NewTime(time.Now().Add(-2 * time.Minute)) meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeProgressing, + Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionTrue, - Reason: ocv1.ReasonSucceeded, + Reason: ocv1.ClusterObjectSetReasonProbesSucceeded, ObservedGeneration: rev1.Generation, }) rev1.Status.CompletedAt = metav1.Now() @@ -1144,8 +1159,8 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { Name: clusterObjectSetName, }, rev) require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) - require.Equal(t, metav1.ConditionTrue, cnd.Status) + cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) + require.Equal(t, metav1.ConditionFalse, cnd.Status) require.Equal(t, ocv1.ReasonRollingOut, cnd.Reason) }, }, @@ -1608,10 +1623,10 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi rev := &ocv1.ClusterObjectSet{} require.NoError(t, testClient.Get(t.Context(), client.ObjectKey{Name: tc.reconcilingRevisionName}, rev)) - cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeProgressing) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable) require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonRetrying, cond.Reason) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason) require.Contains(t, cond.Message, "revision object collisions") } else { require.Equal(t, ctrl.Result{}, result) diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index 29f897ebe4..e623aedda2 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -24,7 +24,6 @@ import ( "io/fs" "slices" - apimeta "k8s.io/apimachinery/pkg/api/meta" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -134,39 +133,12 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e return nil, err } - ext.Status.ActiveRevisions = []ocv1.RevisionStatus{} - // Mirror Available/Progressing conditions from the installed revision - if i := state.revisionStates.Installed; i != nil { - for _, cndType := range []string{ocv1.ClusterObjectSetTypeAvailable, ocv1.ClusterObjectSetTypeProgressing} { - if cnd := apimeta.FindStatusCondition(i.Conditions, cndType); cnd != nil { - cnd.ObservedGeneration = ext.GetGeneration() - apimeta.SetStatusCondition(&ext.Status.Conditions, *cnd) - } - } - ext.Status.Install = &ocv1.ClusterExtensionInstallStatus{ - Bundle: i.BundleMetadata, - } - ext.Status.ActiveRevisions = []ocv1.RevisionStatus{{Name: i.RevisionName}} - } - for idx, r := range state.revisionStates.RollingOut { - rs := ocv1.RevisionStatus{Name: r.RevisionName} - for _, cndType := range []string{ocv1.ClusterObjectSetTypeAvailable, ocv1.ClusterObjectSetTypeProgressing} { - if cnd := apimeta.FindStatusCondition(r.Conditions, cndType); cnd != nil { - cnd.ObservedGeneration = ext.GetGeneration() - apimeta.SetStatusCondition(&rs.Conditions, *cnd) - } - } - // Mirror Progressing condition from the latest active revision - if idx == len(state.revisionStates.RollingOut)-1 { - if pcnd := apimeta.FindStatusCondition(r.Conditions, ocv1.ClusterObjectSetTypeProgressing); pcnd != nil { - pcnd.ObservedGeneration = ext.GetGeneration() - apimeta.SetStatusCondition(&ext.Status.Conditions, *pcnd) - } - } - ext.Status.ActiveRevisions = append(ext.Status.ActiveRevisions, rs) - } + setActiveRevisionsFromRevisionStates(ext, state.revisionStates) + setAvailableFromRevisionStates(ext, state.revisionStates) + setProgressingFromRevisionStates(ext, state.revisionStates) setInstalledStatusFromRevisionStates(ext, state.revisionStates) + return nil, nil } } diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go index 78adb70090..7b90f509e6 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go @@ -23,6 +23,7 @@ import ( "testing/fstest" "github.com/stretchr/testify/require" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ocv1 "github.com/operator-framework/operator-controller/api/v1" @@ -30,11 +31,13 @@ import ( func TestApplyBundleWithBoxcutter(t *testing.T) { type args struct { - activeRevisions []ocv1.RevisionStatus - revisionStates *RevisionStates + activeRevisions []ocv1.RevisionStatus + initialConditions []metav1.Condition + revisionStates *RevisionStates } type want struct { activeRevisions []ocv1.RevisionStatus + progressing *metav1.Condition } for _, tc := range []struct { @@ -108,6 +111,40 @@ func TestApplyBundleWithBoxcutter(t *testing.T) { }, }, }, + { + name: "rolling revision without Available resets stale Progressing to RollingOut default", + args: args{ + activeRevisions: []ocv1.RevisionStatus{ + {Name: "ce-1"}, + }, + initialConditions: []metav1.Condition{ + { + Type: ocv1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonSucceeded, + Message: "Desired state reached", + }, + }, + revisionStates: &RevisionStates{ + RollingOut: []*RevisionMetadata{ + // Freshly created revision that has not reconciled yet: + // it has no Available condition. + {RevisionName: "ce-1"}, + }, + }, + }, + want: want{ + activeRevisions: []ocv1.RevisionStatus{ + {Name: "ce-1"}, + }, + progressing: &metav1.Condition{ + Type: ocv1.TypeProgressing, + Status: metav1.ConditionTrue, + Reason: ocv1.ReasonRollingOut, + Message: "Revision is rolling out.", + }, + }, + }, } { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() @@ -119,6 +156,7 @@ func TestApplyBundleWithBoxcutter(t *testing.T) { }, Status: ocv1.ClusterExtensionStatus{ ActiveRevisions: tc.args.activeRevisions, + Conditions: tc.args.initialConditions, }, } @@ -145,6 +183,14 @@ func TestApplyBundleWithBoxcutter(t *testing.T) { require.Equal(t, expected.Name, ext.Status.ActiveRevisions[i].Name, "ActiveRevisions[%d].Name mismatch", i) } + + if tc.want.progressing != nil { + got := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(t, got, "Progressing condition not found") + require.Equal(t, tc.want.progressing.Status, got.Status, "Progressing.Status mismatch") + require.Equal(t, tc.want.progressing.Reason, got.Reason, "Progressing.Reason mismatch") + require.Equal(t, tc.want.progressing.Message, got.Message, "Progressing.Message mismatch") + } }) } } diff --git a/internal/operator-controller/controllers/common_controller.go b/internal/operator-controller/controllers/common_controller.go index 46197c6c36..4c3c77b803 100644 --- a/internal/operator-controller/controllers/common_controller.go +++ b/internal/operator-controller/controllers/common_controller.go @@ -69,21 +69,76 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image)) } +// 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)) + if i := revisionStates.Installed; i != nil { + ext.Status.ActiveRevisions = append(ext.Status.ActiveRevisions, ocv1.RevisionStatus{Name: i.RevisionName}) + } + for _, r := range revisionStates.RollingOut { + rs := ocv1.RevisionStatus{Name: r.RevisionName} + avail := apimeta.FindStatusCondition(r.Conditions, ocv1.ClusterObjectSetTypeAvailable) + if avail != nil { + a := *avail + a.ObservedGeneration = ext.GetGeneration() + apimeta.SetStatusCondition(&rs.Conditions, a) + } + ext.Status.ActiveRevisions = append(ext.Status.ActiveRevisions, rs) + } +} + +// setAvailableFromRevisionStates sets the Available status condition based on the given revision states +func setAvailableFromRevisionStates(ext *ocv1.ClusterExtension, revisionStates *RevisionStates) { + if i := revisionStates.Installed; i != nil { + avail := apimeta.FindStatusCondition(i.Conditions, ocv1.ClusterObjectSetTypeAvailable) + if avail != nil { + a := *avail + a.ObservedGeneration = ext.GetGeneration() + apimeta.SetStatusCondition(&ext.Status.Conditions, a) + } + } +} + +// setProgressingFromRevisionStates sets the Progressing status condition based on the given revision states +// The Progressing condition is derived from the Available condition of either: +// - the latest rolling out revision (when one exists); OR +// - the installed revision +// When the source revision has no Available condition yet (e.g. a freshly created revision that has +// not reconciled), a nil condition is passed so the helper applies the RollingOut default rather than +// leaving Progressing stale. +func setProgressingFromRevisionStates(ext *ocv1.ClusterExtension, revisionStates *RevisionStates) { + if len(revisionStates.RollingOut) > 0 { + revisionMeta := revisionStates.RollingOut[len(revisionStates.RollingOut)-1] + avail := apimeta.FindStatusCondition(revisionMeta.Conditions, ocv1.ClusterObjectSetTypeAvailable) + setProgressingFromAvailable(ext, avail, false) + } else if revisionStates.Installed != nil { + setProgressingFromAvailable(ext, apimeta.FindStatusCondition(revisionStates.Installed.Conditions, ocv1.ClusterObjectSetTypeAvailable), true) + } +} + +// setProgressingFromAvailable derives the correct Progressing condition for the ClusterExtension from the given +// revision Available condition, and whether the revision is complete or not +func setProgressingFromAvailable(ext *ocv1.ClusterExtension, availableCond *metav1.Condition, isRevisionCompleted bool) { + prog := progressingFromAvailable(availableCond, isRevisionCompleted) + prog.ObservedGeneration = ext.GetGeneration() + apimeta.SetStatusCondition(&ext.Status.Conditions, prog) +} + // determineFailureReason determines the appropriate reason for the Installed condition // when no bundle is installed (Installed: False). // // Returns Failed when: // - No rolling revisions exist (nothing to install) -// - The latest rolling revision has Reason: Retrying (indicates an error occurred) +// - The latest rolling revision has Available condition with Reason: Reconciling (indicates an error occurred) // // Returns Absent when: -// - Rolling revisions exist with the latest having Reason: RollingOut (healthy phased rollout in progress) +// - Rolling revisions exist with the latest not having Available=Reconciling (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) -// - Retrying reason indicates an error (config validation, apply failure, etc.) -// - RollingOut reason indicates healthy progress (not an error) +// - Reconciling reason on Available indicates an error (config validation, apply failure, etc.) +// - Other Available 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 // // Note: rollingRevisions are sorted in ascending order by Spec.Revision (oldest to newest), @@ -92,14 +147,11 @@ func determineFailureReason(rollingRevisions []*RevisionMetadata) string { if len(rollingRevisions) == 0 { return ocv1.ReasonFailed } - - // Check if the LATEST rolling revision indicates an error (Retrying reason) - // Latest revision is the last element in the array (sorted ascending by Spec.Revision) + // Latest revision is the last element (sorted ascending by Spec.Revision). latestRevision := rollingRevisions[len(rollingRevisions)-1] - progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterObjectSetTypeProgressing) - if progressingCond != nil && progressingCond.Reason == string(ocv1.ClusterObjectSetReasonRetrying) { - // Retrying indicates an error occurred (config, apply, validation, etc.) - // Use Failed for semantic correctness: installation failed due to error + availableCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterObjectSetTypeAvailable) + // Reconciling is the new home of the old Retrying signal: it indicates an error occurred. + if availableCond != nil && availableCond.Reason == ocv1.ClusterObjectSetReasonReconciling { return ocv1.ReasonFailed } @@ -174,3 +226,41 @@ func setStatusProgressing(ext *ocv1.ClusterExtension, err error) { SetStatusCondition(&ext.Status.Conditions, progressingCond) } + +// progressingFromAvailable reconstructs the ClusterExtension Progressing condition +// for a single revision from that revision's ClusterObjectSet Available condition +// and whether the revision has completed its rollout (status.completedAt set). +// +// This replaces the previous behavior of mirroring the ClusterObjectSet Progressing +// condition, which no longer exists. ObservedGeneration is left unset; callers set it. +func progressingFromAvailable(available *metav1.Condition, completed bool) metav1.Condition { + cond := metav1.Condition{ + Type: ocv1.TypeProgressing, + Status: metav1.ConditionTrue, + } + if completed { + cond.Reason = ocv1.ReasonSucceeded + cond.Message = "Desired state reached" + return cond + } + if available == nil { + cond.Reason = ocv1.ReasonRollingOut + cond.Message = "Revision is rolling out." + return cond + } + cond.Message = available.Message + switch available.Reason { + case ocv1.ClusterObjectSetReasonBlocked: + cond.Status = metav1.ConditionFalse + cond.Reason = ocv1.ReasonBlocked + case ocv1.ReasonProgressDeadlineExceeded: + cond.Status = metav1.ConditionFalse + cond.Reason = ocv1.ReasonProgressDeadlineExceeded + case ocv1.ClusterObjectSetReasonReconciling: + cond.Reason = ocv1.ReasonRetrying + default: + // ProbeFailure, RollingOut, or ProbesSucceeded-but-not-yet-complete. + cond.Reason = ocv1.ReasonRollingOut + } + return cond +} diff --git a/internal/operator-controller/controllers/common_controller_test.go b/internal/operator-controller/controllers/common_controller_test.go index 6edf8751ba..a5cb360943 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 (Retrying) - uses Failed", + name: "rolling revision with error (Reconciling) - uses Failed", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -325,9 +325,9 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T RevisionName: "rev-1", Conditions: []metav1.Condition{ { - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterObjectSetReasonRetrying, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: ocv1.ClusterObjectSetReasonReconciling, Message: "some error occurred", }, }, @@ -341,7 +341,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, { - name: "multiple rolling revisions with one Retrying - uses Failed", + name: "multiple rolling revisions with one Reconciling - uses Failed", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -349,8 +349,8 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T RevisionName: "rev-1", Conditions: []metav1.Condition{ { - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: metav1.ConditionFalse, Reason: ocv1.ReasonRollingOut, Message: "Revision is rolling out", }, @@ -360,9 +360,9 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T RevisionName: "rev-2", Conditions: []metav1.Condition{ { - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterObjectSetReasonRetrying, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: ocv1.ClusterObjectSetReasonReconciling, Message: "validation error occurred", }, }, @@ -384,8 +384,8 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T RevisionName: "rev-1", Conditions: []metav1.Condition{ { - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: metav1.ConditionFalse, Reason: ocv1.ReasonRollingOut, Message: "Revision is rolling out", }, @@ -400,7 +400,7 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }, }, { - name: "old revision with Retrying superseded by latest healthy - uses Absent", + name: "old revision with Reconciling superseded by latest healthy - uses Absent", revisionStates: &RevisionStates{ Installed: nil, RollingOut: []*RevisionMetadata{ @@ -408,9 +408,9 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T RevisionName: "rev-1", Conditions: []metav1.Condition{ { - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterObjectSetReasonRetrying, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: ocv1.ClusterObjectSetReasonReconciling, Message: "old error that was superseded", }, }, @@ -419,8 +419,8 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T RevisionName: "rev-2", Conditions: []metav1.Condition{ { - Type: ocv1.ClusterObjectSetTypeProgressing, - Status: metav1.ConditionTrue, + Type: ocv1.ClusterObjectSetTypeAvailable, + Status: metav1.ConditionFalse, Reason: ocv1.ReasonRollingOut, Message: "Latest revision is rolling out healthy", }, @@ -454,3 +454,97 @@ func TestSetInstalledStatusFromRevisionStates_ConfigValidationError(t *testing.T }) } } + +func TestDetermineFailureReason(t *testing.T) { + availCond := func(status metav1.ConditionStatus, reason string) []metav1.Condition { + return []metav1.Condition{{Type: ocv1.ClusterObjectSetTypeAvailable, Status: status, Reason: reason}} + } + for _, tc := range []struct { + name string + rolling []*RevisionMetadata + expected string + }{ + {name: "no rolling revisions -> Failed", rolling: nil, expected: ocv1.ReasonFailed}, + { + name: "latest reconciling -> Failed", + rolling: []*RevisionMetadata{{Conditions: availCond(metav1.ConditionUnknown, ocv1.ClusterObjectSetReasonReconciling)}}, + expected: ocv1.ReasonFailed, + }, + { + name: "latest rolling out -> Absent", + rolling: []*RevisionMetadata{{Conditions: availCond(metav1.ConditionFalse, ocv1.ReasonRollingOut)}}, + expected: ocv1.ReasonAbsent, + }, + { + name: "latest blocked -> Absent (Blocked is terminal, handled elsewhere)", + rolling: []*RevisionMetadata{{Conditions: availCond(metav1.ConditionFalse, ocv1.ClusterObjectSetReasonBlocked)}}, + expected: ocv1.ReasonAbsent, + }, + { + name: "only the latest revision matters", + rolling: []*RevisionMetadata{ + {Conditions: availCond(metav1.ConditionUnknown, ocv1.ClusterObjectSetReasonReconciling)}, + {Conditions: availCond(metav1.ConditionFalse, ocv1.ReasonRollingOut)}, + }, + expected: ocv1.ReasonAbsent, + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, determineFailureReason(tc.rolling)) + }) + } +} + +func TestProgressingFromAvailable(t *testing.T) { + for _, tc := range []struct { + name string + available *metav1.Condition + completed bool + expected metav1.Condition + }{ + { + name: "completed revision maps to Succeeded", + available: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionTrue, Reason: ocv1.ClusterObjectSetReasonProbesSucceeded, Message: "ok"}, + completed: true, + expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonSucceeded, Message: "Desired state reached"}, + }, + { + name: "nil available on rolling revision defaults to RollingOut", + available: nil, + completed: false, + expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRollingOut, Message: "Revision is rolling out."}, + }, + { + name: "probe failure maps to RollingOut", + available: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectSetReasonProbeFailure, Message: "probe failed"}, + completed: false, + expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRollingOut, Message: "probe failed"}, + }, + { + name: "reconciling maps to Retrying", + available: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionUnknown, Reason: ocv1.ClusterObjectSetReasonReconciling, Message: "boom"}, + completed: false, + expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonRetrying, Message: "boom"}, + }, + { + name: "blocked maps to Blocked", + available: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionFalse, Reason: ocv1.ClusterObjectSetReasonBlocked, Message: "manual fix needed"}, + completed: false, + expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionFalse, Reason: ocv1.ReasonBlocked, Message: "manual fix needed"}, + }, + { + name: "deadline exceeded maps to ProgressDeadlineExceeded", + available: &metav1.Condition{Type: ocv1.ClusterObjectSetTypeAvailable, Status: metav1.ConditionFalse, Reason: ocv1.ReasonProgressDeadlineExceeded, Message: "too slow"}, + completed: false, + expected: metav1.Condition{Type: ocv1.TypeProgressing, Status: metav1.ConditionFalse, Reason: ocv1.ReasonProgressDeadlineExceeded, Message: "too slow"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := progressingFromAvailable(tc.available, tc.completed) + require.Equal(t, tc.expected.Type, got.Type) + require.Equal(t, tc.expected.Status, got.Status) + require.Equal(t, tc.expected.Reason, got.Reason) + require.Equal(t, tc.expected.Message, got.Message) + }) + } +} diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 683d3969df..7526e930e7 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1375,9 +1375,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Available')].status name: Available type: string - - jsonPath: .status.conditions[?(@.type=='Progressing')].status - name: Progressing - type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -1913,19 +1910,19 @@ spec: conditions is an optional list of status conditions describing the state of the ClusterObjectSet. - The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is RollingOut, the ClusterObjectSet rollout is actively making progress and is in transition. - - When status is True and reason is Retrying, the ClusterObjectSet has encountered an error that could be resolved on subsequent reconciliation attempts. - - When status is True and reason is Succeeded, the ClusterObjectSet has reached the desired state. - - When status is False and reason is Blocked, the ClusterObjectSet has encountered an error that requires manual intervention for recovery. - - When status is False and reason is Archived, the ClusterObjectSet is archived and not being actively reconciled. - - The Available condition represents whether the revision has been successfully rolled out and is available: - - When status is True and reason is ProbesSucceeded, the ClusterObjectSet has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - - 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 Available condition represents the state of the revision. + 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. + - 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 available. + - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery. + - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. + - False with reason Reconciling: the revision encountered an error that prevented it from observing the probes. + - False with reason Archived: the revision has been archived and its objects have been torn down. + + Rollout completion is recorded separately by status.completedAt. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 1d13fa0c87..2f98f9b05e 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1336,9 +1336,6 @@ spec: - jsonPath: .status.conditions[?(@.type=='Available')].status name: Available type: string - - jsonPath: .status.conditions[?(@.type=='Progressing')].status - name: Progressing - type: string - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -1874,19 +1871,19 @@ spec: conditions is an optional list of status conditions describing the state of the ClusterObjectSet. - The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is RollingOut, the ClusterObjectSet rollout is actively making progress and is in transition. - - When status is True and reason is Retrying, the ClusterObjectSet has encountered an error that could be resolved on subsequent reconciliation attempts. - - When status is True and reason is Succeeded, the ClusterObjectSet has reached the desired state. - - When status is False and reason is Blocked, the ClusterObjectSet has encountered an error that requires manual intervention for recovery. - - When status is False and reason is Archived, the ClusterObjectSet is archived and not being actively reconciled. - - The Available condition represents whether the revision has been successfully rolled out and is available: - - When status is True and reason is ProbesSucceeded, the ClusterObjectSet has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - - 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 Available condition represents the state of the revision. + 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. + - 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 available. + - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery. + - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. + - False with reason Reconciling: the revision encountered an error that prevented it from observing the probes. + - False with reason Archived: the revision has been archived and its objects have been torn down. + + Rollout completion is recorded separately by status.completedAt. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index 2611edeec0..f62191b9f9 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -370,7 +370,7 @@ Feature: Install ClusterExtension matchLabels: "olm.operatorframework.io/metadata.name": ${CATALOG:test} """ - Then ClusterObjectSet "${NAME}-1" reports Progressing as False with Reason ProgressDeadlineExceeded + Then ClusterObjectSet "${NAME}-1" reports Available as False with Reason ProgressDeadlineExceeded And ClusterExtension reports Progressing as False with Reason ProgressDeadlineExceeded and Message: """ Revision has not rolled out for 1 minute(s). Last status: Revision 1 is rolling out. @@ -404,7 +404,7 @@ Feature: Install ClusterExtension matchLabels: "olm.operatorframework.io/metadata.name": ${CATALOG:test} """ - Then ClusterObjectSet "${NAME}-1" reports Progressing as False with Reason ProgressDeadlineExceeded + Then ClusterObjectSet "${NAME}-1" reports Available as False with Reason ProgressDeadlineExceeded And ClusterExtension reports Progressing as False with Reason ProgressDeadlineExceeded and Message: """ Revision has not rolled out for 1 minute(s). Last status: Revision 1 is rolling out. diff --git a/test/e2e/features/revision.feature b/test/e2e/features/revision.feature index 0a4ab87627..cd44d8b168 100644 --- a/test/e2e/features/revision.feature +++ b/test/e2e/features/revision.feature @@ -147,8 +147,7 @@ Feature: Install ClusterObjectSet revision: 1 """ - Then ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded - And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded + Then ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded And resource "persistentvolume/test-pv" is installed And resource "persistentvolumeclaim/test-pvc" is installed And resource "configmap/test-configmap" is installed @@ -323,7 +322,6 @@ 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 Progressing as True with Reason Succeeded And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded Scenario: User can install a ClusterObjectSet with objects stored in Secrets @@ -420,8 +418,7 @@ Feature: Install ClusterObjectSet key: deployment revision: 1 """ - Then ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded - And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded + Then ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded 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 @@ -468,7 +465,7 @@ Feature: Install ClusterObjectSet key: configmap revision: 1 """ - Then ClusterObjectSet "${COS_NAME}" reports Progressing as False with Reason Blocked and Message: + Then ClusterObjectSet "${COS_NAME}" reports Available as False with Reason Blocked and Message: """ the following secrets are not immutable (referenced secrets must have immutable set to true): ${TEST_NAMESPACE}/${COS_NAME}-mutable-secret """ @@ -516,8 +513,7 @@ Feature: Install ClusterObjectSet key: configmap revision: 1 """ - Then ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded - And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded + Then ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded 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 @@ -545,7 +541,7 @@ Feature: Install ClusterObjectSet } """ And ClusterObjectSet "${COS_NAME}" reconciliation is triggered - Then ClusterObjectSet "${COS_NAME}" reports Progressing as False with Reason Blocked and Message includes: + Then ClusterObjectSet "${COS_NAME}" reports Available as False with Reason Blocked and Message includes: """ resolved content of 1 phase(s) has changed: phase "resources" """ @@ -575,7 +571,7 @@ Feature: Install ClusterObjectSet } """ And ClusterObjectSet "${COS_NAME}" reconciliation is triggered - Then ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded + Then ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded @ProgressDeadline @@ -637,7 +633,7 @@ Feature: Install ClusterObjectSet """ Then resource "configmap/test-configmap" is installed And resource "deployment/test-deployment" is installed - And ClusterObjectSet "${COS_NAME}" reports Progressing as False with Reason ProgressDeadlineExceeded + And ClusterObjectSet "${COS_NAME}" reports Available as False with Reason ProgressDeadlineExceeded When ClusterObjectSet "${COS_NAME}" lifecycle is set to "Archived" Then ClusterObjectSet "${COS_NAME}" is archived And resource "configmap/test-configmap" is eventually not found @@ -706,5 +702,5 @@ Feature: Install ClusterObjectSet type: RuntimeDefault revision: 1 """ - Then ClusterObjectSet "${COS_NAME}" reports Progressing as False with Reason ProgressDeadlineExceeded - And ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded + Then ClusterObjectSet "${COS_NAME}" reports Available as False with Reason ProgressDeadlineExceeded + And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded diff --git a/test/e2e/features/update.feature b/test/e2e/features/update.feature index e7ba0251ef..40edd84274 100644 --- a/test/e2e/features/update.feature +++ b/test/e2e/features/update.feature @@ -315,7 +315,6 @@ 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 Progressing as True with Reason Succeeded And ClusterObjectSet "${NAME}-2" reports Available as True with Reason ProbesSucceeded And ClusterObjectSet "${NAME}-1" is archived And ClusterObjectSet "${NAME}-1" phase objects are not found or not owned by the revision @@ -344,7 +343,6 @@ Feature: Update ClusterExtension And ClusterExtension is available When ClusterExtension version is updated to "1.0.2" Then ClusterExtension reports "${NAME}-1, ${NAME}-2" as active revisions - And ClusterObjectSet "${NAME}-2" reports Progressing as True with Reason RollingOut And ClusterObjectSet "${NAME}-2" reports Available as False with Reason ProbeFailure Scenario: Clearing deprecated serviceAccount field is reconciled without warnings diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index d3edd5169c..7495bbabcd 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -972,10 +972,10 @@ func ClusterObjectSetHasObservedPhase(ctx context.Context, cosName, phaseName st return nil } -// ClusterObjectSetIsArchived waits for the named ClusterObjectSet to have Progressing=False +// ClusterObjectSetIsArchived waits for the named ClusterObjectSet to have Available=False // with reason Archived. Polls with timeout. func ClusterObjectSetIsArchived(ctx context.Context, revisionName string) error { - return waitForCondition(ctx, "clusterobjectset", substituteScenarioVars(revisionName, scenarioCtx(ctx)), "Progressing", "False", ptr.To("Archived"), nil) + return waitForCondition(ctx, "clusterobjectset", substituteScenarioVars(revisionName, scenarioCtx(ctx)), "Available", "False", ptr.To("Archived"), nil) } // ClusterObjectSetHasAnnotationWithValue waits for the named ClusterObjectSet to have the specified