Do not merge: Ocp 88331 88332 automation manual check#753
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>
Regenerate the tests-extension metadata after adding the ClusterExtension progress deadline QE coverage. Signed-off-by: Bruno Andrade <bruno.balint@gmail.com>
|
Skipping CI for Draft Pull Request. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughA new Ginkgo QE test file is added for the OLMv1 ChangesOLMv1 ClusterExtension Progress Deadline QE Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in :\n\tgithub.com/BurntSushi/toml@v1.6.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/Masterminds/semver/v3@v3.5.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/blang/semver/v4@v4.0.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/cert-manager/cert-manager@v1.20.2: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/containerd/containerd@v1.7.32: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/cucumber/godog@v0.15.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/evanphx/json-patch@v5.9.11+incompatib ... [truncated 33104 characters] ... txt\n\tk8s.io/kubectl: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/kubelet: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/kubernetes: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/metrics: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/mount-utils: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/pod-security-admission: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-apiserver: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n" 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: dtfranz 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 |
|
/test e2e-aws-olmv1-ext |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@openshift/tests-extension/test/qe/specs/olmv1_ce_progress_deadline.go`:
- Around line 263-265: The oc.AsAdmin().WithoutNamespace().Run("start-build")
command with Output() call lacks a timeout/cancellation boundary and can block
indefinitely if the build stalls. Refactor this code to use context.Context with
a timeout (via context.WithTimeout) to enforce a bounded wait duration on the
start-build operation. Pass the timeout context through the command execution
chain and update error handling to properly handle context.DeadlineExceeded
errors that may occur when the timeout is exceeded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3f12419e-b7e6-4d88-a56b-1b62142a16af
📒 Files selected for processing (2)
openshift/tests-extension/.openshift-tests-extension/openshift_payload_olmv1.jsonopenshift/tests-extension/test/qe/specs/olmv1_ce_progress_deadline.go
| output, err := oc.AsAdmin().WithoutNamespace().Run("start-build").Args(name, "-n", namespace, "--from-archive="+archive, "--wait").Output() | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "failed to build image %s: %s", name, output) | ||
| } |
There was a problem hiding this comment.
Add a timeout/cancellation boundary around oc start-build --wait.
Output() can block indefinitely when a build stalls, which can hang the spec and tie up CI capacity. Please enforce a bounded wait path for this external call.
As per coding guidelines, "Go security (prodsec-skills): ... context.Context for cancellation and timeouts".
🤖 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 263 - 265, The oc.AsAdmin().WithoutNamespace().Run("start-build") command
with Output() call lacks a timeout/cancellation boundary and can block
indefinitely if the build stalls. Refactor this code to use context.Context with
a timeout (via context.WithTimeout) to enforce a bounded wait duration on the
start-build operation. Pass the timeout context through the command execution
chain and update error handling to properly handle context.DeadlineExceeded
errors that may occur when the timeout is exceeded.
Source: Coding guidelines
|
/test e2e-aws-techpreview-olmv1-ext |
1 similar comment
|
/test e2e-aws-techpreview-olmv1-ext |
1c6aca5 to
ae1e369
Compare
|
Running tests again with progression timeout set to CRD minimum of 10. |
ae1e369 to
a43d503
Compare
|
/test e2e-aws-techpreview-olmv1-ext |
Adding new tests to ReleaseGate suite to exercise them since they don't run in presubmits. Signed-off-by: Daniel Franz <dfranz@redhat.com>
a43d503 to
44a3970
Compare
|
/test e2e-aws-techpreview-olmv1-ext |
|
@dtfranz: 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. |
Exercising new tests manually since they won't run in the presubmits
Summary by CodeRabbit