From 04de6035a66d175d8d47ba172c024e493c7c535c Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Tue, 18 Nov 2025 13:37:40 +0100 Subject: [PATCH] Update ClusterExtensionRevision status conditions Co-authored-by: Per Goncalves da Silva Co-authored-by: Predrag Knezevic --- api/v1/clusterextension_types.go | 3 + api/v1/clusterextensionrevision_types.go | 55 +-- api/v1/common_types.go | 6 +- docs/api-reference/olmv1-api-reference.md | 2 +- ...ramework.io_clusterextensionrevisions.yaml | 24 +- ...peratorframework.io_clusterextensions.yaml | 2 + .../operator-controller/applier/boxcutter.go | 9 +- .../conditionsets/conditionsets.go | 2 +- .../clusterextension_reconcile_steps.go | 2 +- .../clusterextensionrevision_controller.go | 223 +++++----- ...lusterextensionrevision_controller_test.go | 411 ++++++++++++++---- manifests/experimental-e2e.yaml | 26 +- manifests/experimental.yaml | 26 +- test/e2e/cluster_extension_revision_test.go | 209 +++++++++ test/e2e/e2e_suite_test.go | 5 + 15 files changed, 733 insertions(+), 272 deletions(-) create mode 100644 test/e2e/cluster_extension_revision_test.go diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6de62b0e12..7f8b55fa31 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -482,6 +482,9 @@ type ClusterExtensionStatus struct { // When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state. // When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts. // When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery. + // + // When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out. + // // // When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition. // These are indications from a package owner to guide users away from a particular package, channel, or bundle. diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 69a116300f..678fc93077 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -24,26 +24,19 @@ import ( const ( ClusterExtensionRevisionKind = "ClusterExtensionRevision" - // ClusterExtensionRevisionTypeAvailable is the condition type that represents whether the - // ClusterExtensionRevision is available and has been successfully rolled out. - ClusterExtensionRevisionTypeAvailable = "Available" - - // ClusterExtensionRevisionTypeSucceeded is the condition type that represents whether the - // ClusterExtensionRevision rollout has succeeded. - ClusterExtensionRevisionTypeSucceeded = "Succeeded" - - // Condition reasons - ClusterExtensionRevisionReasonAvailable = "Available" - ClusterExtensionRevisionReasonReconcileFailure = "ReconcileFailure" - ClusterExtensionRevisionReasonRevisionValidationFailure = "RevisionValidationFailure" - ClusterExtensionRevisionReasonPhaseValidationError = "PhaseValidationError" - ClusterExtensionRevisionReasonObjectCollisions = "ObjectCollisions" - ClusterExtensionRevisionReasonRolloutSuccess = "RolloutSuccess" - ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" - ClusterExtensionRevisionReasonIncomplete = "Incomplete" - ClusterExtensionRevisionReasonProgressing = "Progressing" - ClusterExtensionRevisionReasonArchived = "Archived" - ClusterExtensionRevisionReasonMigrated = "Migrated" + // Condition Types + ClusterExtensionRevisionTypeAvailable = "Available" + ClusterExtensionRevisionTypeProgressing = "Progressing" + ClusterExtensionRevisionTypeSucceeded = "Succeeded" + + // Condition Reasons + ClusterExtensionRevisionReasonArchived = "Archived" + ClusterExtensionRevisionReasonBlocked = "Blocked" + ClusterExtensionRevisionReasonMigrated = "Migrated" + ClusterExtensionRevisionReasonProbeFailure = "ProbeFailure" + ClusterExtensionRevisionReasonProbesSucceeded = "ProbesSucceeded" + ClusterExtensionRevisionReasonReconciling = "Reconciling" + ClusterExtensionRevisionReasonRetrying = "Retrying" ) // ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. @@ -190,22 +183,21 @@ type ClusterExtensionRevisionStatus struct { // ClusterExtensionRevision. // // The Progressing condition represents whether the revision is actively rolling out: - // - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - // - When Progressing is not present, the revision is not currently in transition. + // - When status is True and reason is RollingOut, the ClusterExtensionRevision rollout is actively making progress and is in transition. + // - When status is True and reason is Retrying, the ClusterExtensionRevision has encountered an error that could be resolved on subsequent reconciliation attempts. + // - When status is True and reason is Succeeded, the ClusterExtensionRevision has reached the desired state. + // - When status is False and reason is Blocked, the ClusterExtensionRevision has encountered an error that requires manual intervention for recovery. + // - When status is False and reason is Archived, the ClusterExtensionRevision 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 Available, the revision has been successfully rolled out and all objects pass their readiness probes. - // - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. + // - When status is True and reason is ProbesSucceeded, the ClusterExtensionRevision 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 False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - // - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - // - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - // - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - // - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - // - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. + // - When status is Unknown and reason is Reconciling, the ClusterExtensionRevision has encountered an error that prevented it from observing the probes. + // - When status is Unknown and reason is Archived, the ClusterExtensionRevision has been archived and its objects have been torn down. + // - When status is Unknown and reason is Migrated, the ClusterExtensionRevision was migrated from an existing release and object status probe results have not yet been observed. // // The Succeeded condition represents whether the revision has successfully completed its rollout: - // - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. + // - When status is True and reason is Succeeded, the ClusterExtensionRevision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. // // +listType=map // +listMapKey=type @@ -217,6 +209,7 @@ type ClusterExtensionRevisionStatus 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` // ClusterExtensionRevision represents an immutable snapshot of Kubernetes objects diff --git a/api/v1/common_types.go b/api/v1/common_types.go index 6ab5336ac2..115836b10c 100644 --- a/api/v1/common_types.go +++ b/api/v1/common_types.go @@ -24,9 +24,9 @@ const ( ReasonAbsent = "Absent" // Progressing reasons - ReasonRolloutInProgress = "RolloutInProgress" - ReasonRetrying = "Retrying" - ReasonBlocked = "Blocked" + ReasonRollingOut = "RollingOut" + ReasonRetrying = "Retrying" + ReasonBlocked = "Blocked" // Deprecation reasons ReasonDeprecated = "Deprecated" diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 317b46a00c..aa51ec71ea 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -359,7 +359,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.
When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
Deprecated is a rollup condition that is present when any of the deprecated conditions are present. | | | +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.
When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.

When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.

When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
Deprecated is a rollup condition that is present when any of the deprecated conditions are present. | | | | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | | diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml index 1a3a8b0212..c448a4da46 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -19,6 +19,9 @@ 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 @@ -190,22 +193,21 @@ spec: ClusterExtensionRevision. The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - - When Progressing is not present, the revision is not currently in transition. + - When status is True and reason is RollingOut, the ClusterExtensionRevision rollout is actively making progress and is in transition. + - When status is True and reason is Retrying, the ClusterExtensionRevision has encountered an error that could be resolved on subsequent reconciliation attempts. + - When status is True and reason is Succeeded, the ClusterExtensionRevision has reached the desired state. + - When status is False and reason is Blocked, the ClusterExtensionRevision has encountered an error that requires manual intervention for recovery. + - When status is False and reason is Archived, the ClusterExtensionRevision 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 Available, the revision has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. + - When status is True and reason is ProbesSucceeded, the ClusterExtensionRevision 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 False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. + - When status is Unknown and reason is Reconciling, the ClusterExtensionRevision has encountered an error that prevented it from observing the probes. + - When status is Unknown and reason is Archived, the ClusterExtensionRevision has been archived and its objects have been torn down. + - When status is Unknown and reason is Migrated, the ClusterExtensionRevision was migrated from an existing release and object status probe results have not yet been observed. The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. + - When status is True and reason is Succeeded, the ClusterExtensionRevision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 1038b7fdf0..a58e58cc88 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -518,6 +518,8 @@ spec: When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts. When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery. + When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out. + When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition. These are indications from a package owner to guide users away from a particular package, channel, or bundle. BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog. diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 6abcd0c43b..af41caa613 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -397,7 +397,14 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust if progressingCondition == nil && availableCondition == nil && succeededCondition == nil { return false, "New revision created", nil } else if progressingCondition != nil && progressingCondition.Status == metav1.ConditionTrue { - return false, progressingCondition.Message, nil + switch progressingCondition.Reason { + case ocv1.ReasonSucceeded: + return true, "", nil + case ocv1.ClusterExtensionRevisionReasonRetrying: + return false, "", errors.New(progressingCondition.Message) + default: + return false, progressingCondition.Message, nil + } } else if availableCondition != nil && availableCondition.Status != metav1.ConditionTrue { return false, "", errors.New(availableCondition.Message) } else if succeededCondition != nil && succeededCondition.Status != metav1.ConditionTrue { diff --git a/internal/operator-controller/conditionsets/conditionsets.go b/internal/operator-controller/conditionsets/conditionsets.go index 0d63e1abb2..6c33b1c8f9 100644 --- a/internal/operator-controller/conditionsets/conditionsets.go +++ b/internal/operator-controller/conditionsets/conditionsets.go @@ -40,5 +40,5 @@ var ConditionReasons = []string{ ocv1.ReasonBlocked, ocv1.ReasonRetrying, ocv1.ReasonAbsent, - ocv1.ReasonRolloutInProgress, + ocv1.ReasonRollingOut, } diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 2e1e9232ac..c4cdbdddc8 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -191,7 +191,7 @@ func ApplyBundle(a Applier) ReconcileStepFunc { apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{ Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, - Reason: ocv1.ReasonRolloutInProgress, + Reason: ocv1.ReasonRollingOut, Message: rolloutStatus, ObservedGeneration: ext.GetGeneration(), }) diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index ec035eee78..9efb845e00 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -117,13 +117,7 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev revision, opts, err := c.toBoxcutterRevision(ctx, rev) if err != nil { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) + setRetryingConditions(rev, err.Error()) return ctrl.Result{}, fmt.Errorf("converting to boxcutter revision: %v", err) } @@ -131,77 +125,42 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev return c.teardown(ctx, rev, revision) } + revVersion := rev.GetAnnotations()[labels.BundleVersionKey] // // Reconcile // if err := c.ensureFinalizer(ctx, rev, clusterExtensionRevisionTeardownFinalizer); err != nil { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) return ctrl.Result{}, fmt.Errorf("error ensuring teardown finalizer: %v", err) } if err := c.establishWatch(ctx, rev, revision); err != nil { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) - return ctrl.Result{}, fmt.Errorf("establish watch: %v", err) + werr := fmt.Errorf("establish watch: %v", err) + setRetryingConditions(rev, werr.Error()) + return ctrl.Result{}, werr } rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...) if err != nil { if rres != nil { - l.Error(err, "revision reconcile failed") - l.V(1).Info("reconcile failure report", "report", rres.String()) - } else { - l.Error(err, "revision reconcile failed") + // Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity. + l.V(1).Info("reconcile report", "report", rres.String()) } - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) + setRetryingConditions(rev, err.Error()) return ctrl.Result{}, fmt.Errorf("revision reconcile: %v", err) } - // Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity. - l.V(1).Info("reconcile report", "report", rres.String()) // Retry failing preflight checks with a flat 10s retry. // TODO: report status, backoff? if verr := rres.GetValidationError(); verr != nil { l.Error(fmt.Errorf("%w", verr), "preflight validation failed, retrying after 10s") - - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonRevisionValidationFailure, - Message: fmt.Sprintf("revision validation error: %s", verr), - ObservedGeneration: rev.Generation, - }) + setRetryingConditions(rev, fmt.Sprintf("revision validation error: %s", verr)) 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) - - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonPhaseValidationError, - Message: fmt.Sprintf("phase %d validation error: %s", i, verr), - ObservedGeneration: rev.Generation, - }) + setRetryingConditions(rev, fmt.Sprintf("phase %d validation error: %s", i, verr)) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } @@ -214,18 +173,17 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev if len(collidingObjs) > 0 { l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs) - - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonObjectCollisions, - Message: fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), - ObservedGeneration: rev.Generation, - }) + setRetryingConditions(rev, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n"))) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } } + if !rres.InTransistion() { + markAsProgressing(rev, ocv1.ReasonSucceeded, fmt.Sprintf("Revision %s has rolled out.", revVersion)) + } else { + markAsProgressing(rev, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion)) + } + //nolint:nestif if rres.IsComplete() { // Archive previous revisions @@ -243,23 +201,18 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev } } - // Report status. + markAsAvailable(rev, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, "Objects are available and pass all probes.") + + // We'll probably only want to remove this once we are done updating the ClusterExtension conditions + // as its one of the interfaces between the revision and the extension. If we still have the Succeeded for now + // that's fine. meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Type: ocv1.ClusterExtensionRevisionTypeSucceeded, Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonAvailable, - Message: "Object is available and passes all probes.", + Reason: ocv1.ReasonSucceeded, + Message: "Revision succeeded rolling out.", ObservedGeneration: rev.Generation, }) - if !meta.IsStatusConditionTrue(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeSucceeded, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonRolloutSuccess, - Message: "Revision succeeded rolling out.", - ObservedGeneration: rev.Generation, - }) - } } else { var probeFailureMsgs []string for _, pres := range rres.GetPhases() { @@ -267,6 +220,8 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev continue } for _, ores := range pres.GetObjects() { + // we probably want an AvailabilityProbeType and run through all of them independently of whether + // the revision is complete or not pr := ores.Probes()[boxcutter.ProgressProbeType] if pr.Success { continue @@ -274,6 +229,8 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev obj := ores.Object() gvk := obj.GetObjectKind().GroupVersionKind() + // I think these can be pretty large and verbose. We may want to + // work a little on the formatting...? probeFailureMsgs = append(probeFailureMsgs, fmt.Sprintf( "Object %s.%s %s/%s: %v", gvk.Kind, gvk.GroupVersion().String(), @@ -282,35 +239,13 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev break } } + if len(probeFailureMsgs) > 0 { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonProbeFailure, - Message: strings.Join(probeFailureMsgs, "\n"), - ObservedGeneration: rev.Generation, - }) + markAsUnavailable(rev, ocv1.ClusterExtensionRevisionReasonProbeFailure, strings.Join(probeFailureMsgs, "\n")) } else { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonIncomplete, - Message: "Revision has not been rolled out completely.", - ObservedGeneration: rev.Generation, - }) + markAsUnavailable(rev, ocv1.ReasonRollingOut, fmt.Sprintf("Revision %s is rolling out.", revVersion)) } } - if rres.InTransistion() { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.TypeProgressing, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonProgressing, - Message: "Rollout in progress.", - ObservedGeneration: rev.Generation, - }) - } else { - meta.RemoveStatusCondition(&rev.Status.Conditions, ocv1.TypeProgressing) - } return ctrl.Result{}, nil } @@ -321,18 +256,9 @@ func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev * tres, err := c.RevisionEngine.Teardown(ctx, *revision) if err != nil { if tres != nil { - l.Error(err, "revision teardown failed") l.V(1).Info("teardown failure report", "report", tres.String()) - } else { - l.Error(err, "revision teardown failed") } - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) + markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error()) return ctrl.Result{}, fmt.Errorf("revision teardown: %v", err) } @@ -346,26 +272,12 @@ func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev * } if err := c.TrackingCache.Free(ctx, rev); err != nil { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionFalse, - Reason: ocv1.ClusterExtensionRevisionReasonReconcileFailure, - Message: err.Error(), - ObservedGeneration: rev.Generation, - }) + markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error()) return ctrl.Result{}, fmt.Errorf("error stopping informers: %v", err) } - // Ensure Available condition is set to Unknown before removing the finalizer when archiving - if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived && - !meta.IsStatusConditionPresentAndEqual(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable, metav1.ConditionUnknown) { - meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterExtensionRevisionTypeAvailable, - Status: metav1.ConditionUnknown, - Reason: ocv1.ClusterExtensionRevisionReasonArchived, - Message: "revision is archived", - ObservedGeneration: rev.Generation, - }) + // Ensure conditions are set before removing the finalizer when archiving + if rev.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived && markAsArchived(rev) { return ctrl.Result{}, nil } @@ -605,3 +517,66 @@ var ( FieldB: ".status.replicas", } ) + +func setRetryingConditions(cer *ocv1.ClusterExtensionRevision, message string) { + markAsProgressing(cer, ocv1.ClusterExtensionRevisionReasonRetrying, message) + if meta.FindStatusCondition(cer.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) != nil { + markAsAvailableUnknown(cer, ocv1.ClusterExtensionRevisionReasonReconciling, message) + } +} + +func markAsProgressing(cer *ocv1.ClusterExtensionRevision, reason, message string) { + meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionTrue, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsNotProgressing(cer *ocv1.ClusterExtensionRevision, reason, message string) bool { + return meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsAvailable(cer *ocv1.ClusterExtensionRevision, reason, message string) bool { + return meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsUnavailable(cer *ocv1.ClusterExtensionRevision, reason, message string) { + meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsAvailableUnknown(cer *ocv1.ClusterExtensionRevision, reason, message string) bool { + return meta.SetStatusCondition(&cer.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionUnknown, + Reason: reason, + Message: message, + ObservedGeneration: cer.Generation, + }) +} + +func markAsArchived(cer *ocv1.ClusterExtensionRevision) bool { + const msg = "revision is archived" + updated := markAsNotProgressing(cer, ocv1.ClusterExtensionRevisionReasonArchived, msg) + return markAsAvailableUnknown(cer, ocv1.ClusterExtensionRevisionReasonArchived, msg) || updated +} diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go index e88051537b..e54827962a 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -2,16 +2,18 @@ package controllers_test import ( "context" + "errors" "fmt" "testing" "time" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -32,26 +34,26 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/labels" ) -func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *testing.T) { - const ( - clusterExtensionRevisionName = "test-ext-1" - ) +const clusterExtensionRevisionName = "test-ext-1" +func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t *testing.T) { testScheme := newScheme(t) for _, tc := range []struct { - name string - existingObjs func() []client.Object - revisionResult machinery.RevisionResult - validate func(*testing.T, client.Client) + name string + reconcilingRevisionName string + existingObjs func() []client.Object + revisionResult machinery.RevisionResult + revisionReconcileErr error + validate func(*testing.T, client.Client) }{ { - name: "sets teardown finalizer", - revisionResult: mockRevisionResult{}, + name: "sets teardown finalizer", + reconcilingRevisionName: clusterExtensionRevisionName, + revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) return []client.Object{ext, rev1} }, validate: func(t *testing.T, c client.Client) { @@ -64,12 +66,154 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, }, { - name: "set Available:False:InComplete status condition during rollout when no probe failures are detected", - revisionResult: mockRevisionResult{}, + name: "Available condition is not updated on error if its not already set", + reconcilingRevisionName: clusterExtensionRevisionName, + revisionResult: mockRevisionResult{}, + revisionReconcileErr: errors.New("some error"), + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.Nil(t, cond) + }, + }, + { + name: "Available condition is updated to Unknown on error if its been already set", + reconcilingRevisionName: clusterExtensionRevisionName, + revisionResult: mockRevisionResult{}, + revisionReconcileErr: errors.New("some error"), + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) + meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeAvailable, + Status: metav1.ConditionTrue, + Reason: ocv1.ClusterExtensionRevisionReasonProbesSucceeded, + Message: "Revision 1.0.0 is rolled out.", + ObservedGeneration: 1, + }) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionUnknown, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonReconciling, cond.Reason) + require.Equal(t, "some error", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set Available:False:RollingOut status condition during rollout when no probe failures are detected", + reconcilingRevisionName: clusterExtensionRevisionName, + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ReasonRollingOut, cond.Reason) + require.Equal(t, "Revision 1.0.0 is rolling out.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set Available:False:ProbeFailure condition when probe failures are detected and revision is in transition", + reconcilingRevisionName: clusterExtensionRevisionName, + revisionResult: mockRevisionResult{ + inTransition: true, + isComplete: false, + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "somephase", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + success: true, + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: true, + }, + }, + }, + mockObjectResult{ + success: false, + 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: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: false, + Messages: []string{ + "something bad happened", + "something worse happened", + }, + }, + }, + }, + }, + }, + mockPhaseResult{ + name: "someotherphase", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + success: false, + object: func() client.Object { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-configmap", + Namespace: "my-namespace", + }, + } + obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) + return obj + }(), + probes: map[string]machinery.ObjectProbeResult{ + boxcutter.ProgressProbeType: { + Success: false, + Messages: []string{ + "we have a problem", + }, + }, + }, + }, + }, + }, + }, + }, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) return []client.Object{ext, rev1} }, validate: func(t *testing.T, c client.Client) { @@ -81,14 +225,17 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) require.NotNil(t, cond) require.Equal(t, metav1.ConditionFalse, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonIncomplete, cond.Reason) - require.Equal(t, "Revision has not been rolled out completely.", cond.Message) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonProbeFailure, cond.Reason) + require.Equal(t, "Object Service.v1 my-namespace/my-service: something bad happened and something worse happened\nObject ConfigMap.v1 my-namespace/my-configmap: we have a problem", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "set Available:False:ProbeFailure condition when probe failures are detected", + name: "set Available:False:ProbeFailure condition when probe failures are detected and revision is not in transition", + reconcilingRevisionName: clusterExtensionRevisionName, revisionResult: mockRevisionResult{ + inTransition: false, + isComplete: false, phases: []machinery.PhaseResult{ mockPhaseResult{ name: "somephase", @@ -157,8 +304,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) return []client.Object{ext, rev1} }, validate: func(t *testing.T, c client.Client) { @@ -176,14 +322,37 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, }, { - name: "set Progressing:True:Progressing condition while revision is transitioning", + name: "set Progressing:True:Retrying when there's an error reconciling the revision", + revisionReconcileErr: errors.New("some error"), + reconcilingRevisionName: clusterExtensionRevisionName, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) + return []client.Object{ext, rev1} + }, + validate: func(t *testing.T, c client.Client) { + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, 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.ClusterExtensionRevisionReasonRetrying, 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", revisionResult: mockRevisionResult{ inTransition: true, }, + reconcilingRevisionName: clusterExtensionRevisionName, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) return []client.Object{ext, rev1} }, validate: func(t *testing.T, c client.Client) { @@ -195,25 +364,25 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonProgressing, cond.Reason) - require.Equal(t, "Rollout in progress.", cond.Message) + require.Equal(t, ocv1.ReasonRollingOut, cond.Reason) + require.Equal(t, "Revision 1.0.0 is rolling out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "remove Progressing condition once transition rollout is finished", + name: "set Progressing:True:Succeeded once transition rollout is finished", revisionResult: mockRevisionResult{ inTransition: false, }, + reconcilingRevisionName: clusterExtensionRevisionName, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, - Reason: ocv1.ClusterExtensionRevisionReasonProgressing, - Message: "some message", + Reason: ocv1.ReasonSucceeded, + Message: "Revision 1.0.0 is rolling out.", ObservedGeneration: 1, }) return []client.Object{ext, rev1} @@ -225,18 +394,22 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te }, rev) require.NoError(t, err) cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing) - require.Nil(t, cond) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, ocv1.ReasonSucceeded, cond.Reason) + require.Equal(t, "Revision 1.0.0 has rolled out.", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "set Available:True:Available and Succeeded:True:RolloutSuccess conditions on successful revision rollout", + name: "set Available:True:ProbesSucceeded and Succeeded:True:Succeeded conditions on successful revision rollout", revisionResult: mockRevisionResult{ isComplete: true, }, + reconcilingRevisionName: clusterExtensionRevisionName, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) return []client.Object{ext, rev1} }, validate: func(t *testing.T, c client.Client) { @@ -248,14 +421,21 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonAvailable, cond.Reason) - require.Equal(t, "Object is available and passes all probes.", cond.Message) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + 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.ClusterExtensionRevisionTypeProgressing) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionTrue, cond.Status) + require.Equal(t, ocv1.ReasonSucceeded, cond.Reason) + require.Equal(t, "Revision 1.0.0 has rolled out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded) require.NotNil(t, cond) require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1.ClusterExtensionRevisionReasonRolloutSuccess, cond.Reason) + require.Equal(t, ocv1.ReasonSucceeded, cond.Reason) require.Equal(t, "Revision succeeded rolling out.", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) }, @@ -265,30 +445,33 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te revisionResult: mockRevisionResult{ isComplete: true, }, + reconcilingRevisionName: "test-ext-3", existingObjs: func() []client.Object { ext := newTestClusterExtension() - prevRev1 := newTestClusterExtensionRevision(t, "prev-rev-1") - require.NoError(t, controllerutil.SetControllerReference(ext, prevRev1, testScheme)) - prevRev2 := newTestClusterExtensionRevision(t, "prev-rev-2") - require.NoError(t, controllerutil.SetControllerReference(ext, prevRev2, testScheme)) - currentRev := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - currentRev.Spec.Revision = 3 - require.NoError(t, controllerutil.SetControllerReference(ext, currentRev, testScheme)) - return []client.Object{ext, prevRev1, prevRev2, currentRev} + prevRev1 := newTestClusterExtensionRevision(t, "test-ext-1", ext, testScheme) + prevRev2 := newTestClusterExtensionRevision(t, "test-ext-2", ext, testScheme) + rev := newTestClusterExtensionRevision(t, "test-ext-3", ext, testScheme) + return []client.Object{ext, prevRev1, prevRev2, rev} }, validate: func(t *testing.T, c client.Client) { rev := &ocv1.ClusterExtensionRevision{} err := c.Get(t.Context(), client.ObjectKey{ - Name: "prev-rev-1", + Name: "test-ext-1", }, rev) require.NoError(t, err) require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateArchived, rev.Spec.LifecycleState) err = c.Get(t.Context(), client.ObjectKey{ - Name: "prev-rev-2", + Name: "test-ext-2", }, rev) require.NoError(t, err) require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateArchived, rev.Spec.LifecycleState) + + err = c.Get(t.Context(), client.ObjectKey{ + Name: "test-ext-3", + }, rev) + require.NoError(t, err) + require.Equal(t, ocv1.ClusterExtensionRevisionLifecycleStateActive, rev.Spec.LifecycleState) }, }, } { @@ -305,19 +488,23 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionProgression(t *te Client: testClient, RevisionEngine: &mockRevisionEngine{ reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { - return tc.revisionResult, nil + return tc.revisionResult, tc.revisionReconcileErr }, }, TrackingCache: &mockTrackingCache{client: testClient}, }).Reconcile(t.Context(), ctrl.Request{ NamespacedName: types.NamespacedName{ - Name: clusterExtensionRevisionName, + Name: tc.reconcilingRevisionName, }, }) - // reconcile cluster extensionr evision + // reconcile cluster extension revision require.Equal(t, ctrl.Result{}, result) - require.NoError(t, err) + if tc.revisionReconcileErr == nil { + require.NoError(t, err) + } else { + require.Contains(t, err.Error(), tc.revisionReconcileErr.Error()) + } // validate test case tc.validate(t, testClient) @@ -406,8 +593,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t } { t.Run(tc.name, func(t *testing.T) { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) // create extension and cluster extension testClient := fake.NewClientBuilder(). @@ -431,7 +617,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t }, }) - // reconcile cluster extensionr evision + // reconcile cluster extension revision require.Equal(t, ctrl.Result{ RequeueAfter: 10 * time.Second, }, result) @@ -454,13 +640,15 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { revisionResult machinery.RevisionResult revisionEngineTeardownFn func(*testing.T) func(context.Context, machinerytypes.Revision, ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) validate func(*testing.T, client.Client) + trackingCacheFreeFn func(context.Context, client.Object) error expectedErr string }{ { name: "teardown finalizer is removed", revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) rev1.Finalizers = []string{ "olm.operatorframework.io/teardown", } @@ -483,12 +671,11 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) rev1.Finalizers = []string{ "olm.operatorframework.io/teardown", } rev1.DeletionTimestamp = &metav1.Time{Time: time.Now()} - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) return []client.Object{rev1, ext} }, revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { @@ -505,20 +692,19 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { Name: clusterExtensionRevisionName, }, rev) require.Error(t, err) - require.True(t, errors.IsNotFound(err)) + require.True(t, apierrors.IsNotFound(err)) }, }, { - name: "surfaces tear down errors when deleted", + name: "set Available:Unknown:Reconciling and surface tear down errors when deleted", revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) rev1.Finalizers = []string{ "olm.operatorframework.io/teardown", } rev1.DeletionTimestamp = &metav1.Time{Time: time.Now()} - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) return []client.Object{rev1, ext} }, revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { @@ -535,19 +721,62 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { }, rev) require.NoError(t, err) require.NotContains(t, "olm.operatorframework.io/teardown", rev.Finalizers) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionUnknown, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonReconciling, cond.Reason) + require.Equal(t, "some teardown error", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { - name: "set Available condition to Unknown with reason Archived when archiving revision", + name: "set Available:Unknown:Reconciling and surface tracking cache cleanup errors when deleted", revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) + rev1.Finalizers = []string{ + "olm.operatorframework.io/teardown", + } + rev1.DeletionTimestamp = &metav1.Time{Time: time.Now()} + return []client.Object{rev1, ext} + }, + revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { + return &mockRevisionTeardownResult{ + isComplete: true, + }, nil + } + }, + trackingCacheFreeFn: func(ctx context.Context, object client.Object) error { + return fmt.Errorf("some tracking cache cleanup error") + }, + expectedErr: "some tracking cache cleanup error", + validate: func(t *testing.T, c client.Client) { + t.Log("cluster revision is not deleted and still contains finalizer") + rev := &ocv1.ClusterExtensionRevision{} + err := c.Get(t.Context(), client.ObjectKey{ + Name: clusterExtensionRevisionName, + }, rev) + require.NoError(t, err) + cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionUnknown, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonReconciling, cond.Reason) + require.Equal(t, "some tracking cache cleanup error", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) + }, + }, + { + name: "set Available:Archived:Unknown and Progressing:False:Archived conditions when a revision is archived", + revisionResult: mockRevisionResult{}, + existingObjs: func() []client.Object { + ext := newTestClusterExtension() + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) rev1.Finalizers = []string{ "olm.operatorframework.io/teardown", } rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) return []client.Object{rev1, ext} }, revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { @@ -569,6 +798,13 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { require.Equal(t, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) require.Equal(t, "revision is archived", cond.Message) require.Equal(t, int64(1), cond.ObservedGeneration) + + cond = meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) + require.Equal(t, "revision is archived", cond.Message) + require.Equal(t, int64(1), cond.ObservedGeneration) }, }, { @@ -576,7 +812,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) rev1.Finalizers = []string{ "olm.operatorframework.io/teardown", } @@ -588,7 +824,13 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { Message: "revision is archived", ObservedGeneration: rev1.Generation, }) - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) + meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterExtensionRevisionTypeProgressing, + Status: metav1.ConditionFalse, + Reason: ocv1.ClusterExtensionRevisionReasonArchived, + Message: "revision is archived", + ObservedGeneration: rev1.Generation, + }) return []client.Object{rev1, ext} }, revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { @@ -612,12 +854,11 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { revisionResult: mockRevisionResult{}, existingObjs: func() []client.Object { ext := newTestClusterExtension() - rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName) + rev1 := newTestClusterExtensionRevision(t, clusterExtensionRevisionName, ext, testScheme) rev1.Finalizers = []string{ "olm.operatorframework.io/teardown", } rev1.Spec.LifecycleState = ocv1.ClusterExtensionRevisionLifecycleStateArchived - require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) return []client.Object{rev1, ext} }, revisionEngineTeardownFn: func(t *testing.T) func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error) { @@ -654,7 +895,10 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) { }, teardown: tc.revisionEngineTeardownFn(t), }, - TrackingCache: &mockTrackingCache{client: testClient}, + TrackingCache: &mockTrackingCache{ + client: testClient, + freeFn: tc.trackingCacheFreeFn, + }, }).Reconcile(t.Context(), ctrl.Request{ NamespacedName: types.NamespacedName{ Name: clusterExtensionRevisionName, @@ -696,23 +940,30 @@ func newTestClusterExtension() *ocv1.ClusterExtension { } } -func newTestClusterExtensionRevision(t *testing.T, name string) *ocv1.ClusterExtensionRevision { +func newTestClusterExtensionRevision(t *testing.T, revisionName string, ext *ocv1.ClusterExtension, scheme *runtime.Scheme) *ocv1.ClusterExtensionRevision { t.Helper() // Extract revision number from name (e.g., "rev-1" -> 1, "test-ext-10" -> 10) - revNum := controllers.ExtractRevisionNumber(t, name) + revNum := controllers.ExtractRevisionNumber(t, revisionName) - return &ocv1.ClusterExtensionRevision{ + rev := &ocv1.ClusterExtensionRevision{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - UID: types.UID(name), + Name: revisionName, + UID: types.UID(revisionName), Generation: int64(1), + Annotations: map[string]string{ + labels.PackageNameKey: "some-package", + labels.BundleNameKey: "some-package.v1.0.0", + labels.BundleReferenceKey: "registry.io/some-repo/some-package:v1.0.0", + labels.BundleVersionKey: "1.0.0", + }, Labels: map[string]string{ labels.OwnerNameKey: "test-ext", }, }, Spec: ocv1.ClusterExtensionRevisionSpec{ - Revision: revNum, + LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive, + Revision: revNum, Phases: []ocv1.ClusterExtensionRevisionPhase{ { Name: "everything", @@ -733,6 +984,8 @@ func newTestClusterExtensionRevision(t *testing.T, name string) *ocv1.ClusterExt }, }, } + require.NoError(t, controllerutil.SetControllerReference(ext, rev, scheme)) + return rev } type mockRevisionEngine struct { @@ -883,6 +1136,7 @@ func (m mockRevisionTeardownResult) String() string { type mockTrackingCache struct { client client.Client + freeFn func(context.Context, client.Object) error } func (m *mockTrackingCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { @@ -902,5 +1156,8 @@ func (m *mockTrackingCache) Watch(ctx context.Context, user client.Object, gvks } func (m *mockTrackingCache) Free(ctx context.Context, user client.Object) error { + if m.freeFn != nil { + return m.freeFn(ctx, user) + } return nil } diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index e536cd72a6..968b274b77 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -644,6 +644,9 @@ 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 @@ -815,22 +818,21 @@ spec: ClusterExtensionRevision. The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - - When Progressing is not present, the revision is not currently in transition. + - When status is True and reason is RollingOut, the ClusterExtensionRevision rollout is actively making progress and is in transition. + - When status is True and reason is Retrying, the ClusterExtensionRevision has encountered an error that could be resolved on subsequent reconciliation attempts. + - When status is True and reason is Succeeded, the ClusterExtensionRevision has reached the desired state. + - When status is False and reason is Blocked, the ClusterExtensionRevision has encountered an error that requires manual intervention for recovery. + - When status is False and reason is Archived, the ClusterExtensionRevision 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 Available, the revision has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. + - When status is True and reason is ProbesSucceeded, the ClusterExtensionRevision 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 False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. + - When status is Unknown and reason is Reconciling, the ClusterExtensionRevision has encountered an error that prevented it from observing the probes. + - When status is Unknown and reason is Archived, the ClusterExtensionRevision has been archived and its objects have been torn down. + - When status is Unknown and reason is Migrated, the ClusterExtensionRevision was migrated from an existing release and object status probe results have not yet been observed. The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. + - When status is True and reason is Succeeded, the ClusterExtensionRevision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. @@ -1416,6 +1418,8 @@ spec: When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts. When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery. + When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out. + When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition. These are indications from a package owner to guide users away from a particular package, channel, or bundle. BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index f88debab0f..d8fc64f84f 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -609,6 +609,9 @@ 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 @@ -780,22 +783,21 @@ spec: ClusterExtensionRevision. The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - - When Progressing is not present, the revision is not currently in transition. + - When status is True and reason is RollingOut, the ClusterExtensionRevision rollout is actively making progress and is in transition. + - When status is True and reason is Retrying, the ClusterExtensionRevision has encountered an error that could be resolved on subsequent reconciliation attempts. + - When status is True and reason is Succeeded, the ClusterExtensionRevision has reached the desired state. + - When status is False and reason is Blocked, the ClusterExtensionRevision has encountered an error that requires manual intervention for recovery. + - When status is False and reason is Archived, the ClusterExtensionRevision 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 Available, the revision has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. + - When status is True and reason is ProbesSucceeded, the ClusterExtensionRevision 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 False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. + - When status is Unknown and reason is Reconciling, the ClusterExtensionRevision has encountered an error that prevented it from observing the probes. + - When status is Unknown and reason is Archived, the ClusterExtensionRevision has been archived and its objects have been torn down. + - When status is Unknown and reason is Migrated, the ClusterExtensionRevision was migrated from an existing release and object status probe results have not yet been observed. The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. + - When status is True and reason is Succeeded, the ClusterExtensionRevision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. @@ -1381,6 +1383,8 @@ spec: When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts. When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery. + When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out. + When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition. These are indications from a package owner to guide users away from a particular package, channel, or bundle. BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog. diff --git a/test/e2e/cluster_extension_revision_test.go b/test/e2e/cluster_extension_revision_test.go new file mode 100644 index 0000000000..322b6fd211 --- /dev/null +++ b/test/e2e/cluster_extension_revision_test.go @@ -0,0 +1,209 @@ +package e2e + +import ( + "context" + "fmt" + "os" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/remotecommand" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" + . "github.com/operator-framework/operator-controller/internal/shared/util/testutils" + . "github.com/operator-framework/operator-controller/test/helpers" +) + +func TestClusterExtensionRevision(t *testing.T) { + SkipIfFeatureGateDisabled(t, string(features.BoxcutterRuntime)) + t.Log("When a cluster extension is installed from a catalog") + t.Log("When the extension bundle format is registry+v1") + + clusterExtension, extensionCatalog, sa, ns := TestInit(t) + defer TestCleanup(t, extensionCatalog, clusterExtension, sa, ns) + defer CollectTestArtifacts(t, artifactName, c, cfg) + + clusterExtension.Spec = ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: "test", + Version: "1.0.1", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name}, + }, + }, + }, + Namespace: ns.Name, + ServiceAccount: ocv1.ServiceAccountReference{ + Name: sa.Name, + }, + } + t.Log("It resolves the specified package with correct bundle path") + t.Log("By creating the ClusterExtension resource") + require.NoError(t, c.Create(context.Background(), clusterExtension)) + + t.Log("By eventually reporting a successful resolution and bundle path") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + }, pollDuration, pollInterval) + + t.Log("By revision-1 eventually reporting Progressing:True:Succeeded and Available:True:ProbesSucceeded conditions") + var clusterExtensionRevision ocv1.ClusterExtensionRevision + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually reporting progressing as True") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually installing the package successfully") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + require.Contains(ct, cond.Message, "Installed bundle") + require.NotEmpty(ct, clusterExtension.Status.Install.Bundle) + }, pollDuration, pollInterval) + + t.Log("Check Deployment Availability Probe") + t.Log("By making the operator pod not ready") + podName := getPodName(t, clusterExtension.Spec.Namespace, client.MatchingLabels{"app": "olme2etest"}) + podExec(t, clusterExtension.Spec.Namespace, podName, []string{"rm", "/var/www/ready"}) + + t.Log("By revision-1 eventually reporting Progressing:True:Succeeded and Available:False:ProbeFailure conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbeFailure, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By making the operator pod ready") + podName = getPodName(t, clusterExtension.Spec.Namespace, client.MatchingLabels{"app": "olme2etest"}) + podExec(t, clusterExtension.Spec.Namespace, podName, []string{"touch", "/var/www/ready"}) + + t.Log("By revision-1 eventually reporting Progressing:True:Succeeded and Available:True:ProbesSucceeded conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("Check archiving") + t.Log("By upgrading the cluster extension to v1.2.0") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + clusterExtension.Spec.Source.Catalog.Version = "1.2.0" + require.NoError(t, c.Update(context.Background(), clusterExtension)) + }, pollDuration, pollInterval) + + t.Log("By revision-2 eventually reporting Progressing:True:Succeeded and Available:True:ProbesSucceeded conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-2", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonProbesSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually reporting progressing, available, and installed as True") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + require.Contains(ct, cond.Message, "Installed bundle") + require.NotEmpty(ct, clusterExtension.Status.Install.Bundle) + }, pollDuration, pollInterval) + + t.Log("By revision-1 eventually reporting Progressing:False:Archived and Available:Unknown:Archived conditions") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("%s-1", clusterExtension.Name)}, &clusterExtensionRevision)) + cond := apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionFalse, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) + + cond = apimeta.FindStatusCondition(clusterExtensionRevision.Status.Conditions, ocv1.ClusterExtensionRevisionTypeAvailable) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionUnknown, cond.Status) + require.Equal(ct, ocv1.ClusterExtensionRevisionReasonArchived, cond.Reason) + }, pollDuration, pollInterval) +} + +func getPodName(t *testing.T, podNamespace string, matchingLabels client.MatchingLabels) string { + var podList corev1.PodList + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.List(context.Background(), &podList, client.InNamespace(podNamespace), matchingLabels)) + podList.Items = slices.DeleteFunc(podList.Items, func(pod corev1.Pod) bool { + // Ignore terminating pods + return pod.DeletionTimestamp != nil + }) + require.Len(ct, podList.Items, 1) + }, pollDuration, pollInterval) + return podList.Items[0].Name +} + +func podExec(t *testing.T, podNamespace string, podName string, cmd []string) { + req := cs.CoreV1().RESTClient().Post().Resource("pods").Name(podName).Namespace(podNamespace).SubResource("exec") + req.VersionedParams(&corev1.PodExecOptions{ + Command: cmd, + Stdout: true, + }, scheme.ParameterCodec) + exec, err := remotecommand.NewSPDYExecutor(ctrl.GetConfigOrDie(), "POST", req.URL()) + require.NoError(t, err) + err = exec.StreamWithContext(context.Background(), remotecommand.StreamOptions{Stdout: os.Stdout}) + require.NoError(t, err) +} diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index aa033a2f1e..bb28ccdf77 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -8,6 +8,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,6 +21,7 @@ import ( var ( cfg *rest.Config c client.Client + cs *kubernetes.Clientset ) const ( @@ -35,6 +37,9 @@ func TestMain(m *testing.M) { c, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) utilruntime.Must(err) + cs, err = kubernetes.NewForConfig(cfg) + utilruntime.Must(err) + res := m.Run() path := os.Getenv(testSummaryOutputEnvVar) if path == "" {