HIVE-2391: vSphere zonal support #2731

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

This PR is a re-do of #2541

Codecov Report

❌ Patch coverage is 19.91701% with 193 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.19%. Comparing base (78f50cd) to head (f248f65).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
contrib/pkg/createcluster/create.go	0.00%	48 Missing ⚠️
...om/openshift/hive/apis/hive/v1/vsphere/platform.go	0.00%	47 Missing ⚠️
pkg/controller/machinepool/vsphereactuator.go	52.94%	18 Missing and 6 partials ⚠️
.../clusterdeployment/clusterdeployment_controller.go	0.00%	18 Missing and 1 partial ⚠️
...g/controller/clusterpool/clusterpool_controller.go	0.00%	8 Missing and 1 partial ⚠️
pkg/controller/utils/credentials.go	0.00%	9 Missing ⚠️
contrib/pkg/deprovision/vsphere.go	0.00%	8 Missing ⚠️
pkg/installmanager/installmanager.go	0.00%	8 Missing ⚠️
.../v1/clusterdeployment_validating_admission_hook.go	53.33%	5 Missing and 2 partials ⚠️
pkg/install/generate.go	0.00%	6 Missing ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2731      +/-   ##
==========================================
- Coverage   50.40%   50.19%   -0.21%     
==========================================
  Files         279      280       +1     
  Lines       34194    34282      +88     
==========================================
- Hits        17236    17209      -27     
- Misses      15597    15706     +109     
- Partials     1361     1367       +6

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% <ø> (ø)`
...oller/clusterdeployment/installconfigvalidation.go	`91.66% <40.00%> (-8.34%)`	⬇️
...s/hive/v1/clusterpool_validating_admission_hook.go	`78.08% <28.57%> (-2.77%)`	⬇️
pkg/install/generate.go	`45.49% <0.00%> (-0.36%)`	⬇️
.../v1/clusterdeployment_validating_admission_hook.go	`84.64% <53.33%> (-0.10%)`	⬇️
contrib/pkg/deprovision/vsphere.go	`0.00% <0.00%> (ø)`
pkg/installmanager/installmanager.go	`35.85% <0.00%> (-0.14%)`	⬇️
...g/controller/clusterpool/clusterpool_controller.go	`58.83% <0.00%> (-0.22%)`	⬇️
pkg/controller/utils/credentials.go	`0.00% <0.00%> (ø)`
... and 4 more

🚀 New features to boost your workflow:

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

config/crds/hive.openshift.io_clusterdeployments.yaml

contrib/pkg/createcluster/create.go

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

pkg/controller/clusterpool/clusterpool_controller.go

 		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

apis/hive/v1/vsphere/platform.go

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

pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go

 	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.

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

pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go

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

pkg/controller/machinepool/awsactuator.go

pkg/controller/machinepool/nutanixactuator_test.go

dlom · 2025-10-22T21:24:14Z

go.mod

+
+replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.9.6
+
+replace github.com/openshift/installer => github.com/dlom/installer v0.0.0-20251022025850-76157f125d29


Currently pointing at my fork (openshift/installer#10025) (actually it's this https://github.com/dlom/installer/tree/add-deepcopy-to-types-1.4.19-ec5)

openshift-ci-robot · 2025-10-23T00:55:33Z

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

Details

In response to this:

xref: HIVE-2391

This PR is a re-do of #2541

See also: openshift/installer#10025

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.

dlom · 2025-10-23T04:58:38Z

/retest

jianping-shu · 2025-10-23T08:51:18Z

/retest

jianping-shu · 2025-10-23T12:31:00Z

jobs still failed. @dlom Pls. ping me when it is ready for testing, thanks

dlom · 2026-01-15T00:35:03Z

/override ci/prow/security

to be fixed in another PR

dlom · 2026-01-15T00:35:33Z

@2uasimojo this is ready for review (again)

/assign @2uasimojo

openshift-ci · 2026-01-15T00:35:38Z

@dlom: Overrode contexts on behalf of dlom: ci/prow/security

Details

In response to this:

/override ci/prow/security

to be fixed in another PR

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 · 2026-01-15T00:35:40Z

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

dlom · 2026-01-15T00:36:07Z

@jianping-shu please re-verify this PR, it is ready again

jianping-shu · 2026-01-15T01:33:36Z

Super! I'm on it.

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

config/crds/hive.openshift.io_clusterdeployments.yaml Outdated Show resolved Hide resolved

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

contrib/pkg/createcluster/create.go Show resolved Hide resolved

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

apis/hive/v1/vsphere/platform.go Outdated Show resolved Hide resolved

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 4f8c0bc to 0ce7d86 Compare October 22, 2025 21:06

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

dlom force-pushed the HIVE-2391 branch from 0ce7d86 to 29fc6e8 Compare October 22, 2025 21:17

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

dlom commented Oct 22, 2025

View reviewed changes

pkg/controller/machinepool/awsactuator.go Outdated Show resolved Hide resolved

dlom commented Oct 22, 2025

View reviewed changes

pkg/controller/machinepool/nutanixactuator_test.go Show resolved Hide resolved

dlom commented Oct 22, 2025

View reviewed changes

dlom force-pushed the HIVE-2391 branch from 29fc6e8 to ed0952f Compare October 22, 2025 23:41

dlom force-pushed the HIVE-2391 branch 2 times, most recently from 4c488c2 to 1714e7c Compare October 23, 2025 20:38

2uasimojo mentioned this pull request Jan 2, 2026

HIVE-3020: add MachinePool adoption documentation. #2815

Open

HIVE-2391: vSphere zonal support

f248f65

dlom force-pushed the HIVE-2391 branch from 1714e7c to f248f65 Compare January 14, 2026 21:27

openshift-ci bot assigned 2uasimojo Jan 15, 2026


		replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.9.6

		replace github.com/openshift/installer => github.com/dlom/installer v0.0.0-20251022025850-76157f125d29

HIVE-2391: vSphere zonal support #2731

Are you sure you want to change the base?

HIVE-2391: vSphere zonal support #2731

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

Uh oh!

Uh oh!

dlom Oct 22, 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!

openshift-ci-robot commented Oct 23, 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 Oct 23, 2025

Uh oh!

jianping-shu commented Oct 23, 2025

Uh oh!

jianping-shu commented Oct 23, 2025

Uh oh!

dlom commented Jan 15, 2026

Uh oh!

dlom commented Jan 15, 2026

Uh oh!

openshift-ci bot commented Jan 15, 2026

Uh oh!

openshift-ci bot commented Jan 15, 2026

Uh oh!

dlom commented Jan 15, 2026

Uh oh!

jianping-shu commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

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

dlom Oct 22, 2025 •

edited

Loading

openshift-ci-robot commented Oct 23, 2025 •

edited by openshift-ci bot

Loading