Skip to content
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

SPLAT-1452: Changed vcenter MaxItems to be 3 for TechPreview. #1842

Merged
merged 3 commits into from May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,6 +1,7 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "Infrastructure"
crdName: infrastructures.config.openshift.io
featureGate: -VSphereMultiVCenters
tests:
onCreate:
- name: Should be able to create a minimal Infrastructure
Expand Down Expand Up @@ -182,6 +183,40 @@ tests:
machineNetworks:
- 192.0.2.1
expectedError: "spec.platformSpec.baremetal.machineNetworks[0]: Invalid value: \"string\": value must be a valid CIDR network address"
- name: Should not be able to pass more than 1 entries to vcenters in the vsphere platform spec
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
type: VSphere
vsphere:
failureDomains:
- name: generated-failure-domain
region: generated-region
server: server1.dev.cluster.com
topology:
computeCluster: /IBMCloud/host/vcs-8e-workload
datacenter: IBMCloud
datastore: /IBMCloud/datastore/mdcnc-ds-shared
networks:
- ocp-ci-seg-13
resourcePool: /IBMCloud/host/vcs-8e-workload/Resources
template: /IBMCloud/vm/ngirard-dev-rqh5s-rhcos-generated-region-generated-zone
zone: generated-zone
nodeNetworking:
external: {}
internal: {}
vcenters:
- datacenters:
- IBMCloud
port: 443
server: server1.dev.cluster.com
- datacenters:
- IBMCloud2
port: 443
server: server2.dev.cluster.com
expectedError: "Too many: 2: must have at most 1 items"
onUpdate:
- name: Should be able to change External platformName from unknown to something else
initial: |
Expand Down
@@ -0,0 +1,141 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "Infrastructure"
crdName: infrastructures.config.openshift.io
featureGate: VSphereMultiVCenters
tests:
onCreate:
- name: Should be able to create a minimal Infrastructure
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec: {} # No spec is required for a Infrastructure
expected: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec: {}
- name: Should be able to pass more than 1 entries to vcenters in the vsphere platform spec
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
cloudConfig:
key: config
name: cloud-provider-config
platformSpec:
type: VSphere
vsphere:
failureDomains:
- name: generated-failure-domain
region: generated-region
server: server1.dev.cluster.com
topology:
computeCluster: /IBMCloud/host/vcs-8e-workload
datacenter: IBMCloud
datastore: /IBMCloud/datastore/mdcnc-ds-shared
networks:
- ocp-ci-seg-13
resourcePool: /IBMCloud/host/vcs-8e-workload/Resources
template: /IBMCloud/vm/ngirard-dev-rqh5s-rhcos-generated-region-generated-zone
zone: generated-zone
nodeNetworking:
external: {}
internal: {}
vcenters:
- datacenters:
- IBMCloud
port: 443
server: server1.dev.cluster.com
- datacenters:
- IBMCloud2
port: 443
server: server2.dev.cluster.com
expected: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
cloudConfig:
key: config
name: cloud-provider-config
platformSpec:
type: VSphere
vsphere:
failureDomains:
- name: generated-failure-domain
region: generated-region
server: server1.dev.cluster.com
topology:
computeCluster: /IBMCloud/host/vcs-8e-workload
datacenter: IBMCloud
datastore: /IBMCloud/datastore/mdcnc-ds-shared
networks:
- ocp-ci-seg-13
resourcePool: /IBMCloud/host/vcs-8e-workload/Resources
template: /IBMCloud/vm/ngirard-dev-rqh5s-rhcos-generated-region-generated-zone
zone: generated-zone
nodeNetworking:
external: {}
internal: {}
vcenters:
- datacenters:
- IBMCloud
port: 443
server: server1.dev.cluster.com
- datacenters:
- IBMCloud2
port: 443
server: server2.dev.cluster.com
onUpdate:
- name: Should be able to set vCenters to multiple
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
cloudConfig:
key: config
name: cloud-provider-config
platformSpec:
type: VSphere
vsphere:
vcenters:
- datacenters:
- IBMCloud
port: 443
server: vcs8e-vc.ocp2.dev.cluster.com
updated: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
cloudConfig:
key: config
name: cloud-provider-config
platformSpec:
type: VSphere
vsphere:
vcenters:
- datacenters:
- IBMCloud
port: 443
server: vcs8e-vc.ocp2.dev.cluster.com
- datacenters:
- IBMCloud
port: 443
server: v8c-2-vcenter.ocp2.dev.cluster.com
expected: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
cloudConfig:
key: config
name: cloud-provider-config
platformSpec:
type: VSphere
vsphere:
vcenters:
- datacenters:
- IBMCloud
port: 443
server: vcs8e-vc.ocp2.dev.cluster.com
- datacenters:
- IBMCloud
port: 443
server: v8c-2-vcenter.ocp2.dev.cluster.com
3 changes: 2 additions & 1 deletion config/v1/types_infrastructure.go
Expand Up @@ -1343,8 +1343,9 @@ type VSpherePlatformSpec struct {
// ---
// + If VCenters is not defined use the existing cloud-config configmap defined
// + in openshift-config.
// +kubebuilder:validation:MaxItems=1
// +kubebuilder:validation:MinItems=0
// +openshift:validation:FeatureGateAwareMaxItems:featureGate="",maxItems=1
// +openshift:validation:FeatureGateAwareMaxItems:featureGate=VSphereMultiVCenters,maxItems=3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vr4manta just curious why only three? Is there a documented maximum?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, is in the epic. Not sure the reasoning for 3 being the max, but for now, that's what we have it being set to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this feature gate already exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had the variable before where we capped it to 1 max. We never supported multiple vCenters. This feature gate is to allow us to provide that function and use the gate in multiple operators where today we are limiting vCenter count to 1. In fact, multiple operators have hard coded limit set to one. With this feature gate, we'll be updating those logic to now allow multiple if this new FeatureGate is enabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature gate will need to be initialised in features/features.go else the tooling (and operators) won't actually know the feature gate exists

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoelSpeed , yes, I updated that file in this PR already. Is that the change you are thinking or did I miss something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently I just couldn't find it, ignore me

// +listType=atomic
// +optional
VCenters []VSpherePlatformVCenterSpec `json:"vcenters,omitempty"`
Expand Down
Expand Up @@ -898,7 +898,7 @@ spec:
- datacenters
- server
type: object
maxItems: 1
maxItems: 3
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down
Expand Up @@ -898,7 +898,7 @@ spec:
- datacenters
- server
type: object
maxItems: 1
maxItems: 3
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down
Expand Up @@ -898,7 +898,7 @@ spec:
- datacenters
- server
type: object
maxItems: 1
maxItems: 3
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down
1 change: 1 addition & 0 deletions config/v1/zz_generated.featuregated-crd-manifests.yaml
Expand Up @@ -303,6 +303,7 @@ infrastructures.config.openshift.io:
- GCPClusterHostedDNS
- GCPLabelsTags
- VSphereControlPlaneMachineSet
- VSphereMultiVCenters
FilenameOperatorName: config-operator
FilenameOperatorOrdering: "01"
FilenameRunLevel: "0000_10"
Expand Down
Expand Up @@ -870,7 +870,6 @@ spec:
- datacenters
- server
type: object
maxItems: 1
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down
Expand Up @@ -870,7 +870,6 @@ spec:
- datacenters
- server
type: object
maxItems: 1
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down
Expand Up @@ -870,7 +870,6 @@ spec:
- datacenters
- server
type: object
maxItems: 1
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down
Expand Up @@ -886,7 +886,6 @@ spec:
- datacenters
- server
type: object
maxItems: 1
minItems: 0
type: array
x-kubernetes-list-type: atomic
Expand Down