From 59597e895dd06f7dd4136e45eacb25612fec0aa4 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Wed, 28 Oct 2026 17:09:51 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9A=A0=EF=B8=8F=20Remove=20the=20progression?= =?UTF-8?q?=20timeout=20from=20the=20ClusterObjectSet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The progress deadline is now owned solely by the ClusterObjectDeployment (COD). Since the COD owns revision lifecycle and enforces its own progress deadline (spec.template.spec.progressDeadlineMinutes, reported on the COD's Progressing condition) without propagating one to the ClusterObjectSet revisions it creates, the COS's own deadline enforcement is redundant. - Remove spec.progressDeadlineMinutes from the ClusterObjectSet API (experimental-only). - Remove the COS controller's deadline enforcement: the Ready=False/ProgressDeadlineExceeded marking, its precedence handling (now Superseded > ObjectsNotReady), and the deadline-aware rate-limiter wiring (reverts to the default controller rate limiter). - Remove the COS-only pieces of the shared deadline helper (durationUntilDeadline, newDeadlineAwareRateLimiter); the shared deadlineAwareRateLimiter struct remains, still used by the COD. - Stop the Helm-storage migration revision builder from setting the field. - Remove the two COS @ProgressDeadline e2e scenarios. - Regenerate CRDs, manifests, applyconfigurations, the API reference, and the ClusterObjectSets concept doc. The ClusterObjectDeployment deadline, the ClusterExtension progressDeadlineMinutes field and its routing to the COD template, the ProgressDeadlineExceeded reason constant, and status.completedAt are unchanged. Signed-off-by: Per G. da Silva Co-Authored-By: Claude Opus 4.8 --- api/v1/clusterobjectset_types.go | 12 - api/v1/validation_test.go | 71 ----- .../api/v1/clusterobjectsetspec.go | 15 - .../api/v1/clusterobjectsetstatus.go | 1 - applyconfigurations/internal/internal.go | 3 - docs/draft/concepts/clusterobjectsets.md | 22 +- ...peratorframework.io_clusterobjectsets.yaml | 11 - ...clusterobjectdeployment_controller_test.go | 1 - .../clusterobjectdeployment_deadline_test.go | 1 - .../clusterobjectset_controller.go | 50 +--- .../clusterobjectset_controller_test.go | 264 ------------------ .../controllers/progress_deadline.go | 72 +---- .../controllers/progress_deadline_test.go | 237 ---------------- .../operator-controller/applier/boxcutter.go | 3 - manifests/experimental-e2e.yaml | 11 - manifests/experimental.yaml | 11 - test/e2e/features/revision.feature | 133 --------- 17 files changed, 24 insertions(+), 894 deletions(-) diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index c23a5328b5..dc361ad766 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -114,17 +114,6 @@ type ClusterObjectSetSpec struct { // +optional Phases []ClusterObjectSetPhase `json:"phases,omitempty"` - // progressDeadlineMinutes is an optional field that defines the maximum period - // of time in minutes after which an installation should be considered failed and - // require manual intervention. This functionality is disabled when no value - // is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours). - // - // +kubebuilder:validation:Minimum:=10 - // +kubebuilder:validation:Maximum:=720 - // +optional - // - ProgressDeadlineMinutes int32 `json:"progressDeadlineMinutes,omitempty"` - // progressionProbes is an optional field which provides the ability to define custom readiness probes // for objects defined within spec.phases. As documented in that field, most kubernetes-native objects // within the phases already have some kind of readiness check built-in, but this field allows for checks @@ -527,7 +516,6 @@ type ClusterObjectSetStatus struct { // - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them. // - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met. // - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed. - // - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. // - False with reason Archiving: the revision is being archived and its owned objects are still being torn down. // - False with reason Archived: the revision has been archived and its objects have been torn down. // diff --git a/api/v1/validation_test.go b/api/v1/validation_test.go index 705ebe2670..48279e6a1e 100644 --- a/api/v1/validation_test.go +++ b/api/v1/validation_test.go @@ -102,77 +102,6 @@ func TestValidate(t *testing.T) { }, want: want{valid: true}, }, - "ClusterObjectSet: invalid progress deadline < 10": { - args: args{ - object: ClusterObjectSetSpec{ - Group: "validation-1", - Revision: 1, - CollisionProtection: CollisionProtectionPrevent, - LifecycleState: ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 9, - }, - }, - want: want{valid: false, errContains: "spec.progressDeadlineMinutes"}, - }, - "ClusterObjectSet: valid progress deadline = 10": { - args: args{ - object: ClusterObjectSetSpec{ - Group: "validation-2", - Revision: 1, - CollisionProtection: CollisionProtectionPrevent, - LifecycleState: ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 10, - }, - }, - want: want{valid: true}, - }, - "ClusterObjectSet: valid progress deadline = 360": { - args: args{ - object: ClusterObjectSetSpec{ - Group: "validation-3", - Revision: 1, - CollisionProtection: CollisionProtectionPrevent, - LifecycleState: ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 360, - }, - }, - want: want{valid: true}, - }, - "ClusterObjectSet: valid progress deadline = 720": { - args: args{ - object: ClusterObjectSetSpec{ - Group: "validation-4", - Revision: 1, - CollisionProtection: CollisionProtectionPrevent, - LifecycleState: ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 720, - }, - }, - want: want{valid: true}, - }, - "ClusterObjectSet: invalid progress deadline > 720": { - args: args{ - object: ClusterObjectSetSpec{ - Group: "validation-5", - Revision: 1, - CollisionProtection: CollisionProtectionPrevent, - LifecycleState: ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 721, - }, - }, - want: want{valid: false, errContains: "spec.progressDeadlineMinutes"}, - }, - "ClusterObjectSet: no progress deadline set": { - args: args{ - object: ClusterObjectSetSpec{ - Group: "validation-6", - Revision: 1, - CollisionProtection: CollisionProtectionPrevent, - LifecycleState: ClusterObjectSetLifecycleStateActive, - }, - }, - want: want{valid: true}, - }, "ClusterObjectDeployment: invalid progress deadline < 10": { args: args{ object: ClusterObjectDeploymentSpec{ diff --git a/applyconfigurations/api/v1/clusterobjectsetspec.go b/applyconfigurations/api/v1/clusterobjectsetspec.go index cdbb97bc22..84df6e87ac 100644 --- a/applyconfigurations/api/v1/clusterobjectsetspec.go +++ b/applyconfigurations/api/v1/clusterobjectsetspec.go @@ -69,13 +69,6 @@ type ClusterObjectSetSpecApplyConfiguration struct { // // Each phase in the list must have a unique name. The maximum number of phases is 20. Phases []ClusterObjectSetPhaseApplyConfiguration `json:"phases,omitempty"` - // progressDeadlineMinutes is an optional field that defines the maximum period - // of time in minutes after which an installation should be considered failed and - // require manual intervention. This functionality is disabled when no value - // is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours). - // - // - ProgressDeadlineMinutes *int32 `json:"progressDeadlineMinutes,omitempty"` // progressionProbes is an optional field which provides the ability to define custom readiness probes // for objects defined within spec.phases. As documented in that field, most kubernetes-native objects // within the phases already have some kind of readiness check built-in, but this field allows for checks @@ -144,14 +137,6 @@ func (b *ClusterObjectSetSpecApplyConfiguration) WithPhases(values ...*ClusterOb return b } -// WithProgressDeadlineMinutes sets the ProgressDeadlineMinutes field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the ProgressDeadlineMinutes field is set to the value of the last call. -func (b *ClusterObjectSetSpecApplyConfiguration) WithProgressDeadlineMinutes(value int32) *ClusterObjectSetSpecApplyConfiguration { - b.ProgressDeadlineMinutes = &value - return b -} - // WithProgressionProbes adds the given value to the ProgressionProbes field in the declarative configuration // and returns the receiver, so that objects can be build by chaining "With" function invocations. // If called multiple times, values provided by each call will be appended to the ProgressionProbes field. diff --git a/applyconfigurations/api/v1/clusterobjectsetstatus.go b/applyconfigurations/api/v1/clusterobjectsetstatus.go index 435dbfe41a..440cece5b4 100644 --- a/applyconfigurations/api/v1/clusterobjectsetstatus.go +++ b/applyconfigurations/api/v1/clusterobjectsetstatus.go @@ -46,7 +46,6 @@ type ClusterObjectSetStatusApplyConfiguration struct { // - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them. // - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met. // - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed. - // - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. // - False with reason Archiving: the revision is being archived and its owned objects are still being torn down. // - False with reason Archived: the revision has been archived and its objects have been torn down. // diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index a71f087ad8..8f564a4eda 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -392,9 +392,6 @@ var schemaYAML = typed.YAMLObject(`types: elementRelationship: associative keys: - name - - name: progressDeadlineMinutes - type: - scalar: numeric - name: progressionProbes type: list: diff --git a/docs/draft/concepts/clusterobjectsets.md b/docs/draft/concepts/clusterobjectsets.md index df492d1d9e..fddcf9a049 100644 --- a/docs/draft/concepts/clusterobjectsets.md +++ b/docs/draft/concepts/clusterobjectsets.md @@ -6,7 +6,7 @@ This document explains what ClusterObjectSets are, how they enable safe phased r A `ClusterObjectSet` is a cluster-scoped Kubernetes API that represents a versioned set of Kubernetes resources organized into ordered phases. It provides a declarative way to roll out a group of related resources sequentially, with built-in readiness checks between phases. -The revision content — the `group` identifier, `revision` number, `collisionProtection` strategy, and `phases` (including all objects within them) — is immutable once set. This guarantees that the record of what was deployed at a given revision cannot change after creation. Other fields like `lifecycleState` can change over the object's lifecycle (e.g. transitioning from `Active` to `Archived`), and optional fields like `progressDeadlineMinutes` and `progressionProbes` can be configured independently. +The revision content — the `group` identifier, `revision` number, `collisionProtection` strategy, and `phases` (including all objects within them) — is immutable once set. This guarantees that the record of what was deployed at a given revision cannot change after creation. Other fields like `lifecycleState` can change over the object's lifecycle (e.g. transitioning from `Active` to `Archived`), and optional fields like `progressionProbes` can be configured independently. Each ClusterObjectSet has: @@ -226,7 +226,6 @@ Indicates whether all objects in the revision have been successfully rolled out | False | `ObjectCollision` | One or more objects are controlled by another owner, or are unowned and collision protection prevents adoption | | False | `PreconditionsNotMet` | A precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met | | False | `InvalidSpecification` | The revision spec is invalid (a malformed revision/phase object or progression probe); requires a spec change | -| False | `ProgressDeadlineExceeded` | The revision did not roll out within `spec.progressDeadlineMinutes` | | False | `Archiving` | The revision is being archived; owned objects are still being torn down | | False | `Archived` | Objects torn down after archival | | Unknown | `Pending` | The revision has not yet been evaluated (initial state) | @@ -235,17 +234,14 @@ On a revision's first reconciliation the `Ready` condition is initialized to `Un ### Progress deadline -`spec.progressDeadlineMinutes` sets one time budget for the entire -ClusterObjectSet (COS) revision, measured from its creation timestamp until its -first successful rollout, recorded in `status.completedAt`. Completing a phase -does not reset the timer. - -For a ClusterObjectDeployment (COD), this is the time allowed for the deployment to -progress to its desired new revision. Configure it through -`spec.template.spec.progressDeadlineMinutes`. The COD enforces this budget without -setting `spec.progressDeadlineMinutes` on the ClusterObjectSets it creates. Time -spent waiting for the ClusterObjectSet to be created is not counted. Each new -revision receives its own budget. +The progress deadline is a ClusterObjectDeployment (COD) concept: the time allowed +for the deployment to progress to its desired new revision. Configure it through +`spec.template.spec.progressDeadlineMinutes`. The COD enforces this budget itself; +the ClusterObjectSets it creates do not carry a deadline. Time spent waiting for +the ClusterObjectSet to be created is not counted, and each new revision receives +its own budget, measured from the revision's creation timestamp until its first +successful rollout (recorded in `status.completedAt`). Completing a phase does not +reset the timer. If the desired revision is still incomplete when that budget expires, the COD reports `Progressing=False` with reason `ProgressDeadlineExceeded`. A five-phase diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml index 8661270922..2e49df9e3f 100644 --- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml +++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml @@ -284,16 +284,6 @@ spec: x-kubernetes-validations: - message: phases is immutable rule: self == oldSelf || oldSelf.size() == 0 - progressDeadlineMinutes: - description: |- - progressDeadlineMinutes is an optional field that defines the maximum period - of time in minutes after which an installation should be considered failed and - require manual intervention. This functionality is disabled when no value - is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours). - format: int32 - maximum: 720 - minimum: 10 - type: integer progressionProbes: description: |- progressionProbes is an optional field which provides the ability to define custom readiness probes @@ -609,7 +599,6 @@ spec: - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them. - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met. - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed. - - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. - False with reason Archiving: the revision is being archived and its owned objects are still being torn down. - False with reason Archived: the revision has been archived and its objects have been torn down. diff --git a/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go b/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go index 7f798e9239..8b7b752088 100644 --- a/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectdeployment_controller_test.go @@ -250,7 +250,6 @@ func Test_ClusterObjectDeploymentReconciler_CreatesFirstRevision(t *testing.T) { require.Equal(t, int64(1), cos.Spec.Revision) require.Equal(t, ocv1.ClusterObjectSetLifecycleStateActive, cos.Spec.LifecycleState) require.Equal(t, ocv1.CollisionProtectionPrevent, cos.Spec.CollisionProtection) - require.Zero(t, cos.Spec.ProgressDeadlineMinutes, "the deployment enforces its own progress deadline") require.NotEmpty(t, cos.Labels[labels.TemplateHashKey]) ref := metav1.GetControllerOf(cos) diff --git a/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go b/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go index 87544298c1..b5df220080 100644 --- a/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go +++ b/internal/object-controller/controllers/clusterobjectdeployment_deadline_test.go @@ -80,7 +80,6 @@ func TestClusterObjectDeploymentReconcilerDeadlineCoversEntireRevision(t *testin cos, found := getCOS(t, c, cosName) require.True(t, found) require.True(t, cos.CreationTimestamp.Time.Equal(creation)) - require.Zero(t, cos.Spec.ProgressDeadlineMinutes, "the COD owns the deadline budget") require.Len(t, cos.Spec.Phases, 5) require.Equal(t, metav1.ConditionTrue, progressing(t, c).Status) // Start after finalizer initialization; the fake client's metadata patch diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index 31308a5cab..9897ca2cac 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -149,15 +149,11 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl return c.delete(ctx, cos) } - remaining, hasDeadline := durationUntilDeadline(c.Clock, cos) - isDeadlineExceeded := hasDeadline && remaining <= 0 - if err := c.verifyReferencedSecretsImmutable(ctx, cos); err != nil { var mutErr *mutableSecretError if errors.As(err, &mutErr) { // A confirmed mutable Secret requires user action (replacing the immutable Secret), // which is not watched, so keep retrying so the revision can recover. - // PreconditionsNotMet takes precedence over ProgressDeadlineExceeded: it is more actionable. l.Error(err, "referenced Secret is mutable, marking revision not ready") markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, err.Error()) return ctrl.Result{RequeueAfter: validationRetryInterval}, nil @@ -307,9 +303,8 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl if len(collidingObjs) > 0 { l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", pres.GetName(), "collisions", collidingObjs) - setReadyWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonObjectCollision, - fmt.Sprintf("Cannot take ownership of %d object(s) in phase %q because they are owned by another controller or already exist. Remove the conflicting owner or object, or create a new ClusterObjectSet with a more permissive collisionProtection setting.", len(collidingObjs), pres.GetName()), - isDeadlineExceeded) + markAsNotReady(cos, ocv1.ClusterObjectSetReasonObjectCollision, + fmt.Sprintf("Cannot take ownership of %d object(s) in phase %q because they are owned by another controller or already exist. Remove the conflicting owner or object, or create a new ClusterObjectSet with a more permissive collisionProtection setting.", len(collidingObjs), pres.GetName())) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } } @@ -377,18 +372,15 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl } } - // Generic summary used as the ProgressDeadlineExceeded "last status", kept - // stable (not the volatile per-object detail) so the deadline message does not - // churn. The non-deadline path reports the count. Per-object detail is kept out - // of the condition message and logged at V(1) for debugging. + // Stable generic summary used when there is no per-object detail; the detailed + // path reports the count. Per-object detail is kept out of the condition message + // and logged at V(1) for debugging. const genericNotReadyMsg = "Objects are not yet ready." if len(notReadyMsgs) > 0 { l.V(1).Info("objects not yet ready", "count", len(notReadyMsgs), "details", notReadyMsgs) } - // Precedence: ProgressDeadlineExceeded > Superseded > ObjectsNotReady. + // Precedence: Superseded > ObjectsNotReady. switch { - case isDeadlineExceeded: - setReadyWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonObjectsNotReady, genericNotReadyMsg, true) case superseded: markAsNotReady(cos, ocv1.ClusterObjectSetReasonSuperseded, "Object(s) have been adopted by a newer revision of the object set.") default: @@ -396,10 +388,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl if len(notReadyMsgs) > 0 { notReadyMsg = fmt.Sprintf("%d object(s) are not yet ready.", len(notReadyMsgs)) } - setReadyWithDeadline(cos, metav1.ConditionFalse, ocv1.ClusterObjectSetReasonObjectsNotReady, notReadyMsg, false) - } - if hasDeadline && !isDeadlineExceeded { - return ctrl.Result{RequeueAfter: remaining}, nil + markAsNotReady(cos, ocv1.ClusterObjectSetReasonObjectsNotReady, notReadyMsg) } } @@ -449,11 +438,7 @@ func (c *ClusterObjectSetReconciler) SetupWithManager(mgr ctrl.Manager) error { c.Clock = clock.RealClock{} return ctrl.NewControllerManagedBy(mgr). WithOptions(controller.Options{ - RateLimiter: newDeadlineAwareRateLimiter( - workqueue.DefaultTypedControllerRateLimiter[ctrl.Request](), - mgr.GetClient(), - c.Clock, - ), + RateLimiter: workqueue.DefaultTypedControllerRateLimiter[ctrl.Request](), }). For( &ocv1.ClusterObjectSet{}, @@ -767,25 +752,6 @@ func buildProgressionProbes(progressionProbes []ocv1.ProgressionProbe) (probing. return userProbes, nil } -// setReadyWithDeadline sets the Ready condition to the supplied status/reason/message, -// unless the progress deadline has been exceeded — in which case it reports -// Ready=False/ProgressDeadlineExceeded instead. This centralises the progress -// deadline enforcement for the Ready condition. -func setReadyWithDeadline(cos *ocv1.ClusterObjectSet, status metav1.ConditionStatus, reason, message string, isDeadlineExceeded bool) { // nolint:unparam - if isDeadlineExceeded { - markAsNotReady(cos, ocv1.ReasonProgressDeadlineExceeded, - fmt.Sprintf("Revision has not rolled out for %d minute(s). Last status: %s", cos.Spec.ProgressDeadlineMinutes, message)) - return - } - meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeReady, - Status: status, - Reason: reason, - Message: message, - ObservedGeneration: cos.Generation, - }) -} - func markAsReady(cos *ocv1.ClusterObjectSet, reason, message string) bool { return meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{ Type: ocv1.ClusterObjectSetTypeReady, diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index 51f3c8502d..b2bf86c141 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -1442,270 +1442,6 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) } } -func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { - const ( - clusterObjectSetName = "test-ext-1" - ) - - testScheme := newScheme(t) - require.NoError(t, corev1.AddToScheme(testScheme)) - mockCtrl := gomock.NewController(t) - - for _, tc := range []struct { - name string - existingObjs func() []client.Object - revisionResult machinery.RevisionResult - validate func(*testing.T, client.Client) - reconcileErr error - reconcileResult ctrl.Result - clock clock.Clock - }{ - { - name: "Ready set to false/ProgressDeadlineExceeded when progress deadline is exceeded", - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - rev1.Spec.ProgressDeadlineMinutes = 1 - rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) - return []client.Object{rev1, ext} - }, - // 61sec elapsed since the creation of the revision - clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 1, 1, 0, time.UTC)), - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - inTransition: true, - }), - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady) - require.Equal(t, metav1.ConditionFalse, cnd.Status) - require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason) - }, - }, - { - // A rolling revision with failing probes AND an exceeded progress deadline must - // set Ready=False/ProgressDeadlineExceeded (deadline wins over ObjectsNotReady). - name: "Ready set to false/ProgressDeadlineExceeded when deadline is exceeded during probe failure (deadline wins over ObjectsNotReady)", - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - rev1.Spec.ProgressDeadlineMinutes = 1 - rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) - return []client.Object{rev1, ext} - }, - // 61sec elapsed since the creation of the revision - clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 1, 1, 0, time.UTC)), - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - inTransition: false, - isComplete: false, - phases: []machinery.PhaseResult{ - newMockPhaseResult(mockCtrl, phaseResultConfig{ - name: "somephase", - isComplete: false, - objects: []machinery.ObjectResult{ - newMockObjectResult(mockCtrl, objectResultConfig{ - object: func() client.Object { - obj := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-service", - Namespace: "my-namespace", - }, - } - obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) - return obj - }(), - probes: machinerytypes.ProbeResultContainer{ - boxcutter.ProgressProbeType: { - Status: machinerytypes.ProbeStatusFalse, - Messages: []string{"probe failed"}, - }, - }, - }), - }, - }), - }, - }), - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady) - require.NotNil(t, cnd) - require.Equal(t, metav1.ConditionFalse, cnd.Status) - require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason) - // The deadline "last status" must be the generic not-ready summary, not the - // per-object probe detail, so the deadline message stays stable. - require.Equal(t, "Revision has not rolled out for 1 minute(s). Last status: Objects are not yet ready.", cnd.Message) - }, - }, - { - // A progressed (superseded) object AND an exceeded progress deadline must - // set Ready=False/ProgressDeadlineExceeded (deadline wins over Superseded). - name: "Ready set to false/ProgressDeadlineExceeded when deadline is exceeded with a superseded object (deadline wins over Superseded)", - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - rev1.Spec.ProgressDeadlineMinutes = 1 - rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) - return []client.Object{rev1, ext} - }, - clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 1, 1, 0, time.UTC)), - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - inTransition: true, - isComplete: false, - phases: []machinery.PhaseResult{ - newMockPhaseResult(mockCtrl, phaseResultConfig{ - name: "somephase", - isComplete: false, - objects: []machinery.ObjectResult{ - newMockObjectResult(mockCtrl, objectResultConfig{ - action: machinery.ActionProgressed, - object: func() client.Object { - obj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm", Namespace: "ns"}} - obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) - return obj - }(), - }), - }, - }), - }, - }), - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - require.NoError(t, c.Get(t.Context(), client.ObjectKey{Name: clusterObjectSetName}, rev)) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady) - require.NotNil(t, cnd) - require.Equal(t, metav1.ConditionFalse, cnd.Status) - require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason) - }, - }, - { - name: "requeue after progressDeadline time for final progression deadline check", - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - rev1.Spec.ProgressDeadlineMinutes = 1 - rev1.CreationTimestamp = metav1.NewTime(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)) - return []client.Object{rev1, ext} - }, - clock: clocktesting.NewFakeClock(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - inTransition: true, - }), - reconcileResult: ctrl.Result{RequeueAfter: 60 * time.Second}, - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady) - require.Equal(t, metav1.ConditionFalse, cnd.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonObjectsNotReady, cnd.Reason) - }, - }, - { - name: "recovery from ProgressDeadlineExceeded to AllObjectsReady when revision completes", - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - rev1.Spec.ProgressDeadlineMinutes = 1 - rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)) - meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeReady, - Status: metav1.ConditionFalse, - Reason: ocv1.ReasonProgressDeadlineExceeded, - Message: "Revision has not rolled out for 1 minute(s). Last status: Objects are not yet ready.", - ObservedGeneration: rev1.Generation, - }) - return []client.Object{rev1, ext} - }, - clock: clocktesting.NewFakeClock(time.Date(2022, 1, 1, 0, 5, 0, 0, time.UTC)), - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - isComplete: true, - }), - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady) - require.NotNil(t, cnd) - require.Equal(t, metav1.ConditionTrue, cnd.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonAllObjectsReady, cnd.Reason) - require.Equal(t, "All objects are at the desired state", cnd.Message) - }, - }, - { - name: "no progression deadline checks on revision recovery", - existingObjs: func() []client.Object { - ext := newTestClusterExtension() - rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) - rev1.Spec.ProgressDeadlineMinutes = 1 - rev1.CreationTimestamp = metav1.NewTime(time.Now().Add(-2 * time.Minute)) - meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{ - Type: ocv1.ClusterObjectSetTypeReady, - Status: metav1.ConditionTrue, - Reason: ocv1.ClusterObjectSetReasonAllObjectsReady, - ObservedGeneration: rev1.Generation, - }) - rev1.Status.CompletedAt = metav1.Now() - return []client.Object{rev1, ext} - }, - revisionResult: newMockRevisionResult(mockCtrl, revisionResultConfig{ - inTransition: true, - }), - validate: func(t *testing.T, c client.Client) { - rev := &ocv1.ClusterObjectSet{} - err := c.Get(t.Context(), client.ObjectKey{ - Name: clusterObjectSetName, - }, rev) - require.NoError(t, err) - cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady) - require.Equal(t, metav1.ConditionFalse, cnd.Status) - require.Equal(t, ocv1.ClusterObjectSetReasonObjectsNotReady, cnd.Reason) - }, - }, - } { - t.Run(tc.name, func(t *testing.T) { - mockCtrl := gomock.NewController(t) - - // create extension and cluster extension - testClient := testutil.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). - WithStatusSubresource(&ocv1.ClusterObjectSet{}). - WithObjects(tc.existingObjs()...). - Build() - - // reconcile cluster extension revision - mockEngine := newMockRevisionEngineWithReconcile(mockCtrl, - func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { - return tc.revisionResult, nil - }, nil, - ) - result, err := (&controllers.ClusterObjectSetReconciler{ - Client: testClient, - RevisionEngineFactory: newMockRevisionEngineFactoryWithEngine(mockCtrl, mockEngine, nil), - TrackingCache: newMockTrackingCache(mockCtrl, testClient, nil), - Clock: tc.clock, - }).Reconcile(t.Context(), ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: clusterObjectSetName, - }, - }) - require.Equal(t, tc.reconcileResult, result) - require.Equal(t, tc.reconcileErr, err) - - tc.validate(t, testClient) - }) - } -} - func Test_ClusterObjectSetReconciler_Reconcile_InitializesReadyPending(t *testing.T) { testScheme := newScheme(t) diff --git a/internal/object-controller/controllers/progress_deadline.go b/internal/object-controller/controllers/progress_deadline.go index 1684b030f3..3242cbc122 100644 --- a/internal/object-controller/controllers/progress_deadline.go +++ b/internal/object-controller/controllers/progress_deadline.go @@ -3,50 +3,28 @@ package controllers import ( - "context" "sync" "time" "k8s.io/client-go/util/workqueue" - "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" ) // deadlineAwareRateLimiter wraps a delegate rate limiter and caps the backoff -// duration to the time remaining until a revision's progress deadline, ensuring -// that ProgressDeadlineExceeded is set promptly even during exponential backoff. +// duration to the time remaining until the deadline reported by its remaining +// function, ensuring that a progress deadline is observed promptly even during +// exponential backoff. The deadline source is supplied by the constructor (the +// ClusterObjectDeployment controller's progress deadline). // -// After the deadline passes, it allows one immediate requeue (returning 0) so -// the reconciler can set the ProgressDeadlineExceeded condition, then falls -// back to the delegate's normal backoff. This avoids both tight-looping and -// coupling to status conditions. +// After the deadline passes, it allows one immediate requeue (returning 0) so the +// reconciler can act on the expiry, then falls back to the delegate's normal +// backoff. This avoids both tight-looping and coupling to status conditions. type deadlineAwareRateLimiter struct { delegate workqueue.TypedRateLimiter[ctrl.Request] remaining func(ctrl.Request) (time.Duration, bool, error) pastDeadline sync.Map } -func newDeadlineAwareRateLimiter( - delegate workqueue.TypedRateLimiter[ctrl.Request], - c client.Reader, - clk clock.Clock, -) *deadlineAwareRateLimiter { - return &deadlineAwareRateLimiter{ - delegate: delegate, - remaining: func(req ctrl.Request) (time.Duration, bool, error) { - cos := &ocv1.ClusterObjectSet{} - if err := c.Get(context.Background(), req.NamespacedName, cos); err != nil { - return 0, false, client.IgnoreNotFound(err) - } - remaining, hasDeadline := durationUntilDeadline(clk, cos) - return remaining, hasDeadline, nil - }, - } -} - func (r *deadlineAwareRateLimiter) When(item ctrl.Request) time.Duration { backoff := r.delegate.When(item) @@ -82,39 +60,3 @@ func (r *deadlineAwareRateLimiter) Forget(item ctrl.Request) { func (r *deadlineAwareRateLimiter) NumRequeues(item ctrl.Request) int { return r.delegate.NumRequeues(item) } - -// durationUntilDeadline returns how much time remains before the progress deadline -// expires. A negative duration means the deadline has already passed. -// -// It derives the deadline from spec and metadata only, with one exception: -// it checks status.completedAt so that a revision recovering from drift is not -// penalised by the original deadline. -// -// completedAt is a latch: there is no way to deduce from current cluster state -// alone that a COS became ready in the past. It is set once and never cleared, -// so once observed ready a revision is never subject to the deadline again. -// -// Returns (0, false) when there is no active deadline: -// - progressDeadlineMinutes is 0 -// - the revision has already been observed ready (completedAt set) -// - the revision is archived (deadline is irrelevant) -// - the revision is being deleted -func durationUntilDeadline(clk clock.Clock, cos *ocv1.ClusterObjectSet) (time.Duration, bool) { - pd := cos.Spec.ProgressDeadlineMinutes - if pd <= 0 { - return 0, false - } - // Once the revision has been observed ready (completedAt set), the deadline - // no longer applies. - if !cos.Status.CompletedAt.IsZero() { - return 0, false - } - if cos.Spec.LifecycleState == ocv1.ClusterObjectSetLifecycleStateArchived { - return 0, false - } - if !cos.DeletionTimestamp.IsZero() { - return 0, false - } - deadline := cos.CreationTimestamp.Add(time.Duration(pd) * time.Minute) - return deadline.Sub(clk.Now()), true -} diff --git a/internal/object-controller/controllers/progress_deadline_test.go b/internal/object-controller/controllers/progress_deadline_test.go index 7cb061af27..6a3167f7a1 100644 --- a/internal/object-controller/controllers/progress_deadline_test.go +++ b/internal/object-controller/controllers/progress_deadline_test.go @@ -23,86 +23,6 @@ import ( "github.com/operator-framework/operator-controller/internal/shared/labels" ) -func TestDurationUntilDeadline(t *testing.T) { - creation := time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC) - now := creation.Add(30 * time.Second) - clk := clocktesting.NewFakeClock(now) - - for _, tc := range []struct { - name string - cos ocv1.ClusterObjectSet - expectDuration time.Duration - expectHasDeadline bool - }{ - { - name: "progressDeadlineMinutes is 0 — no deadline", - cos: ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, - Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 0, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, - }, - expectDuration: 0, - expectHasDeadline: false, - }, - { - name: "completedAt is set — no deadline", - cos: ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, - Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, - Status: ocv1.ClusterObjectSetStatus{ - CompletedAt: metav1.NewTime(creation), - }, - }, - expectDuration: 0, - expectHasDeadline: false, - }, - { - name: "lifecycleState is Archived — no deadline", - cos: ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, - Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived}, - }, - expectDuration: 0, - expectHasDeadline: false, - }, - { - name: "DeletionTimestamp is set — no deadline", - cos: ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.NewTime(creation), - DeletionTimestamp: &metav1.Time{Time: now}, - }, - Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, - }, - expectDuration: 0, - expectHasDeadline: false, - }, - { - name: "deadline not yet exceeded — returns positive remaining", - cos: ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation)}, - Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, - }, - expectDuration: 30 * time.Second, - expectHasDeadline: true, - }, - { - name: "deadline already exceeded — returns negative remaining", - cos: ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(creation.Add(-2 * time.Minute))}, - Spec: ocv1.ClusterObjectSetSpec{ProgressDeadlineMinutes: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive}, - }, - expectDuration: -90 * time.Second, - expectHasDeadline: true, - }, - } { - t.Run(tc.name, func(t *testing.T) { - duration, hasDeadline := durationUntilDeadline(clk, &tc.cos) - require.Equal(t, tc.expectHasDeadline, hasDeadline) - require.Equal(t, tc.expectDuration, duration) - }) - } -} - type fixedRateLimiter struct { duration time.Duration } @@ -111,163 +31,6 @@ func (f *fixedRateLimiter) When(_ ctrl.Request) time.Duration { return f.duratio func (f *fixedRateLimiter) Forget(_ ctrl.Request) {} func (f *fixedRateLimiter) NumRequeues(_ ctrl.Request) int { return 0 } -func TestDeadlineAwareRateLimiter(t *testing.T) { - scheme := runtime.NewScheme() - require.NoError(t, ocv1.AddToScheme(scheme)) - - creation := time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC) - req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "test-cos"}} - - for _, tc := range []struct { - name string - backoff time.Duration - cos *ocv1.ClusterObjectSet - clockTime time.Time - expectDuration time.Duration - }{ - { - name: "no deadline configured — uses delegate backoff", - backoff: 30 * time.Second, - clockTime: creation, - cos: &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cos", - CreationTimestamp: metav1.NewTime(creation), - }, - Spec: ocv1.ClusterObjectSetSpec{ - LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, - }, - }, - expectDuration: 30 * time.Second, - }, - { - name: "deadline not exceeded and backoff is shorter — uses delegate backoff", - backoff: 5 * time.Second, - clockTime: creation, - cos: &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cos", - CreationTimestamp: metav1.NewTime(creation), - }, - Spec: ocv1.ClusterObjectSetSpec{ - LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 1, - }, - }, - expectDuration: 5 * time.Second, - }, - { - name: "deadline not exceeded and backoff is longer — caps at deadline", - backoff: 5 * time.Minute, - clockTime: creation, - cos: &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cos", - CreationTimestamp: metav1.NewTime(creation), - }, - Spec: ocv1.ClusterObjectSetSpec{ - LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 1, - }, - }, - expectDuration: 60 * time.Second, - }, - { - name: "deadline exceeded — first call returns immediate requeue", - backoff: 30 * time.Second, - clockTime: creation.Add(61 * time.Second), - cos: &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cos", - CreationTimestamp: metav1.NewTime(creation), - }, - Spec: ocv1.ClusterObjectSetSpec{ - LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 1, - }, - }, - expectDuration: 0, - }, - { - name: "COS not found — uses delegate backoff", - backoff: 30 * time.Second, - clockTime: creation, - cos: nil, - expectDuration: 30 * time.Second, - }, - } { - t.Run(tc.name, func(t *testing.T) { - builder := fake.NewClientBuilder().WithScheme(scheme) - if tc.cos != nil { - builder = builder.WithObjects(tc.cos) - } - - limiter := newDeadlineAwareRateLimiter( - &fixedRateLimiter{duration: tc.backoff}, - builder.Build(), - clocktesting.NewFakeClock(tc.clockTime), - ) - - testReq := req - if tc.cos == nil { - testReq.Name = "nonexistent" - } - - result := limiter.When(testReq) - require.Equal(t, tc.expectDuration, result) - }) - } - - t.Run("deadline exceeded — second call uses delegate backoff (one-shot)", func(t *testing.T) { - cos := &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cos", - CreationTimestamp: metav1.NewTime(creation), - }, - Spec: ocv1.ClusterObjectSetSpec{ - LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 1, - }, - } - limiter := newDeadlineAwareRateLimiter( - &fixedRateLimiter{duration: 30 * time.Second}, - fake.NewClientBuilder().WithScheme(scheme).WithObjects(cos).Build(), - clocktesting.NewFakeClock(creation.Add(61*time.Second)), - ) - - first := limiter.When(req) - require.Equal(t, time.Duration(0), first) - - second := limiter.When(req) - require.Equal(t, 30*time.Second, second) - }) - - t.Run("Forget resets the one-shot flag", func(t *testing.T) { - cos := &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cos", - CreationTimestamp: metav1.NewTime(creation), - }, - Spec: ocv1.ClusterObjectSetSpec{ - LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, - ProgressDeadlineMinutes: 1, - }, - } - limiter := newDeadlineAwareRateLimiter( - &fixedRateLimiter{duration: 30 * time.Second}, - fake.NewClientBuilder().WithScheme(scheme).WithObjects(cos).Build(), - clocktesting.NewFakeClock(creation.Add(61*time.Second)), - ) - - require.Equal(t, time.Duration(0), limiter.When(req)) - require.Equal(t, 30*time.Second, limiter.When(req)) - - limiter.Forget(req) - - require.Equal(t, time.Duration(0), limiter.When(req)) - }) -} - func TestCODDeadlineAwareRateLimiter(t *testing.T) { creation := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) for _, tc := range []struct { diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index e558e206ff..b2621f11d9 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -221,9 +221,6 @@ func (r *SimpleRevisionGenerator) buildClusterObjectSet( WithLifecycleState(ocv1.ClusterObjectSetLifecycleStateActive). WithPhases(phases...). WithProgressionProbes(defaultProgressionProbes...) - if p := ext.Spec.ProgressDeadlineMinutes; p > 0 { - spec.WithProgressDeadlineMinutes(p) - } return ocv1ac.ClusterObjectSet(""). WithAnnotations(annotations). diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index d96a444aae..cf121ecd37 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -2253,16 +2253,6 @@ spec: x-kubernetes-validations: - message: phases is immutable rule: self == oldSelf || oldSelf.size() == 0 - progressDeadlineMinutes: - description: |- - progressDeadlineMinutes is an optional field that defines the maximum period - of time in minutes after which an installation should be considered failed and - require manual intervention. This functionality is disabled when no value - is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours). - format: int32 - maximum: 720 - minimum: 10 - type: integer progressionProbes: description: |- progressionProbes is an optional field which provides the ability to define custom readiness probes @@ -2578,7 +2568,6 @@ spec: - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them. - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met. - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed. - - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. - False with reason Archiving: the revision is being archived and its owned objects are still being torn down. - False with reason Archived: the revision has been archived and its objects have been torn down. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 0f13d68f83..2952ff5723 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -2214,16 +2214,6 @@ spec: x-kubernetes-validations: - message: phases is immutable rule: self == oldSelf || oldSelf.size() == 0 - progressDeadlineMinutes: - description: |- - progressDeadlineMinutes is an optional field that defines the maximum period - of time in minutes after which an installation should be considered failed and - require manual intervention. This functionality is disabled when no value - is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours). - format: int32 - maximum: 720 - minimum: 10 - type: integer progressionProbes: description: |- progressionProbes is an optional field which provides the ability to define custom readiness probes @@ -2539,7 +2529,6 @@ spec: - False with reason ObjectCollision: one or more objects are controlled by another owner, or are unowned while collision protection prevents adoption, so this revision cannot take ownership of them. - False with reason PreconditionsNotMet: a precondition for applying the revision is not yet satisfied (a referenced Secret is mutable or has invalid or changed content, or a preflight dry-run was rejected, e.g. an immutable field, a conflicting object, or RBAC/dependency CRDs not yet propagated); the revision keeps retrying and recovers once the precondition is met. - False with reason InvalidSpecification: the revision spec is invalid (a malformed revision/phase object or progression probe) and requires a spec change to proceed. - - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes. - False with reason Archiving: the revision is being archived and its owned objects are still being torn down. - False with reason Archived: the revision has been archived and its objects have been torn down. diff --git a/test/e2e/features/revision.feature b/test/e2e/features/revision.feature index 633440547c..86861c044d 100644 --- a/test/e2e/features/revision.feature +++ b/test/e2e/features/revision.feature @@ -668,139 +668,6 @@ Feature: Install ClusterObjectSet """ - @ProgressDeadline - Scenario: Archiving a COS with ProgressDeadlineExceeded cleans up its resources - Given namespace "${TEST_NAMESPACE}" is available - And min value for ClusterObjectSet .spec.progressDeadlineMinutes is set to 1 - When ClusterObjectSet is applied - """ - apiVersion: olm.operatorframework.io/v1 - kind: ClusterObjectSet - metadata: - name: ${COS_NAME} - spec: - group: ${NAME} - lifecycleState: Active - 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: v1 - kind: ConfigMap - metadata: - name: test-configmap - namespace: ${TEST_NAMESPACE} - data: - foo: bar - - object: - apiVersion: apps/v1 - kind: Deployment - metadata: - name: test-deployment - 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 - revision: 1 - """ - Then resource "configmap/test-configmap" is installed - And resource "deployment/test-deployment" is installed - And ClusterObjectSet "${COS_NAME}" reports Ready as False with Reason ProgressDeadlineExceeded - When ClusterObjectSet "${COS_NAME}" lifecycle is set to "Archived" - Then ClusterObjectSet "${COS_NAME}" is archived - And resource "configmap/test-configmap" is eventually not found - And resource "deployment/test-deployment" is eventually not found - - @ProgressDeadline - @Serial - Scenario: COS recovers from ProgressDeadlineExceeded to Succeeded when probes pass - Given namespace "${TEST_NAMESPACE}" is available - And min value for ClusterObjectSet .spec.progressDeadlineMinutes is set to 1 - When ClusterObjectSet is applied - """ - apiVersion: olm.operatorframework.io/v1 - kind: ClusterObjectSet - metadata: - name: ${COS_NAME} - spec: - group: ${NAME} - lifecycleState: Active - 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: test-deployment - namespace: ${TEST_NAMESPACE} - spec: - replicas: 1 - selector: - matchLabels: - app: delayed-ready - template: - metadata: - labels: - app: delayed-ready - spec: - containers: - - name: delayed-ready - image: busybox:1.36 - imagePullPolicy: IfNotPresent - command: ["sleep", "1000"] - readinessProbe: - exec: - command: ["true"] - initialDelaySeconds: 65 - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - seccompProfile: - type: RuntimeDefault - revision: 1 - """ - Then ClusterObjectSet "${COS_NAME}" reports Ready as False with Reason ProgressDeadlineExceeded - And ClusterObjectSet "${COS_NAME}" reports Ready as True with Reason AllObjectsReady - Scenario: Report Object Counts for managed workloads Given namespace "${TEST_NAMESPACE}" is available When ClusterObjectSet is applied