HIVE-2391: vsphere zonal by 2uasimojo · Pull Request #2851 · openshift/hive

2uasimojo · 2026-02-10T22:09:50Z

Co-Authored-By: @dlom

Summary by CodeRabbit

New Features
- Multi-vCenter support and new Infrastructure-based vSphere platform (including richer failure-domain/topology and machine-pool disk/zone options).
- CLI: support for an installer platform JSON input and multi-vCenter flags.
Deprecations
- Legacy per-field vSphere settings (vCenter, datacenter, datastore, folder, cluster, network, etc.) deprecated in favor of Infrastructure/VCenters.
Documentation
- Updated docs and examples, secret format now supports a vcenters list and migration guidance included.

openshift-ci · 2026-02-10T22:11:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo

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~~ [2uasimojo]

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

openshift-ci-robot · 2026-02-10T22:57:06Z

@2uasimojo: This pull request references HIVE-2391 which is a valid jira issue.

Details

In response to this:

Co-Authored-By: @dlom

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.

codecov · 2026-02-10T23:39:25Z

Codecov Report

❌ Patch coverage is 26.71480% with 203 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.21%. Comparing base (aa1db74) to head (5e671cb).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
contrib/pkg/createcluster/create.go	0.00%	78 Missing ⚠️
pkg/controller/utils/credentials.go	0.00%	33 Missing ⚠️
.../clusterdeployment/clusterdeployment_controller.go	0.00%	20 Missing and 1 partial ⚠️
contrib/pkg/deprovision/vsphere.go	0.00%	15 Missing ⚠️
pkg/creds/vsphere/vsphere.go	0.00%	15 Missing ⚠️
pkg/installmanager/installmanager.go	0.00%	13 Missing ⚠️
...g/controller/clusterpool/clusterpool_controller.go	0.00%	11 Missing and 1 partial ⚠️
pkg/install/generate.go	0.00%	6 Missing ⚠️
pkg/clusterresource/vsphere.go	83.33%	4 Missing ⚠️
pkg/controller/utils/vsphereutils/vsphere.go	77.77%	2 Missing and 2 partials ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2851      +/-   ##
==========================================
- Coverage   50.28%   50.21%   -0.07%     
==========================================
  Files         280      281       +1     
  Lines       34332    34365      +33     
==========================================
- Hits        17264    17258       -6     
- Misses      15707    15751      +44     
+ Partials     1361     1356       -5

Files with missing lines	Coverage Δ
pkg/constants/constants.go	`100.00% <ø> (ø)`
...oller/clusterdeployment/installconfigvalidation.go	`100.00% <100.00%> (ø)`
...g/controller/machinepool/machinepool_controller.go	`64.33% <100.00%> (+0.52%)`	⬆️
pkg/controller/machinepool/vsphereactuator.go	`77.14% <100.00%> (+3.57%)`	⬆️
...hift/hive/apis/hive/v1/clusterdeprovision_types.go	`0.00% <ø> (ø)`
.../v1/clusterdeployment_validating_admission_hook.go	`86.29% <87.50%> (+1.56%)`	⬆️
pkg/clusterresource/vsphere.go	`92.95% <83.33%> (+2.83%)`	⬆️
pkg/controller/utils/vsphereutils/vsphere.go	`77.77% <77.77%> (ø)`
pkg/install/generate.go	`45.56% <0.00%> (-0.29%)`	⬇️
...g/controller/clusterpool/clusterpool_controller.go	`58.83% <0.00%> (-0.22%)`	⬇️
... and 6 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

2uasimojo · 2026-02-11T20:31:11Z

@jianping-shu this passed e2e-vsphere, so I reckon it's probably ready for you to take another stab at it!

2uasimojo · 2026-02-13T19:36:36Z

/hold

Looks like I missed refactoring the preflight auth check for the new creds shape.

2uasimojo · 2026-02-13T20:21:18Z

/hold cancel

dlom · 2026-02-18T20:31:47Z

The new multi-creds changes LGTM. My only concern (as noted in review comments) is that there are some additional fields (at least Folder, potentially more) on the machinepool that end users may want to use

2uasimojo · 2026-02-19T22:51:31Z

/hold for QE

2uasimojo · 2026-02-26T19:02:41Z

/test e2e security

e2e: infra flake
security: upstream bug with the packageURL check again (though possibly slightly different this time).

2uasimojo · 2026-02-26T22:22:40Z

Allowable to override security job (buggy upstream tool) if necessary when ready to merge.

dlom · 2026-03-24T19:33:56Z

+	}
+	// Scrub credentials
+	i := b.infrastructure.DeepCopy()
+	i.DeprecatedPassword = ""


Thoughts on clearing usernames here too? (both the deprecated path and in the array of VCenters)

Jianping brought up the same thing. I wasn't concerned about the username being sensitive; but I suppose for consistency with what we're doing elsewhere, it makes sense.

Oh, I just thought of a better reason: I think in some places we're only setting the username if it's unset. So there's a potential failure mode if the username is present but incorrect. (Though arguably that's a user error...)

