Repository navigation
⚠️ Remove the progression timeout from the ClusterObjectSet - #3078
perdasilva wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Remove the deadline from the ClusterObjectSet contract api/v1/clusterobjectset_types.go, api/v1/validation_test.go, applyconfigurations/api/v1/*, applyconfigurations/internal/internal.go, helm/olmv1/base/object-controller/crd/experimental/*, manifests/experimental*.yaml, internal/operator-controller/applier/boxcutter.go, docs/draft/concepts/clusterobjectsets.md |
The ClusterObjectSet API, generated configurations, and CRD schemas no longer include progressDeadlineMinutes. The documentation describes the deadline configured through spec.template.spec.progressDeadlineMinutes on the COD. |
Remove deadline-based reconciliation internal/object-controller/controllers/clusterobjectset_controller.go, internal/object-controller/controllers/progress_deadline.go, internal/object-controller/controllers/*_test.go, test/e2e/features/revision.feature |
The controller no longer sets ProgressDeadlineExceeded or uses a deadline-aware rate limiter. Tests and assertions for the removed deadline behavior are deleted. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Other
Suggested labels: api-diff-lint-override, go-apidiff-override
Suggested reviewers: fao89, dtfranz
Merge Risk: 
99f7e
-
No actionable issue is established by this review. The documented CRD-diff override remains a required merge step.
Pre-merge checks: 
-
Check name Status Explanation Description check The description explains what changed, why it changed, affected components, testing changes, compatibility impact, and the required CRD diff override. It does not include the reviewer checklist or rel… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly summarizes the primary change: removing progress-deadline behavior from ClusterObjectSet. Full details: Description check
-
Explanation
The description explains what changed, why it changed, affected components, testing changes, compatibility impact, and the required CRD diff override. It does not include the reviewer checklist or related issue links, but the required summary and motivation are complete.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
99f7e82 to
b0e0381
Compare
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 <[email protected]> Co-Authored-By: Claude Opus 4.8 <[email protected]>
perdasilva
changed the title
b0e0381 to
59597e8
Compare
What
Removes the progress-deadline ("progression timeout") behavior and
spec.progressDeadlineMinutesfield from theClusterObjectSet. The progress deadline is now owned solely by theClusterObjectDeployment(COD).Why
Since the COD owns revision lifecycle (creation, rollout, archival, retention) and enforces its own progress deadline (
spec.template.spec.progressDeadlineMinutes, reported on the COD'sProgressingcondition), the COS's own deadline enforcement is redundant — the COD does not propagate a deadline onto the ClusterObjectSet revisions it creates. This leaves a clean split: theClusterObjectSetrolls out its objects; theClusterObjectDeploymentowns the deadline.Changes
spec.progressDeadlineMinutesfrom theClusterObjectSetAPI (experimental-only).Ready=False/ProgressDeadlineExceededmarking, its precedence handling (nowSuperseded > ObjectsNotReady), and the deadline-aware rate-limiter wiring (reverts to the default controller rate limiter).durationUntilDeadline,newDeadlineAwareRateLimiter); the shareddeadlineAwareRateLimiterstruct remains, still used by the COD.@ProgressDeadlinee2e scenarios.Unchanged: the ClusterObjectDeployment deadline, the ClusterExtension
progressDeadlineMinutesfield and its routing to the COD template, theProgressDeadlineExceededreason constant, andstatus.completedAt.Notes
ClusterObjectSetis an experimental API; this is an intentional incompatible change with no migration support, per the project's experimental-API policy. Standard manifests are unaffected (the field was experimental-channel-only).crd-diffCI will fail by design (removal of an existing field). Merging requires thecrd-diff-overridelabel from a maintainer.🤖 Generated with Claude Code
Summary by CodeRabbit
ProgressDeadlineExceeded. Rollout deadline enforcement is handled by each ClusterObjectDeployment.