Skip to content

OSASINFRA-3179: openstack failureDomain: Add RootVolume type #1496

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
Jun 20, 2023
Merged

OSASINFRA-3179: openstack failureDomain: Add RootVolume type #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions machine/v1/0000_10_controlplanemachineset.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,11 @@ spec:
maxLength: 63
minLength: 1
pattern: ^[^ ]*$
volumeType:
description: volumeType specifies the type of the root volume that will be provisioned. If not specifified, the root volume will be created as the type in the machine template. The maximum length of a volume type name is 255 characters, as per the OpenStack limit.
type: string
maxLength: 255
minLength: 1
platform:
description: Platform identifies the platform for which the FailureDomain represents. Currently supported values are AWS, Azure, and GCP.
type: string
Expand Down
130 changes: 130 additions & 0 deletions machine/v1/stable.controlplanemachineset.openstack.testsuite.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,57 @@ tests:
openstack:
- rootVolume:
availabilityZone: foo
- name: Should accept an OpenStack failure domain with only the root volume type provided
initial: |
apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
spec:
selector:
matchLabels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
template:
machineType: machines_v1beta1_machine_openshift_io
machines_v1beta1_machine_openshift_io:
metadata:
labels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
machine.openshift.io/cluster-api-cluster: cluster
spec:
providerSpec: {}
failureDomains:
platform: OpenStack
openstack:
- rootVolume:
volumeType: typeone
expected: |
apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
spec:
replicas: 3
state: Inactive
strategy:
type: RollingUpdate
selector:
matchLabels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
template:
machineType: machines_v1beta1_machine_openshift_io
machines_v1beta1_machine_openshift_io:
metadata:
labels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
machine.openshift.io/cluster-api-cluster: cluster
spec:
providerSpec: {}
failureDomains:
platform: OpenStack
openstack:
- rootVolume:
volumeType: typeone
- name: Should accept an OpenStack failure domain with both availabilityZone and rootVolume provided
initial: |
apiVersion: machine.openshift.io/v1
Expand Down Expand Up @@ -248,6 +299,85 @@ tests:
- availabilityZone: foo
rootVolume:
availabilityZone: foo
- name: Should accept an OpenStack failure domain with both availabilityZone and root volume type provided
initial: |
apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
spec:
selector:
matchLabels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
template:
machineType: machines_v1beta1_machine_openshift_io
machines_v1beta1_machine_openshift_io:
metadata:
labels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
machine.openshift.io/cluster-api-cluster: cluster
spec:
providerSpec: {}
failureDomains:
platform: OpenStack
openstack:
- availabilityZone: foo
rootVolume:
volumeType: bar
expected: |
apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
spec:
replicas: 3
state: Inactive
strategy:
type: RollingUpdate
selector:
matchLabels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
template:
machineType: machines_v1beta1_machine_openshift_io
machines_v1beta1_machine_openshift_io:
metadata:
labels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
machine.openshift.io/cluster-api-cluster: cluster
spec:
providerSpec: {}
failureDomains:
platform: OpenStack
openstack:
- availabilityZone: foo
rootVolume:
volumeType: bar
- name: Should reject an OpenStack failure domain with too long a rootVolume volumeType name
initial: |
apiVersion: machine.openshift.io/v1
kind: ControlPlaneMachineSet
spec:
selector:
matchLabels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
template:
machineType: machines_v1beta1_machine_openshift_io
machines_v1beta1_machine_openshift_io:
metadata:
labels:
machine.openshift.io/cluster-api-machine-role: master
machine.openshift.io/cluster-api-machine-type: master
machine.openshift.io/cluster-api-cluster: cluster
spec:
providerSpec: {}
failureDomains:
platform: OpenStack
openstack:
- availabilityZone: foo
rootVolume:
volumeType: a123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345
expectedError: "spec.template.machines_v1beta1_machine_openshift_io.failureDomains.openstack[0].rootVolume.volumeType: Too long: may not be longer than 255"
- name: Should reject an OpenStack failure domain with an empty rootVolume provided
initial: |
apiVersion: machine.openshift.io/v1
Expand Down
8 changes: 8 additions & 0 deletions machine/v1/types_controlplanemachineset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,14 @@ type RootVolume struct {
// +kubebuilder:validation:Pattern=`^[^ ]*$`
// +optional
AvailabilityZone string `json:"availabilityZone,omitempty"`

// volumeType specifies the type of the root volume that will be provisioned.
// If not specifified, the root volume will be created as the type in the machine template.
// The maximum length of a volume type name is 255 characters, as per the OpenStack limit.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=255
// +optional
VolumeType string `json:"volumeType,omitempty"`
Copy link
Member

@EmilienM EmilienM Jun 15, 2023

Choose a reason for hiding this comment

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

This corresponds exactly with our machinev1alpha1 API:
https://github.com/openshift/api/blob/master/machine/v1alpha1/types_openstack.go#L358

Copy link
Contributor

@JoelSpeed JoelSpeed Jun 20, 2023

Choose a reason for hiding this comment

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

Are there definitely no character limitations on this? Are there any examples of valid volume types we can include in the godoc to give the user an idea of a correct value?

Copy link
Member Author

@pierreprinetti pierreprinetti Jun 20, 2023

Choose a reason for hiding this comment

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

There are no documented character limitations. Volume types are typically created by the OpenStack infrastructure administrators, and we are a tenant payload. This means that in practice, OpenShift administrators put here ready-made volume types that their OpenStack administrators make available to them.

For instance, as an admin of my cloud I just did this:

 $ openstack volume type create '432,+?^%$_ ù\f ~`helluva name'
+-------------+--------------------------------------+
| Field       | Value                                |
+-------------+--------------------------------------+
| description | None                                 |
| id          | ab3c8647-1bca-4ab8-89dd-f1ce1453ec3e |
| is_public   | True                                 |
| name        | 432,+?^%$_ ù\f ~`helluva name        |
+-------------+--------------------------------------+

Copy link
Contributor

@JoelSpeed JoelSpeed Jun 20, 2023

Choose a reason for hiding this comment

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

Ok, in that case I guess there's not a lot we can do

}

// ControlPlaneMachineSetStatus represents the status of the ControlPlaneMachineSet CRD.
Expand Down
1 change: 1 addition & 0 deletions machine/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17708,6 +17708,10 @@
"availabilityZone": {
"description": "availabilityZone specifies the Cinder availability zone where the root volume will be created. If not specifified, the root volume will be created in the availability zone specified by the volume type in the cinder configuration. If the volume type (configured in the OpenStack cluster) does not specify an availability zone, the root volume will be created in the default availability zone specified in the cinder configuration. See https://docs.openstack.org/cinder/latest/admin/availability-zone-type.html for more details. If the OpenStack cluster is deployed with the cross_az_attach configuration option set to false, the root volume will have to be in the same availability zone as the VM (defined by OpenStackFailureDomain.AvailabilityZone). Availability zone names must NOT contain spaces otherwise it will lead to volume that belongs to this availability zone register failure, see kubernetes/cloud-provider-openstack#1379 for further information. The maximum length of availability zone name is 63 as per labels limits.",
"type": "string"
},
"volumeType": {
"description": "volumeType specifies the type of the root volume that will be provisioned. If not specifified, the root volume will be created as the type in the machine template. The maximum length of a volume type name is 255 characters, as per the OpenStack limit.",
"type": "string"
}
}
},
Expand Down