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 22.22222% with 154 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.32%. Comparing base (b0ef85f) to head (1714e7c).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...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 ⚠️
contrib/pkg/createcluster/create.go	0.00%	18 Missing ⚠️
.../clusterdeployment/clusterdeployment_controller.go	0.00%	9 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	36.36%	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.45%   50.32%   -0.14%     
==========================================
  Files         284      285       +1     
  Lines       33968    34007      +39     
==========================================
- Hits        17140    17113      -27     
- Misses      15489    15549      +60     
- Partials     1339     1345       +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	`46.85% <0.00%> (-0.40%)`	⬇️
.../v1/clusterdeployment_validating_admission_hook.go	`86.92% <36.36%> (-0.11%)`	⬇️
contrib/pkg/deprovision/vsphere.go	`0.00% <0.00%> (ø)`
pkg/installmanager/installmanager.go	`35.53% <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.

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

openshift-ci · 2025-10-24T00:29:18Z

@dlom: all tests passed!

Full PR test history. Your PR dashboard.

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

Open

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 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 from ed0952f to 4c488c2 Compare October 23, 2025 18:55

HIVE-2391: vSphere zonal support

1714e7c

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


		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

Uh oh!

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!

openshift-ci bot commented Oct 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 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

dlom Oct 22, 2025 •

edited

Loading

openshift-ci-robot commented Oct 23, 2025 •

edited by openshift-ci bot

Loading