Skip to content

MCO-2470: Disable scale-up test support for AWS and vSphere - #6344

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
proietfb:MCO-2470_Disable_scale-up_test_support_for_AWS_and_vSphere
Jul 28, 2026
Merged

MCO-2470: Disable scale-up test support for AWS and vSphere#6344
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
proietfb:MCO-2470_Disable_scale-up_test_support_for_AWS_and_vSphere

Conversation

@proietfb

@proietfb proietfb commented Jul 27, 2026

Copy link
Copy Markdown
Member

- 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

  • Tests
    • Simplified the irreconcilable-changes scale-up scenario flow.
    • Limited verification to GCP and Azure only.
    • Removed AWS-specific NVMe by-path discovery and probe-node handling.
    • Standardized disk selection and post-scale-up node selection to consistent single-node scaling behavior on the supported platforms.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 27, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@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.

Details

In response to this:

- 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.

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-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 16e76543-c067-4cc2-8b37-8e37bb5d4c2e

📥 Commits

Reviewing files that changed from the base of the PR and between ce87240 and a5384d6.

📒 Files selected for processing (1)
  • test/extended-priv/mco_irreconcilablechanges.go
💤 Files with no reviewable changes (1)
  • test/extended-priv/mco_irreconcilablechanges.go

Walkthrough

The 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.

Changes

Scale-up test simplification

Layer / File(s) Summary
Restrict supported platforms
test/extended-priv/mco_irreconcilablechanges.go
Removes the AWS-only NVMe discovery helper and limits the scale-up scenario to GCP and Azure.
Simplify scale-up flow
test/extended-priv/mco_irreconcilablechanges.go
Selects platform-based disks directly and always selects the first node after scaling to one node.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: dkhater-redhat, harshwardhanpatil07

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Microshift Test Compatibility ⚠️ Warning The scale-up Ginkgo test uses MachineSet creation/scale-to-1 and has no [Skipped:MicroShift]/apigroup guard. Add a MicroShift skip/tag or runtime exutil.IsMicroShiftCluster() guard, or exclude this scale-up test from MicroShift CI.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: disabling the scale-up test on AWS and vSphere.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The only Ginkgo titles in the changed file are static literals; no fmt.Sprintf, concatenation, timestamps, node names, or other dynamic data appear.
Test Structure And Quality ✅ Passed The updated scale-up test keeps bounded waits, cleans up MC/MachineSet resources via defers, and matches repository test patterns.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS: The scale-up test calls SkipIfCompactOrSNO in JustBeforeEach, and that helper skips SNO/compact clusters before any multi-node scaling logic runs.
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The PR only adjusts an e2e test’s platform skips/disk enumeration; it adds no pod scheduling constraints, replicas, affinities, selectors, or PDBs.
Ote Binary Stdout Contract ✅ Passed The only changed file adds no main/init/suite stdout writes; all logging stays inside It blocks, which the contract allows.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The scale-up test only manipulates disks on GCP/Azure; no IPv4 literals, IP parsing, localhost use, or external network calls were found.
No-Weak-Crypto ✅ Passed The only changed file is a test, and it contains no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or custom-crypto logic.
Container-Privileges ✅ Passed PR only edits a Go test file; no container/K8s manifests or privilege settings are changed.
No-Sensitive-Data-In-Logs ✅ Passed Patch only removes a scale-up skip call; the diff adds no logging and no new sensitive data exposure.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 27, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3b4a5c7 and ce87240.

📒 Files selected for processing (1)
  • test/extended-priv/mco_irreconcilablechanges.go

Comment on lines +146 to +158
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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.

@proietfb

Copy link
Copy Markdown
Member Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

  • The changed files don't match any pipeline_run_if_changed patterns
  • All files match pipeline_skip_if_only_changed patterns
  • No pipeline-controlled jobs are defined for the main branch

Use /test ? to see all available tests.

@proietfb

Copy link
Copy Markdown
Member Author

/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

@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@proietfb: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • 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

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/57dbfde0-89e4-11f1-8312-5cdf9b125441-0

@proietfb

Copy link
Copy Markdown
Member Author

/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

@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@proietfb: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • 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

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f75d1f20-89e4-11f1-81de-a8527297dd33-0

@isabella-janssen isabella-janssen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

  • The changed files don't match any pipeline_run_if_changed patterns
  • All files match pipeline_skip_if_only_changed patterns
  • No pipeline-controlled jobs are defined for the main branch

Use /test ? to see all available tests.

@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

[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

Details Needs approval from an approver in each of these files:
  • OWNERS [isabella-janssen,proietfb]

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

@isabella-janssen

Copy link
Copy Markdown
Member

/override ci/prow/unit

This only touches payload tests

@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@isabella-janssen: Overrode contexts on behalf of isabella-janssen: ci/prow/unit

Details

In response to this:

/override ci/prow/unit

This only touches payload tests

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

Copy link
Copy Markdown
Member Author

/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive-techpreview

@openshift-ci

openshift-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@proietfb: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/58040690-8a58-11f1-812e-39f85e34002f-0

@proietfb

proietfb commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

@proietfb: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-vsphere-mco-disruptive-techpreview

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.

Ref: longrun tests artifacts logs 1 2

@proietfb

Copy link
Copy Markdown
Member Author

/verified by CI

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 28, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@proietfb: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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.

@isabella-janssen

Copy link
Copy Markdown
Member

/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

@openshift-ci

openshift-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@isabella-janssen: Overrode contexts on behalf of isabella-janssen: ci/prow/bootstrap-unit

Details

In response to this:

/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

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

openshift-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@proietfb: all tests passed!

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-merge-bot
openshift-merge-bot Bot merged commit ceaa575 into openshift:main Jul 28, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants