HIVE-2391: vSphere zonal support by dlom · Pull Request #2731 · openshift/hive

dlom · 2025-08-11T19:10:12Z

This PR is a re-do of #2541

Codecov Report

❌ Patch coverage is 26.31579% with 154 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.29%. Comparing base (78f50cd) to head (beb96b3).
⚠️ Report is 57 commits behind head on master.

Files with missing lines	Patch %	Lines
contrib/pkg/createcluster/create.go	0.00%	48 Missing ⚠️
pkg/controller/machinepool/vsphereactuator.go	52.94%	18 Missing and 6 partials ⚠️
.../clusterdeployment/clusterdeployment_controller.go	0.00%	20 Missing and 1 partial ⚠️
...g/controller/clusterpool/clusterpool_controller.go	0.00%	10 Missing and 1 partial ⚠️
pkg/controller/utils/credentials.go	0.00%	9 Missing ⚠️
.../v1/clusterdeployment_validating_admission_hook.go	10.00%	6 Missing and 3 partials ⚠️
contrib/pkg/deprovision/vsphere.go	0.00%	8 Missing ⚠️
pkg/installmanager/installmanager.go	0.00%	8 Missing ⚠️
pkg/install/generate.go	0.00%	6 Missing ⚠️
...s/hive/v1/clusterpool_validating_admission_hook.go	0.00%	4 Missing and 1 partial ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2731      +/-   ##
==========================================
- Coverage   50.40%   50.29%   -0.12%     
==========================================
  Files         279      280       +1     
  Lines       34194    34262      +68     
==========================================
- Hits        17236    17232       -4     
- Misses      15597    15661      +64     
- Partials     1361     1369       +8

Files with missing lines	Coverage Δ
pkg/clusterresource/vsphere.go	`88.05% <100.00%> (-2.07%)`	⬇️
...hift/hive/apis/hive/v1/clusterdeprovision_types.go	`0.00% <ø> (ø)`
pkg/controller/utils/vsphereutils/vsphere.go	`88.88% <88.88%> (ø)`
...oller/clusterdeployment/installconfigvalidation.go	`91.66% <40.00%> (-8.34%)`	⬇️
...s/hive/v1/clusterpool_validating_admission_hook.go	`78.08% <0.00%> (-2.77%)`	⬇️
pkg/install/generate.go	`45.49% <0.00%> (-0.36%)`	⬇️
contrib/pkg/deprovision/vsphere.go	`0.00% <0.00%> (ø)`
pkg/installmanager/installmanager.go	`35.85% <0.00%> (-0.14%)`	⬇️
pkg/controller/utils/credentials.go	`0.00% <0.00%> (ø)`
.../v1/clusterdeployment_validating_admission_hook.go	`84.32% <10.00%> (-0.41%)`	⬇️
... and 4 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

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

dlom · 2025-09-10T17:45:39Z

/retest

jianping-shu · 2025-09-11T07:45:45Z

@dlom I tried the new commit, install vsphere cd with old format i.e. without platform.vsphere.vSphere still failed due to the same error. Pls. check the comments and hive controlller log attached on https://issues.redhat.com/browse/HIVE-2917.

newtonheath · 2025-09-16T09:07:20Z

@dlom can you pick up this again now private image work is in review - need to get this wrapped up

jianping-shu · 2025-10-17T06:39:22Z

 		return cloudBuilder, nil
 	case platform.VSphere != nil:
