Irreconcilable Differences scale-up test: apply disk discovering on vSphere scale-up test

[proietfb]

[PolarionID:84219] vSphere scale-up test by discovering stable disk paths at runtime

On vSphere, the PVSCSI controller's PCI address is not guaranteed to be fixed across all VM hardware versions, making /dev/disk/by-path/ entries non-predictable at test-write time. Additionally, the raw /dev/sdb /dev/sdc names passed to the Ignition MachineConfig caused the newly provisioned VM to boot without ever obtaining a network IP address, timing out the WaitUntilReady("10m") call with a context deadline exceeded error.

Apply the same probe-node pattern already used for AWS NVMe disks.

- What I did
In place discovery of provisioned disks by-path instead of by-name

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of non-boot disk discovery validation during irreconcilable machine configuration scenarios on AWS and vSphere.
  • Updated scaling validation to handle platform-specific disk paths and confirm that exactly two non-boot disks are detected.
  • Extended scaling scenario checks so vSphere applies the same non-boot disk correctness expectations as AWS.
  • Excluded partitioned devices from disk discovery to improve validation accuracy.

[openshift-merge-bot]

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]

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: b5b42bc2-b4e8-4dd1-be46-ca2714aa5291

📥 Commits

Reviewing files that changed from the base of the PR and between 7ff337a and a5ccaac.

📒 Files selected for processing (1)

• test/extended-priv/mco_irreconcilablechanges.go

🚧 Files skipped from review as they are similar to previous changes (1)

• test/extended-priv/mco_irreconcilablechanges.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

Walkthrough

The irreconcilable machine config test adds runtime discovery of non-boot vSphere SCSI disks, extends probe-node setup to vSphere, selects disk discovery by platform, validates two disks, and broadens scale-up node selection.

Changes

Platform-specific disk discovery and scaling

Layer / File(s)	Summary
Disk discovery and scale-up flow test/extended-priv/mco_irreconcilablechanges.go	The test probes non-boot vSphere SCSI paths, selects AWS NVMe or vSphere SCSI paths, requires two disks, and applies scale-up handling to both platforms.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to a5cca

This PR updates a single vSphere scale-up test to discover provisioned disks at runtime; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: umohnani8, isabella-janssen

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	The new INFO log prints node.GetName() and runtime SCSI paths; the node name may be an internal hostname exposed in collected test logs.	Log only the disk count, or redact the node identifier and path list before logging.

✅ 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 identifies the irreconcilable differences scale-up test and the disk discovery update for vSphere.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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 PR changes only disk discovery and scaling logic. All five Ginkgo It titles remain unchanged static literals, with no node, namespace, IP, timestamp, UUID, or generated value interpolation.
Test Structure And Quality	✅ Passed	The vSphere path follows the existing probe pattern, adds cleanup through the deferred MachineSet removal, uses 10m waits and bounded Eventually calls, and gives new discovery assertions diagnostic...
Microshift Test Compatibility	✅ Passed	The PR adds no Ginkgo test declaration. Its changes extend an existing test with Node debug probing and vSphere disk paths, without introducing a listed MicroShift-unavailable API or resource.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR modifies an existing test, not a new Ginkgo test. Its enclosing Describe calls SkipIfCompactOrSNO, which invokes g.Skip for SNO before the changed node-scaling paths run.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff modifies only an extended test's disk discovery and MachineSet scaling; it adds no manifests, controllers, replicas, affinity, topology spread, selectors, tolerations, or PDBs.
Ote Binary Stdout Contract	✅ Passed	The PR adds no direct stdout write. Its new logger.Infof routes to GinkgoWriter, and the SCSI probe runs from an It body; remote stdout is captured.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The diff modifies an existing Ginkgo test and local node disk probing only; it adds no IPv4 literals, IP parsing, URL requests, public hosts, or external connectivity.
No-Weak-Crypto	✅ Passed	The PR changes only vSphere disk-path discovery and variable scope; the diff adds no MD5, SHA1, DES, RC4, Blowfish, ECB, crypto, or secret-comparison code.
Container-Privileges	✅ Passed	The PR changes only one Go test file and adds no container/Kubernetes manifest or listed privilege setting; it reuses the existing node-debug helper.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 [proietfb]

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

[proietfb]

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

[openshift-ci]

@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/18b2b8f0-89c3-11f1-9de2-5f4a7266e8dc-0

[coderabbitai]

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 248-258: Replace the branch-local := declarations in the platform
conditional with assignments to the outer discoveredDisks declared before it, so
the AWS and vSphere discovery results remain available when assigning disks
afterward. Preserve the existing discovery functions and length assertions.

🪄 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: 6cd1507a-817e-4412-a440-79731d148d7b

📥 Commits

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

📒 Files selected for processing (1)

• test/extended-priv/mco_irreconcilablechanges.go

[proietfb]

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

[openshift-ci]

@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/b34f25a0-89c4-11f1-8dad-1493eca28f53-0

[proietfb]

/hold

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[openshift-ci]

@proietfb: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/unit	a5ccaac	link	true	/test unit

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.