OPRUN-4228: Add Boxcutter OTE tests by anik120 · Pull Request #560 · openshift/operator-framework-operator-controller

anik120 · 2025-11-18T16:48:02Z

openshift-ci-robot · 2025-11-18T16:48:07Z

@anik120: This pull request references OPRUN-4228 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set.

Details

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-11-18T16:48:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anik120
Once this PR has been reviewed and has the lgtm label, please assign grokspawn for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

DOWNSTREAM_OWNERS

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

anik120 · 2025-11-18T16:49:07Z

/hold

for openshift/cluster-olm-operator#151 to merge first

openshift-ci-robot · 2025-11-18T16:49:36Z

@anik120: This pull request references OPRUN-4228 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set.

Details

In response to this:

For EP: openshift/enhancements#1890

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 openshift-eng/jira-lifecycle-plugin repository.

camilamacedo86 · 2025-11-18T16:54:34Z

+	mkdir -p $(@D)
+	$(GO_BINDATA) -pkg boxcuttercatalog -o $@ -prefix "testdata/boxcutter/catalog" testdata/boxcutter/catalog/...
+	go fmt ./$(@D)/...
+


I can see that we have a lot of it already :-(
Not a great solution
@tmshort I think we will need to look for a way to improve it at some stage
it is not maintainable.

camilamacedo86 · 2025-11-19T18:36:57Z

+ Expect(foundOwnerRef).To(BeTrue(), "ClusterExtension should be in owner references")
+
+ By("verifying the revision has proper labels")
+ Expect(revision.Labels).To(HaveKeyWithValue("olm.operatorframework.io/owner", name),


That will be owner-name and can we have a const on top of the file.
I think we will use more than once

camilamacedo86 · 2025-11-19T18:38:50Z

+ err = k8sClient.Get(ctx, client.ObjectKey{Name: revision2Name}, &revision2)
+ Expect(err).ToNot(HaveOccurred(), "revision-2 should exist")
+
+ By("verifying revision-2 is Active")


Shoild we not check that the revision-1 is Archived ?

@camilamacedo86
https://github.com/operator-framework/operator-controller/blob/main/internal/operator-controller/applier/boxcutter.go#L311-L407
From what I can see in the code that's in main, the apply() function creates a new revision when upgrading, but doesn't modify the lifecycle state of previous revisions. Unit tests also show that revisions must be manually set to Archived state. And then GC logic (setPreviousRevisions) only deletes revisions that are already in Archived state.

Can you please check with the main changes? See the demo: #560 (comment)

camilamacedo86 · 2025-11-19T18:45:31Z

+ // NOTE: GC only deletes ARCHIVED revisions beyond the limit of 5.
+ // Active revisions are NEVER deleted regardless of count.
+ // We archive 6 revisions here, so when we later trigger a new revision,
+ // the oldest archived revision (revision-1) should be deleted,
+ // keeping 5 archived revisions (2-6) + active revisions (7-9).


All of this workaround should not be required after we get sync with downstream; operator-framework/operator-controller#2315
(see the demo in the PR)

camilamacedo86 · 2025-11-19T18:49:30Z

+ err := k8sClient.Get(ctx, client.ObjectKey{Name: revisionName}, &revision)
+ g.Expect(err).ToNot(HaveOccurred())
+
+ revision.Spec.LifecycleState = olmv1.ClusterExtensionRevisionLifecycleStateArchived


We shouldn’t archive it manually — that should be handled by OLM, right?
So we need to test the expected behavior, and if it’s failing, then we’ll know it needs to be fixed. I believe operator-framework/operator-controller#2315 will address it.

Oh so looks like this isn't in downstream yet. However, based on a quick look, looks like the archiving behavior is unchanged - ie there's still no automatic archiving behavior - @perdasilva is this intended?
Ack that once this PR downstreams I need to change incorporate the switch from spec.previous field to label-based discovery using olm.operatorframework.io/owner label

ie there's still no automatic archiving behavior
Can you please test it out?
See the demo: https://asciinema.org/a/qupccwlIdeprj8VvjF7OOko9t
It is fixed with: operator-framework/operator-controller#2315

camilamacedo86 · 2025-11-19T18:50:46Z

+ ext.Annotations["test.boxcutter/trigger-gc"] = "true"
+ err = k8sClient.Update(ctx, &ext)
+ g.Expect(err).ToNot(HaveOccurred())
+ }).WithTimeout(helpers.DefaultTimeout).WithPolling(helpers.DefaultPolling).Should(Succeed())


I do not think that we should test it.
What IHMO we need to check is when we uninstall a ClusterExtension all revisions are not left orphoned and are either removed.

Signed-off-by: Anik Bhattacharjee <anbhatta@redhat.com>

openshift-ci · 2025-11-20T17:59:03Z

@anik120: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-olmv1-ext	`9dac49c`	link	true	`/test e2e-aws-olmv1-ext`
ci/prow/e2e-aws-techpreview-olmv1-ext	`9dac49c`	link	true	`/test e2e-aws-techpreview-olmv1-ext`
ci/prow/openshift-e2e-aws	`9dac49c`	link	true	`/test openshift-e2e-aws`
ci/prow/tests-extension	`9dac49c`	link	true	`/test tests-extension`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-bot · 2026-02-20T01:01:12Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-merge-robot · 2026-02-20T01:01:21Z

PR needs rebase.

Details

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.

openshift-bot · 2026-03-22T08:30:23Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

coderabbitai · 2026-03-22T08:30:40Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7586f2b9-f6c2-492a-b8f7-cc41ad2428b8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

camilamacedo86 · 2026-03-26T13:25:55Z

Close this one in favour of: #672

/close

openshift-ci · 2026-03-26T13:27:15Z

@camilamacedo86: Closed this PR.

Details

In response to this:

Close this one in favour of: #672
/close

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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 18, 2025

openshift-ciBot requested review from bentito and perdasilva November 18, 2025 16:48

openshift-ciBot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2025

camilamacedo86 reviewed Nov 18, 2025
View reviewed changes

Comment threadopenshift/tests-extension/go.mod

camilamacedo86 reviewed Nov 18, 2025
View reviewed changes

Comment threadopenshift/tests-extension/testdata/boxcutter/bundle/tests/scorecard/config.yaml Outdated

camilamacedo86 reviewed Nov 18, 2025
View reviewed changes

Comment threadopenshift/tests-extension/testdata/boxcutter/bundle/metadata/properties.yaml Outdated

anik120 force-pushed the add-boxcutter-ote-tests branch 3 times, most recently from 72637ae to 7d4cb15Compare November 18, 2025 17:23