+		if platform.VSphere.VSphere == nil {
+			return nil, errors.New("VSphere CD with deprecated fields has not been updated by CD controller yet, requeueing...")


It should be CP/CP controller instead of CD/CD controller

I didn't see the code logic of converting platform.vsphere.attributes to platform.vsphere.vSphere in clusterpool_controller.go like in clusterpool_deployment.go

Since ClusterPool support for vSphere is NOT supported officially yet(HIVE-2801 not closed and hive doc not updated). One choice is if we can make the judgement/statement that the ClusterPool only support the new format of platform.vsphere.vSphere, this part of code change can be avoided. In that case we need at least make the error in condition/log appropriate and clearer.

I think ClusterPool support will have to be bundled into this PR, to prevent a mismatch in functionality between our CD/CP controllers. I'm going to try to add a similar approach for the CP that I used on the CD

2uasimojo · 2025-10-21T22:30:01Z

 	contextLogger.Data["oldObject.Name"] = oldObject.Name

+	// HIVE-2391
+	if oldObject.Spec.Platform.VSphere != nil && cd.Spec.Platform.VSphere != nil {


This allows/enforces a model where you can transition from old => old+new. I think it would be better if we instead enforced old => new -- i.e. you're forced to scrub the deprecated fields when upconverting. I realize this is going to entail more code because we have to look at each field individually and make sure it's "" (empty string). But IMHO the end result is a lot cleaner.

BTW, I did notice your comment about performance. We're really not concerned about adding more local comparison logic -- the resource consumption, wallclock time, etc is negligible against the overall operation: most of the burden comes from shoving objects over the network.

Fixed this up so you can do it either way (remove deprecated content or leave it and it'll be ignored).

Added UT for these paths as well.

2uasimojo · 2025-10-21T22:30:39Z

+		if oldObject.Spec.Platform.VSphere.VSphere == nil && cd.Spec.Platform.VSphere.VSphere != nil {
+			contextLogger.Debug("Passed validation: HIVE-2391")
+			return &admissionv1beta1.AdmissionResponse{
+				Allowed: true,


As written I think this short circuit would allow you to stealth-edit the other otherwise-immutable fields as part of upconverting. That is undesirable :)

Please add UT to confirm that this ⬆️ can't happen.

While this is true, it's not a problem, as the deprecated fields will be ignored after converting anyway.

UT is added to cover these upconversion special cases.

2uasimojo · 2026-02-03T16:58:01Z

/test e2e-vsphere

infra flake during IPI workflow

openshift-ci · 2026-02-03T18:57:33Z

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

jianping-shu · 2026-02-04T02:40:25Z

@dlom I didn't see the code changes regarding topology in MachinePool and creds for multi-vcenters. Pls. let me know when PR is ready for testing, thanks

Followon addressing review from openshift#2731. MachinePool: - Removed `Topology` override - Restored ResourcePool and TagIDs overrides 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.

dlom · 2026-02-25T20:05:08Z

Superseded by #2851

Followon addressing review from openshift#2731. MachinePool: - Removed `Topology` override - Restored ResourcePool and TagIDs overrides 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.

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.

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

openshift-ci Bot requested review from 2uasimojo and jstuever August 11, 2025 19:10

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2025

jianping-shu reviewed Sep 8, 2025

View reviewed changes

Comment thread config/crds/hive.openshift.io_clusterdeployments.yaml Outdated

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2025

jianping-shu reviewed Sep 8, 2025

View reviewed changes

Comment thread contrib/pkg/createcluster/create.go

dlom force-pushed the HIVE-2391 branch from 23e8a39 to 57323f4 Compare September 9, 2025 01:01

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2025

dlom force-pushed the HIVE-2391 branch 2 times, most recently from 556f051 to afcf08f Compare September 9, 2025 18:02

This comment was marked as resolved.

Sign in to view

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2025

dlom force-pushed the HIVE-2391 branch from afcf08f to 222e9fb Compare September 20, 2025 00:02

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2025

dlom force-pushed the HIVE-2391 branch 2 times, most recently from 3b426a3 to 981416a Compare September 25, 2025 05:53

jianping-shu reviewed Oct 17, 2025

View reviewed changes

2uasimojo reviewed Oct 17, 2025

View reviewed changes

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

dlom force-pushed the HIVE-2391 branch from 981416a to 4f8c0bc Compare October 18, 2025 00:36

2uasimojo reviewed Oct 21, 2025

View reviewed changes

dlom mentioned this pull request Oct 22, 2025

CORS-4266: Add generated DeepCopy implementations for pkg/types/installconfig openshift/installer#10025

Merged

dlom force-pushed the HIVE-2391 branch from f248f65 to beb96b3 Compare January 29, 2026 20:35

dlom closed this Feb 25, 2026

Conversation

dlom commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

openshift-ci Bot commented Aug 11, 2025

Uh oh!

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

Uh oh!

Uh oh!

dlom commented Aug 11, 2025 • 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 Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

dlom commented Sep 10, 2025

Uh oh!

This comment was marked as resolved.

jianping-shu commented Sep 11, 2025

Uh oh!

newtonheath commented Sep 16, 2025

Uh oh!

jianping-shu Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

jianping-shu Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlom Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

2uasimojo Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2uasimojo Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

2uasimojo Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2uasimojo Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

2uasimojo commented Feb 3, 2026

Uh oh!

openshift-ci Bot commented Feb 3, 2026

Uh oh!

jianping-shu commented Feb 4, 2026

Uh oh!

dlom commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dlom commented Aug 11, 2025 •

edited

Loading

openshift-ci-robot commented Aug 11, 2025 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Aug 11, 2025 •

edited by openshift-ci Bot

Loading

dlom commented Aug 11, 2025 •

edited by openshift-ci Bot

Loading

codecov Bot commented Aug 11, 2025 •

edited

Loading

jianping-shu Oct 17, 2025 •

edited

Loading

2uasimojo Oct 21, 2025 •

edited

Loading

2uasimojo Oct 21, 2025 •

edited

Loading