OCPBUGS-99903: Fixing test issue where error result was for other resource type - #1524
OCPBUGS-99903: Fixing test issue where error result was for other resource type#1524vr4manta wants to merge 2 commits into
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
jira/severity-important
|
@vr4manta: This pull request references Jira Issue OCPBUGS-99903, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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)
WalkthroughThe vSphere end-to-end tests now poll dry-run Infrastructure updates before selected live updates, synchronize denied and successful failure-domain removal checks, and accept non-deterministic reference errors from Machine, MachineSet, or ControlPlaneMachineSet resources. ChangesvSphere failure-domain validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant E2ETest
participant InfrastructureAPI
participant InfrastructureVAP
E2ETest->>InfrastructureAPI: Submit dry-run update
InfrastructureAPI->>InfrastructureVAP: Validate failure-domain removal
InfrastructureVAP-->>InfrastructureAPI: Deny or allow update
InfrastructureAPI-->>E2ETest: Return validation result
E2ETest->>InfrastructureAPI: Submit synchronized live update
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
@vr4manta: This pull request references Jira Issue OCPBUGS-99903, which is valid. 3 validation(s) were run on this bug
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. |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-rhcos10-techpreview-serial |
|
@vr4manta: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/afde40d0-89cf-11f1-84fa-63d7986a1b2b-0 |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-rhcos10-techpreview-serial |
|
@vr4manta: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/78648070-8a82-11f1-88c3-9b1991469a41-0 |
|
Previous payload-job tests did not hit the issue. Failures were from other tests. Rerunning in hopes of some green. |
|
/assign @damdo |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
This test failed: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/openshift-machine-api-operator-1524-nightly-5.0-e2e-vsphere-ovn-zones-techpreview-serial/2082085520304771072. I need to investigate more on why this one failed. |
|
New changes are detected. LGTM label has been removed. |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-rhcos10-techpreview-serial |
|
@vr4manta: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1bdeab00-8ab4-11f1-96c6-0731cd0e5156-0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/e2e/vsphere/failure_domain_vap.go`:
- Line 569: After deleting the MachineSet in the test flow, add a dry-run update
poll that waits for the VAP cache to accept the update before proceeding. Then
refetch the Infrastructure and pass that fresh object to the live update,
replacing the current direct waitForInfraUpdateToBeDenied call’s stale-object
path while preserving the existing post-deletion assertions.
- Around line 255-266: Update waitForInfraUpdateToBeDenied so its polling
callback succeeds only when the dry-run Update returns an admission denial
identified by apierrors.IsInvalid(err) or apierrors.IsForbidden(err), rather
than any error. Use a per-attempt timeout context for each
Infrastructures().Update call so Eventually can regain control when an attempt
blocks, while preserving the existing polling timeout and denial assertion.
🪄 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: 25772021-e49b-4bb4-8042-878749d54b05
📒 Files selected for processing (1)
test/e2e/vsphere/failure_domain_vap.go
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 `@test/e2e/vsphere/failure_domain_vap.go`:
- Around line 281-289: Update waitForInfraUpdateToSucceed so each polling
attempt creates a bounded context with a per-request timeout and uses it for
both Infrastructures().Get and Infrastructures().Update. Follow the existing
deny-path pattern, ensuring stalled API calls cannot outlive vapTestWaitTimeout
while preserving the current retry behavior.
🪄 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: 26089f02-9570-4c4d-9747-cb493920b6df
📒 Files selected for processing (1)
test/e2e/vsphere/failure_domain_vap.go
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-rhcos10-techpreview-serial |
|
@vr4manta: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/034444d0-8ab7-11f1-9a4e-b735348bca4c-0 |
4 participants
OCPBUGS-99903
Changes
Summary by CodeRabbit