dlom · 2026-03-24T19:35:15Z

+	// DeprecatedVCenter is the vSphere vCenter hostname.
+	// Deprecated: use VCenters instead.
+	// +optional
+	DeprecatedVCenter string `json:"vCenter"`


Needs omitempty in the json tag (and VCenters too)

Since neither is a pointer type and both have +optional, omitempty has no effect. I don't mind including the tag though.

dlom · 2026-03-24T19:37:40Z

+	}
+
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {


Probably worth it to test this more vigorously, I bet the LLM could generate tons of theoretical test cases

I'm a little meh about testing upstream functionality. Having a smoke test like this to make sure we're calling upstream correctly (or at all) is sane, but trying to cover all its paths seems like the responsibility of the repo in which it lives, no?

2uasimojo · 2026-03-24T20:26:40Z

Rebase only. Fixes pending.

2uasimojo · 2026-03-24T21:17:32Z

Okay, fixes are in. Thanks for the review @dlom

jianping-shu · 2026-03-25T03:32:46Z

regression test in progress...

2uasimojo · 2026-03-25T19:06:27Z

/test e2e-vsphere

2uasimojo · 2026-03-25T22:43:36Z

Unfortunately, since the issue in question here is important for security, I think it would be prudent to block this PR until the above is resolved.

That's merged.

Getting real close here. Currently just waiting on:

Regression QE (@jianping-shu)
/lgtm if all is well (@dlom)

jianping-shu · 2026-04-01T08:21:47Z

@dlom I've finished the round of regression tests.
Here are 2 findings:

For vsphere credential secret, data.vCenters works but data.vcenters is NOT working.
(1)data.vcenters worked for the previous commit but didn't work for the latest commit
(2) For using-hive.md, the cmd sample still uses 'vcenters'
oc create secret generic mycluster-vsphere-creds -n mynamespace --from-file=vcenters=/home/me/vsphere/vcenters.yaml
(3)For the vsphere credential secret generated by hiveutil, it uses 'vCenters'

- apiVersion: v1
  data:
    password: aaa
    username: bbb
    vCenters: ccc

The admission validation doesn't work for clusterpool, this is OK since HIVE-3131 change is NOT included in this PR yet.

I think we have 2 alternatives:
(A) Wait for Eric to review and address the issue 1
(B) If we expected to merge the PR sooner, then it is possible to merge the PR now and create one follow up PR to change the command is using-hive.md from 'vcenters' to 'vCenters' as temporary solution.
Pls. check and comment

cc @newtonheath @2uasimojo

2uasimojo · 2026-04-16T15:05:01Z

For vsphere credential secret, data.vCenters works but data.vcenters is NOT working.
(1)data.vcenters worked for the previous commit but didn't work for the latest commit

Looks like I "fixed" that here. This isn't the same as the json unmarshaling issue we were seeing previously: this is about the spelling of the key in the Secret, which is definitely case-sensitive. We just need to decide which spelling we want to use and make it consistent.

(Later) So my plan was to look at the installer's incarnations of this string and use whichever spelling they favor. Alas, it is not consistent there either -- there are cases of vcenters, vCenters, and even VCenters (I had thought capitalized keys weren't allowed in JSON for some reason).

However, the singular (deprecated in install-config platform, but still viable in metadata) is always spelled vCenter, which I guess is as good a reason as any to go with camel case here.

TL;DR: I'll update the doc to match the code.

2uasimojo · 2026-04-16T15:07:04Z

2. The admission validation doesn't work for clusterpool, this is OK since HIVE-3131 change is NOT included in this PR yet.

Okay, that PR is merged, so I'll just rebase this one...

2uasimojo · 2026-04-16T15:09:17Z

@dlom Given that Jianping already validated using vCenters as the creds Secret key, and the latest commit is just a doc update & rebase, and the clusterpool webhook was validated via HIVE-3131, I think we should be okay to merge this now.

Followon addressing review from openshift#2731. MachinePool: - Removed `Topology` override - Restored ResourcePool and TagIDs overrides - Removed `osImage` detected from arbitrary master; using whatever's passed through from FD Topology (which defaults sanely if unset). Deprovision: - Changed `--vsphere-vcenter` StringVar to `--vsphere-vcenters` StringSliceVar Platform Creds: - Redesigned to take `vcenters`, a list of vcenter server/username/password, matching (and unmarshaling into) the corresponding chunk of metadata.json. Docs: - Updated install-config sample to zonal shape. - Documented new creds shape.

jcpowermac · 2026-04-17T14:41:03Z

/test e2e-vsphere

2uasimojo · 2026-04-17T19:27:00Z

/test e2e-vsphere

...since openshift/release#77981

/test periodic-images

(flake)

/override ci/prow/security

Known, to be dealt with elsewhere.

openshift-ci · 2026-04-17T19:28:01Z

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security

Details

In response to this:

/test e2e-vsphere

...since openshift/release#77981

