OPRUN-4392,OPRUN-4393: Add OLMv1 progress deadline QE tests#745
OPRUN-4392,OPRUN-4393: Add OLMv1 progress deadline QE tests#745bandrade wants to merge 2 commits into
Conversation
Automate the ClusterExtension rollout failure coverage for OCP-88331 and OCP-88332 by building in-cluster bundle and catalog images for successful and failing bundle versions. The new QE specs verify ProgressDeadlineExceeded on an initial failed rollout and ProbeFailure while upgrading to a bad revision under the BoxCutter runtime. Signed-off-by: Bruno Andrade <bruno.balint@gmail.com>
WalkthroughAdded a new Ginkgo test spec validating ClusterExtension progress deadline and rollout failure behavior. The suite constructs synthetic bundle and catalog OCI images, defines scenarios where a ClusterExtension transitions to ProgressDeadlineExceeded and where upgrades maintain multiple active revisions while reporting failures, and provides polling-based assertions for condition verification. ChangesClusterExtension Progress Deadline Test Suite
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bandrade 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openshift/tests-extension/test/qe/specs/olmv1_ce_progress_deadline.go (1)
34-36: 💤 Low valueRemove ineffective
GinkgoRecover()call.
defer g.GinkgoRecover()at the Describe level is dead code. The Describe callback executes during test tree construction, not test execution, so this deferred call runs before any specs execute. GinkgoRecover is designed to catch panics within goroutines spawned fromItblocks, not at the suite construction level.🧹 Suggested fix
var _ = g.Describe("[sig-olmv1][Jira:OLM][OCPFeatureGate:NewOLMBoxCutterRuntime] clusterextension progress deadline", g.Label("NonHyperShiftHOST"), func() { - defer g.GinkgoRecover() - var oc = exutil.NewCLIWithoutNamespace("default")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openshift/tests-extension/test/qe/specs/olmv1_ce_progress_deadline.go` around lines 34 - 36, Remove the ineffective deferred call to g.GinkgoRecover() from the Describe callback: locate the anonymous function passed to g.Describe (the function literal in the declaration starting with var _ = g.Describe(..., func() { ... })) and delete the line containing defer g.GinkgoRecover(); if panic recovery is needed for goroutines spawned during tests, move a g.GinkgoRecover() call into the appropriate It/BeforeEach/AfterEach block or the goroutine entrypoint instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openshift/tests-extension/test/qe/specs/olmv1_ce_progress_deadline.go`:
- Around line 34-36: Remove the ineffective deferred call to g.GinkgoRecover()
from the Describe callback: locate the anonymous function passed to g.Describe
(the function literal in the declaration starting with var _ = g.Describe(...,
func() { ... })) and delete the line containing defer g.GinkgoRecover(); if
panic recovery is needed for goroutines spawned during tests, move a
g.GinkgoRecover() call into the appropriate It/BeforeEach/AfterEach block or the
goroutine entrypoint instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9db03a8f-8565-4538-ba1f-c5d77f3957d3
📒 Files selected for processing (1)
openshift/tests-extension/test/qe/specs/olmv1_ce_progress_deadline.go
Regenerate the tests-extension metadata after adding the ClusterExtension progress deadline QE coverage. Signed-off-by: Bruno Andrade <bruno.balint@gmail.com>
|
/test openshift-e2e-aws |
|
/test openshift-e2e-aws-techpreview |
|
@bandrade: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
Needs a Jira ticket or |
|
/retitle OPRUN-4392,OPRUN-4393: Add OLMv1 progress deadline QE tests |
|
@bandrade: This pull request references OPRUN-4392 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 story to target the "5.0.0" version, but no target version was set. This pull request references OPRUN-4393 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 story to target the "5.0.0" version, but no target version was set. DetailsIn 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. |
| }) | ||
|
|
||
| g.By("creating a ClusterExtension with a 1-minute progress deadline for the failing bundle") | ||
| ce := fixture.newClusterExtension("test-ce-install-timeout-"+caseID, "1.0.2", "olm-sa", ptr.To(int32(1))) |
There was a problem hiding this comment.
How are you running the tests? I may be running this wrong - this step fails for me because the progressDeadlineMinutes must be >= 10. We solved that in the upstream tests by patching the CRD, but I don't see that happening in here.
Summary
Validation
go test -mod=vendor ./test/qe/specsmake build./bin/olmv1-tests-ext list -o names | rg '88331|88332'\n-make verify GO=/Users/bandrade/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.7.darwin-arm64/bin/goSummary by CodeRabbit