-
Notifications
You must be signed in to change notification settings - Fork 819
SPLAT-2826: Promote VSphereMultiVCenterDay2 to GA #2968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1722,12 +1722,10 @@ type VSpherePlatformNodeNetworking struct { | |
| type VSpherePlatformSpec struct { | ||
| // vcenters holds the connection details for services to communicate with vCenter. | ||
| // Up to 3 vCenters are supported. | ||
| // Once the cluster has been installed, you are unable to change the current number of defined | ||
| // vCenters except when 1.) the cluster has been upgraded from a version of OpenShift | ||
| // where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and | ||
| // remove vCenters but may not remove all vCenters. You may make modifications to the existing | ||
| // vCenters that are defined in the vcenters list in order to match with any added or modified | ||
| // failure domains. | ||
| // After installation, you can add or change vCenters, or remove some of them, but you must keep | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The godoc describes that users can "add or change vCenters, or remove some of them" but does not mention that these are mutually exclusive per-update operations. The FeatureGateAwareXValidation rules on lines 1709-1710 enforce that adds and removes cannot happen in the same update, e.g. Consider adding something like: "Adding and removing vCenters must be done as separate updates."
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'll make that change. |
||
| // at least one and may not add and remove vCenters during the same update. You may make modifications | ||
| // to the existing vCenters that are defined in the vcenters list in order to match with any added or | ||
| //modified failure domains. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing space after |
||
| // --- | ||
| // + If VCenters is not defined use the existing cloud-config configmap defined | ||
| // + in openshift-config. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: patching by array index (
x-kubernetes-validations/0) is fragile; if the rule order changes in a future generation, this silently patches out the wrong rule. Not a blocker since it matches the current generated output, but worth noting.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree. i wasn't sure of a way to patch it out for ratcheting tests. Is there something better i can do for this test?