SPLAT-1582: Multi vCenter Support #1660
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@vr4manta: This pull request references SPLAT-1582 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set. In response to this:
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. |
1 similar comment
|
@vr4manta: This pull request references SPLAT-1582 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set. In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| deferred to a later phase of implementation that may call for its own | ||
| enhancement. | ||
|
|
||
| - Updating cloud config to yaml format for existing clusters (upgrading OCP to 4.17+) |
There was a problem hiding this comment.
Should we call out that only clusters installed on 4.17(or the y-stream where this is GA'ed) or later can use this feature?
There was a problem hiding this comment.
Sounds good. I'll update that.
| The multiple vCenter feature will begin allowing more than one vCenter to be | ||
| configured in the infrastructure resource. We will be controlling this via | ||
| a new feature (VSphereMultiVCenters) and will have different CRDs installed | ||
| based on this gate. | ||
|
|
||
| Initially, the plans are to allow a max of 3 vCenters to be configured when the | ||
| feature gate is enabled. The way we are going to control this is by adding new | ||
| control annotations to the model objects. | ||
|
|
||
| The OpenShift controller tools will be enhanced to allow a new Feature Gate | ||
| Aware config option for max size. | ||
|
|
||
| Example: | ||
| ```go | ||
| // +kubebuilder:validation:MinItems=0 | ||
| // +openshift:validation:FeatureGateAwareMaxItems:featureGate="",maxItems=1 | ||
| // +openshift:validation:FeatureGateAwareMaxItems:featureGate=VSphereMultiVCenters,maxItems=3 | ||
| // +listType=atomic | ||
| // +optional | ||
| VCenters []VSpherePlatformVCenterSpec `json:"vcenters,omitempty"` |
There was a problem hiding this comment.
Should we consider a webhook to block updates if the cluster doesnt have a yaml cloud config?
There was a problem hiding this comment.
We recently updated the CRD to block changes once this field is set in 4.17. Once we finish migration testing and fixes, the CRD will be updated to not have those blockers anymore. Right now I do believe the cluster admin will have to manually migrate this config. I am hoping we can maybe create an automated process for this.
| The OpenShift controller tools will be enhanced to allow a new Feature Gate | ||
| Aware config option for max size. |
There was a problem hiding this comment.
This was already done right?
There was a problem hiding this comment.
Yes, I am working on updating verbage to not be future tense based on reviews from others. Sorry for the confusion.
| Next the operator was enhanced to be able to support using the upstream vSphere | ||
| YAML cloud provider config format. There is some logic that uses our old legacy | ||
| style config. To preserve this, we created a wrapper config object that attempts | ||
| to load the cloud provider config as either INI or YAML. If its INI, we will | ||
| also store the INI data into the `LegacyConfig` field so we can access it in | ||
| certain situations. |
There was a problem hiding this comment.
We have had people confused by the errors from trying to read old vs new, is there anything we can do to mitigate these errors from being produced?
There was a problem hiding this comment.
I am working with upstream to fix these outputs: kubernetes/cloud-provider-vsphere#1225
| ### Multiple vCenters Configured as Day 2 | ||
|
|
||
| NOTE: This section is placeholder for future design / work. |
There was a problem hiding this comment.
What do we actually support In OCP 4.17? It is possible to edit Infrastructure after installation and all the operators try to respect it. But is it supported? Should we block such edits until we figure out what we actually support?
There was a problem hiding this comment.
In 4.17, we are not supporting migrating single vCenter to multiple. That work is still being validated to see what all is not working. We did have a PR to make sure in 4.17 that the CRD prevents user from changing vCenters list after install.
Co-authored-by: Richard Vanderpool <[email protected]>
Co-authored-by: Richard Vanderpool <[email protected]>
Co-authored-by: Richard Vanderpool <[email protected]>
|
@vr4manta: 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. |
SPLAT-1582
Changes