From 2f40ac813b43e950a5019e9ebc0f8153e139f002 Mon Sep 17 00:00:00 2001 From: Matt Pryor Date: Wed, 13 Sep 2023 14:27:13 +0100 Subject: [PATCH 1/4] UPSTREAM 1668: Additional data volumes for machines We now have the ability for a machine to have additional block devices to be attached. Here is an example on how to use `additionalBlockDevices` for adding additional Cinder volumes attached to the server instance: ```yaml additionalBlockDevices: - nameSuffix: dbvol1 size: 50 volumeType: my-volume-type availabilityZone: az0 tag: database ``` Co-authored-by: Matt Pryor Co-authored-by: Emilien Macchi (cherry picked from commit 94d96906bed2d2322d2d4c23303220ef7d1805c1) --- api/v1alpha5/conversion.go | 4 + api/v1alpha5/zz_generated.conversion.go | 16 +- api/v1alpha6/conversion.go | 11 +- api/v1alpha6/zz_generated.conversion.go | 16 +- api/v1alpha7/openstackmachine_types.go | 5 + api/v1alpha7/types.go | 14 + api/v1alpha7/zz_generated.deepcopy.go | 20 ++ ...re.cluster.x-k8s.io_openstackclusters.yaml | 30 ++ ...er.x-k8s.io_openstackclustertemplates.yaml | 34 +++ ...re.cluster.x-k8s.io_openstackmachines.yaml | 30 ++ ...er.x-k8s.io_openstackmachinetemplates.yaml | 30 ++ controllers/openstackmachine_controller.go | 23 +- .../crd-changes/v1alpha6-to-v1alpha7.md | 18 +- pkg/cloud/services/compute/instance.go | 263 +++++++++++------- pkg/cloud/services/compute/instance_test.go | 246 ++++++++++++++-- pkg/cloud/services/compute/instance_types.go | 31 ++- .../patch-machine-template-control-plane.yaml | 5 + .../patch-machine-template-worker.yaml | 7 + test/e2e/suites/e2e/e2e_test.go | 59 +++- 19 files changed, 668 insertions(+), 194 deletions(-) diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index 8fec9715e8..3f41962b92 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -433,3 +433,7 @@ func Convert_v1alpha5_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterStatus( return nil } + +func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *infrav1.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error { + return autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in, out, s) +} diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index ef43a3c824..0ad34a27d8 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -194,11 +194,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha7.OpenStackMachineSpec)(nil), (*OpenStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(a.(*v1alpha7.OpenStackMachineSpec), b.(*OpenStackMachineSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*OpenStackMachineStatus)(nil), (*v1alpha7.OpenStackMachineStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha5_OpenStackMachineStatus_To_v1alpha7_OpenStackMachineStatus(a.(*OpenStackMachineStatus), b.(*v1alpha7.OpenStackMachineStatus), scope) }); err != nil { @@ -394,6 +389,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha7.OpenStackMachineSpec)(nil), (*OpenStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(a.(*v1alpha7.OpenStackMachineSpec), b.(*OpenStackMachineSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha7.PortOpts)(nil), (*PortOpts)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha7_PortOpts_To_v1alpha5_PortOpts(a.(*v1alpha7.PortOpts), b.(*PortOpts), scope) }); err != nil { @@ -1134,16 +1134,12 @@ func autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec( out.ServerMetadata = *(*map[string]string)(unsafe.Pointer(&in.ServerMetadata)) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) + // WARNING: in.AdditionalBlockDevices requires manual conversion: does not exist in peer-type out.ServerGroupID = in.ServerGroupID out.IdentityRef = (*OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } -// Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec is an autogenerated conversion function. -func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *v1alpha7.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error { - return autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in, out, s) -} - func autoConvert_v1alpha5_OpenStackMachineStatus_To_v1alpha7_OpenStackMachineStatus(in *OpenStackMachineStatus, out *v1alpha7.OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index 379eb17eac..0882aac11c 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -60,13 +60,12 @@ func restorev1alpha7MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *inf // PropagateUplinkStatus has been added in v1alpha7. // We restore the whole Ports since they are anyway immutable. dst.Ports = previous.Ports + dst.AdditionalBlockDevices = previous.AdditionalBlockDevices } func restorev1alpha7Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) { - // PropagateUplinkStatus has been added in v1alpha7. - // We restore the whole Ports since they are anyway immutable. - if *previous != nil && (*previous).Instance.Ports != nil && *dst != nil && (*dst).Instance.Ports != nil { - (*dst).Instance.Ports = (*previous).Instance.Ports + if *previous != nil && *dst != nil { + restorev1alpha7MachineSpec(&(*previous).Instance, &(*dst).Instance) } } @@ -646,3 +645,7 @@ func Convert_v1alpha6_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterStatus( return nil } + +func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *infrav1.OpenStackMachineSpec, out *OpenStackMachineSpec, s apiconversion.Scope) error { + return autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in, out, s) +} diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index 79eedbedce..dc87a6d961 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -204,11 +204,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha7.OpenStackMachineSpec)(nil), (*OpenStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(a.(*v1alpha7.OpenStackMachineSpec), b.(*OpenStackMachineSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*OpenStackMachineStatus)(nil), (*v1alpha7.OpenStackMachineStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_OpenStackMachineStatus_To_v1alpha7_OpenStackMachineStatus(a.(*OpenStackMachineStatus), b.(*v1alpha7.OpenStackMachineStatus), scope) }); err != nil { @@ -404,6 +399,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha7.OpenStackMachineSpec)(nil), (*OpenStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(a.(*v1alpha7.OpenStackMachineSpec), b.(*OpenStackMachineSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha7.PortOpts)(nil), (*PortOpts)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha7_PortOpts_To_v1alpha6_PortOpts(a.(*v1alpha7.PortOpts), b.(*PortOpts), scope) }); err != nil { @@ -1157,16 +1157,12 @@ func autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec( out.ServerMetadata = *(*map[string]string)(unsafe.Pointer(&in.ServerMetadata)) out.ConfigDrive = (*bool)(unsafe.Pointer(in.ConfigDrive)) out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) + // WARNING: in.AdditionalBlockDevices requires manual conversion: does not exist in peer-type out.ServerGroupID = in.ServerGroupID out.IdentityRef = (*OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } -// Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec is an autogenerated conversion function. -func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *v1alpha7.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error { - return autoConvert_v1alpha7_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in, out, s) -} - func autoConvert_v1alpha6_OpenStackMachineStatus_To_v1alpha7_OpenStackMachineStatus(in *OpenStackMachineStatus, out *v1alpha7.OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) diff --git a/api/v1alpha7/openstackmachine_types.go b/api/v1alpha7/openstackmachine_types.go index 5ef4a39b67..0ed7ca2bc0 100644 --- a/api/v1alpha7/openstackmachine_types.go +++ b/api/v1alpha7/openstackmachine_types.go @@ -84,6 +84,11 @@ type OpenStackMachineSpec struct { // The volume metadata to boot from RootVolume *RootVolume `json:"rootVolume,omitempty"` + // AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance + // +listType=map + // +listMapKey=name + AdditionalBlockDevices []AdditionalBlockDevice `json:"additionalBlockDevices,omitempty"` + // The server group to assign the machine to ServerGroupID string `json:"serverGroupID,omitempty"` diff --git a/api/v1alpha7/types.go b/api/v1alpha7/types.go index 0a9d31f570..049cae2965 100644 --- a/api/v1alpha7/types.go +++ b/api/v1alpha7/types.go @@ -163,6 +163,20 @@ type RootVolume struct { AvailabilityZone string `json:"availabilityZone,omitempty"` } +type AdditionalBlockDevice struct { + // Name of the Cinder volume in the context of a machine. + // It will be combined with the machine name to make the actual volume name. + Name string `json:"name"` + // Size is the size in GB of the volume. + Size int `json:"diskSize"` + // VolumeType is the volume type of the volume. + // If omitted, the default type will be used. + VolumeType string `json:"volumeType,omitempty"` + // AvailabilityZone is the volume availability zone to create the volume in. + // If omitted, the availability zone of the server will be used. + AvailabilityZone string `json:"availabilityZone,omitempty"` +} + // NetworkStatus contains basic information about an existing neutron network. type NetworkStatus struct { Name string `json:"name"` diff --git a/api/v1alpha7/zz_generated.deepcopy.go b/api/v1alpha7/zz_generated.deepcopy.go index bbccd1e7d6..a6ba842aa7 100644 --- a/api/v1alpha7/zz_generated.deepcopy.go +++ b/api/v1alpha7/zz_generated.deepcopy.go @@ -52,6 +52,21 @@ func (in *APIServerLoadBalancer) DeepCopy() *APIServerLoadBalancer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AdditionalBlockDevice) DeepCopyInto(out *AdditionalBlockDevice) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdditionalBlockDevice. +func (in *AdditionalBlockDevice) DeepCopy() *AdditionalBlockDevice { + if in == nil { + return nil + } + out := new(AdditionalBlockDevice) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AddressPair) DeepCopyInto(out *AddressPair) { *out = *in @@ -628,6 +643,11 @@ func (in *OpenStackMachineSpec) DeepCopyInto(out *OpenStackMachineSpec) { *out = new(RootVolume) **out = **in } + if in.AdditionalBlockDevices != nil { + in, out := &in.AdditionalBlockDevices, &out.AdditionalBlockDevices + *out = make([]AdditionalBlockDevice, len(*in)) + copy(*out, *in) + } if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef *out = new(OpenStackIdentityReference) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index 5dcbcf227a..b5df40bd86 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -3770,6 +3770,36 @@ spec: instance: description: Instance for the bastion itself properties: + additionalBlockDevices: + description: AdditionalBlockDevices is a list of specifications + for additional block devices to attach to the server instance + items: + properties: + availabilityZone: + description: AvailabilityZone is the volume availability + zone to create the volume in. If omitted, the availability + zone of the server will be used. + type: string + diskSize: + description: Size is the size in GB of the volume. + type: integer + name: + description: Name of the Cinder volume in the context + of a machine. It will be combined with the machine + name to make the actual volume name. + type: string + volumeType: + description: VolumeType is the volume type of the volume. + If omitted, the default type will be used. + type: string + required: + - diskSize + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map cloudName: description: The name of the cloud to use from the clouds secret diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 3cfafc4600..f115f6f7fc 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -1610,6 +1610,40 @@ spec: instance: description: Instance for the bastion itself properties: + additionalBlockDevices: + description: AdditionalBlockDevices is a list of specifications + for additional block devices to attach to the server + instance + items: + properties: + availabilityZone: + description: AvailabilityZone is the volume + availability zone to create the volume in. + If omitted, the availability zone of the server + will be used. + type: string + diskSize: + description: Size is the size in GB of the volume. + type: integer + name: + description: Name of the Cinder volume in the + context of a machine. It will be combined + with the machine name to make the actual volume + name. + type: string + volumeType: + description: VolumeType is the volume type of + the volume. If omitted, the default type will + be used. + type: string + required: + - diskSize + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map cloudName: description: The name of the cloud to use from the clouds secret diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index 14801337bc..8bc2df59c6 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1159,6 +1159,36 @@ spec: spec: description: OpenStackMachineSpec defines the desired state of OpenStackMachine. properties: + additionalBlockDevices: + description: AdditionalBlockDevices is a list of specifications for + additional block devices to attach to the server instance + items: + properties: + availabilityZone: + description: AvailabilityZone is the volume availability zone + to create the volume in. If omitted, the availability zone + of the server will be used. + type: string + diskSize: + description: Size is the size in GB of the volume. + type: integer + name: + description: Name of the Cinder volume in the context of a machine. + It will be combined with the machine name to make the actual + volume name. + type: string + volumeType: + description: VolumeType is the volume type of the volume. If + omitted, the default type will be used. + type: string + required: + - diskSize + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map cloudName: description: The name of the cloud to use from the clouds secret type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index a7c6c89146..31b90927ff 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -958,6 +958,36 @@ spec: description: Spec is the specification of the desired behavior of the machine. properties: + additionalBlockDevices: + description: AdditionalBlockDevices is a list of specifications + for additional block devices to attach to the server instance + items: + properties: + availabilityZone: + description: AvailabilityZone is the volume availability + zone to create the volume in. If omitted, the availability + zone of the server will be used. + type: string + diskSize: + description: Size is the size in GB of the volume. + type: integer + name: + description: Name of the Cinder volume in the context + of a machine. It will be combined with the machine + name to make the actual volume name. + type: string + volumeType: + description: VolumeType is the volume type of the volume. + If omitted, the default type will be used. + type: string + required: + - diskSize + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map cloudName: description: The name of the cloud to use from the clouds secret diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index a00a9033b5..ebb4e83c16 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -455,17 +455,18 @@ func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *cl func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, userData string) *compute.InstanceSpec { instanceSpec := compute.InstanceSpec{ - Name: openStackMachine.Name, - Image: openStackMachine.Spec.Image, - ImageUUID: openStackMachine.Spec.ImageUUID, - Flavor: openStackMachine.Spec.Flavor, - SSHKeyName: openStackMachine.Spec.SSHKeyName, - UserData: userData, - Metadata: openStackMachine.Spec.ServerMetadata, - ConfigDrive: openStackMachine.Spec.ConfigDrive != nil && *openStackMachine.Spec.ConfigDrive, - RootVolume: openStackMachine.Spec.RootVolume, - ServerGroupID: openStackMachine.Spec.ServerGroupID, - Trunk: openStackMachine.Spec.Trunk, + Name: openStackMachine.Name, + Image: openStackMachine.Spec.Image, + ImageUUID: openStackMachine.Spec.ImageUUID, + Flavor: openStackMachine.Spec.Flavor, + SSHKeyName: openStackMachine.Spec.SSHKeyName, + UserData: userData, + Metadata: openStackMachine.Spec.ServerMetadata, + ConfigDrive: openStackMachine.Spec.ConfigDrive != nil && *openStackMachine.Spec.ConfigDrive, + RootVolume: openStackMachine.Spec.RootVolume, + AdditionalBlockDevices: openStackMachine.Spec.AdditionalBlockDevices, + ServerGroupID: openStackMachine.Spec.ServerGroupID, + Trunk: openStackMachine.Spec.Trunk, } // Add the failure domain only if specified diff --git a/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md b/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md index 53d5c15396..726642cfdd 100644 --- a/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md +++ b/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md @@ -14,6 +14,7 @@ - [Removal of securityGroups](#removal-of-securitygroups) - [Removal of tenantId and projectId](#removal-of-tenantid-and-projectid) - [Change to profile](#change-to-profile) + - [Creation of additionalBlockDevices](#creation-of-additionalblockdevices) - [`OpenStackCluster`](#openstackcluster) - [Change to externalRouterIPs.subnet](#change-to-externalrouteripssubnet) @@ -213,6 +214,21 @@ profile: TrustedVF: true ``` +#### Creation of additionalBlockDevices + +We now have the ability for a machine to have additional block devices to be attached. + +Here is an example on how to use `additionalBlockDevices` for adding additional Cinder volumes attached +to the server instance: + +```yaml +additionalBlockDevices: +- name: database + size: 50 + volumeType: my-volume-type + availabilityZone: az0 +``` + ### `OpenStackCluster` #### Change to externalRouterIPs.subnet @@ -293,4 +309,4 @@ becomes cidr: 192.168.100.0/24 ``` -Nothing will currently create more than a single subnet, but there may be multiple subnets in the future. Similarly, code should no longer assume that the CIDR is an IPv4 CIDR, although nothing will currently create anything other than an IPv4 CIDR. \ No newline at end of file +Nothing will currently create more than a single subnet, but there may be multiple subnets in the future. Similarly, code should no longer assume that the CIDR is an IPv4 CIDR, although nothing will currently create anything other than an IPv4 CIDR. diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index 980e0c7d18..72122d2562 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -294,42 +294,12 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste }) } - volume, err := s.getOrCreateRootVolume(eventObject, instanceSpec, imageID) - if err != nil { - return nil, fmt.Errorf("error in get or create root volume: %w", err) - } - instanceCreateTimeout := getTimeout("CLUSTER_API_OPENSTACK_INSTANCE_CREATE_TIMEOUT", timeoutInstanceCreate) instanceCreateTimeout *= time.Minute - // Wait for volume to become available - if volume != nil { - err = wait.PollUntilContextTimeout(context.TODO(), retryIntervalInstanceStatus, instanceCreateTimeout, true, func(_ context.Context) (bool, error) { - createdVolume, err := s.getVolumeClient().GetVolume(volume.ID) - if err != nil { - if capoerrors.IsRetryable(err) { - return false, nil - } - return false, err - } - - switch createdVolume.Status { - case "available": - return true, nil - case "error": - return false, fmt.Errorf("volume %s is in error state", volume.ID) - default: - return false, nil - } - }) - if err != nil { - return nil, fmt.Errorf("volume %s did not become available: %w", volume.ID, err) - } - } - // Don't set ImageRef on the server if we're booting from volume var serverImageRef string - if volume == nil { + if !hasRootVolume(instanceSpec) { serverImageRef = imageID } @@ -345,7 +315,16 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste ConfigDrive: &instanceSpec.ConfigDrive, } - serverCreateOpts = applyRootVolume(serverCreateOpts, volume) + blockDevices, err := s.getBlockDevices(eventObject, instanceSpec, imageID, instanceCreateTimeout, retryInterval) + if err != nil { + return nil, err + } + if len(blockDevices) > 0 { + serverCreateOpts = bootfromvolume.CreateOptsExt{ + CreateOptsBuilder: serverCreateOpts, + BlockDevice: blockDevices, + } + } serverCreateOpts = applyServerGroupID(serverCreateOpts, instanceSpec.ServerGroupID) @@ -380,12 +359,12 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste return createdInstance, nil } -func rootVolumeName(instanceName string) string { - return fmt.Sprintf("%s-root", instanceName) +func volumeName(instanceName string, nameSuffix string) string { + return fmt.Sprintf("%s-%s", instanceName, nameSuffix) } -func hasRootVolume(rootVolume *infrav1.RootVolume) bool { - return rootVolume != nil && rootVolume.Size > 0 +func hasRootVolume(instanceSpec *InstanceSpec) bool { + return instanceSpec.RootVolume != nil && instanceSpec.RootVolume.Size > 0 } func (s *Service) getVolumeByName(name string) (*volumes.Volume, error) { @@ -407,68 +386,136 @@ func (s *Service) getVolumeByName(name string) (*volumes.Volume, error) { return &volumeList[0], nil } -func (s *Service) getOrCreateRootVolume(eventObject runtime.Object, instanceSpec *InstanceSpec, imageID string) (*volumes.Volume, error) { - rootVolume := instanceSpec.RootVolume - if !hasRootVolume(rootVolume) { - return nil, nil +// getOrCreateVolume gets or creates a volume with the given options. It returns the volume that already exists or the +// newly created one. It returns an error if the volume creation failed or if the expected volume size is different from +// the one that already exists. +func (s *Service) getOrCreateVolume(eventObject runtime.Object, opts volumes.CreateOpts) (*volumes.Volume, error) { + existingVolume, err := s.getVolumeByName(opts.Name) + if err != nil { + return nil, err } + if existingVolume != nil { + // TODO(emilien): Improve the checks here, there is an ongoing discussion in the community about how to do this + // which would involve adding metadata to the volume. + if existingVolume.Size != opts.Size { + return nil, fmt.Errorf("expected to find volume %s with size %d; found size %d", opts.Name, opts.Size, existingVolume.Size) + } - name := rootVolumeName(instanceSpec.Name) - size := rootVolume.Size + s.scope.Logger().V(3).Info("Using existing volume", "name", opts.Name, "id", existingVolume.ID) + return existingVolume, nil + } - volume, err := s.getVolumeByName(name) + createdVolume, err := s.getVolumeClient().CreateVolume(opts) if err != nil { + record.Eventf(eventObject, "FailedCreateVolume", "Failed to create volume; name=%s size=%d err=%v", opts.Name, opts.Size, err) return nil, err } - if volume != nil { - if volume.Size != size { - return nil, fmt.Errorf("exected to find volume %s with size %d; found size %d", name, size, volume.Size) + record.Eventf(eventObject, "SuccessfulCreateVolume", "Created volume; id=%s", createdVolume.ID) + return createdVolume, err +} + +func (s *Service) waitForVolume(volumeID string, timeout time.Duration, retryInterval time.Duration) error { + return wait.PollUntilContextTimeout(context.TODO(), retryInterval, timeout, true, func(_ context.Context) (bool, error) { + volume, err := s.getVolumeClient().GetVolume(volumeID) + if err != nil { + if capoerrors.IsRetryable(err) { + return false, nil + } + return false, err } - s.scope.Logger().V(3).Info("Using existing root volume", "name", name) - return volume, nil - } + switch volume.Status { + case "available": + return true, nil + case "error": + return false, fmt.Errorf("volume %s is in error state", volumeID) + default: + return false, nil + } + }) +} +// getOrCreateVolumeBuilder gets or creates a volume with the given options. It returns the volume that already exists or the newly created one. +// It returns an error if the volume creation failed or if the expected volume is different from the one that already exists. +func (s *Service) getOrCreateVolumeBuilder(eventObject runtime.Object, instanceSpec *InstanceSpec, blockDevice infrav1.AdditionalBlockDevice, imageID string, description string) (*volumes.Volume, error) { availabilityZone := instanceSpec.FailureDomain - if rootVolume.AvailabilityZone != "" { - availabilityZone = rootVolume.AvailabilityZone + if blockDevice.AvailabilityZone != "" { + availabilityZone = blockDevice.AvailabilityZone } createOpts := volumes.CreateOpts{ - Size: rootVolume.Size, - Description: fmt.Sprintf("Root volume for %s", instanceSpec.Name), - Name: rootVolumeName(instanceSpec.Name), + Name: volumeName(instanceSpec.Name, blockDevice.Name), + Description: description, + Size: blockDevice.Size, ImageID: imageID, Multiattach: false, AvailabilityZone: availabilityZone, - VolumeType: rootVolume.VolumeType, + VolumeType: blockDevice.VolumeType, } - volume, err = s.getVolumeClient().CreateVolume(createOpts) - if err != nil { - record.Eventf(eventObject, "FailedCreateVolume", "Failed to create root volume; size=%d imageID=%s err=%v", size, imageID, err) - return nil, err - } - record.Eventf(eventObject, "SuccessfulCreateVolume", "Created root volume; id=%s", volume.ID) - return volume, err + + return s.getOrCreateVolume(eventObject, createOpts) } -// applyRootVolume sets a root volume if the root volume Size is not 0. -func applyRootVolume(opts servers.CreateOptsBuilder, volume *volumes.Volume) servers.CreateOptsBuilder { - if volume == nil { - return opts +// getBlockDevices returns a list of block devices that were created and attached to the instance. It returns an error +// if the root volume or any of the additional block devices could not be created. +func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *InstanceSpec, imageID string, timeout time.Duration, retryInterval time.Duration) ([]bootfromvolume.BlockDevice, error) { + blockDevices := []bootfromvolume.BlockDevice{} + + if hasRootVolume(instanceSpec) { + rootVolumeToBlockDevice := infrav1.AdditionalBlockDevice{ + Name: "root", + AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, + Size: instanceSpec.RootVolume.Size, + VolumeType: instanceSpec.RootVolume.VolumeType, + } + rootVolume, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, rootVolumeToBlockDevice, imageID, fmt.Sprintf("Root volume for %s", instanceSpec.Name)) + if err != nil { + return []bootfromvolume.BlockDevice{}, err + } + blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ + SourceType: bootfromvolume.SourceVolume, + DestinationType: bootfromvolume.DestinationVolume, + UUID: rootVolume.ID, + BootIndex: 0, + DeleteOnTermination: true, + }) + } else { + blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ + SourceType: bootfromvolume.SourceImage, + DestinationType: bootfromvolume.DestinationLocal, + UUID: imageID, + BootIndex: 0, + DeleteOnTermination: true, + }) } - block := bootfromvolume.BlockDevice{ - SourceType: bootfromvolume.SourceVolume, - BootIndex: 0, - UUID: volume.ID, - DeleteOnTermination: true, - DestinationType: bootfromvolume.DestinationVolume, + for _, blockDeviceSpec := range instanceSpec.AdditionalBlockDevices { + blockDevice, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, blockDeviceSpec, "", fmt.Sprintf("Additional block device for %s", instanceSpec.Name)) + if err != nil { + return []bootfromvolume.BlockDevice{}, err + } + blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ + SourceType: bootfromvolume.SourceVolume, + DestinationType: bootfromvolume.DestinationVolume, + UUID: blockDevice.ID, + BootIndex: -1, + DeleteOnTermination: true, + Tag: blockDeviceSpec.Name, + }) } - return bootfromvolume.CreateOptsExt{ - CreateOptsBuilder: opts, - BlockDevice: []bootfromvolume.BlockDevice{block}, + + // Wait for any volumes in the block devices to become available + if len(blockDevices) > 0 { + for _, bd := range blockDevices { + if bd.SourceType == bootfromvolume.SourceVolume { + if err := s.waitForVolume(bd.UUID, timeout, retryInterval); err != nil { + return []bootfromvolume.BlockDevice{}, fmt.Errorf("volume %s did not become available: %w", bd.UUID, err) + } + } + } } + + return blockDevices, nil } // applyServerGroupID adds a scheduler hint to the CreateOptsBuilder, if the @@ -545,38 +592,24 @@ func (s *Service) GetManagementPort(openStackCluster *infrav1.OpenStackCluster, func (s *Service) DeleteInstance(openStackCluster *infrav1.OpenStackCluster, eventObject runtime.Object, instanceStatus *InstanceStatus, instanceSpec *InstanceSpec) error { if instanceStatus == nil { /* - We create a boot-from-volume instance in 2 steps: - 1. Create the volume - 2. Create the instance with the created root volume and set DeleteOnTermination + Attaching volumes to an instance is a two-step process: + + 1. Create the volume + 2. Create the instance with the created volumes in RootVolume and AdditionalBlockDevices fields with DeleteOnTermination=true - This introduces a new failure mode which has implications for safely deleting instances: we - might create the volume, but the instance create fails. This would leave us with a dangling - volume with no instance. + This has a possible failure mode where creating the volume succeeds but creating the instance + fails. In this case, we want to make sure that the dangling volumes are cleaned up. To handle this safely, we ensure that we never remove a machine finalizer until all resources - associated with the instance, including a root volume, have been deleted. To achieve this: - * We always call DeleteInstance when reconciling a delete, regardless of - whether the instance exists or not. - * If the instance was already deleted we check that the volume is also gone. + associated with the instance, including volumes, have been deleted. To achieve this: - Note that we don't need to separately delete the root volume when deleting the instance because + * We always call DeleteInstance when reconciling a delete, even if the instance does not exist + * If the instance was already deleted we check that the volumes are also gone + + Note that we don't need to separately delete the volumes when deleting the instance because DeleteOnTermination will ensure it is deleted in that case. */ - if hasRootVolume(instanceSpec.RootVolume) { - name := rootVolumeName(instanceSpec.Name) - volume, err := s.getVolumeByName(name) - if err != nil { - return err - } - if volume == nil { - return nil - } - - s.scope.Logger().V(2).Info("Deleting dangling root volume", "name", volume.Name, "ID", volume.ID) - return s.getVolumeClient().DeleteVolume(volume.ID, volumes.DeleteOpts{}) - } - - return nil + return s.deleteVolumes(instanceSpec) } instanceInterfaces, err := s.getComputeClient().ListAttachedInterfaces(instanceStatus.ID()) @@ -625,6 +658,34 @@ func (s *Service) DeleteInstance(openStackCluster *infrav1.OpenStackCluster, eve return s.deleteInstance(eventObject, instanceStatus.InstanceIdentifier()) } +func (s *Service) deleteVolumes(instanceSpec *InstanceSpec) error { + if hasRootVolume(instanceSpec) { + if err := s.deleteVolume(instanceSpec.Name, "root"); err != nil { + return err + } + } + for _, volumeSpec := range instanceSpec.AdditionalBlockDevices { + if err := s.deleteVolume(instanceSpec.Name, volumeSpec.Name); err != nil { + return err + } + } + return nil +} + +func (s *Service) deleteVolume(instanceName string, nameSuffix string) error { + volumeName := volumeName(instanceName, nameSuffix) + volume, err := s.getVolumeByName(volumeName) + if err != nil { + return err + } + if volume == nil { + return nil + } + + s.scope.Logger().V(2).Info("Deleting dangling volume", "name", volume.Name, "ID", volume.ID) + return s.getVolumeClient().DeleteVolume(volume.ID, volumes.DeleteOpts{}) +} + func (s *Service) deletePorts(eventObject runtime.Object, nets []servers.Network) error { trunkSupported, err := s.isTrunkExtSupported() if err != nil { diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 698392ad85..f0d93b5737 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -219,17 +219,18 @@ func TestService_getImageID(t *testing.T) { } const ( - networkUUID = "d412171b-9fd7-41c1-95a6-c24e5953974d" - subnetUUID = "d2d8d98d-b234-477e-a547-868b7cb5d6a5" - portUUID = "e7b7f3d1-0a81-40b1-bfa6-a22a31b17816" - trunkUUID = "2cf74a9f-3e89-4546-9779-20f2503c4ae8" - imageUUID = "652b5a05-27fa-41d4-ac82-3e63cf6f7ab7" - flavorUUID = "6dc820db-f912-454e-a1e3-1081f3b8cc72" - instanceUUID = "383a8ec1-b6ea-4493-99dd-fc790da04ba9" - controlPlaneSecurityGroupUUID = "c9817a91-4821-42db-8367-2301002ab659" - workerSecurityGroupUUID = "9c6c0d28-03c9-436c-815d-58440ac2c1c8" - serverGroupUUID = "7b940d62-68ef-4e42-a76a-1a62e290509c" - volumeUUID = "d84fe775-e25d-4f80-9888-f701e996c689" + networkUUID = "d412171b-9fd7-41c1-95a6-c24e5953974d" + subnetUUID = "d2d8d98d-b234-477e-a547-868b7cb5d6a5" + portUUID = "e7b7f3d1-0a81-40b1-bfa6-a22a31b17816" + trunkUUID = "2cf74a9f-3e89-4546-9779-20f2503c4ae8" + imageUUID = "652b5a05-27fa-41d4-ac82-3e63cf6f7ab7" + flavorUUID = "6dc820db-f912-454e-a1e3-1081f3b8cc72" + instanceUUID = "383a8ec1-b6ea-4493-99dd-fc790da04ba9" + controlPlaneSecurityGroupUUID = "c9817a91-4821-42db-8367-2301002ab659" + workerSecurityGroupUUID = "9c6c0d28-03c9-436c-815d-58440ac2c1c8" + serverGroupUUID = "7b940d62-68ef-4e42-a76a-1a62e290509c" + rootVolumeUUID = "d84fe775-e25d-4f80-9888-f701e996c689" + additionalBlockDeviceVolumeUUID = "1d1d5a56-c433-41dd-8446-cba2077e96e9" openStackMachineName = "test-openstack-machine" portName = "test-openstack-machine-0" @@ -298,6 +299,15 @@ func TestService_ReconcileInstance(t *testing.T) { "test-metadata": "test-value", }, "user_data": &userData, + "block_device_mapping_v2": []map[string]interface{}{ + { + "delete_on_termination": true, + "destination_type": "local", + "source_type": "image", + "uuid": imageUUID, + "boot_index": float64(0), + }, + }, }, "os:scheduler_hints": map[string]interface{}{ "group": serverGroupUUID, @@ -407,22 +417,22 @@ func TestService_ReconcileInstance(t *testing.T) { expectServerPoll(computeRecorder, []string{"ACTIVE"}) } - returnedVolume := func(status string) *volumes.Volume { + returnedVolume := func(uuid string, status string) *volumes.Volume { return &volumes.Volume{ - ID: volumeUUID, + ID: uuid, Status: status, } } // Expected calls when polling for server creation - expectVolumePoll := func(volumeRecorder *mock.MockVolumeClientMockRecorder, states []string) { + expectVolumePoll := func(volumeRecorder *mock.MockVolumeClientMockRecorder, uuid string, states []string) { for _, state := range states { - volumeRecorder.GetVolume(volumeUUID).Return(returnedVolume(state), nil) + volumeRecorder.GetVolume(uuid).Return(returnedVolume(uuid, state), nil) } } - expectVolumePollSuccess := func(volumeRecorder *mock.MockVolumeClientMockRecorder) { - expectVolumePoll(volumeRecorder, []string{"available"}) + expectVolumePollSuccess := func(volumeRecorder *mock.MockVolumeClientMockRecorder, uuid string) { + expectVolumePoll(volumeRecorder, uuid, []string{"available"}) } // ******************* @@ -541,8 +551,8 @@ func TestService_ReconcileInstance(t *testing.T) { Name: fmt.Sprintf("%s-root", openStackMachineName), ImageID: imageUUID, Multiattach: false, - }).Return(&volumes.Volume{ID: volumeUUID}, nil) - expectVolumePollSuccess(r.volume) + }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, rootVolumeUUID) createMap := getDefaultServerMap() serverMap := createMap["server"].(map[string]interface{}) @@ -552,7 +562,7 @@ func TestService_ReconcileInstance(t *testing.T) { "delete_on_termination": true, "destination_type": "volume", "source_type": "volume", - "uuid": volumeUUID, + "uuid": rootVolumeUUID, "boot_index": float64(0), }, } @@ -588,8 +598,8 @@ func TestService_ReconcileInstance(t *testing.T) { Name: fmt.Sprintf("%s-root", openStackMachineName), ImageID: imageUUID, Multiattach: false, - }).Return(&volumes.Volume{ID: volumeUUID}, nil) - expectVolumePollSuccess(r.volume) + }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, rootVolumeUUID) createMap := getDefaultServerMap() serverMap := createMap["server"].(map[string]interface{}) @@ -599,7 +609,7 @@ func TestService_ReconcileInstance(t *testing.T) { "delete_on_termination": true, "destination_type": "volume", "source_type": "volume", - "uuid": volumeUUID, + "uuid": rootVolumeUUID, "boot_index": float64(0), }, } @@ -632,13 +642,195 @@ func TestService_ReconcileInstance(t *testing.T) { Name: fmt.Sprintf("%s-root", openStackMachineName), ImageID: imageUUID, Multiattach: false, - }).Return(&volumes.Volume{ID: volumeUUID}, nil) - expectVolumePoll(r.volume, []string{"creating", "error"}) + }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) + expectVolumePoll(r.volume, rootVolumeUUID, []string{"creating", "error"}) expectCleanupDefaultPort(r.network) }, wantErr: true, }, + { + name: "Root volume with additional block device success", + getInstanceSpec: func() *InstanceSpec { + s := getDefaultInstanceSpec() + s.RootVolume = &infrav1.RootVolume{ + Size: 50, + } + s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "etcd", + Size: 50, + VolumeType: "test-volume-type", + }, + } + return s + }, + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) + + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). + Return([]volumes.Volume{}, nil) + r.volume.CreateVolume(volumes.CreateOpts{ + Size: 50, + AvailabilityZone: failureDomain, + Description: fmt.Sprintf("Root volume for %s", openStackMachineName), + Name: fmt.Sprintf("%s-root", openStackMachineName), + ImageID: imageUUID, + Multiattach: false, + }).Return(&volumes.Volume{ID: rootVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, rootVolumeUUID) + + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). + Return([]volumes.Volume{}, nil) + r.volume.CreateVolume(volumes.CreateOpts{ + Size: 50, + AvailabilityZone: failureDomain, + Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), + Name: fmt.Sprintf("%s-etcd", openStackMachineName), + Multiattach: false, + VolumeType: "test-volume-type", + }).Return(&volumes.Volume{ID: additionalBlockDeviceVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, additionalBlockDeviceVolumeUUID) + + createMap := getDefaultServerMap() + serverMap := createMap["server"].(map[string]interface{}) + serverMap["imageRef"] = "" + serverMap["block_device_mapping_v2"] = []map[string]interface{}{ + { + "source_type": "volume", + "uuid": rootVolumeUUID, + "boot_index": float64(0), + "delete_on_termination": true, + "destination_type": "volume", + }, + { + "source_type": "volume", + "uuid": additionalBlockDeviceVolumeUUID, + "boot_index": float64(-1), + "delete_on_termination": true, + "destination_type": "volume", + "tag": "etcd", + }, + } + expectCreateServer(r.compute, createMap, false) + expectServerPollSuccess(r.compute) + + // Don't delete ports because the server is created: DeleteInstance will do it + }, + wantErr: false, + }, + { + name: "Additional block device success", + getInstanceSpec: func() *InstanceSpec { + s := getDefaultInstanceSpec() + s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "etcd", + Size: 50, + VolumeType: "test-volume-type", + }, + } + return s + }, + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) + + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). + Return([]volumes.Volume{}, nil) + r.volume.CreateVolume(volumes.CreateOpts{ + Size: 50, + AvailabilityZone: failureDomain, + Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), + Name: fmt.Sprintf("%s-etcd", openStackMachineName), + Multiattach: false, + VolumeType: "test-volume-type", + }).Return(&volumes.Volume{ID: additionalBlockDeviceVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, additionalBlockDeviceVolumeUUID) + + createMap := getDefaultServerMap() + serverMap := createMap["server"].(map[string]interface{}) + serverMap["block_device_mapping_v2"] = []map[string]interface{}{ + { + "source_type": "image", + "uuid": imageUUID, + "boot_index": float64(0), + "delete_on_termination": true, + "destination_type": "local", + }, + { + "source_type": "volume", + "uuid": additionalBlockDeviceVolumeUUID, + "boot_index": float64(-1), + "delete_on_termination": true, + "destination_type": "volume", + "tag": "etcd", + }, + } + expectCreateServer(r.compute, createMap, false) + expectServerPollSuccess(r.compute) + + // Don't delete ports because the server is created: DeleteInstance will do it + }, + wantErr: false, + }, + { + name: "Additional block device success with explicit AZ", + getInstanceSpec: func() *InstanceSpec { + s := getDefaultInstanceSpec() + s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "etcd", + Size: 50, + VolumeType: "test-volume-type", + AvailabilityZone: "test-alternate-az", + }, + } + return s + }, + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) + + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). + Return([]volumes.Volume{}, nil) + r.volume.CreateVolume(volumes.CreateOpts{ + Size: 50, + AvailabilityZone: "test-alternate-az", + Description: fmt.Sprintf("Additional block device for %s", openStackMachineName), + Name: fmt.Sprintf("%s-etcd", openStackMachineName), + Multiattach: false, + VolumeType: "test-volume-type", + }).Return(&volumes.Volume{ID: additionalBlockDeviceVolumeUUID}, nil) + expectVolumePollSuccess(r.volume, additionalBlockDeviceVolumeUUID) + + createMap := getDefaultServerMap() + serverMap := createMap["server"].(map[string]interface{}) + serverMap["block_device_mapping_v2"] = []map[string]interface{}{ + { + "source_type": "image", + "uuid": imageUUID, + "boot_index": float64(0), + "delete_on_termination": true, + "destination_type": "local", + }, + { + "source_type": "volume", + "uuid": additionalBlockDeviceVolumeUUID, + "boot_index": float64(-1), + "delete_on_termination": true, + "destination_type": "volume", + "tag": "etcd", + }, + } + expectCreateServer(r.compute, createMap, false) + expectServerPollSuccess(r.compute) + + // Don't delete ports because the server is created: DeleteInstance will do it + }, + wantErr: false, + }, { name: "Delete trunks on port creation error", getInstanceSpec: func() *InstanceSpec { @@ -795,12 +987,12 @@ func TestService_DeleteInstance(t *testing.T) { Name: volumeName, TenantID: "", }).Return([]volumes.Volume{{ - ID: volumeUUID, + ID: rootVolumeUUID, Name: volumeName, }}, nil) // Delete volume - r.volume.DeleteVolume(volumeUUID, volumes.DeleteOpts{}).Return(nil) + r.volume.DeleteVolume(rootVolumeUUID, volumes.DeleteOpts{}).Return(nil) }, wantErr: false, }, diff --git a/pkg/cloud/services/compute/instance_types.go b/pkg/cloud/services/compute/instance_types.go index c483e71ecb..0524e79774 100644 --- a/pkg/cloud/services/compute/instance_types.go +++ b/pkg/cloud/services/compute/instance_types.go @@ -33,21 +33,22 @@ import ( // InstanceSpec does not contain all of the fields of infrav1.Instance, as not // all of them can be set on a new instance. type InstanceSpec struct { - Name string - Image string - ImageUUID string - Flavor string - SSHKeyName string - UserData string - Metadata map[string]string - ConfigDrive bool - FailureDomain string - RootVolume *infrav1.RootVolume - ServerGroupID string - Trunk bool - Tags []string - SecurityGroups []infrav1.SecurityGroupFilter - Ports []infrav1.PortOpts + Name string + Image string + ImageUUID string + Flavor string + SSHKeyName string + UserData string + Metadata map[string]string + ConfigDrive bool + FailureDomain string + RootVolume *infrav1.RootVolume + AdditionalBlockDevices []infrav1.AdditionalBlockDevice + ServerGroupID string + Trunk bool + Tags []string + SecurityGroups []infrav1.SecurityGroupFilter + Ports []infrav1.PortOpts } // InstanceIdentifier describes an instance which has not necessarily been fetched. diff --git a/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml b/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml index f9af057890..73e908b4cc 100644 --- a/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml +++ b/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml @@ -3,3 +3,8 @@ path: /spec/template/spec/rootVolume value: diskSize: 15 +- op: add + path: /spec/template/spec/additionalBlockDevices + value: + - diskSize: 1 + name: extravol diff --git a/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml b/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml index f86b3ac976..f3e0be60cd 100644 --- a/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml +++ b/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml @@ -5,3 +5,10 @@ diskSize: 15 volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} +- op: add + path: /spec/template/spec/additionalBlockDevices + value: + - diskSize: 1 + name: extravol + volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} + availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} diff --git a/test/e2e/suites/e2e/e2e_test.go b/test/e2e/suites/e2e/e2e_test.go index 7ca787e470..c45313378c 100644 --- a/test/e2e/suites/e2e/e2e_test.go +++ b/test/e2e/suites/e2e/e2e_test.go @@ -560,7 +560,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() { allServerNames = append(allServerNames, name) } - bootVolumes := make(map[string]*volumes.Volume) + rootVolumes := make(map[string]*volumes.Volume) + additionalVolumes := make(map[string]*volumes.Volume) for _, machine := range allMachines { // The output of a HaveKey() failure against @@ -575,33 +576,61 @@ var _ = Describe("e2e tests [PR-Blocking]", func() { Equal(*machine.Spec.FailureDomain), fmt.Sprintf("Server %s was not scheduled in the correct AZ", machine.Name)) - // Check that all machines have the expected boot volume + // Check that all machines have the expected volumes: + // - 1 root volume + // - 1 additional volume volumes := server.AttachedVolumes - Expect(volumes).To(HaveLen(1)) + Expect(volumes).To(HaveLen(2)) - bootVolume, err := shared.GetOpenStackVolume(e2eCtx, volumes[0].ID) + // nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid does not guarantee order of the volumes + // so we need to find the boot volume by checking the "bootable" flag for now. + firstVolumeFound, err := shared.GetOpenStackVolume(e2eCtx, volumes[0].ID) Expect(err).NotTo(HaveOccurred(), "failed to get OpenStack volume %s for machine %s", volumes[0].ID, machine.Name) - bootVolumes[machine.Name] = bootVolume + secondVolumeFound, err := shared.GetOpenStackVolume(e2eCtx, volumes[1].ID) + Expect(err).NotTo(HaveOccurred(), "failed to get OpenStack volume %s for machine %s", volumes[1].ID, machine.Name) + + rootVolume := firstVolumeFound + additionalVolume := secondVolumeFound + // The boot volume is the one with the "bootable" flag set. + if firstVolumeFound.Bootable != "true" { // This is genuinely a string, not a bool + rootVolume = secondVolumeFound + additionalVolume = firstVolumeFound + } - Expect(*bootVolume).To(MatchFields(IgnoreExtras, Fields{ + rootVolumes[machine.Name] = rootVolume + Expect(*rootVolume).To(MatchFields(IgnoreExtras, Fields{ "Name": Equal(fmt.Sprintf("%s-root", server.Name)), "Size": Equal(15), "Bootable": Equal("true"), // This is genuinely a string, not a bool - }), "Boot volume %s for machine %s not as expected", bootVolume.ID, machine.Name) + }), "Boot volume %s for machine %s not as expected", rootVolume.ID, machine.Name) + + additionalVolumes[machine.Name] = additionalVolume + Expect(*additionalVolume).To(MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-extravol", server.Name)), + "Size": Equal(1), + }), "Additional block device %s for machine %s not as expected", additionalVolume.ID, machine.Name) } - // Expect all control plane machines to have a root volume in the same AZ as the machine, and the default volume type + // Expect all control plane machines to have volumes in the same AZ as the machine, and the default volume type for _, machine := range controlPlaneMachines { - bootVolume := bootVolumes[machine.Name] - Expect(bootVolume.AvailabilityZone).To(Equal(*machine.Spec.FailureDomain)) - Expect(bootVolume.VolumeType).NotTo(Equal(volumeTypeAlt)) + rootVolume := rootVolumes[machine.Name] + Expect(rootVolume.AvailabilityZone).To(Equal(*machine.Spec.FailureDomain)) + Expect(rootVolume.VolumeType).NotTo(Equal(volumeTypeAlt)) + + additionalVolume := additionalVolumes[machine.Name] + Expect(additionalVolume.AvailabilityZone).To(Equal(*machine.Spec.FailureDomain)) + Expect(additionalVolume.VolumeType).NotTo(Equal(volumeTypeAlt)) } - // Expect all worker machines to have a root volume in the primary AZ, and the test volume type + // Expect all worker machines to have volumes in the primary AZ, and the test volume type for _, machine := range workerMachines { - bootVolume := bootVolumes[machine.Name] - Expect(bootVolume.AvailabilityZone).To(Equal(failureDomain)) - Expect(bootVolume.VolumeType).To(Equal(volumeTypeAlt)) + rootVolume := rootVolumes[machine.Name] + Expect(rootVolume.AvailabilityZone).To(Equal(failureDomain)) + Expect(rootVolume.VolumeType).To(Equal(volumeTypeAlt)) + + additionalVolume := additionalVolumes[machine.Name] + Expect(additionalVolume.AvailabilityZone).To(Equal(failureDomain)) + Expect(additionalVolume.VolumeType).To(Equal(volumeTypeAlt)) } }) }) From 9315e2b19fe5c00c13a2dbe6b1d740bd948ff0c7 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Fri, 29 Sep 2023 15:12:25 -0400 Subject: [PATCH 2/4] Add ephemeral storage support to the `AdditionalBlockDevices` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emilien Macchi Co-authored-by: Martin André (cherry picked from commit 1efeaf76b071ba64386ceecab91c4308356fa8e8) --- api/v1alpha7/openstackmachine_types.go | 6 ++ api/v1alpha7/openstackmachine_webhook.go | 8 ++ api/v1alpha7/types.go | 81 ++++++++++++++-- api/v1alpha7/zz_generated.deepcopy.go | 40 +++++++- ...re.cluster.x-k8s.io_openstackclusters.yaml | 83 +++++++++++++---- ...er.x-k8s.io_openstackclustertemplates.yaml | 92 +++++++++++++++---- ...re.cluster.x-k8s.io_openstackmachines.yaml | 78 ++++++++++++---- ...er.x-k8s.io_openstackmachinetemplates.yaml | 83 +++++++++++++---- .../crd-changes/v1alpha6-to-v1alpha7.md | 35 ++++++- hack/ci/cloud-init/controller.yaml.tpl | 6 +- pkg/cloud/services/compute/instance.go | 61 +++++++++--- pkg/cloud/services/compute/instance_test.go | 91 +++++++++++++++--- .../patch-machine-template-control-plane.yaml | 10 +- .../patch-machine-template-worker.yaml | 15 ++- 14 files changed, 574 insertions(+), 115 deletions(-) diff --git a/api/v1alpha7/openstackmachine_types.go b/api/v1alpha7/openstackmachine_types.go index 0ed7ca2bc0..91b0bcbf60 100644 --- a/api/v1alpha7/openstackmachine_types.go +++ b/api/v1alpha7/openstackmachine_types.go @@ -85,8 +85,14 @@ type OpenStackMachineSpec struct { RootVolume *RootVolume `json:"rootVolume,omitempty"` // AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance + // + // + --- + // + While it's possible to have unlimited number of block devices attached to a server instance, the number is + // + limited to 255 to avoid rule cost exceeded error in CRD validation. + // +kubebuilder:validation:MaxItems=255 // +listType=map // +listMapKey=name + // +optional AdditionalBlockDevices []AdditionalBlockDevice `json:"additionalBlockDevices,omitempty"` // The server group to assign the machine to diff --git a/api/v1alpha7/openstackmachine_webhook.go b/api/v1alpha7/openstackmachine_webhook.go index 1ec2849fba..6470589a76 100644 --- a/api/v1alpha7/openstackmachine_webhook.go +++ b/api/v1alpha7/openstackmachine_webhook.go @@ -62,6 +62,14 @@ func (r *OpenStackMachine) ValidateCreate() (admission.Warnings, error) { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret")) } + if r.Spec.RootVolume != nil && r.Spec.AdditionalBlockDevices != nil { + for _, device := range r.Spec.AdditionalBlockDevices { + if device.Name == "root" { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "additionalBlockDevices"), "cannot contain a device named \"root\" when rootVolume is set")) + } + } + } + return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) } diff --git a/api/v1alpha7/types.go b/api/v1alpha7/types.go index 049cae2965..dc75cbec6d 100644 --- a/api/v1alpha7/types.go +++ b/api/v1alpha7/types.go @@ -163,20 +163,83 @@ type RootVolume struct { AvailabilityZone string `json:"availabilityZone,omitempty"` } -type AdditionalBlockDevice struct { - // Name of the Cinder volume in the context of a machine. - // It will be combined with the machine name to make the actual volume name. - Name string `json:"name"` - // Size is the size in GB of the volume. - Size int `json:"diskSize"` - // VolumeType is the volume type of the volume. - // If omitted, the default type will be used. - VolumeType string `json:"volumeType,omitempty"` +// BlockDeviceStorage is the storage type of a block device to create and +// contains additional storage options. +// +kubebuilder:validation:XValidation:rule="self.type == 'Volume' || !has(self.volume)",message="volume is forbidden when type is not Volume" +// +union +// +//nolint:godot +type BlockDeviceStorage struct { + // Type is the type of block device to create. + // This can be either "Volume" or "Local". + // +kubebuilder:validation:Enum="Volume";"Local" + // +kubebuilder:validation:Required + // +unionDiscriminator + Type BlockDeviceType `json:"type"` + + // Volume contains additional storage options for a volume block device. + // +optional + // +unionMember,optional + Volume *BlockDeviceVolume `json:"volume,omitempty"` +} + +// BlockDeviceVolume contains additional storage options for a volume block device. +type BlockDeviceVolume struct { + // Type is the Cinder volume type of the volume. + // If omitted, the default Cinder volume type that is configured in the OpenStack cloud + // will be used. + // 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 + Type string `json:"type,omitempty"` + // AvailabilityZone is the volume availability zone to create the volume in. // If omitted, the availability zone of the server will be used. + // The availability zone 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. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:XValidation:rule="!self.contains(' ')",message="availabilityZone may not contain spaces" + // +optional AvailabilityZone string `json:"availabilityZone,omitempty"` } +// AdditionalBlockDevice is a block device to attach to the server. +type AdditionalBlockDevice struct { + // Name of the block device in the context of a machine. + // If the block device is a volume, the Cinder volume will be named + // as a combination of the machine name and this name. + // Also, this name will be used for tagging the block device. + // Information about the block device tag can be obtained from the OpenStack + // metadata API or the config drive. + // +kubebuilder:validation:Required + Name string `json:"name"` + + // SizeGiB is the size of the block device in gibibytes (GiB). + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Required + SizeGiB int `json:"sizeGiB"` + + // Storage specifies the storage type of the block device and + // additional storage options. + // +kubebuilder:validation:Required + Storage BlockDeviceStorage `json:"storage"` +} + +// BlockDeviceType defines the type of block device to create. +type BlockDeviceType string + +const ( + // LocalBlockDevice is an ephemeral block device attached to the server. + LocalBlockDevice BlockDeviceType = "Local" + + // VolumeBlockDevice is a volume block device attached to the server. + VolumeBlockDevice BlockDeviceType = "Volume" +) + // NetworkStatus contains basic information about an existing neutron network. type NetworkStatus struct { Name string `json:"name"` diff --git a/api/v1alpha7/zz_generated.deepcopy.go b/api/v1alpha7/zz_generated.deepcopy.go index a6ba842aa7..1d81a83410 100644 --- a/api/v1alpha7/zz_generated.deepcopy.go +++ b/api/v1alpha7/zz_generated.deepcopy.go @@ -55,6 +55,7 @@ func (in *APIServerLoadBalancer) DeepCopy() *APIServerLoadBalancer { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AdditionalBlockDevice) DeepCopyInto(out *AdditionalBlockDevice) { *out = *in + in.Storage.DeepCopyInto(&out.Storage) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdditionalBlockDevice. @@ -128,6 +129,41 @@ func (in *BindingProfile) DeepCopy() *BindingProfile { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BlockDeviceStorage) DeepCopyInto(out *BlockDeviceStorage) { + *out = *in + if in.Volume != nil { + in, out := &in.Volume, &out.Volume + *out = new(BlockDeviceVolume) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BlockDeviceStorage. +func (in *BlockDeviceStorage) DeepCopy() *BlockDeviceStorage { + if in == nil { + return nil + } + out := new(BlockDeviceStorage) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BlockDeviceVolume) DeepCopyInto(out *BlockDeviceVolume) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BlockDeviceVolume. +func (in *BlockDeviceVolume) DeepCopy() *BlockDeviceVolume { + if in == nil { + return nil + } + out := new(BlockDeviceVolume) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalRouterIPParam) DeepCopyInto(out *ExternalRouterIPParam) { *out = *in @@ -646,7 +682,9 @@ func (in *OpenStackMachineSpec) DeepCopyInto(out *OpenStackMachineSpec) { if in.AdditionalBlockDevices != nil { in, out := &in.AdditionalBlockDevices, &out.AdditionalBlockDevices *out = make([]AdditionalBlockDevice, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index b5df40bd86..f3fadf6a3a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -3774,28 +3774,79 @@ spec: description: AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device to + attach to the server. properties: - availabilityZone: - description: AvailabilityZone is the volume availability - zone to create the volume in. If omitted, the availability - zone of the server will be used. - type: string - diskSize: - description: Size is the size in GB of the volume. - type: integer name: - description: Name of the Cinder volume in the context - of a machine. It will be combined with the machine - name to make the actual volume name. - type: string - volumeType: - description: VolumeType is the volume type of the volume. - If omitted, the default type will be used. + description: Name of the block device in the context + of a machine. If the block device is a volume, the + Cinder volume will be named as a combination of the + machine name and this name. Also, this name will be + used for tagging the block device. Information about + the block device tag can be obtained from the OpenStack + metadata API or the config drive. type: string + sizeGiB: + description: SizeGiB is the size of the block device + in gibibytes (GiB). + minimum: 1 + type: integer + storage: + description: Storage specifies the storage type of the + block device and additional storage options. + properties: + type: + description: Type is the type of block device to + create. This can be either "Volume" or "Local". + enum: + - Volume + - Local + type: string + volume: + description: Volume contains additional storage + options for a volume block device. + properties: + availabilityZone: + description: AvailabilityZone is the volume + availability zone to create the volume in. + If omitted, the availability zone of the server + will be used. The availability zone 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. + maxLength: 63 + minLength: 1 + type: string + x-kubernetes-validations: + - message: availabilityZone may not contain + spaces + rule: '!self.contains('' '')' + type: + description: Type is the Cinder volume type + of the volume. If omitted, the default Cinder + volume type that is configured in the OpenStack + cloud will be used. The maximum length of + a volume type name is 255 characters, as per + the OpenStack limit. + maxLength: 255 + minLength: 1 + type: string + type: object + required: + - type + type: object + x-kubernetes-validations: + - message: volume is forbidden when type is not Volume + rule: self.type == 'Volume' || !has(self.volume) required: - - diskSize - name + - sizeGiB + - storage type: object + maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index f115f6f7fc..491d861f54 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -1615,31 +1615,85 @@ spec: for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device + to attach to the server. properties: - availabilityZone: - description: AvailabilityZone is the volume - availability zone to create the volume in. - If omitted, the availability zone of the server - will be used. - type: string - diskSize: - description: Size is the size in GB of the volume. - type: integer name: - description: Name of the Cinder volume in the - context of a machine. It will be combined - with the machine name to make the actual volume - name. - type: string - volumeType: - description: VolumeType is the volume type of - the volume. If omitted, the default type will - be used. + description: Name of the block device in the + context of a machine. If the block device + is a volume, the Cinder volume will be named + as a combination of the machine name and this + name. Also, this name will be used for tagging + the block device. Information about the block + device tag can be obtained from the OpenStack + metadata API or the config drive. type: string + sizeGiB: + description: SizeGiB is the size of the block + device in gibibytes (GiB). + minimum: 1 + type: integer + storage: + description: Storage specifies the storage type + of the block device and additional storage + options. + properties: + type: + description: Type is the type of block device + to create. This can be either "Volume" + or "Local". + enum: + - Volume + - Local + type: string + volume: + description: Volume contains additional + storage options for a volume block device. + properties: + availabilityZone: + description: AvailabilityZone is the + volume availability zone to create + the volume in. If omitted, the availability + zone of the server will be used. The + availability zone 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. + maxLength: 63 + minLength: 1 + type: string + x-kubernetes-validations: + - message: availabilityZone may not + contain spaces + rule: '!self.contains('' '')' + type: + description: Type is the Cinder volume + type of the volume. If omitted, the + default Cinder volume type that is + configured in the OpenStack cloud + will be used. The maximum length of + a volume type name is 255 characters, + as per the OpenStack limit. + maxLength: 255 + minLength: 1 + type: string + type: object + required: + - type + type: object + x-kubernetes-validations: + - message: volume is forbidden when type is + not Volume + rule: self.type == 'Volume' || !has(self.volume) required: - - diskSize - name + - sizeGiB + - storage type: object + maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index 8bc2df59c6..7422af2d47 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1163,28 +1163,74 @@ spec: description: AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device to attach to + the server. properties: - availabilityZone: - description: AvailabilityZone is the volume availability zone - to create the volume in. If omitted, the availability zone - of the server will be used. - type: string - diskSize: - description: Size is the size in GB of the volume. - type: integer name: - description: Name of the Cinder volume in the context of a machine. - It will be combined with the machine name to make the actual - volume name. - type: string - volumeType: - description: VolumeType is the volume type of the volume. If - omitted, the default type will be used. + description: Name of the block device in the context of a machine. + If the block device is a volume, the Cinder volume will be + named as a combination of the machine name and this name. + Also, this name will be used for tagging the block device. + Information about the block device tag can be obtained from + the OpenStack metadata API or the config drive. type: string + sizeGiB: + description: SizeGiB is the size of the block device in gibibytes + (GiB). + minimum: 1 + type: integer + storage: + description: Storage specifies the storage type of the block + device and additional storage options. + properties: + type: + description: Type is the type of block device to create. + This can be either "Volume" or "Local". + enum: + - Volume + - Local + type: string + volume: + description: Volume contains additional storage options + for a volume block device. + properties: + availabilityZone: + description: AvailabilityZone is the volume availability + zone to create the volume in. If omitted, the availability + zone of the server will be used. The availability + zone 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. + maxLength: 63 + minLength: 1 + type: string + x-kubernetes-validations: + - message: availabilityZone may not contain spaces + rule: '!self.contains('' '')' + type: + description: Type is the Cinder volume type of the volume. + If omitted, the default Cinder volume type that is + configured in the OpenStack cloud will be used. The + maximum length of a volume type name is 255 characters, + as per the OpenStack limit. + maxLength: 255 + minLength: 1 + type: string + type: object + required: + - type + type: object + x-kubernetes-validations: + - message: volume is forbidden when type is not Volume + rule: self.type == 'Volume' || !has(self.volume) required: - - diskSize - name + - sizeGiB + - storage type: object + maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index 31b90927ff..635213a6c1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -962,28 +962,79 @@ spec: description: AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device to + attach to the server. properties: - availabilityZone: - description: AvailabilityZone is the volume availability - zone to create the volume in. If omitted, the availability - zone of the server will be used. - type: string - diskSize: - description: Size is the size in GB of the volume. - type: integer name: - description: Name of the Cinder volume in the context - of a machine. It will be combined with the machine - name to make the actual volume name. - type: string - volumeType: - description: VolumeType is the volume type of the volume. - If omitted, the default type will be used. + description: Name of the block device in the context + of a machine. If the block device is a volume, the + Cinder volume will be named as a combination of the + machine name and this name. Also, this name will be + used for tagging the block device. Information about + the block device tag can be obtained from the OpenStack + metadata API or the config drive. type: string + sizeGiB: + description: SizeGiB is the size of the block device + in gibibytes (GiB). + minimum: 1 + type: integer + storage: + description: Storage specifies the storage type of the + block device and additional storage options. + properties: + type: + description: Type is the type of block device to + create. This can be either "Volume" or "Local". + enum: + - Volume + - Local + type: string + volume: + description: Volume contains additional storage + options for a volume block device. + properties: + availabilityZone: + description: AvailabilityZone is the volume + availability zone to create the volume in. + If omitted, the availability zone of the server + will be used. The availability zone 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. + maxLength: 63 + minLength: 1 + type: string + x-kubernetes-validations: + - message: availabilityZone may not contain + spaces + rule: '!self.contains('' '')' + type: + description: Type is the Cinder volume type + of the volume. If omitted, the default Cinder + volume type that is configured in the OpenStack + cloud will be used. The maximum length of + a volume type name is 255 characters, as per + the OpenStack limit. + maxLength: 255 + minLength: 1 + type: string + type: object + required: + - type + type: object + x-kubernetes-validations: + - message: volume is forbidden when type is not Volume + rule: self.type == 'Volume' || !has(self.volume) required: - - diskSize - name + - sizeGiB + - storage type: object + maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md b/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md index 726642cfdd..25937afa52 100644 --- a/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md +++ b/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md @@ -218,17 +218,44 @@ profile: We now have the ability for a machine to have additional block devices to be attached. -Here is an example on how to use `additionalBlockDevices` for adding additional Cinder volumes attached +Here is an example on how to use `additionalBlockDevices` for adding an additional Cinder volume attached to the server instance: ```yaml additionalBlockDevices: - name: database - size: 50 - volumeType: my-volume-type - availabilityZone: az0 + sizeGiB: 50 + storage: + type: volume ``` +Here is an example on how to use `additionalBlockDevices` for adding an additional Cinder volume attached +to the server instance with an availability zone and a cinder type: + +```yaml +additionalBlockDevices: +- name: database + sizeGiB: 50 + storage: + type: volume + volume: + type: my-volume-type + availabilityZone: az0 +``` + +Here is an example on how to attach a ephemeral disk to the instance: + +```yaml +additionalBlockDevices +- name: disk1 + sizeGiB: 1 + storage: + type: local +``` + +Adding more than one ephemeral disk to an instance is possible but you should use it at your own risks, it has been +known to cause some issues in some environments. + ### `OpenStackCluster` #### Change to externalRouterIPs.subnet diff --git a/hack/ci/cloud-init/controller.yaml.tpl b/hack/ci/cloud-init/controller.yaml.tpl index bca56cc475..abfc2e8523 100644 --- a/hack/ci/cloud-init/controller.yaml.tpl +++ b/hack/ci/cloud-init/controller.yaml.tpl @@ -149,11 +149,11 @@ # the flavors are created in a way that we can execute at least 2 e2e tests in parallel (overall we have 32 vCPUs) openstack flavor delete m1.tiny - openstack flavor create --ram 512 --disk 1 --vcpus 1 --public --id 1 m1.tiny --property hw_rng:allowed='True' + openstack flavor create --ram 512 --disk 1 --ephemeral 1 --vcpus 1 --public --id 1 m1.tiny --property hw_rng:allowed='True' openstack flavor delete m1.small - openstack flavor create --ram 4192 --disk 20 --vcpus 2 --public --id 2 m1.small --property hw_rng:allowed='True' + openstack flavor create --ram 4192 --disk 20 --ephemeral 5 --vcpus 2 --public --id 2 m1.small --property hw_rng:allowed='True' openstack flavor delete m1.medium - openstack flavor create --ram 6144 --disk 20 --vcpus 4 --public --id 3 m1.medium --property hw_rng:allowed='True' + openstack flavor create --ram 6144 --disk 20 --ephemeral 5 --vcpus 4 --public --id 3 m1.medium --property hw_rng:allowed='True' # Adjust the CPU quota openstack quota set --cores 32 demo diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index 72122d2562..3ee0bae2dd 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -438,19 +438,24 @@ func (s *Service) waitForVolume(volumeID string, timeout time.Duration, retryInt // getOrCreateVolumeBuilder gets or creates a volume with the given options. It returns the volume that already exists or the newly created one. // It returns an error if the volume creation failed or if the expected volume is different from the one that already exists. func (s *Service) getOrCreateVolumeBuilder(eventObject runtime.Object, instanceSpec *InstanceSpec, blockDevice infrav1.AdditionalBlockDevice, imageID string, description string) (*volumes.Volume, error) { + var volumeType string availabilityZone := instanceSpec.FailureDomain - if blockDevice.AvailabilityZone != "" { - availabilityZone = blockDevice.AvailabilityZone + + if blockDevice.Storage.Volume != nil { + if blockDevice.Storage.Volume.AvailabilityZone != "" { + availabilityZone = blockDevice.Storage.Volume.AvailabilityZone + } + volumeType = blockDevice.Storage.Volume.Type } createOpts := volumes.CreateOpts{ Name: volumeName(instanceSpec.Name, blockDevice.Name), Description: description, - Size: blockDevice.Size, + Size: blockDevice.SizeGiB, ImageID: imageID, Multiattach: false, AvailabilityZone: availabilityZone, - VolumeType: blockDevice.VolumeType, + VolumeType: volumeType, } return s.getOrCreateVolume(eventObject, createOpts) @@ -463,10 +468,15 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst if hasRootVolume(instanceSpec) { rootVolumeToBlockDevice := infrav1.AdditionalBlockDevice{ - Name: "root", - AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, - Size: instanceSpec.RootVolume.Size, - VolumeType: instanceSpec.RootVolume.VolumeType, + Name: "root", + SizeGiB: instanceSpec.RootVolume.Size, + Storage: infrav1.BlockDeviceStorage{ + Type: infrav1.VolumeBlockDevice, + Volume: &infrav1.BlockDeviceVolume{ + AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, + Type: instanceSpec.RootVolume.VolumeType, + }, + }, } rootVolume, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, rootVolumeToBlockDevice, imageID, fmt.Sprintf("Root volume for %s", instanceSpec.Name)) if err != nil { @@ -490,16 +500,39 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst } for _, blockDeviceSpec := range instanceSpec.AdditionalBlockDevices { - blockDevice, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, blockDeviceSpec, "", fmt.Sprintf("Additional block device for %s", instanceSpec.Name)) - if err != nil { - return []bootfromvolume.BlockDevice{}, err + var bdUUID string + var localDiskSizeGiB int + var sourceType bootfromvolume.SourceType + var destinationType bootfromvolume.DestinationType + + // There is also a validation in the openstackmachine webhook. + if blockDeviceSpec.Name == "root" { + return []bootfromvolume.BlockDevice{}, fmt.Errorf("block device name 'root' is reserved") } + + if blockDeviceSpec.Storage.Type == infrav1.VolumeBlockDevice { + blockDevice, err := s.getOrCreateVolumeBuilder(eventObject, instanceSpec, blockDeviceSpec, "", fmt.Sprintf("Additional block device for %s", instanceSpec.Name)) + if err != nil { + return []bootfromvolume.BlockDevice{}, err + } + bdUUID = blockDevice.ID + sourceType = bootfromvolume.SourceVolume + destinationType = bootfromvolume.DestinationVolume + } else if blockDeviceSpec.Storage.Type == infrav1.LocalBlockDevice { + sourceType = bootfromvolume.SourceBlank + destinationType = bootfromvolume.DestinationLocal + localDiskSizeGiB = blockDeviceSpec.SizeGiB + } else { + return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.Storage.Type) + } + blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ - SourceType: bootfromvolume.SourceVolume, - DestinationType: bootfromvolume.DestinationVolume, - UUID: blockDevice.ID, + SourceType: sourceType, + DestinationType: destinationType, + UUID: bdUUID, BootIndex: -1, DeleteOnTermination: true, + VolumeSize: localDiskSizeGiB, Tag: blockDeviceSpec.Name, }) } diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index f0d93b5737..62e88c836f 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -658,9 +658,21 @@ func TestService_ReconcileInstance(t *testing.T) { } s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", + Name: "etcd", + SizeGiB: 50, + Storage: infrav1.BlockDeviceStorage{ + Type: "Volume", + Volume: &infrav1.BlockDeviceVolume{ + Type: "test-volume-type", + }, + }, + }, + { + Name: "local-device", + SizeGiB: 10, + Storage: infrav1.BlockDeviceStorage{ + Type: "Local", + }, }, } return s @@ -712,6 +724,14 @@ func TestService_ReconcileInstance(t *testing.T) { "destination_type": "volume", "tag": "etcd", }, + { + "source_type": "blank", + "destination_type": "local", + "boot_index": float64(-1), + "delete_on_termination": true, + "volume_size": float64(10), + "tag": "local-device", + }, } expectCreateServer(r.compute, createMap, false) expectServerPollSuccess(r.compute) @@ -721,14 +741,26 @@ func TestService_ReconcileInstance(t *testing.T) { wantErr: false, }, { - name: "Additional block device success", + name: "Additional block devices success", getInstanceSpec: func() *InstanceSpec { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", + Name: "etcd", + SizeGiB: 50, + Storage: infrav1.BlockDeviceStorage{ + Type: "Volume", + Volume: &infrav1.BlockDeviceVolume{ + Type: "test-volume-type", + }, + }, + }, + { + Name: "data", + SizeGiB: 10, + Storage: infrav1.BlockDeviceStorage{ + Type: "Local", + }, }, } return s @@ -767,6 +799,14 @@ func TestService_ReconcileInstance(t *testing.T) { "destination_type": "volume", "tag": "etcd", }, + { + "source_type": "blank", + "destination_type": "local", + "boot_index": float64(-1), + "delete_on_termination": true, + "volume_size": float64(10), + "tag": "data", + }, } expectCreateServer(r.compute, createMap, false) expectServerPollSuccess(r.compute) @@ -781,10 +821,15 @@ func TestService_ReconcileInstance(t *testing.T) { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", - AvailabilityZone: "test-alternate-az", + Name: "etcd", + SizeGiB: 50, + Storage: infrav1.BlockDeviceStorage{ + Type: "Volume", + Volume: &infrav1.BlockDeviceVolume{ + AvailabilityZone: "test-alternate-az", + Type: "test-volume-type", + }, + }, }, } return s @@ -831,6 +876,30 @@ func TestService_ReconcileInstance(t *testing.T) { }, wantErr: false, }, + { + name: "Additional block device error when using wrong type", + getInstanceSpec: func() *InstanceSpec { + s := getDefaultInstanceSpec() + s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Name: "oops", + SizeGiB: 1, + Storage: infrav1.BlockDeviceStorage{ + Type: "doesnt-exist", + }, + }, + } + return s + }, + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) + + // Make sure we delete ports + expectCleanupDefaultPort(r.network) + }, + wantErr: true, + }, { name: "Delete trunks on port creation error", getInstanceSpec: func() *InstanceSpec { diff --git a/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml b/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml index 73e908b4cc..4cc8f7d772 100644 --- a/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml +++ b/test/e2e/data/kustomize/multi-az/patch-machine-template-control-plane.yaml @@ -6,5 +6,11 @@ - op: add path: /spec/template/spec/additionalBlockDevices value: - - diskSize: 1 - name: extravol + - name: extravol + sizeGiB: 1 + storage: + type: Volume + - name: etcd + sizeGiB: 1 + storage: + type: Local diff --git a/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml b/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml index f3e0be60cd..761f8a72dc 100644 --- a/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml +++ b/test/e2e/data/kustomize/multi-az/patch-machine-template-worker.yaml @@ -8,7 +8,14 @@ - op: add path: /spec/template/spec/additionalBlockDevices value: - - diskSize: 1 - name: extravol - volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} - availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + - name: extravol + sizeGiB: 1 + storage: + type: Volume + volume: + type: ${OPENSTACK_VOLUME_TYPE_ALT} + availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + - name: etcd + sizeGiB: 1 + storage: + type: Local \ No newline at end of file From 656fd0c817e16dabb929968f13010f571455c21d Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 23 Oct 2023 13:24:11 -0400 Subject: [PATCH 3/4] ci: relax nolintlint Hitting https://github.com/golangci/golangci-lint/issues/3228 when adding nolint. So allow to ignore unused nolints. (cherry picked from commit 4336f38166f305e267aeb645a67b2e4c770d68b8) --- .golangci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index b6e4ffb230..db45eb6083 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -126,6 +126,9 @@ linters-settings: - pkg: sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1 alias: controlplanev1 + nolintlint: + # https://github.com/golangci/golangci-lint/issues/3228 + allow-unused: true staticcheck: go: "1.17" stylecheck: From e36505bb3dcd39fec5d00226f7939f8ae75f525a Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Thu, 26 Oct 2023 13:10:46 -0400 Subject: [PATCH 4/4] api: remove CEL validations for `AdditionalBlockDevices` I've put that into a separate commit so we can re-add it later. It also serves as documentation for the work we have done to add CEL. However and unfortunately, we don't have CEL tests in place now so we decided to not have CEL validations for now; add them later and then we'll be able to revert this commit. (cherry picked from commit 71fa48fa3fac6f3c30a4955357d8da11c970da76) --- api/v1alpha7/openstackmachine_types.go | 5 ---- api/v1alpha7/types.go | 14 ----------- ...re.cluster.x-k8s.io_openstackclusters.yaml | 24 ++---------------- ...er.x-k8s.io_openstackclustertemplates.yaml | 25 ++----------------- ...re.cluster.x-k8s.io_openstackmachines.yaml | 22 ++-------------- ...er.x-k8s.io_openstackmachinetemplates.yaml | 24 ++---------------- 6 files changed, 8 insertions(+), 106 deletions(-) diff --git a/api/v1alpha7/openstackmachine_types.go b/api/v1alpha7/openstackmachine_types.go index 91b0bcbf60..f5f4f698aa 100644 --- a/api/v1alpha7/openstackmachine_types.go +++ b/api/v1alpha7/openstackmachine_types.go @@ -85,11 +85,6 @@ type OpenStackMachineSpec struct { RootVolume *RootVolume `json:"rootVolume,omitempty"` // AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance - // - // + --- - // + While it's possible to have unlimited number of block devices attached to a server instance, the number is - // + limited to 255 to avoid rule cost exceeded error in CRD validation. - // +kubebuilder:validation:MaxItems=255 // +listType=map // +listMapKey=name // +optional diff --git a/api/v1alpha7/types.go b/api/v1alpha7/types.go index dc75cbec6d..3ef5305ff2 100644 --- a/api/v1alpha7/types.go +++ b/api/v1alpha7/types.go @@ -165,15 +165,12 @@ type RootVolume struct { // BlockDeviceStorage is the storage type of a block device to create and // contains additional storage options. -// +kubebuilder:validation:XValidation:rule="self.type == 'Volume' || !has(self.volume)",message="volume is forbidden when type is not Volume" // +union // //nolint:godot type BlockDeviceStorage struct { // Type is the type of block device to create. // This can be either "Volume" or "Local". - // +kubebuilder:validation:Enum="Volume";"Local" - // +kubebuilder:validation:Required // +unionDiscriminator Type BlockDeviceType `json:"type"` @@ -188,9 +185,6 @@ type BlockDeviceVolume struct { // Type is the Cinder volume type of the volume. // If omitted, the default Cinder volume type that is configured in the OpenStack cloud // will be used. - // 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 Type string `json:"type,omitempty"` @@ -199,10 +193,6 @@ type BlockDeviceVolume struct { // The availability zone 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. - // +kubebuilder:validation:MinLength=1 - // +kubebuilder:validation:MaxLength=63 - // +kubebuilder:validation:XValidation:rule="!self.contains(' ')",message="availabilityZone may not contain spaces" // +optional AvailabilityZone string `json:"availabilityZone,omitempty"` } @@ -215,17 +205,13 @@ type AdditionalBlockDevice struct { // Also, this name will be used for tagging the block device. // Information about the block device tag can be obtained from the OpenStack // metadata API or the config drive. - // +kubebuilder:validation:Required Name string `json:"name"` // SizeGiB is the size of the block device in gibibytes (GiB). - // +kubebuilder:validation:Minimum=1 - // +kubebuilder:validation:Required SizeGiB int `json:"sizeGiB"` // Storage specifies the storage type of the block device and // additional storage options. - // +kubebuilder:validation:Required Storage BlockDeviceStorage `json:"storage"` } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index f3fadf6a3a..9d07fa358b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -3789,7 +3789,6 @@ spec: sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). - minimum: 1 type: integer storage: description: Storage specifies the storage type of the @@ -3798,9 +3797,6 @@ spec: type: description: Type is the type of block device to create. This can be either "Volume" or "Local". - enum: - - Volume - - Local type: string volume: description: Volume contains additional storage @@ -3814,39 +3810,23 @@ spec: 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. - maxLength: 63 - minLength: 1 + for further information. type: string - x-kubernetes-validations: - - message: availabilityZone may not contain - spaces - rule: '!self.contains('' '')' type: description: Type is the Cinder volume type of the volume. If omitted, the default Cinder volume type that is configured in the OpenStack - cloud will be used. The maximum length of - a volume type name is 255 characters, as per - the OpenStack limit. - maxLength: 255 - minLength: 1 + cloud will be used. type: string type: object required: - type type: object - x-kubernetes-validations: - - message: volume is forbidden when type is not Volume - rule: self.type == 'Volume' || !has(self.volume) required: - name - sizeGiB - storage type: object - maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 491d861f54..6a08b3be7d 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -1631,7 +1631,6 @@ spec: sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). - minimum: 1 type: integer storage: description: Storage specifies the storage type @@ -1642,9 +1641,6 @@ spec: description: Type is the type of block device to create. This can be either "Volume" or "Local". - enum: - - Volume - - Local type: string volume: description: Volume contains additional @@ -1659,41 +1655,24 @@ spec: 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. - maxLength: 63 - minLength: 1 + for further information. type: string - x-kubernetes-validations: - - message: availabilityZone may not - contain spaces - rule: '!self.contains('' '')' type: description: Type is the Cinder volume type of the volume. If omitted, the default Cinder volume type that is configured in the OpenStack cloud - will be used. The maximum length of - a volume type name is 255 characters, - as per the OpenStack limit. - maxLength: 255 - minLength: 1 + will be used. type: string type: object required: - type type: object - x-kubernetes-validations: - - message: volume is forbidden when type is - not Volume - rule: self.type == 'Volume' || !has(self.volume) required: - name - sizeGiB - storage type: object - maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index 7422af2d47..e5171cd868 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1177,7 +1177,6 @@ spec: sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). - minimum: 1 type: integer storage: description: Storage specifies the storage type of the block @@ -1186,9 +1185,6 @@ spec: type: description: Type is the type of block device to create. This can be either "Volume" or "Local". - enum: - - Volume - - Local type: string volume: description: Volume contains additional storage options @@ -1201,36 +1197,22 @@ spec: zone 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. - maxLength: 63 - minLength: 1 + for further information. type: string - x-kubernetes-validations: - - message: availabilityZone may not contain spaces - rule: '!self.contains('' '')' type: description: Type is the Cinder volume type of the volume. If omitted, the default Cinder volume type that is - configured in the OpenStack cloud will be used. The - maximum length of a volume type name is 255 characters, - as per the OpenStack limit. - maxLength: 255 - minLength: 1 + configured in the OpenStack cloud will be used. type: string type: object required: - type type: object - x-kubernetes-validations: - - message: volume is forbidden when type is not Volume - rule: self.type == 'Volume' || !has(self.volume) required: - name - sizeGiB - storage type: object - maxItems: 255 type: array x-kubernetes-list-map-keys: - name diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index 635213a6c1..bc88d84fa2 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -977,7 +977,6 @@ spec: sizeGiB: description: SizeGiB is the size of the block device in gibibytes (GiB). - minimum: 1 type: integer storage: description: Storage specifies the storage type of the @@ -986,9 +985,6 @@ spec: type: description: Type is the type of block device to create. This can be either "Volume" or "Local". - enum: - - Volume - - Local type: string volume: description: Volume contains additional storage @@ -1002,39 +998,23 @@ spec: 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. - maxLength: 63 - minLength: 1 + for further information. type: string - x-kubernetes-validations: - - message: availabilityZone may not contain - spaces - rule: '!self.contains('' '')' type: description: Type is the Cinder volume type of the volume. If omitted, the default Cinder volume type that is configured in the OpenStack - cloud will be used. The maximum length of - a volume type name is 255 characters, as per - the OpenStack limit. - maxLength: 255 - minLength: 1 + cloud will be used. type: string type: object required: - type type: object - x-kubernetes-validations: - - message: volume is forbidden when type is not Volume - rule: self.type == 'Volume' || !has(self.volume) required: - name - sizeGiB - storage type: object - maxItems: 255 type: array x-kubernetes-list-map-keys: - name