[perdasilva]

Description

When RevisionEngine.Teardown returns an error during archival, ClusterObjectSetReconciler.archive() returned the error for rate-limited backoff but left the Ready condition unchanged. A revision whose teardown fails on the first attempt — before any Archiving status had been set — keeps displaying its prior condition (e.g. AllObjectsReady), which masks that cleanup is still in progress.

This sets Ready=False/Archiving in the teardown error branch before returning the error, so the surfaced status is honest during teardown retries. It is consistent with the incomplete-but-no-error branch directly below, which already reports Archiving.

The condition is never set to Archived on this path, so the ClusterObjectDeployment garbage collector — which prunes only revisions whose Ready reason is Archived — still cannot select a revision whose teardown is incomplete. The GC invariant (reason == Archived ⟹ teardown complete) is preserved.

Follow-up to a review comment on #3052.

Reviewer notes

• Status-accuracy improvement, not a data-integrity fix: the archived-vs-archiving distinction that gates GC eligibility was already correct.
• The separate revision-engine factory-error path is unchanged — it errors before archive() and correctly leaves Ready untouched.

Testing

• Updated the existing archival unit test case to assert Ready=False/Archiving after a teardown error.
• go build, go vet, and the full ./internal/object-controller/... suite pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • When revision teardown encounters an error during archiving, its status now reflects that archiving is in progress instead of remaining marked ready.
  • The teardown error is still reported, and the finalizer remains in place while the error is handled.

[netlify]

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	de75b8e
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6adb5833a39db3ccbe39c15f
😎 Deploy Preview	https://deploy-preview-3059--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 87b091dc-23a1-47e2-b1ce-8ff8e8006618

📥 Commits

Reviewing files that changed from the base of the PR and between 1edb343 and de75b8e.

📒 Files selected for processing: 2

• internal/object-controller/controllers/clusterobjectset_controller.go
• internal/object-controller/controllers/clusterobjectset_controller_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

When teardown fails during archival, the controller now marks the revision as Archiving before returning the error. The test checks the Ready condition and retained finalizer.

Changes: Archive teardown status

Layer / File(s)	Summary
Teardown failure status internal/object-controller/controllers/clusterobjectset_controller.go, internal/object-controller/controllers/clusterobjectset_controller_test.go	The archive path sets Ready=False with reason Archiving when teardown fails, then returns the error. The test verifies the condition and retained finalizer.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: fgiudici

Merge Risk: ⚪ Minimal · up to de75b

The change is ready to merge with normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the change: reporting Archiving status when ClusterObjectSet teardown retries fail.
Description check		The description explains the problem, motivation, implementation, preserved garbage-collection invariant, testing, and related issue. It does not include the template's Reviewer Checklist section, but…
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.

Full details: Description check

Explanation

The description explains the problem, motivation, implementation, preserved garbage-collection invariant, testing, and related issue. It does not include the template's Reviewer Checklist section, but the required change and validation details are otherwise complete.

✨ 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.}

[fao89]

/lgtm

[tmshort]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment