MCO-2470: Disable scale-up test support for AWS and vSphere - #6344
Conversation
|
@proietfb: This pull request references MCO-2470 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. |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughThe scale-up irreconcilable-changes test removes AWS NVMe discovery, restricts execution to GCP and Azure, selects disks directly from platform defaults, and simplifies post-scale-up node selection. ChangesScale-up test simplification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 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 |
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/extended-priv/mco_irreconcilablechanges.go`:
- Around line 146-158: Move the skipSupportedPlatformNotMatched call in the
irreconcilable scale-up test before SkipTestIfWorkersCannotBeScaled, so
unsupported platforms are skipped without triggering the scalability assertion;
preserve the existing supported-platform restriction and subsequent test setup.
🪄 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: Pro Plus
Run ID: 73243af5-a3d4-4b6d-92b2-b669e6f7d8cf
📒 Files selected for processing (1)
test/extended-priv/mco_irreconcilablechanges.go
| g.It("[PolarionID:84219] Verify irreconcilable changes on new and existing nodes after scale up [Disruptive]", g.Label("Platform:gce", "Platform:azure"), func() { | ||
| var ( | ||
| machineconfiguration = GetMachineConfiguration(oc) | ||
| mcName = "irreconcilable-scaleup-test" | ||
| initialMcSpecs = machineconfiguration.GetSpecOrFail() | ||
| ) | ||
|
|
||
| SkipTestIfWorkersCannotBeScaled(oc) | ||
|
|
||
| // Only GCPPlatform and AzurePlatform are able to enumerate disks in a reliable manner right now. | ||
| // See https://redhat.atlassian.net/browse/MCO-2470 | ||
| skipTestIfSupportedPlatformNotMatched(oc, GCPPlatform, AzurePlatform) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Skip unsupported platforms before checking scalability.
Line 153 runs an assertion-backed scaling capability check on AWS, vSphere, and other unsupported platforms before Line 157 skips them. A WorkersCanBeScaled error therefore fails the test instead of disabling it as intended. Move the platform guard before SkipTestIfWorkersCannotBeScaled.
Proposed fix
- SkipTestIfWorkersCannotBeScaled(oc)
-
// Only GCPPlatform and AzurePlatform are able to enumerate disks in a reliable manner right now.
// See https://redhat.atlassian.net/browse/MCO-2470
skipTestIfSupportedPlatformNotMatched(oc, GCPPlatform, AzurePlatform)
+ SkipTestIfWorkersCannotBeScaled(oc)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| g.It("[PolarionID:84219] Verify irreconcilable changes on new and existing nodes after scale up [Disruptive]", g.Label("Platform:gce", "Platform:azure"), func() { | |
| var ( | |
| machineconfiguration = GetMachineConfiguration(oc) | |
| mcName = "irreconcilable-scaleup-test" | |
| initialMcSpecs = machineconfiguration.GetSpecOrFail() | |
| ) | |
| SkipTestIfWorkersCannotBeScaled(oc) | |
| // Only GCPPlatform and AzurePlatform are able to enumerate disks in a reliable manner right now. | |
| // See https://redhat.atlassian.net/browse/MCO-2470 | |
| skipTestIfSupportedPlatformNotMatched(oc, GCPPlatform, AzurePlatform) | |
| g.It("[PolarionID:84219] Verify irreconcilable changes on new and existing nodes after scale up [Disruptive]", g.Label("Platform:gce", "Platform:azure"), func() { | |
| var ( | |
| machineconfiguration = GetMachineConfiguration(oc) | |
| mcName = "irreconcilable-scaleup-test" | |
| initialMcSpecs = machineconfiguration.GetSpecOrFail() | |
| ) | |
| // Only GCPPlatform and AzurePlatform are able to enumerate disks in a reliable manner right now. | |
| // See https://redhat.atlassian.net/browse/MCO-2470 | |
| skipTestIfSupportedPlatformNotMatched(oc, GCPPlatform, AzurePlatform) | |
| SkipTestIfWorkersCannotBeScaled(oc) |
🤖 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 `@test/extended-priv/mco_irreconcilablechanges.go` around lines 146 - 158, Move
the skipSupportedPlatformNotMatched call in the irreconcilable scale-up test
before SkipTestIfWorkersCannotBeScaled, so unsupported platforms are skipped
without triggering the scalability assertion; preserve the existing
supported-platform restriction and subsequent test setup.
|
/pipeline required |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-dualstack-mco-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive-techpreview periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-azure-mco-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-gcp-mco-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive-techpreview-1of3 |
|
@proietfb: 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/57dbfde0-89e4-11f1-8312-5cdf9b125441-0 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-ipv4-mco-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-metal-ipi-ovn-ipv6-mco-disruptive-techpreview-1of3 |
|
@proietfb: trigger 2 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/f75d1f20-89e4-11f1-81de-a8527297dd33-0 |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: isabella-janssen, proietfb 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 |
|
/override ci/prow/unit This only touches payload tests |
|
@isabella-janssen: Overrode contexts on behalf of isabella-janssen: ci/prow/unit 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 kubernetes-sigs/prow repository. |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive-techpreview |
|
@proietfb: trigger 1 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/58040690-8a58-11f1-812e-39f85e34002f-0 |
Despite job failure, underlying tests related to this PR test suite passed/skipped. |
|
/verified by CI |
|
@proietfb: This PR has been marked as verified by 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. |
|
/override ci/prow/bootstrap-unit This has passed on this PR before and the PR does not impact any functionality in the MCO, just disruptive tests |
|
@isabella-janssen: Overrode contexts on behalf of isabella-janssen: ci/prow/bootstrap-unit 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 kubernetes-sigs/prow repository. |
|
@proietfb: 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. |
- What I did
Disabled scale-up test on AWS and vSphere.
- How to verify it
This test should skip on AWS and vSphere platforms
- Description for the changelog
AWS and vSphere does not offer right now a reliable manner to enumerate disks. For that reason, apart of some consuming workaround, automations cannot rely on gived disks name by prior. I'm disabling scale up tests to avoid failures on tests not related to the Irreconcilable Differences feature itself.
Summary by CodeRabbit