/test periodic-images

(flake)

/override ci/prow/security

Known, to be dealt with elsewhere.

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.

2uasimojo · 2026-04-17T22:56:51Z

/test e2e-vsphere

Gonna hope this was a temporary DNS flake...

2uasimojo · 2026-04-21T18:43:58Z

/assign @suhanime

2uasimojo · 2026-04-22T12:43:54Z

/hold cancel

This should be ready to go now.

2uasimojo · 2026-04-23T14:40:43Z

/test e2e-vsphere

Trying to confirm this test infra fix: we want to see a successful run that uses vcenter-120.ci.ibmc.devcluster.openshift.com.

openshift-ci · 2026-04-23T16:40:25Z

@2uasimojo: 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-ci Bot requested review from dlom and suhanime February 10, 2026 22:10

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

2uasimojo changed the title ~~Hive 2391: vsphere zonal~~ HIVE2391: vsphere zonal Feb 10, 2026

2uasimojo changed the title ~~HIVE2391: vsphere zonal~~ HIVE-2391: vsphere zonal Feb 10, 2026

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

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 29af27f to 5249404 Compare February 11, 2026 18:20

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 13, 2026

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 5249404 to 39cf13e Compare February 13, 2026 20:21

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 13, 2026

dlom reviewed Feb 18, 2026

View reviewed changes

Comment thread apis/hive/v1/vsphere/machinepools.go Outdated

dlom reviewed Feb 18, 2026

View reviewed changes

Comment thread apis/hive/v1/vsphere/machinepools.go Outdated

dlom reviewed Feb 18, 2026

View reviewed changes

Comment thread apis/hive/v1/vsphere/machinepools.go Outdated

dlom reviewed Feb 18, 2026

View reviewed changes

Comment thread pkg/controller/machinepool/vsphereactuator_test.go

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 39cf13e to 896e851 Compare February 19, 2026 22:49

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 19, 2026

dlom mentioned this pull request Feb 25, 2026

HIVE-2391: vSphere zonal support #2731

Closed

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 896e851 to 7996e22 Compare February 26, 2026 15:21

jianping-shu reviewed Feb 27, 2026

View reviewed changes

Comment thread pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go Outdated

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 7996e22 to 066a4e4 Compare February 27, 2026 16:46

dlom reviewed Mar 24, 2026

View reviewed changes

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from afded4e to 870fdd5 Compare March 24, 2026 20:26

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 870fdd5 to d3042d8 Compare March 24, 2026 21:17

2uasimojo mentioned this pull request Mar 25, 2026

HIVE-3131: Add ValidatingWebhookConfiguration for ClusterPool #2879

Merged

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from d3042d8 to 3eafe07 Compare April 16, 2026 15:04

HIVE-2391: vSphere zonal support

70ed4b3

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from 3eafe07 to dd75c19 Compare April 16, 2026 15:07

2uasimojo force-pushed the HIVE-2391/vsphere-zonal branch from dd75c19 to 5e671cb Compare April 16, 2026 16:01

openshift-ci Bot assigned suhanime Apr 21, 2026

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 22, 2026

Conversation

2uasimojo commented Feb 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-ci Bot commented Feb 10, 2026

Uh oh!

openshift-ci-robot commented Feb 10, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

2uasimojo commented Feb 11, 2026

Uh oh!

2uasimojo commented Feb 13, 2026

Uh oh!

2uasimojo commented Feb 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlom commented Feb 18, 2026

Uh oh!

2uasimojo commented Feb 19, 2026

Uh oh!

2uasimojo commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

2uasimojo commented Feb 26, 2026

Uh oh!

Uh oh!

dlom Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

2uasimojo Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dlom Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

2uasimojo Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dlom Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

2uasimojo Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

2uasimojo commented Mar 24, 2026

Uh oh!

2uasimojo commented Mar 24, 2026

Uh oh!

jianping-shu commented Mar 25, 2026

Uh oh!

2uasimojo commented Mar 25, 2026

Uh oh!

2uasimojo commented Mar 25, 2026

Uh oh!

jianping-shu commented Apr 1, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

2uasimojo commented Apr 16, 2026

Uh oh!

2uasimojo commented Apr 16, 2026

Uh oh!

2uasimojo commented Apr 16, 2026

Uh oh!

jcpowermac commented Apr 17, 2026

Uh oh!

2uasimojo commented Apr 17, 2026

Uh oh!

openshift-ci Bot commented Apr 17, 2026

Uh oh!

2uasimojo commented Apr 17, 2026

Uh oh!

2uasimojo commented Apr 21, 2026

2uasimojo commented Feb 10, 2026 •

edited by coderabbitai Bot

Loading

openshift-ci-robot commented Feb 10, 2026 •

edited by openshift-ci Bot

Loading

codecov Bot commented Feb 10, 2026 •

edited

Loading

2uasimojo commented Feb 26, 2026 •

edited

Loading

jianping-shu commented Apr 1, 2026 •

edited by openshift-ci Bot

Loading