✨ Set Availability condition to Unknown on archived revisions #2261

perdasilva · 2025-10-10T13:39:13Z

Description

This PR updates the ClusterExtensionRevision to set the Available condition to Unknown with reason Archived and message revision is archived on archived revisions.

It also adds the AVAILABLE print column to the ClusterExtensionRevision for visibility in kubectl get clusterextensionrevision ...

NAME              AVAILABLE   AGE
test-operator-1   Unknown     17s
test-operator-2   True        3s

We also remove the condition setting on finalizer errors due to transience. No need to hit the api server on something that will in all likelihood be fixed in the next reconciliation.

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

Signed-off-by: Per Goncalves da Silva <[email protected]>

netlify · 2025-10-10T13:39:19Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`0b26ee5`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/68e91abe5a30180007d3fb2a
😎 Deploy Preview	https://deploy-preview-2261--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

codecov · 2025-10-10T13:59:17Z

Codecov Report

❌ Patch coverage is 72.91667% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.85%. Comparing base (95c5934) to head (0b26ee5).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...controllers/clusterextensionrevision_controller.go	72.91%	11 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2261      +/-   ##
==========================================
- Coverage   72.87%   72.85%   -0.02%     
==========================================
  Files          88       88              
  Lines        8733     8738       +5     
==========================================
+ Hits         6364     6366       +2     
- Misses       1955     1957       +2     
- Partials      414      415       +1

Flag	Coverage Δ
e2e	`39.30% <0.00%> (+0.05%)`	⬆️
experimental-e2e	`46.18% <52.08%> (-0.14%)`	⬇️
unit	`58.10% <60.41%> (+0.10%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pedjak · 2025-10-10T13:51:27Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go


 	revision, opts, previous := toBoxcutterRevision(rev)

+	// nolint:nestif


let's fix the reporting linting issue if possible?

I couldn't figure out a way to do it that made sense.

I've moved the teardown procedure to its own method - that solved the problem

pedjak · 2025-10-10T13:59:21Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

 			meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{
-				Type:               "Available",
+				Type:               ocv1.ClusterExtensionRevisionTypeAvailable,
 				Status:             metav1.ConditionFalse,
-				Reason:             "ReconcileFailure",
+				Reason:             ocv1.ClusterExtensionRevisionReasonReconcileFailure,


IMHO we do not need to report in the status if there is an error in removing finalizer, because such errors are transient. to this point we have archived the revision, and only thing left is to remove the finalizer (btw, why we would like to remove it, if deletion is not requested)?

I don't think this is unreasonable. But could we address it as a follow up to keep this PR as small as possible? Also, since I didn't really write this bit, it would be good to get Nico involved in the discussion (more so on the finalizer side). I don't have strong opinions here. Given that the resource is not managing anything and there's no need for the finalizer when archived, both can make sense - unless there's good reasons to block deletion if the controller isn't up.

I've removed the condition setting - but let's come back to the finalizer on Monday

Signed-off-by: Per Goncalves da Silva <[email protected]>

tmshort · 2025-10-10T20:36:25Z

/approve

pedjak

/lgtm

openshift-ci · 2025-10-13T11:41:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pedjak, tmshort

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [tmshort]
~~helm/OWNERS~~ [tmshort]
~~internal/operator-controller/OWNERS~~ [tmshort]
~~manifests/OWNERS~~ [tmshort]

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

tmshort · 2025-10-13T14:17:21Z

/override codecov/patch
/override patch

openshift-ci · 2025-10-13T14:17:25Z

@tmshort: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

patch

Only the following failed contexts/checkruns were expected:

Verify PR title
codecov/patch
crd-diff
e2e-kind
extension-developer-e2e
go-apidiff
go-verdiff
goreleaser
lint
netlify/olmv1/deploy-preview
tide
unit-test-basic
upgrade-e2e
verify

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override codecov/patch
/override patch

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

perdasilva · 2025-10-13T14:31:29Z

/override codecov/patch

openshift-ci · 2025-10-13T14:31:34Z

@perdasilva: Overrode contexts on behalf of perdasilva: codecov/patch

In response to this:

/override codecov/patch

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

pedjak · 2025-10-13T14:33:18Z

/override codecov/patch

shall we investigate this job?

tmshort · 2025-10-13T15:12:14Z

No, it was a matter of the size of the patch coverage. Admittedly, we have a threshold, and the difference (72% vs 76%) exceeded that.

pedjak · 2025-10-14T09:30:52Z

No, it was a matter of the size of the patch coverage. Admittedly, we have a threshold, and the difference (72% vs 76%) exceeded that.

How strictly we try to keep the coverage on a certain level? cc @perdasilva

Per Goncalves da Silva added 2 commits October 10, 2025 15:34

Add AVAILABLE print column to ClusterExtensionRevision

997ff47

Signed-off-by: Per Goncalves da Silva <[email protected]>

Update resources for print column

e781e2d

Signed-off-by: Per Goncalves da Silva <[email protected]>

perdasilva requested a review from a team as a code owner October 10, 2025 13:39

openshift-ci bot requested review from OchiengEd and trgeiger October 10, 2025 13:39

pedjak changed the title ~~:spakles: Set Availability condition to Unknown on archived revisions~~ ✨ Set Availability condition to Unknown on archived revisions Oct 10, 2025

perdasilva force-pushed the revision-status branch from c32e56c to 8de1c8e Compare October 10, 2025 13:48

pedjak reviewed Oct 10, 2025

View reviewed changes

perdasilva force-pushed the revision-status branch 2 times, most recently from ceb0255 to 75853a8 Compare October 10, 2025 14:37

Per Goncalves da Silva added 2 commits October 10, 2025 16:38

Set Available condition to Unknown on archived revisions

d6acdc1

Signed-off-by: Per Goncalves da Silva <[email protected]>

Remove condition setting on finalizer removal

0b26ee5

Signed-off-by: Per Goncalves da Silva <[email protected]>

perdasilva force-pushed the revision-status branch from 75853a8 to 0b26ee5 Compare October 10, 2025 14:39

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 10, 2025

pedjak approved these changes Oct 13, 2025

View reviewed changes

openshift-ci bot assigned pedjak Oct 13, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2025

openshift-merge-bot bot merged commit 05375cb into operator-framework:main Oct 13, 2025
25 of 26 checks passed


		revision, opts, previous := toBoxcutterRevision(rev)

		// nolint:nestif

✨ Set Availability condition to Unknown on archived revisions #2261

✨ Set Availability condition to Unknown on archived revisions #2261

Uh oh!

Conversation

perdasilva commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Checklist

Uh oh!

netlify bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

codecov bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pedjak Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

perdasilva Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

perdasilva Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

pedjak Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

perdasilva Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

tmshort commented Oct 10, 2025

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 13, 2025

Uh oh!

tmshort commented Oct 13, 2025

Uh oh!

openshift-ci bot commented Oct 13, 2025

Uh oh!

perdasilva commented Oct 13, 2025

Uh oh!

openshift-ci bot commented Oct 13, 2025

Uh oh!

pedjak commented Oct 13, 2025

Uh oh!

Uh oh!

tmshort commented Oct 13, 2025

Uh oh!

pedjak commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

perdasilva commented Oct 10, 2025 •

edited

Loading

netlify bot commented Oct 10, 2025 •

edited

Loading

codecov bot commented Oct 10, 2025 •

edited

Loading

perdasilva Oct 10, 2025 •

edited

Loading