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 both AWS and vSphere.
  • Updated scaling test coverage to ensure platform-appropriate disk path handling and to verify that exactly two non-boot disks are detected.
  • Extended scaling scenario validation so vSphere follows the same non-boot disk correctness expectations as AWS.

[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: e438b17c-2f22-4ad0-b022-e00a48bf2f99

📥 Commits

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

📒 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

---

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
Platform disk discovery test/extended-priv/mco_irreconcilablechanges.go	Adds vSphere SCSI by-path disk probing and clarifies platform-specific disk path behavior.
Scale-up probe flow test/extended-priv/mco_irreconcilablechanges.go	Extends probe-node creation and later node selection to vSphere, uses platform-specific disk discovery, and asserts two non-boot disks.

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

Suggested reviewers: umohnani8, isabella-janssen

---

Important

Pre-merge checks failed

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

❌ Failed checks (1 error, 2 warnings)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	The PR adds logger.Infof calls that print probeNode.GetName()/node.GetName(), which can expose internal hostnames.	Remove or redact node names from these logs, or downgrade them to non-identifying messages before merging.
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Microshift Test Compatibility	⚠️ Warning	The scale-up Ginkgo test still uses MachineSet/node scaling and has no MicroShift skip or apigroup tag, so it would run on MicroShift unsupported APIs/features.	Add [apigroup:machine.openshift.io] or [Skipped:MicroShift], or guard with exutil.IsMicroShiftCluster() before any scaling logic.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change by calling out vSphere scale-up test disk discovery.
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	All Ginkgo titles in the changed file are static strings; no generated names, timestamps, node/namespace values, or other dynamic data appear.
Test Structure And Quality	✅ Passed	PASS: the new vSphere probe-node flow uses bounded waits, cleans up MachineConfigs/MachineSets via defers, and follows existing extended-priv test patterns.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: the suite calls SkipIfCompactOrSNO in JustBeforeEach, which skips SNO/compact clusters before the scale-up node assumptions run.
Topology-Aware Scheduling Compatibility	✅ Passed	Only test disk-discovery logic changed; no manifests, replicas, affinities, nodeSelectors, or topology-based scheduling constraints were added.
Ote Binary Stdout Contract	✅ Passed	No new stdout writes in main/init/TestMain/top-level init code; changes are confined to test helpers and It bodies, with logging routed via GinkgoWriter.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No IPv4-only IP handling or external internet access in the modified e2e tests; the new probe-node logic only reads local disk paths on cluster nodes.
No-Weak-Crypto	✅ Passed	Only disk-path discovery logic changed; the patch/file contain no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons.
Container-Privileges	✅ Passed	Only a Go test file changed; no manifests or security fields like privileged, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation were added.

✨ 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