From 082aa80ae3bf95fb7f4c4f7c606b12f5563be08d Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Tue, 27 Oct 2026 15:25:53 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20ClusterObjectDeployment:=20Progress?= =?UTF-8?q?ing=20reasons=20for=20impeded=20head=20revision=20rollouts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Map the head active ClusterObjectSet revision's Ready reason onto the ClusterObjectDeployment (COD) Progressing condition so a user can tell a rollout that is merely in progress from one stalled on something they must fix: - RevisionObjectCollision (False, hard stall) - objects owned by another controller or already exist; resolve ownership. - RevisionInvalidSpecification (False, hard stall) - the revision's rendered objects are invalid; fix the spec. Distinct from the COD's own InvalidSpecification (invalid spec.template.metadata). - RevisionPreconditionsNotMet (True, keeps retrying) - an unmet precondition (e.g. RBAC, dependency CRD, referenced Secret); governed by the deadline. Hard stalls short-circuit the progress deadline (the actionable cause is more useful than ProgressDeadlineExceeded). The COD reports its own generic message naming the revision and directs the user to inspect it, rather than echoing the ClusterObjectSet message. Updates the status conditions docstring (regenerating CRDs, apply configurations, and the API reference), adds unit tests for every branch and the hard-stall-beats-deadline precedence, adds an e2e feature, and documents the reasons in the ClusterObjectSets concept doc. Co-Authored-By: Claude Opus 4.8 --- api/v1/clusterobjectdeployment_types.go | 14 + .../api/v1/clusterobjectdeploymentstatus.go | 11 + docs/api-reference/olmv1-api-reference.md | 2 +- docs/draft/concepts/clusterobjectsets.md | 27 ++ ...framework.io_clusterobjectdeployments.yaml | 11 + .../clusterobjectdeployment_controller.go | 39 +++ ...clusterobjectdeployment_controller_test.go | 91 ++++++- .../clusterobjectdeployment_deadline_test.go | 13 +- manifests/experimental-e2e.yaml | 11 + manifests/experimental.yaml | 11 + .../features/cod_progressing_reasons.feature | 243 ++++++++++++++++++ .../features/cod_progression_timeout.feature | 63 ----- test/e2e/steps/steps.go | 87 +++++++ 13 files changed, 555 insertions(+), 68 deletions(-) create mode 100644 test/e2e/features/cod_progressing_reasons.feature delete mode 100644 test/e2e/features/cod_progression_timeout.feature diff --git a/api/v1/clusterobjectdeployment_types.go b/api/v1/clusterobjectdeployment_types.go index 86d02882bf..0e2cea864d 100644 --- a/api/v1/clusterobjectdeployment_types.go +++ b/api/v1/clusterobjectdeployment_types.go @@ -33,6 +33,9 @@ const ( ClusterObjectDeploymentReasonNewClusterObjectSetProgressing = "NewClusterObjectSetProgressing" ClusterObjectDeploymentReasonProgressDeadlineExceeded = ReasonProgressDeadlineExceeded ClusterObjectDeploymentReasonInvalidSpecification = "InvalidSpecification" + ClusterObjectDeploymentReasonRevisionPreconditionsNotMet = "RevisionPreconditionsNotMet" + ClusterObjectDeploymentReasonRevisionObjectCollision = "RevisionObjectCollision" + ClusterObjectDeploymentReasonRevisionInvalidSpecification = "RevisionInvalidSpecification" // Available condition reasons. ClusterObjectDeploymentReasonAvailable = "Available" @@ -150,6 +153,17 @@ type ClusterObjectDeploymentStatus struct { // contains labels or annotations that are not valid for the resulting // ClusterObjectSet; no new revision is created until the metadata is corrected. // + // While a rollout is incomplete, Progressing also reflects why the latest (head + // active) ClusterObjectSet revision is not yet Ready. It is True with reason + // RevisionPreconditionsNotMet when the revision's preconditions are not yet met + // (for example RBAC or a dependency CRD that has not propagated); the controller + // keeps retrying. It is False with reason RevisionObjectCollision when the + // revision's objects are owned by another controller or already exist, which the + // user must resolve. It is False with reason RevisionInvalidSpecification when the + // revision's own objects are invalid; this is distinct from InvalidSpecification + // above, which refers to the deployment's spec.template.metadata. Inspect the + // named ClusterObjectSet revision for the specific cause. + // // Available is True with reason Available when at least one active (non-archived) // ClusterObjectSet revision is Ready, so the deployment remains available across // rollouts while a previous revision is still Ready. It is False with reason diff --git a/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go b/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go index 2b93b2f5ab..092520dcac 100644 --- a/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go +++ b/applyconfigurations/api/v1/clusterobjectdeploymentstatus.go @@ -38,6 +38,17 @@ type ClusterObjectDeploymentStatusApplyConfiguration struct { // contains labels or annotations that are not valid for the resulting // ClusterObjectSet; no new revision is created until the metadata is corrected. // + // While a rollout is incomplete, Progressing also reflects why the latest (head + // active) ClusterObjectSet revision is not yet Ready. It is True with reason + // RevisionPreconditionsNotMet when the revision's preconditions are not yet met + // (for example RBAC or a dependency CRD that has not propagated); the controller + // keeps retrying. It is False with reason RevisionObjectCollision when the + // revision's objects are owned by another controller or already exist, which the + // user must resolve. It is False with reason RevisionInvalidSpecification when the + // revision's own objects are invalid; this is distinct from InvalidSpecification + // above, which refers to the deployment's spec.template.metadata. Inspect the + // named ClusterObjectSet revision for the specific cause. + // // Available is True with reason Available when at least one active (non-archived) // ClusterObjectSet revision is Ready, so the deployment remains available across // rollouts while a previous revision is still Ready. It is False with reason diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 2c33623f02..0ce3a4ffdc 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -458,7 +458,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions describes rollout progress and availability for the observed generation.
Progressing is True with reason NewClusterObjectSetProgressing while the desired
rollout is incomplete, or False with reason NewClusterObjectSetProgressed after
completion. Completion remains recorded even if the completed revision later
becomes unhealthy. Progressing is False with reason ProgressDeadlineExceeded when
the latest revision does not roll out within spec.template.spec.progressDeadlineMinutes.
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
Available is True with reason Available when at least one active (non-archived)
ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
NoActiveRevision when no active revision exists. | | Optional: \{\}
| +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions describes rollout progress and availability for the observed generation.
Progressing is True with reason NewClusterObjectSetProgressing while the desired
rollout is incomplete, or False with reason NewClusterObjectSetProgressed after
completion. Completion remains recorded even if the completed revision later
becomes unhealthy. Progressing is False with reason ProgressDeadlineExceeded when
the latest revision does not roll out within spec.template.spec.progressDeadlineMinutes.
Progressing is False with reason InvalidSpecification when spec.template.metadata
contains labels or annotations that are not valid for the resulting
ClusterObjectSet; no new revision is created until the metadata is corrected.
While a rollout is incomplete, Progressing also reflects why the latest (head
active) ClusterObjectSet revision is not yet Ready. It is True with reason
RevisionPreconditionsNotMet when the revision's preconditions are not yet met
(for example RBAC or a dependency CRD that has not propagated); the controller
keeps retrying. It is False with reason RevisionObjectCollision when the
revision's objects are owned by another controller or already exist, which the
user must resolve. It is False with reason RevisionInvalidSpecification when the
revision's own objects are invalid; this is distinct from InvalidSpecification
above, which refers to the deployment's spec.template.metadata. Inspect the
named ClusterObjectSet revision for the specific cause.
Available is True with reason Available when at least one active (non-archived)
ClusterObjectSet revision is Ready, so the deployment remains available across
rollouts while a previous revision is still Ready. It is False with reason
Unavailable when active revisions exist but none are Ready yet, or with reason
NoActiveRevision when no active revision exists. | | Optional: \{\}
| #### ClusterObjectDeploymentTemplate diff --git a/docs/draft/concepts/clusterobjectsets.md b/docs/draft/concepts/clusterobjectsets.md index f7f328d671..df492d1d9e 100644 --- a/docs/draft/concepts/clusterobjectsets.md +++ b/docs/draft/concepts/clusterobjectsets.md @@ -264,6 +264,33 @@ recorded, the COD reports `Progressing=False` with reason timing. The COD can remain `Available=True` if an older active revision is still ready. +### ClusterObjectDeployment Progressing + +While a rollout is incomplete, a ClusterObjectDeployment (COD) reflects *why* its +head active revision is not yet `Ready` on its own `Progressing` condition, so you +can tell a rollout that is merely in progress from one that is stalled on +something you must fix. Each reason names the revision; inspect that +ClusterObjectSet for the specific cause. + +| Status | Reason | Meaning | What to do | +| --- | --- | --- | --- | +| True | `NewClusterObjectSetProgressing` | The head revision is rolling out (`Pending`/`ObjectsNotReady`) | Wait | +| True | `RevisionPreconditionsNotMet` | The head revision's precondition is not yet met (e.g. a mutable/invalid referenced Secret, or RBAC/dependency CRDs not yet propagated); it keeps retrying | Satisfy the precondition (grant RBAC, install the dependency, fix the Secret); it recovers automatically | +| False | `RevisionObjectCollision` | The head revision's objects are owned by another controller or already exist; it will not roll out without intervention | Resolve ownership of the conflicting objects | +| False | `RevisionInvalidSpecification` | The head revision's own objects are invalid | Correct the template so the rendered objects are valid | +| False | `ProgressDeadlineExceeded` | A still-retrying revision exceeded its deadline (see [Progress deadline](#progress-deadline)) | See above | +| False | `InvalidSpecification` | The deployment's own `spec.template.metadata` is invalid, so no revision is created | Fix the template labels/annotations | +| False | `NewClusterObjectSetProgressed` | The head revision completed its rollout | None | + +`RevisionObjectCollision` and `RevisionInvalidSpecification` are *hard stalls*: +they will not resolve on their own, so they report `Progressing=False` and take +precedence over `ProgressDeadlineExceeded` (the actionable cause is more useful +than "the clock ran out"). `RevisionPreconditionsNotMet` keeps retrying, so it +stays `Progressing=True` and the progress deadline still governs it. Note that +`RevisionInvalidSpecification` (the revision's rendered objects) is distinct from +the deployment's own `InvalidSpecification` (invalid `spec.template.metadata`, +reported before any revision is created). + ### Completion (`status.completedAt`) The first time a revision becomes ready, the controller stamps `status.completedAt` with the current time. This timestamp is set once and never changes, marking that this revision successfully rolled out at least once — it persists even if the revision later becomes not ready. The ClusterExtension controller uses it to determine the installed revision. diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml index 1879b3eb4f..48294b7a35 100644 --- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml +++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml @@ -551,6 +551,17 @@ spec: contains labels or annotations that are not valid for the resulting ClusterObjectSet; no new revision is created until the metadata is corrected. + While a rollout is incomplete, Progressing also reflects why the latest (head + active) ClusterObjectSet revision is not yet Ready. It is True with reason + RevisionPreconditionsNotMet when the revision's preconditions are not yet met + (for example RBAC or a dependency CRD that has not propagated); the controller + keeps retrying. It is False with reason RevisionObjectCollision when the + revision's objects are owned by another controller or already exist, which the + user must resolve. It is False with reason RevisionInvalidSpecification when the + revision's own objects are invalid; this is distinct from InvalidSpecification + above, which refers to the deployment's spec.template.metadata. Inspect the + named ClusterObjectSet revision for the specific cause. + Available is True with reason Available when at least one active (non-archived) ClusterObjectSet revision is Ready, so the deployment remains available across rollouts while a previous revision is still Ready. It is False with reason diff --git a/internal/object-controller/controllers/clusterobjectdeployment_controller.go b/internal/object-controller/controllers/clusterobjectdeployment_controller.go index e5bb4302ad..ad3caf37b6 100644 --- a/internal/object-controller/controllers/clusterobjectdeployment_controller.go +++ b/internal/object-controller/controllers/clusterobjectdeployment_controller.go @@ -142,12 +142,41 @@ func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context return ctrl.Result{}, nil } + // Reflect why the head revision is not yet Ready. Hard stalls will not resolve + // without user action, so they report Progressing=False and short-circuit the + // deadline (a more actionable reason than ProgressDeadlineExceeded). The COD + // surfaces its own generic, revision-naming message and does not echo the + // revision's Ready message; the user inspects the named ClusterObjectSet for the + // specific cause. + readyReason := headRevisionReadyReason(latest) + switch readyReason { + case ocv1.ClusterObjectSetReasonInvalidSpecification: + setProgressing(cod, metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonRevisionInvalidSpecification, + fmt.Sprintf("latest revision %q has an invalid specification; inspect the ClusterObjectSet for details", latest.Name)) + return ctrl.Result{}, nil + case ocv1.ClusterObjectSetReasonObjectCollision: + setProgressing(cod, metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonRevisionObjectCollision, + fmt.Sprintf("latest revision %q cannot take ownership of one or more objects owned by another controller or already present; inspect the ClusterObjectSet for details", latest.Name)) + return ctrl.Result{}, nil + } + remaining, hasDeadline := durationUntilDeploymentDeadline(c.Clock, cod, latest) if hasDeadline && remaining <= 0 { setProgressing(cod, metav1.ConditionFalse, ocv1.ClusterObjectDeploymentReasonProgressDeadlineExceeded, "latest revision exceeded its progress deadline") return ctrl.Result{}, nil } + // PreconditionsNotMet is a still-retrying state: the controller keeps retrying, + // so Progressing stays True and the deadline above governs it. + if readyReason == ocv1.ClusterObjectSetReasonPreconditionsNotMet { + setProgressing(cod, metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonRevisionPreconditionsNotMet, + fmt.Sprintf("latest revision %q is waiting for preconditions to be met; inspect the ClusterObjectSet for details", latest.Name)) + if hasDeadline { + return ctrl.Result{RequeueAfter: remaining}, nil + } + return ctrl.Result{}, nil + } + setProgressing(cod, metav1.ConditionTrue, ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, "latest revision is progressing") if hasDeadline { return ctrl.Result{RequeueAfter: remaining}, nil @@ -155,6 +184,16 @@ func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context return ctrl.Result{}, nil } +// headRevisionReadyReason returns the Ready condition reason of the head active +// revision, or "" when the revision has no Ready condition yet. +func headRevisionReadyReason(cos *ocv1.ClusterObjectSet) string { + cond := apimeta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeReady) + if cond == nil { + return "" + } + return cond.Reason +} + // listRevisions returns all ClusterObjectSets in the deployment's group, sorted // ascending by revision number. func (c *ClusterObjectDeploymentReconciler) listRevisions(ctx context.Context, group string) ([]ocv1.ClusterObjectSet, error) { diff --git a/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go b/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go index f1c7284c8a..7f798e9239 100644 --- a/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go @@ -122,6 +122,26 @@ func expireRevisionDeadline(t *testing.T, c client.Client, name string) { require.NoError(t, c.Update(t.Context(), cos)) } +// revisionReadyProbeMessage is a distinctive message written onto a revision's +// Ready condition so tests can assert the COD surfaces its own generic message +// rather than echoing the revision's message. +const revisionReadyProbeMessage = "COS-ONLY-REVISION-READY-MESSAGE" + +// setRevisionReadyReason sets the named revision's Ready condition to False with +// the given reason and a distinctive message (revisionReadyProbeMessage). +func setRevisionReadyReason(t *testing.T, c client.Client, name, reason string) { + t.Helper() + cos, ok := getCOS(t, c, name) + require.True(t, ok) + apimeta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ + Type: ocv1.ClusterObjectSetTypeReady, + Status: metav1.ConditionFalse, + Reason: reason, + Message: revisionReadyProbeMessage, + }) + require.NoError(t, c.Status().Update(t.Context(), cos)) +} + func Test_ClusterObjectDeploymentReconciler_Available(t *testing.T) { t.Run("no active revision before the first revision exists", func(t *testing.T) { c := newCODClient(t, codFixture()) @@ -381,10 +401,15 @@ func Test_TemplateHash_MetadataParticipates(t *testing.T) { func Test_ClusterObjectDeploymentReconciler_NoNewRevisionWhenTemplateUnchanged(t *testing.T) { for _, tc := range []struct { - name string - prep func(t *testing.T, c client.Client, cosName string) + name string + prep func(t *testing.T, c client.Client, cosName string) + wantStatus metav1.ConditionStatus wantReason string + // wantMsgContains, when set, must be a substring of the COD Progressing + // message; wantMsgExcludes, when set, must not be. + wantMsgContains string + wantMsgExcludes string }{ { name: "completed latest revision reports Progressed", @@ -409,6 +434,62 @@ func Test_ClusterObjectDeploymentReconciler_NoNewRevisionWhenTemplateUnchanged(t wantStatus: metav1.ConditionFalse, wantReason: ocv1.ClusterObjectDeploymentReasonProgressDeadlineExceeded, }, + { + name: "head revision ObjectCollision reports RevisionObjectCollision and is not progressing", + prep: func(t *testing.T, c client.Client, cosName string) { + setRevisionReadyReason(t, c, cosName, ocv1.ClusterObjectSetReasonObjectCollision) + }, + wantStatus: metav1.ConditionFalse, + wantReason: ocv1.ClusterObjectDeploymentReasonRevisionObjectCollision, + wantMsgContains: clusterObjectDeploymentName + "-1", + wantMsgExcludes: revisionReadyProbeMessage, + }, + { + name: "head revision InvalidSpecification reports RevisionInvalidSpecification and is not progressing", + prep: func(t *testing.T, c client.Client, cosName string) { + setRevisionReadyReason(t, c, cosName, ocv1.ClusterObjectSetReasonInvalidSpecification) + }, + wantStatus: metav1.ConditionFalse, + wantReason: ocv1.ClusterObjectDeploymentReasonRevisionInvalidSpecification, + wantMsgContains: clusterObjectDeploymentName + "-1", + wantMsgExcludes: revisionReadyProbeMessage, + }, + { + name: "head revision PreconditionsNotMet reports RevisionPreconditionsNotMet and keeps progressing", + prep: func(t *testing.T, c client.Client, cosName string) { + setRevisionReadyReason(t, c, cosName, ocv1.ClusterObjectSetReasonPreconditionsNotMet) + }, + wantStatus: metav1.ConditionTrue, + wantReason: ocv1.ClusterObjectDeploymentReasonRevisionPreconditionsNotMet, + wantMsgContains: clusterObjectDeploymentName + "-1", + wantMsgExcludes: revisionReadyProbeMessage, + }, + { + name: "head revision with no Ready condition falls through to NewClusterObjectSetProgressing", + prep: nil, + wantStatus: metav1.ConditionTrue, + wantReason: ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, + }, + { + name: "head revision with unrecognized Ready reason falls through to NewClusterObjectSetProgressing", + prep: func(t *testing.T, c client.Client, cosName string) { + setRevisionReadyReason(t, c, cosName, ocv1.ClusterObjectSetReasonObjectsNotReady) + }, + wantStatus: metav1.ConditionTrue, + wantReason: ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressing, + }, + { + name: "completed revision that also reports ObjectCollision still reports Progressed", + prep: func(t *testing.T, c client.Client, cosName string) { + setRevisionReadyReason(t, c, cosName, ocv1.ClusterObjectSetReasonObjectCollision) + cos, ok := getCOS(t, c, cosName) + require.True(t, ok) + cos.Status.CompletedAt = metav1.Now() + require.NoError(t, c.Status().Update(t.Context(), cos)) + }, + wantStatus: metav1.ConditionFalse, + wantReason: ocv1.ClusterObjectDeploymentReasonNewClusterObjectSetProgressed, + }, } { t.Run(tc.name, func(t *testing.T) { c := newCODClient(t, codFixture()) @@ -432,6 +513,12 @@ func Test_ClusterObjectDeploymentReconciler_NoNewRevisionWhenTemplateUnchanged(t require.NotNil(t, cond) require.Equal(t, tc.wantStatus, cond.Status) require.Equal(t, tc.wantReason, cond.Reason) + if tc.wantMsgContains != "" { + require.Contains(t, cond.Message, tc.wantMsgContains) + } + if tc.wantMsgExcludes != "" { + require.NotContains(t, cond.Message, tc.wantMsgExcludes) + } }) } } diff --git a/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go b/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go index b95fdb90bf..87544298c1 100644 --- a/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go +++ b/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go @@ -211,11 +211,16 @@ func TestClusterObjectDeploymentReconcilerDeadlineWithoutCOSDeadlineCondition(t name string status metav1.ConditionStatus reason string + // wantReason is the expected COD Progressing reason. Empty means the deadline + // governs the outcome (ProgressDeadlineExceeded); a hard-stall revision reason + // short-circuits the deadline and is named explicitly. + wantReason string }{ {name: "no Ready condition"}, {name: "pending", status: metav1.ConditionUnknown, reason: ocv1.ClusterObjectSetReasonPending}, {name: "preconditions not met", status: metav1.ConditionFalse, reason: ocv1.ClusterObjectSetReasonPreconditionsNotMet}, - {name: "invalid specification", status: metav1.ConditionFalse, reason: ocv1.ClusterObjectSetReasonInvalidSpecification}, + {name: "invalid specification", status: metav1.ConditionFalse, reason: ocv1.ClusterObjectSetReasonInvalidSpecification, wantReason: ocv1.ClusterObjectDeploymentReasonRevisionInvalidSpecification}, + {name: "object collision", status: metav1.ConditionFalse, reason: ocv1.ClusterObjectSetReasonObjectCollision, wantReason: ocv1.ClusterObjectDeploymentReasonRevisionObjectCollision}, {name: "objects not ready", status: metav1.ConditionFalse, reason: ocv1.ClusterObjectSetReasonObjectsNotReady}, } { t.Run(tc.name, func(t *testing.T) { @@ -243,7 +248,11 @@ func TestClusterObjectDeploymentReconcilerDeadlineWithoutCOSDeadlineCondition(t condition := progressing(t, c) require.NotNil(t, condition) require.Equal(t, metav1.ConditionFalse, condition.Status) - require.Equal(t, ocv1.ClusterObjectDeploymentReasonProgressDeadlineExceeded, condition.Reason) + wantReason := tc.wantReason + if wantReason == "" { + wantReason = ocv1.ClusterObjectDeploymentReasonProgressDeadlineExceeded + } + require.Equal(t, wantReason, condition.Reason) cos, found = getCOS(t, c, cos.Name) require.True(t, found) if tc.reason != "" { diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 941c81db1b..d96a444aae 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1880,6 +1880,17 @@ spec: contains labels or annotations that are not valid for the resulting ClusterObjectSet; no new revision is created until the metadata is corrected. + While a rollout is incomplete, Progressing also reflects why the latest (head + active) ClusterObjectSet revision is not yet Ready. It is True with reason + RevisionPreconditionsNotMet when the revision's preconditions are not yet met + (for example RBAC or a dependency CRD that has not propagated); the controller + keeps retrying. It is False with reason RevisionObjectCollision when the + revision's objects are owned by another controller or already exist, which the + user must resolve. It is False with reason RevisionInvalidSpecification when the + revision's own objects are invalid; this is distinct from InvalidSpecification + above, which refers to the deployment's spec.template.metadata. Inspect the + named ClusterObjectSet revision for the specific cause. + Available is True with reason Available when at least one active (non-archived) ClusterObjectSet revision is Ready, so the deployment remains available across rollouts while a previous revision is still Ready. It is False with reason diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 1f56f63d51..0f13d68f83 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1841,6 +1841,17 @@ spec: contains labels or annotations that are not valid for the resulting ClusterObjectSet; no new revision is created until the metadata is corrected. + While a rollout is incomplete, Progressing also reflects why the latest (head + active) ClusterObjectSet revision is not yet Ready. It is True with reason + RevisionPreconditionsNotMet when the revision's preconditions are not yet met + (for example RBAC or a dependency CRD that has not propagated); the controller + keeps retrying. It is False with reason RevisionObjectCollision when the + revision's objects are owned by another controller or already exist, which the + user must resolve. It is False with reason RevisionInvalidSpecification when the + revision's own objects are invalid; this is distinct from InvalidSpecification + above, which refers to the deployment's spec.template.metadata. Inspect the + named ClusterObjectSet revision for the specific cause. + Available is True with reason Available when at least one active (non-archived) ClusterObjectSet revision is Ready, so the deployment remains available across rollouts while a previous revision is still Ready. It is False with reason diff --git a/test/e2e/features/cod_progressing_reasons.feature b/test/e2e/features/cod_progressing_reasons.feature new file mode 100644 index 0000000000..b72885db7c --- /dev/null +++ b/test/e2e/features/cod_progressing_reasons.feature @@ -0,0 +1,243 @@ +@BoxcutterRuntime +Feature: ClusterObjectDeployment impeded-rollout Progressing reasons + + As an OLM user applying a ClusterObjectDeployment, I want the Progressing + condition to tell me when a rollout is impeded by the head active + ClusterObjectSet revision and whether I need to intervene, so that I can tell a + rollout that is merely in progress from one that is stalled on something I must + fix. The deployment reports its own generic message naming the revision and + directs me to inspect it for the specific cause. + + Background: + Given OLM is available + + # A revision that cannot take ownership of its objects will not become Ready + # without user action, so the deployment reports Progressing=False. + Scenario: Progressing is False with RevisionObjectCollision when the head revision collides + Given namespace "${TEST_NAMESPACE}" is available + And resource is applied + """ + apiVersion: v1 + kind: ConfigMap + metadata: + name: cod-collide-cm + namespace: ${TEST_NAMESPACE} + data: + k: pre-existing + """ + When resource is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectDeployment + metadata: + name: ${NAME} + spec: + template: + spec: + collisionProtection: Prevent + phases: + - name: resources + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: cod-collide-cm + namespace: ${TEST_NAMESPACE} + data: + k: managed + """ + Then ClusterObjectSet "${NAME}-1" reports Ready as False with Reason ObjectCollision + And ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason RevisionObjectCollision and Message includes: + """ + ${NAME}-1 + """ + And ClusterObjectDeployment "${NAME}" reports Available as False with Reason Unavailable + # Resolve the collision: remove the conflicting object. The revision takes + # ownership, becomes Ready, and the deployment progresses. + When resource "configmap/cod-collide-cm" is removed + Then ClusterObjectSet "${NAME}-1" reports Ready as True with Reason AllObjectsReady + And ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason NewClusterObjectSetProgressed + And ClusterObjectDeployment "${NAME}" reports Available as True with Reason Available + + # A revision waiting on an unsatisfied precondition keeps retrying, so the + # deployment stays Progressing=True with an actionable reason. + Scenario: Progressing is True with RevisionPreconditionsNotMet when the head revision's precondition is unmet + Given namespace "${TEST_NAMESPACE}" is available + And resource is applied + """ + apiVersion: v1 + kind: Secret + metadata: + name: ${NAME}-mutable-secret + namespace: ${TEST_NAMESPACE} + type: olm.operatorframework.io/object-data + stringData: + configmap: | + { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "cod-precond-cm", + "namespace": "${TEST_NAMESPACE}" + }, + "data": { + "key": "value" + } + } + """ + When resource is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectDeployment + metadata: + name: ${NAME} + spec: + template: + spec: + collisionProtection: Prevent + phases: + - name: resources + objects: + - ref: + name: ${NAME}-mutable-secret + namespace: ${TEST_NAMESPACE} + key: configmap + """ + Then ClusterObjectSet "${NAME}-1" reports Ready as False with Reason PreconditionsNotMet + And ClusterObjectDeployment "${NAME}" reports Progressing as True with Reason RevisionPreconditionsNotMet and Message includes: + """ + ${NAME}-1 + """ + # Satisfy the precondition: replace the mutable Secret with an immutable one + # carrying the same content. The revision recovers and the deployment progresses. + When resource "secret/${NAME}-mutable-secret" is removed + And resource is applied + """ + apiVersion: v1 + kind: Secret + metadata: + name: ${NAME}-mutable-secret + namespace: ${TEST_NAMESPACE} + type: olm.operatorframework.io/object-data + immutable: true + stringData: + configmap: | + { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "cod-precond-cm", + "namespace": "${TEST_NAMESPACE}" + }, + "data": { + "key": "value" + } + } + """ + Then ClusterObjectSet "${NAME}-1" reports Ready as True with Reason AllObjectsReady + And ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason NewClusterObjectSetProgressed + + # A revision that never becomes ready is bounded by the progress deadline: once + # the budget expires the deployment reports Progressing=False/ProgressDeadlineExceeded + # while the revision keeps reporting its own readiness failure. + @ProgressDeadline + Scenario: Progressing is False with ProgressDeadlineExceeded when the revision never becomes ready + Given namespace "${TEST_NAMESPACE}" is available + And min value for ClusterObjectDeployment .spec.template.spec.progressDeadlineMinutes is set to 1 + When resource is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectDeployment + metadata: + name: ${NAME} + spec: + template: + spec: + collisionProtection: Prevent + progressDeadlineMinutes: 1 + progressionProbes: + - selector: + type: GroupKind + groupKind: + group: apps + kind: Deployment + assertions: + - type: ConditionEqual + conditionEqual: + type: Available + status: "True" + phases: + - name: resources + objects: + - object: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: never-ready + namespace: ${TEST_NAMESPACE} + spec: + replicas: 1 + selector: + matchLabels: + app: never-ready + template: + metadata: + labels: + app: never-ready + spec: + containers: + - name: never-ready + image: does-not-exist:latest + """ + Then ClusterObjectSet "${NAME}-1" has group "${NAME}" + And ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason ProgressDeadlineExceeded + And ClusterObjectSet "${NAME}-1" reports Ready as False with Reason ObjectsNotReady + And ClusterObjectDeployment "${NAME}" reports Available as False with Reason Unavailable + + # Hard stalls short-circuit the progress deadline: a collision keeps reporting + # RevisionObjectCollision even once the deadline would have elapsed, rather than + # flipping to ProgressDeadlineExceeded. The consistency window (70s) spans past the + # 1-minute deadline, so a controller that let the deadline win would be caught. + @ProgressDeadline + Scenario: A hard stall is reported instead of the progress deadline even after it elapses + Given namespace "${TEST_NAMESPACE}" is available + And min value for ClusterObjectDeployment .spec.template.spec.progressDeadlineMinutes is set to 1 + And resource is applied + """ + apiVersion: v1 + kind: ConfigMap + metadata: + name: cod-collide-deadline-cm + namespace: ${TEST_NAMESPACE} + data: + k: pre-existing + """ + When resource is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectDeployment + metadata: + name: ${NAME} + spec: + template: + spec: + collisionProtection: Prevent + progressDeadlineMinutes: 1 + phases: + - name: resources + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: cod-collide-deadline-cm + namespace: ${TEST_NAMESPACE} + data: + k: managed + """ + Then ClusterObjectSet "${NAME}-1" reports Ready as False with Reason ObjectCollision + # Wait for the deployment to converge before starting the consistency window, + # so it begins from a known-good state rather than racing the COD reconcile. + And ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason RevisionObjectCollision + And ClusterObjectDeployment "${NAME}" consistently reports Progressing as False with Reason RevisionObjectCollision for "70s" diff --git a/test/e2e/features/cod_progression_timeout.feature b/test/e2e/features/cod_progression_timeout.feature deleted file mode 100644 index 61839e8911..0000000000 --- a/test/e2e/features/cod_progression_timeout.feature +++ /dev/null @@ -1,63 +0,0 @@ -@BoxcutterRuntime -@ProgressDeadline -Feature: ClusterObjectDeployment progression timeout - - As an OLM user applying a ClusterObjectDeployment, I want spec.template.spec.progressDeadlineMinutes - to bound the rollout of the desired ClusterObjectSet revision, so that a revision that does not - roll out within the deadline is reported as stalled while its readiness failures remain visible. - - Background: - Given OLM is available - - Scenario: ClusterObjectDeployment times out while its revision retains its readiness failure - Given namespace "${TEST_NAMESPACE}" is available - And min value for ClusterObjectDeployment .spec.template.spec.progressDeadlineMinutes is set to 1 - When resource is applied - """ - apiVersion: olm.operatorframework.io/v1 - kind: ClusterObjectDeployment - metadata: - name: ${NAME} - spec: - template: - spec: - collisionProtection: Prevent - progressDeadlineMinutes: 1 - progressionProbes: - - selector: - type: GroupKind - groupKind: - group: apps - kind: Deployment - assertions: - - type: ConditionEqual - conditionEqual: - type: Available - status: "True" - phases: - - name: resources - objects: - - object: - apiVersion: apps/v1 - kind: Deployment - metadata: - name: never-ready - namespace: ${TEST_NAMESPACE} - spec: - replicas: 1 - selector: - matchLabels: - app: never-ready - template: - metadata: - labels: - app: never-ready - spec: - containers: - - name: never-ready - image: does-not-exist:latest - """ - Then ClusterObjectSet "${NAME}-1" has group "${NAME}" - And ClusterObjectDeployment "${NAME}" reports Progressing as False with Reason ProgressDeadlineExceeded - And ClusterObjectSet "${NAME}-1" reports Ready as False with Reason ObjectsNotReady - And ClusterObjectDeployment "${NAME}" reports Available as False with Reason Unavailable diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 07ced9664c..123c93213f 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -34,6 +34,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" k8sresource "k8s.io/cli-runtime/pkg/resource" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -129,6 +130,7 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ClusterObjectDeployment "([^"]+)" reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+)$`, ClusterObjectDeploymentReportsConditionWithoutMsg) sc.Step(`^(?i)ClusterObjectDeployment "([^"]+)" reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message includes:$`, ClusterObjectDeploymentReportsConditionWithMessageFragment) + sc.Step(`^(?i)ClusterObjectDeployment "([^"]+)" consistently reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) for "([^"]+)"$`, ClusterObjectDeploymentConsistentlyReportsCondition) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) transition between (\d+) and (\d+) minutes since ClusterObjectSet "([^"]+)" was created$`, ClusterExtensionReportsConditionTransitionTime) sc.Step(`^(?i)ClusterObjectSet is applied(?:\s+.*)?$`, ResourceIsApplied) sc.Step(`^(?i)ClusterObjectSet with Namespace owner "([^"]*)" is applied$`, ClusterObjectSetWithNamespaceOwnerApplied) @@ -995,6 +997,91 @@ func ClusterObjectDeploymentReportsConditionWithMessageFragment(ctx context.Cont return waitForCondition(ctx, "clusterobjectdeployment", substituteScenarioVars(name, scenarioCtx(ctx)), conditionType, conditionStatus, &conditionReason, messageFragmentComparison(ctx, msgFragment)) } +// ClusterObjectDeploymentConsistentlyReportsCondition fails if, over the given +// duration, the named ClusterObjectDeployment's condition ever deviates from the +// expected type/status/reason. Use it to prove a reason persists (e.g. that a hard +// stall keeps its reason past the progress deadline rather than flipping to +// ProgressDeadlineExceeded). +func ClusterObjectDeploymentConsistentlyReportsCondition(ctx context.Context, name, conditionType, conditionStatus, conditionReason, durationStr string) error { + d, err := time.ParseDuration(durationStr) + if err != nil { + return fmt.Errorf("invalid duration %q: %w", durationStr, err) + } + return consistentlyCondition(ctx, "clusterobjectdeployment", substituteScenarioVars(name, scenarioCtx(ctx)), conditionType, conditionStatus, conditionReason, d) +} + +// consistently invokes check every tick until duration elapses. Each call is one +// observation of the live cluster state: check returns a non-nil error to report a +// transient failure whose true state is unknown (e.g. a failed API read), which is +// tolerated only within maxTransientGap; it uses require/assert for hard failures that +// must fail the test immediately (e.g. an observed value that deviates from what is +// expected). A nil return is a successful observation. consistently fails if no +// observation ever succeeds within the window, so it cannot pass vacuously when the +// resource is never successfully read. +func consistently(ctx context.Context, duration time.Duration, check func(context.Context) error) error { + t := godog.T(ctx) + const maxTransientGap = 10 * time.Second + lastObserved := time.Now() + observed := false + // Poll for the whole window rather than until a condition becomes true: the condition + // always returns false, so PollUntilContextTimeout runs check every tick (starting + // immediately) until it times out, which is the success case. A transient check error is + // tolerated only within maxTransientGap; a hard failure inside check fails the test + // immediately via require. + err := wait.PollUntilContextTimeout(ctx, tick, duration, true, func(ctx context.Context) (bool, error) { + if cerr := check(ctx); cerr != nil { + require.LessOrEqualf(t, time.Since(lastObserved), maxTransientGap, + "consistency check could not observe the expected state for over %s during the window: %v", maxTransientGap, cerr) + } else { + observed = true + lastObserved = time.Now() + } + return false, nil + }) + // A full-window run ends with a timeout (or context cancellation), which is expected; + // anything else is a real error. + if err != nil && !wait.Interrupted(err) { + return err + } + require.Truef(t, observed, + "consistency check never observed the expected state during the window") + return nil +} + +// conditionCheck returns a consistently check that verifies a single condition's status +// and reason on the named resource. A failed API read is returned as a transient error +// (the condition's real state is unknown while the read is failing). A successful read +// that does not contain the condition, or shows a different status or reason, fails +// immediately: a condition that disappears or changes is not being reported consistently. +func conditionCheck(resourceType, resourceName, conditionType, conditionStatus, conditionReason string) func(context.Context) error { + return func(ctx context.Context) error { + t := godog.T(ctx) + v, err := k8sClient(ctx, "get", resourceType, resourceName, "-o", + fmt.Sprintf("jsonpath={.status.conditions[?(@.type==\"%s\")]}", conditionType)) + if err != nil { + return fmt.Errorf("reading %s %q: %w", resourceType, resourceName, err) + } + require.NotEmptyf(t, v, + "%s %s condition disappeared during the consistency window", resourceType, conditionType) + var condition metav1.Condition + require.NoErrorf(t, json.Unmarshal([]byte(v), &condition), + "unmarshaling %s %s condition during the consistency window", resourceType, conditionType) + require.Equalf(t, metav1.ConditionStatus(conditionStatus), condition.Status, + "%s %s condition status changed during the consistency window", resourceType, conditionType) + require.Equalf(t, conditionReason, condition.Reason, + "%s %s condition reason changed during the consistency window", resourceType, conditionType) + return nil + } +} + +// consistentlyCondition fails the test if, at any point during duration, the named +// resource's condition deviates from the expected type/status/reason or disappears. +// Transient API read failures are tolerated only briefly; see consistently and +// conditionCheck for the exact semantics. +func consistentlyCondition(ctx context.Context, resourceType, resourceName, conditionType, conditionStatus, conditionReason string, duration time.Duration) error { + return consistently(ctx, duration, conditionCheck(resourceType, resourceName, conditionType, conditionStatus, conditionReason)) +} + // TriggerClusterObjectSetReconciliation annotates the named ClusterObjectSet // to trigger a new reconciliation cycle. func TriggerClusterObjectSetReconciliation(ctx context.Context, cosName string) error {