From a750ce96822441b9b5671e5d5490557519ed9adb 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) (cherry picked from commit edb9ef4dceb93883e00441a61b39648db7304c46) --- 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 46f2d41ca4015c3fece098f1db6bcefaee388768 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Wed, 18 Oct 2023 14:50:37 -0400 Subject: [PATCH 2/4] CARRY: Bump Gophercloud to v1.7.0 Edit go.mod to bump Gophercloud to v1.7.0 instead of v1.3.0 then: ``` go mod tiny go mod vendor ``` (cherry picked from commit bb659abf4cbf3b6fa0d92ac91bdce6782b85acff) --- go.mod | 2 +- go.sum | 4 +- .../gophercloud/gophercloud/CHANGELOG.md | 73 ++++++++++++ .../gophercloud/gophercloud/README.md | 5 +- .../gophercloud/gophercloud/errors.go | 48 ++++++++ .../blockstorage/v3/volumetypes/doc.go | 56 ++++++++++ .../blockstorage/v3/volumetypes/requests.go | 105 ++++++++++++++++++ .../blockstorage/v3/volumetypes/results.go | 97 ++++++++++++++++ .../blockstorage/v3/volumetypes/urls.go | 20 ++++ .../v2/extensions/bootfromvolume/requests.go | 6 + .../imageservice/v2/images/results.go | 2 +- .../openstack/networking/v2/ports/requests.go | 41 +++---- .../gophercloud/provider_client.go | 2 +- vendor/modules.txt | 2 +- 14 files changed, 434 insertions(+), 29 deletions(-) diff --git a/go.mod b/go.mod index db4a954a48..4de8538205 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.5.9 github.com/google/gofuzz v1.2.0 - github.com/gophercloud/gophercloud v1.3.0 + github.com/gophercloud/gophercloud v1.7.0 github.com/gophercloud/utils v0.0.0-20221207145018-e8fba78967ca github.com/hashicorp/go-version v1.4.0 github.com/onsi/ginkgo/v2 v2.11.0 diff --git a/go.sum b/go.sum index 66bd81097a..6d65809d76 100644 --- a/go.sum +++ b/go.sum @@ -255,8 +255,8 @@ github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+ github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/googleapis/google-cloud-go-testing v0.0.0-20200911160855-bcd43fbb19e8/go.mod h1:dvDLG8qkwmyD9a/MJJN3XJcT3xFxOKAvTZGvuZmac9g= github.com/gophercloud/gophercloud v1.1.1/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM= -github.com/gophercloud/gophercloud v1.3.0 h1:RUKyCMiZoQR3VlVR5E3K7PK1AC3/qppsWYo6dtBiqs8= -github.com/gophercloud/gophercloud v1.3.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM= +github.com/gophercloud/gophercloud v1.7.0 h1:fyJGKh0LBvIZKLvBWvQdIgkaV5yTM3Jh9EYUh+UNCAs= +github.com/gophercloud/gophercloud v1.7.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM= github.com/gophercloud/utils v0.0.0-20221207145018-e8fba78967ca h1:HZWyhvVXgS2rx5StixMKhrTjpfUg5vLSNhF/xHkcRNg= github.com/gophercloud/utils v0.0.0-20221207145018-e8fba78967ca/go.mod h1:z4Dey7xsTUXgcB1C8elMvGRKTjV1ez0eoYQlMrduG1g= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= diff --git a/vendor/github.com/gophercloud/gophercloud/CHANGELOG.md b/vendor/github.com/gophercloud/gophercloud/CHANGELOG.md index 9e2567b98b..b470df398b 100644 --- a/vendor/github.com/gophercloud/gophercloud/CHANGELOG.md +++ b/vendor/github.com/gophercloud/gophercloud/CHANGELOG.md @@ -1,3 +1,76 @@ +## v1.7.0 (2023-09-22) + +New features and improvements: + +* [GH-2782](https://github.com/gophercloud/gophercloud/pull/2782) [v1] (manual clean backport) Add tag field to compute block_device_v2 + +CI changes: + +* [GH-2760](https://github.com/gophercloud/gophercloud/pull/2760) [v1 backports] semver auto labels +* [GH-2775](https://github.com/gophercloud/gophercloud/pull/2775) [v1] Fix typos in comments +* [GH-2783](https://github.com/gophercloud/gophercloud/pull/2783) [v1] (clean manual backport) ci/functional: fix ubuntu version & add antelope +* [GH-2785](https://github.com/gophercloud/gophercloud/pull/2785) [v1] Acceptance: Handle numerical version names in version comparison helpers +* [GH-2787](https://github.com/gophercloud/gophercloud/pull/2787) backport-v1: fixes to semver label +* [GH-2788](https://github.com/gophercloud/gophercloud/pull/2788) [v1] Make acceptance tests internal + + +## v1.6.0 (2023-08-30) + +New features and improvements: + +* [GH-2712](https://github.com/gophercloud/gophercloud/pull/2712) [v1] README: minor change to test backport workflow +* [GH-2713](https://github.com/gophercloud/gophercloud/pull/2713) [v1] tests: run MultiAttach with a capable Cinder Type +* [GH-2714](https://github.com/gophercloud/gophercloud/pull/2714) [v1] Add CRUD support for encryption in volume v3 types +* [GH-2715](https://github.com/gophercloud/gophercloud/pull/2715) [v1] Add projectID to fwaas_v2 policy CreateOpts and ListOpts +* [GH-2716](https://github.com/gophercloud/gophercloud/pull/2716) [v1] Add projectID to fwaas_v2 CreateOpts +* [GH-2717](https://github.com/gophercloud/gophercloud/pull/2717) [v1] [manila]: add reset and force delete actions to a snapshot +* [GH-2718](https://github.com/gophercloud/gophercloud/pull/2718) [v1] [cinder]: add reset and force delete actions to volumes and snapshots +* [GH-2721](https://github.com/gophercloud/gophercloud/pull/2721) [v1] orchestration: Explicit error in optionsmap creation +* [GH-2723](https://github.com/gophercloud/gophercloud/pull/2723) [v1] Add conductor API to Baremetal V1 +* [GH-2729](https://github.com/gophercloud/gophercloud/pull/2729) [v1] networking/v2/ports: allow list filter by security group + +CI changes: + +* [GH-2675](https://github.com/gophercloud/gophercloud/pull/2675) [v1][CI] Drop periodic jobs from stable branch +* [GH-2683](https://github.com/gophercloud/gophercloud/pull/2683) [v1] CI tweaks + + +## v1.5.0 (2023-06-21) + +New features and improvements: + +* [GH-2634](https://github.com/gophercloud/gophercloud/pull/2634) baremetal: update inspection inventory with recent additions +* [GH-2635](https://github.com/gophercloud/gophercloud/pull/2635) [manila]: Add Share Replicas support +* [GH-2637](https://github.com/gophercloud/gophercloud/pull/2637) [FWaaS_v2]: Add FWaaS_V2 workflow and enable tests +* [GH-2639](https://github.com/gophercloud/gophercloud/pull/2639) Implement errors.Unwrap() on unexpected status code errors +* [GH-2648](https://github.com/gophercloud/gophercloud/pull/2648) [manila]: implement share transfer API + + +## v1.4.0 (2023-05-25) + +New features and improvements: + +* [GH-2465](https://github.com/gophercloud/gophercloud/pull/2465) keystone: add v3 limits update operation +* [GH-2596](https://github.com/gophercloud/gophercloud/pull/2596) keystone: add v3 limits get operation +* [GH-2618](https://github.com/gophercloud/gophercloud/pull/2618) keystone: add v3 limits delete operation +* [GH-2616](https://github.com/gophercloud/gophercloud/pull/2616) Add CRUD support for register limit APIs +* [GH-2610](https://github.com/gophercloud/gophercloud/pull/2610) Add PUT/HEAD/DELETE for identity/v3/OS-INHERIT +* [GH-2597](https://github.com/gophercloud/gophercloud/pull/2597) Add validation and optimise objects.BulkDelete +* [GH-2602](https://github.com/gophercloud/gophercloud/pull/2602) [swift v1]: introduce a TempURLKey argument for objects.CreateTempURLOpts struct +* [GH-2623](https://github.com/gophercloud/gophercloud/pull/2623) Add the ability to remove ingress/egress policies from fwaas_v2 groups +* [GH-2625](https://github.com/gophercloud/gophercloud/pull/2625) neutron: Support trunk_details extension + +CI changes: + +* [GH-2608](https://github.com/gophercloud/gophercloud/pull/2608) Drop train and ussuri jobs +* [GH-2589](https://github.com/gophercloud/gophercloud/pull/2589) Bump EmilienM/devstack-action from 0.10 to 0.11 +* [GH-2604](https://github.com/gophercloud/gophercloud/pull/2604) Bump mheap/github-action-required-labels from 3 to 4 +* [GH-2620](https://github.com/gophercloud/gophercloud/pull/2620) Pin goimport dep to a version that works with go 1.14 +* [GH-2619](https://github.com/gophercloud/gophercloud/pull/2619) Fix version comparison for acceptance tests +* [GH-2627](https://github.com/gophercloud/gophercloud/pull/2627) Limits: Fix ToDo to create registered limit and use it +* [GH-2629](https://github.com/gophercloud/gophercloud/pull/2629) [manila]: Add share from snapshot restore functional test + + ## v1.3.0 (2023-03-28) * [GH-2464](https://github.com/gophercloud/gophercloud/pull/2464) keystone: add v3 limits create operation diff --git a/vendor/github.com/gophercloud/gophercloud/README.md b/vendor/github.com/gophercloud/gophercloud/README.md index 89b08156fe..4e6e57dadb 100644 --- a/vendor/github.com/gophercloud/gophercloud/README.md +++ b/vendor/github.com/gophercloud/gophercloud/README.md @@ -1,6 +1,5 @@ # Gophercloud: an OpenStack SDK for Go -[![Build Status](https://travis-ci.org/gophercloud/gophercloud.svg?branch=master)](https://travis-ci.org/gophercloud/gophercloud) -[![Coverage Status](https://coveralls.io/repos/github/gophercloud/gophercloud/badge.svg?branch=master)](https://coveralls.io/github/gophercloud/gophercloud?branch=master) +[![Coverage Status](https://coveralls.io/repos/github/gophercloud/gophercloud/badge.svg?branch=v1)](https://coveralls.io/github/gophercloud/gophercloud?branch=v1) Gophercloud is an OpenStack Go SDK. @@ -40,7 +39,7 @@ You will need to retrieve the following: Credentials, a pre-generated token, or any other supported authentication mechanism. -For users that have the OpenStack dashboard installed, there's a shortcut. If +For users who have the OpenStack dashboard installed, there's a shortcut. If you visit the `project/api_access` path in Horizon and click on the "Download OpenStack RC File" button at the top right hand corner, you can download either a `clouds.yaml` file or an `openrc` bash file that exports all diff --git a/vendor/github.com/gophercloud/gophercloud/errors.go b/vendor/github.com/gophercloud/gophercloud/errors.go index edba02badf..8ab592ca49 100644 --- a/vendor/github.com/gophercloud/gophercloud/errors.go +++ b/vendor/github.com/gophercloud/gophercloud/errors.go @@ -116,61 +116,109 @@ type ErrDefault400 struct { ErrUnexpectedResponseCode } +func (e ErrDefault400) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault401 is the default error type returned on a 401 HTTP response code. type ErrDefault401 struct { ErrUnexpectedResponseCode } +func (e ErrDefault401) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault403 is the default error type returned on a 403 HTTP response code. type ErrDefault403 struct { ErrUnexpectedResponseCode } +func (e ErrDefault403) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault404 is the default error type returned on a 404 HTTP response code. type ErrDefault404 struct { ErrUnexpectedResponseCode } +func (e ErrDefault404) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault405 is the default error type returned on a 405 HTTP response code. type ErrDefault405 struct { ErrUnexpectedResponseCode } +func (e ErrDefault405) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault408 is the default error type returned on a 408 HTTP response code. type ErrDefault408 struct { ErrUnexpectedResponseCode } +func (e ErrDefault408) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault409 is the default error type returned on a 409 HTTP response code. type ErrDefault409 struct { ErrUnexpectedResponseCode } +func (e ErrDefault409) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault429 is the default error type returned on a 429 HTTP response code. type ErrDefault429 struct { ErrUnexpectedResponseCode } +func (e ErrDefault429) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault500 is the default error type returned on a 500 HTTP response code. type ErrDefault500 struct { ErrUnexpectedResponseCode } +func (e ErrDefault500) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault502 is the default error type returned on a 502 HTTP response code. type ErrDefault502 struct { ErrUnexpectedResponseCode } +func (e ErrDefault502) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault503 is the default error type returned on a 503 HTTP response code. type ErrDefault503 struct { ErrUnexpectedResponseCode } +func (e ErrDefault503) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + // ErrDefault504 is the default error type returned on a 504 HTTP response code. type ErrDefault504 struct { ErrUnexpectedResponseCode } +func (e ErrDefault504) Unwrap() error { + return e.ErrUnexpectedResponseCode +} + func (e ErrDefault400) Error() string { e.DefaultErrString = fmt.Sprintf( "Bad request with: [%s %s], error message: %s", diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/doc.go b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/doc.go index 55a2170bc2..03cad7ecbd 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/doc.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/doc.go @@ -160,5 +160,61 @@ Example to Remove/Revoke Access to a Volume Type if err != nil { panic(err) } + +Example to Create the Encryption of a Volume Type + + typeID := "7ffaca22-f646-41d4-b79d-d7e4452ef8cc" + volumeType, err := volumetypes.CreateEncryption(client, typeID, .CreateEncryptionOpts{ + KeySize: 256, + Provider: "luks", + ControlLocation: "front-end", + Cipher: "aes-xts-plain64", + }).Extract() + if err != nil{ + panic(err) + } + fmt.Println(volumeType) + +Example to Delete the Encryption of a Volume Type + + typeID := "7ffaca22-f646-41d4-b79d-d7e4452ef8cc" + encryptionID := ""81e069c6-7394-4856-8df7-3b237ca61f74 + err := volumetypes.DeleteEncryption(client, typeID, encryptionID).ExtractErr() + if err != nil{ + panic(err) + } + +Example to Update the Encryption of a Volume Type + + typeID := "7ffaca22-f646-41d4-b79d-d7e4452ef8cc" + volumetype, err = volumetypes.UpdateEncryption(client, typeID, volumetypes.UpdateEncryptionOpts{ + KeySize: 256, + Provider: "luks", + ControlLocation: "front-end", + Cipher: "aes-xts-plain64", + }).Extract() + if err != nil{ + panic(err) + } + fmt.Println(volumetype) + +Example to Show an Encryption of a Volume Type + + typeID := "7ffaca22-f646-41d4-b79d-d7e4452ef8cc" + volumeType, err := volumetypes.GetEncrytpion(client, typeID).Extract() + if err != nil{ + panic(err) + } + fmt.Println(volumeType) + +Example to Show an Encryption Spec of a Volume Type + + typeID := "7ffaca22-f646-41d4-b79d-d7e4452ef8cc" + key := "cipher" + volumeType, err := volumetypes.GetEncrytpionSpec(client, typeID).Extract() + if err != nil{ + panic(err) + } + fmt.Println(volumeType) */ package volumetypes diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/requests.go b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/requests.go index 5b272bf05b..e06f7a4638 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/requests.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/requests.go @@ -304,3 +304,108 @@ func RemoveAccess(client *gophercloud.ServiceClient, id string, opts RemoveAcces _, r.Header, r.Err = gophercloud.ParseResponse(resp, err) return } + +// CreateEncryptionOptsBuilder allows extensions to add additional parameters to the +// Create Encryption request. +type CreateEncryptionOptsBuilder interface { + ToEncryptionCreateMap() (map[string]interface{}, error) +} + +// CreateEncryptionOpts contains options for creating an Encryption Type object. +// This object is passed to the volumetypes.CreateEncryption function. +// For more information about these parameters,see the Encryption Type object. +type CreateEncryptionOpts struct { + // The size of the encryption key. + KeySize int `json:"key_size"` + // The class of that provides the encryption support. + Provider string `json:"provider" required:"true"` + // Notional service where encryption is performed. + ControlLocation string `json:"control_location"` + // The encryption algorithm or mode. + Cipher string `json:"cipher"` +} + +// ToEncryptionCreateMap assembles a request body based on the contents of a +// CreateEncryptionOpts. +func (opts CreateEncryptionOpts) ToEncryptionCreateMap() (map[string]interface{}, error) { + return gophercloud.BuildRequestBody(opts, "encryption") +} + +// CreateEncryption will creates an Encryption Type object based on the CreateEncryptionOpts. +// To extract the Encryption Type object from the response, call the Extract method on the +// EncryptionCreateResult. +func CreateEncryption(client *gophercloud.ServiceClient, id string, opts CreateEncryptionOptsBuilder) (r CreateEncryptionResult) { + b, err := opts.ToEncryptionCreateMap() + if err != nil { + r.Err = err + return + } + resp, err := client.Post(createEncryptionURL(client, id), b, &r.Body, &gophercloud.RequestOpts{ + OkCodes: []int{200}, + }) + _, r.Header, r.Err = gophercloud.ParseResponse(resp, err) + return +} + +// Delete will delete an encryption type for an existing Volume Type with the provided ID. +func DeleteEncryption(client *gophercloud.ServiceClient, id, encryptionID string) (r DeleteEncryptionResult) { + resp, err := client.Delete(deleteEncryptionURL(client, id, encryptionID), nil) + _, r.Header, r.Err = gophercloud.ParseResponse(resp, err) + return +} + +// GetEncryption retrieves the encryption type for an existing VolumeType with the provided ID. +func GetEncryption(client *gophercloud.ServiceClient, id string) (r GetEncryptionResult) { + resp, err := client.Get(getEncryptionURL(client, id), &r.Body, nil) + _, r.Header, r.Err = gophercloud.ParseResponse(resp, err) + return +} + +// GetEncryptionSpecs retrieves the encryption type specs for an existing VolumeType with the provided ID. +func GetEncryptionSpec(client *gophercloud.ServiceClient, id, key string) (r GetEncryptionSpecResult) { + resp, err := client.Get(getEncryptionSpecURL(client, id, key), &r.Body, nil) + _, r.Header, r.Err = gophercloud.ParseResponse(resp, err) + return +} + +// UpdateEncryptionOptsBuilder allows extensions to add additional parameters to the +// Update encryption request. +type UpdateEncryptionOptsBuilder interface { + ToUpdateEncryptionMap() (map[string]interface{}, error) +} + +// Update Encryption Opts contains options for creating an Update Encryption Type. This object is passed to +// the volumetypes.UpdateEncryption function. For more information about these parameters, +// see the Update Encryption Type object. +type UpdateEncryptionOpts struct { + // The size of the encryption key. + KeySize int `json:"key_size"` + // The class of that provides the encryption support. + Provider string `json:"provider"` + // Notional service where encryption is performed. + ControlLocation string `json:"control_location"` + // The encryption algorithm or mode. + Cipher string `json:"cipher"` +} + +// ToEncryptionCreateMap assembles a request body based on the contents of a +// UpdateEncryptionOpts. +func (opts UpdateEncryptionOpts) ToUpdateEncryptionMap() (map[string]interface{}, error) { + return gophercloud.BuildRequestBody(opts, "encryption") +} + +// Update will update an existing encryption for a Volume Type based on the values in UpdateEncryptionOpts. +// To extract the UpdateEncryption Type object from the response, call the Extract method on the +// UpdateEncryptionResult. +func UpdateEncryption(client *gophercloud.ServiceClient, id, encryptionID string, opts UpdateEncryptionOptsBuilder) (r UpdateEncryptionResult) { + b, err := opts.ToUpdateEncryptionMap() + if err != nil { + r.Err = err + return + } + resp, err := client.Put(updateEncryptionURL(client, id, encryptionID), b, &r.Body, &gophercloud.RequestOpts{ + OkCodes: []int{200}, + }) + _, r.Header, r.Err = gophercloud.ParseResponse(resp, err) + return +} diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/results.go b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/results.go index 916b476da5..4d1d1cf2df 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/results.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/results.go @@ -200,3 +200,100 @@ type AddAccessResult struct { type RemoveAccessResult struct { gophercloud.ErrResult } + +type EncryptionType struct { + // Unique identifier for the volume type. + VolumeTypeID string `json:"volume_type_id"` + // Notional service where encryption is performed. + ControlLocation string `json:"control_location"` + // Unique identifier for encryption type. + EncryptionID string `json:"encryption_id"` + // Size of encryption key. + KeySize int `json:"key_size"` + // Class that provides encryption support. + Provider string `json:"provider"` + // The encryption algorithm or mode. + Cipher string `json:"cipher"` +} + +type encryptionResult struct { + gophercloud.Result +} + +func (r encryptionResult) Extract() (*EncryptionType, error) { + var s EncryptionType + err := r.ExtractInto(&s) + return &s, err +} + +// ExtractInto converts our response data into a volume type struct +func (r encryptionResult) ExtractInto(v interface{}) error { + return r.Result.ExtractIntoStructPtr(v, "encryption") +} + +type CreateEncryptionResult struct { + encryptionResult +} + +// UpdateResult contains the response body and error from an UpdateEncryption request. +type UpdateEncryptionResult struct { + encryptionResult +} + +// DeleteEncryptionResult contains the response body and error from a DeleteEncryprion request. +type DeleteEncryptionResult struct { + gophercloud.ErrResult +} + +type GetEncryptionType struct { + // Unique identifier for the volume type. + VolumeTypeID string `json:"volume_type_id"` + // Notional service where encryption is performed. + ControlLocation string `json:"control_location"` + // Shows if the resource is deleted or Notional + Deleted bool `json:"deleted"` + // Shows the date and time the resource was created. + CreatedAt string `json:"created_at"` + // Shows the date and time when resource was updated. + UpdatedAt string `json:"updated_at"` + // Unique identifier for encryption type. + EncryptionID string `json:"encryption_id"` + // Size of encryption key. + KeySize int `json:"key_size"` + // Class that provides encryption support. + Provider string `json:"provider"` + // Shows the date and time the reousrce was deleted. + DeletedAt string `json:"deleted_at"` + // The encryption algorithm or mode. + Cipher string `json:"cipher"` +} + +type encryptionShowResult struct { + gophercloud.Result +} + +// Extract interprets any extraSpecResult as an ExtraSpec, if possible. +func (r encryptionShowResult) Extract() (*GetEncryptionType, error) { + var s GetEncryptionType + err := r.ExtractInto(&s) + return &s, err +} + +type GetEncryptionResult struct { + encryptionShowResult +} + +type encryptionShowSpecResult struct { + gophercloud.Result +} + +// Extract interprets any empty interface Result as an empty interface. +func (r encryptionShowSpecResult) Extract() (map[string]interface{}, error) { + var s map[string]interface{} + err := r.ExtractInto(&s) + return s, err +} + +type GetEncryptionSpecResult struct { + encryptionShowSpecResult +} diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/urls.go b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/urls.go index c63ee47e62..c65478e684 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/urls.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumetypes/urls.go @@ -49,3 +49,23 @@ func accessURL(client *gophercloud.ServiceClient, id string) string { func accessActionURL(client *gophercloud.ServiceClient, id string) string { return client.ServiceURL("types", id, "action") } + +func createEncryptionURL(client *gophercloud.ServiceClient, id string) string { + return client.ServiceURL("types", id, "encryption") +} + +func deleteEncryptionURL(client *gophercloud.ServiceClient, id, encryptionID string) string { + return client.ServiceURL("types", id, "encryption", encryptionID) +} + +func getEncryptionURL(client *gophercloud.ServiceClient, id string) string { + return client.ServiceURL("types", id, "encryption") +} + +func getEncryptionSpecURL(client *gophercloud.ServiceClient, id, key string) string { + return client.ServiceURL("types", id, "encryption", key) +} + +func updateEncryptionURL(client *gophercloud.ServiceClient, id, encryptionID string) string { + return client.ServiceURL("types", id, "encryption", encryptionID) +} diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/bootfromvolume/requests.go b/vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/bootfromvolume/requests.go index 096d8be7ef..17f11b3849 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/bootfromvolume/requests.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/bootfromvolume/requests.go @@ -79,6 +79,12 @@ type BlockDevice struct { // VolumeType is the volume type of the block device. // This requires Compute API microversion 2.67 or later. VolumeType string `json:"volume_type,omitempty"` + + // Tag is an arbitrary string that can be applied to a block device. + // Information about the device tags can be obtained from the metadata API + // and the config drive, allowing devices to be easily identified. + // This requires Compute API microversion 2.42 or later. + Tag string `json:"tag,omitempty"` } // CreateOptsExt is a structure that extends the server `CreateOpts` structure diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/imageservice/v2/images/results.go b/vendor/github.com/gophercloud/gophercloud/openstack/imageservice/v2/images/results.go index 1b27a15495..96fd91a2ca 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/imageservice/v2/images/results.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/imageservice/v2/images/results.go @@ -76,7 +76,7 @@ type Image struct { CreatedAt time.Time `json:"created_at"` // UpdatedAt is the date when the last change has been made to the image or - // it's properties. + // its properties. UpdatedAt time.Time `json:"updated_at"` // File is the trailing path after the glance endpoint that represent the diff --git a/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/ports/requests.go b/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/ports/requests.go index 48f9985643..805f0e5b99 100644 --- a/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/ports/requests.go +++ b/vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/ports/requests.go @@ -21,26 +21,27 @@ type ListOptsBuilder interface { // by a particular port attribute. SortDir sets the direction, and is either // `asc' or `desc'. Marker and Limit are used for pagination. type ListOpts struct { - Status string `q:"status"` - Name string `q:"name"` - Description string `q:"description"` - AdminStateUp *bool `q:"admin_state_up"` - NetworkID string `q:"network_id"` - TenantID string `q:"tenant_id"` - ProjectID string `q:"project_id"` - DeviceOwner string `q:"device_owner"` - MACAddress string `q:"mac_address"` - ID string `q:"id"` - DeviceID string `q:"device_id"` - Limit int `q:"limit"` - Marker string `q:"marker"` - SortKey string `q:"sort_key"` - SortDir string `q:"sort_dir"` - Tags string `q:"tags"` - TagsAny string `q:"tags-any"` - NotTags string `q:"not-tags"` - NotTagsAny string `q:"not-tags-any"` - FixedIPs []FixedIPOpts + Status string `q:"status"` + Name string `q:"name"` + Description string `q:"description"` + AdminStateUp *bool `q:"admin_state_up"` + NetworkID string `q:"network_id"` + TenantID string `q:"tenant_id"` + ProjectID string `q:"project_id"` + DeviceOwner string `q:"device_owner"` + MACAddress string `q:"mac_address"` + ID string `q:"id"` + DeviceID string `q:"device_id"` + Limit int `q:"limit"` + Marker string `q:"marker"` + SortKey string `q:"sort_key"` + SortDir string `q:"sort_dir"` + Tags string `q:"tags"` + TagsAny string `q:"tags-any"` + NotTags string `q:"not-tags"` + NotTagsAny string `q:"not-tags-any"` + SecurityGroups []string `q:"security_groups"` + FixedIPs []FixedIPOpts } type FixedIPOpts struct { diff --git a/vendor/github.com/gophercloud/gophercloud/provider_client.go b/vendor/github.com/gophercloud/gophercloud/provider_client.go index c603d6dbe3..e53b713cd4 100644 --- a/vendor/github.com/gophercloud/gophercloud/provider_client.go +++ b/vendor/github.com/gophercloud/gophercloud/provider_client.go @@ -14,7 +14,7 @@ import ( // DefaultUserAgent is the default User-Agent string set in the request header. const ( - DefaultUserAgent = "gophercloud/v1.3.0" + DefaultUserAgent = "gophercloud/v1.7.0" DefaultMaxBackoffRetries = 60 ) diff --git a/vendor/modules.txt b/vendor/modules.txt index 5f8ff4bfa1..3cdb764add 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -201,7 +201,7 @@ github.com/google/safetext/yamltemplate # github.com/google/uuid v1.3.0 ## explicit github.com/google/uuid -# github.com/gophercloud/gophercloud v1.3.0 +# github.com/gophercloud/gophercloud v1.7.0 ## explicit; go 1.14 github.com/gophercloud/gophercloud github.com/gophercloud/gophercloud/openstack From 110efd974e9760805f804a1d4661c91c2cf3f85d Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Fri, 29 Sep 2023 15:12:25 -0400 Subject: [PATCH 3/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 51b199687e877f66ccb8f96571f16e0b731ebbd6) --- api/v1alpha7/types.go | 33 ++++++++++- ...re.cluster.x-k8s.io_openstackclusters.yaml | 17 +++++- ...er.x-k8s.io_openstackclustertemplates.yaml | 21 +++++-- ...re.cluster.x-k8s.io_openstackmachines.yaml | 17 +++++- ...er.x-k8s.io_openstackmachinetemplates.yaml | 17 +++++- .../crd-changes/v1alpha6-to-v1alpha7.md | 16 +++++- hack/ci/cloud-init/controller.yaml.tpl | 6 +- pkg/cloud/services/compute/instance.go | 28 ++++++++-- pkg/cloud/services/compute/instance_test.go | 56 ++++++++++++++++++- .../patch-machine-template-control-plane.yaml | 4 ++ .../patch-machine-template-worker.yaml | 4 ++ 11 files changed, 191 insertions(+), 28 deletions(-) diff --git a/api/v1alpha7/types.go b/api/v1alpha7/types.go index 049cae2965..695090d2fb 100644 --- a/api/v1alpha7/types.go +++ b/api/v1alpha7/types.go @@ -16,6 +16,18 @@ limitations under the License. package v1alpha7 +// BlockDeviceType defines the type of block device to create. +// +kubebuilder:validation:Enum=volume;local +type BlockDeviceType string + +const ( + // LocalBlockDevice is an ephemeral block deviced attached to the server. + LocalBlockDevice BlockDeviceType = "local" + + // VolumeBlockDevice is a volume block device attached to the server. + VolumeBlockDevice BlockDeviceType = "volume" +) + // OpenStackMachineTemplateResource describes the data needed to create a OpenStackMachine from a template. type OpenStackMachineTemplateResource struct { // Spec is the specification of the desired behavior of the machine. @@ -163,17 +175,32 @@ type RootVolume struct { AvailabilityZone string `json:"availabilityZone,omitempty"` } +// AdditionalBlockDevice is a block device to attach to the server. 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. + // Type is the type of block device to create. + // This can be either "volume" or "local". + // +kubebuilder:validation:Required + // +unionDiscriminator + Type BlockDeviceType `json:"type"` + + // Name of the block device in the context of a machine. + // It will be combined with the machine name to make the actual cinder + // volume name, and will be used for tagging the block device. Name string `json:"name"` - // Size is the size in GB of the volume. + + // Size is the size in GB of the block device. Size int `json:"diskSize"` + // VolumeType is the volume type of the volume. // If omitted, the default type will be used. + // +unionMember=volume,optional + // +optional 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. + // +unionMember=volume,optional + // +optional AvailabilityZone string `json:"availabilityZone,omitempty"` } 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..f59badc30a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -3774,6 +3774,8 @@ 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 @@ -3781,12 +3783,20 @@ spec: zone of the server will be used. type: string diskSize: - description: Size is the size in GB of the volume. + description: Size is the size in GB of the block device. type: integer name: - description: Name of the Cinder volume in the context + description: Name of the block device in the context of a machine. It will be combined with the machine - name to make the actual volume name. + name to make the actual cinder volume name, and will + be used for tagging the block device. + type: string + type: + description: Type is the type of block device to create. + This can be either "volume" or "local". + enum: + - volume + - local type: string volumeType: description: VolumeType is the volume type of the volume. @@ -3795,6 +3805,7 @@ spec: required: - diskSize - name + - type type: object type: array x-kubernetes-list-map-keys: 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..1452e7832d 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -1615,6 +1615,8 @@ 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 @@ -1623,13 +1625,23 @@ spec: will be used. type: string diskSize: - description: Size is the size in GB of the volume. + description: Size is the size in GB of the block + device. type: integer name: - description: Name of the Cinder volume in the + description: Name of the block device in the context of a machine. It will be combined - with the machine name to make the actual volume - name. + with the machine name to make the actual cinder + volume name, and will be used for tagging + the block device. + type: string + type: + description: Type is the type of block device + to create. This can be either "volume" or + "local". + enum: + - volume + - local type: string volumeType: description: VolumeType is the volume type of @@ -1639,6 +1651,7 @@ spec: required: - diskSize - name + - type type: object type: array x-kubernetes-list-map-keys: 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..fa140da9b7 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1163,6 +1163,8 @@ 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 @@ -1170,12 +1172,20 @@ spec: of the server will be used. type: string diskSize: - description: Size is the size in GB of the volume. + description: Size is the size in GB of the block device. type: integer name: - description: Name of the Cinder volume in the context of a machine. + description: Name of the block device in the context of a machine. It will be combined with the machine name to make the actual - volume name. + cinder volume name, and will be used for tagging the block + device. + type: string + type: + description: Type is the type of block device to create. This + can be either "volume" or "local". + enum: + - volume + - local type: string volumeType: description: VolumeType is the volume type of the volume. If @@ -1184,6 +1194,7 @@ spec: required: - diskSize - name + - type type: object type: array x-kubernetes-list-map-keys: 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..1dd79af913 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -962,6 +962,8 @@ 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 @@ -969,12 +971,20 @@ spec: zone of the server will be used. type: string diskSize: - description: Size is the size in GB of the volume. + description: Size is the size in GB of the block device. type: integer name: - description: Name of the Cinder volume in the context + description: Name of the block device in the context of a machine. It will be combined with the machine - name to make the actual volume name. + name to make the actual cinder volume name, and will + be used for tagging the block device. + type: string + type: + description: Type is the type of block device to create. + This can be either "volume" or "local". + enum: + - volume + - local type: string volumeType: description: VolumeType is the volume type of the volume. @@ -983,6 +993,7 @@ spec: required: - diskSize - name + - type type: object type: array x-kubernetes-list-map-keys: 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..c1530fb7e2 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,29 @@ 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 + type: volume + diskSize: 50 volumeType: my-volume-type availabilityZone: az0 ``` +Here is an example on how to attach a ephemeral disk to the instance: + +```yaml +- name: disk1 + type: local + diskSize: 1 +``` + +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..f259ea5949 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -463,6 +463,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst if hasRootVolume(instanceSpec) { rootVolumeToBlockDevice := infrav1.AdditionalBlockDevice{ + Type: infrav1.VolumeBlockDevice, Name: "root", AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, Size: instanceSpec.RootVolume.Size, @@ -490,16 +491,31 @@ 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 sourceType bootfromvolume.SourceType + var destinationType bootfromvolume.DestinationType + if blockDeviceSpec.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.Type == infrav1.LocalBlockDevice { + sourceType = bootfromvolume.SourceBlank + destinationType = bootfromvolume.DestinationLocal + } else { + return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.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: blockDeviceSpec.Size, Tag: blockDeviceSpec.Name, }) } diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index f0d93b5737..6f2152d386 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -658,10 +658,16 @@ func TestService_ReconcileInstance(t *testing.T) { } s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { + Type: "volume", Name: "etcd", Size: 50, VolumeType: "test-volume-type", }, + { + Type: "local", + Name: "local-device", + Size: 10, + }, } return s }, @@ -710,8 +716,17 @@ func TestService_ReconcileInstance(t *testing.T) { "boot_index": float64(-1), "delete_on_termination": true, "destination_type": "volume", + "volume_size": float64(50), "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,15 +736,21 @@ 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{ { + Type: "volume", Name: "etcd", Size: 50, VolumeType: "test-volume-type", }, + { + Type: "local", + Name: "data", + Size: 10, + }, } return s }, @@ -765,8 +786,17 @@ func TestService_ReconcileInstance(t *testing.T) { "boot_index": float64(-1), "delete_on_termination": true, "destination_type": "volume", + "volume_size": float64(50), "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,6 +811,7 @@ func TestService_ReconcileInstance(t *testing.T) { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { + Type: "volume", Name: "etcd", Size: 50, VolumeType: "test-volume-type", @@ -821,6 +852,7 @@ func TestService_ReconcileInstance(t *testing.T) { "boot_index": float64(-1), "delete_on_termination": true, "destination_type": "volume", + "volume_size": float64(50), "tag": "etcd", }, } @@ -831,6 +863,28 @@ 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{ + { + Type: "doesnt-exist", + Name: "oops", + Size: 1, + }, + } + 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..a54f78a883 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 @@ -7,4 +7,8 @@ path: /spec/template/spec/additionalBlockDevices value: - diskSize: 1 + type: volume name: extravol + - diskSize: 1 + type: local + name: etcd 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..b46bfc4848 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 @@ -9,6 +9,10 @@ path: /spec/template/spec/additionalBlockDevices value: - diskSize: 1 + type: volume name: extravol volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + - diskSize: 1 + type: local + name: etcd \ No newline at end of file From 522c65abbce40f47002f2856a7c282dc92884bda Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Fri, 20 Oct 2023 10:33:44 -0400 Subject: [PATCH 4/4] Proposal v2 (cherry picked from commit dce1886454e709158be074d34b340ac0dcca0ec8) --- api/v1alpha7/openstackmachine_types.go | 3 +- api/v1alpha7/types.go | 60 +++++++++------ api/v1alpha7/zz_generated.deepcopy.go | 40 +++++++++- ...re.cluster.x-k8s.io_openstackclusters.yaml | 69 ++++++++++------- ...er.x-k8s.io_openstackclustertemplates.yaml | 76 +++++++++++-------- ...re.cluster.x-k8s.io_openstackmachines.yaml | 66 +++++++++------- ...er.x-k8s.io_openstackmachinetemplates.yaml | 69 ++++++++++------- .../crd-changes/v1alpha6-to-v1alpha7.md | 16 ++-- pkg/cloud/services/compute/instance.go | 26 ++++--- pkg/cloud/services/compute/instance_test.go | 50 ++++++++---- .../patch-machine-template-control-plane.yaml | 14 ++-- .../patch-machine-template-worker.yaml | 19 +++-- 12 files changed, 325 insertions(+), 183 deletions(-) diff --git a/api/v1alpha7/openstackmachine_types.go b/api/v1alpha7/openstackmachine_types.go index 0ed7ca2bc0..b14e647882 100644 --- a/api/v1alpha7/openstackmachine_types.go +++ b/api/v1alpha7/openstackmachine_types.go @@ -84,7 +84,8 @@ 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 + // additionalBlockDevices is a list of specifications for additional block devices to attach to the server instance + // +optional // +listType=map // +listMapKey=name AdditionalBlockDevices []AdditionalBlockDevice `json:"additionalBlockDevices,omitempty"` diff --git a/api/v1alpha7/types.go b/api/v1alpha7/types.go index 695090d2fb..50b0a40eef 100644 --- a/api/v1alpha7/types.go +++ b/api/v1alpha7/types.go @@ -17,15 +17,14 @@ limitations under the License. package v1alpha7 // BlockDeviceType defines the type of block device to create. -// +kubebuilder:validation:Enum=volume;local type BlockDeviceType string const ( - // LocalBlockDevice is an ephemeral block deviced attached to the server. - LocalBlockDevice BlockDeviceType = "local" + // 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" + VolumeBlockDevice BlockDeviceType = "Volume" ) // OpenStackMachineTemplateResource describes the data needed to create a OpenStackMachine from a template. @@ -175,35 +174,52 @@ type RootVolume struct { AvailabilityZone string `json:"availabilityZone,omitempty"` } -// AdditionalBlockDevice is a block device to attach to the server. -type AdditionalBlockDevice struct { - // Type is the type of block device to create. - // This can be either "volume" or "local". +// blockDeviceStorage is the storage type of a block device to create and +// contains additional storage options. +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"` - // Name of the block device in the context of a machine. - // It will be combined with the machine name to make the actual cinder - // volume name, and will be used for tagging the block device. - Name string `json:"name"` - - // Size is the size in GB of the block device. - Size int `json:"diskSize"` + // volume contains additional storage options for a volume block device. + // +optional + Volume *BlockDeviceVolume `json:"volume,omitempty"` +} - // VolumeType is the volume type of the volume. - // If omitted, the default type will be used. - // +unionMember=volume,optional +// blockDeviceVolume contains additional storage options for a volume block device. +type BlockDeviceVolume struct { + // type is the volume type of the volume. + // If omitted, the default volume type will be used. // +optional - VolumeType string `json:"volumeType,omitempty"` + Type string `json:"type,omitempty"` - // AvailabilityZone is the volume availability zone to create the volume in. + // availabilityZone is the volume availability zone to create the volume in. // If omitted, the availability zone of the server will be used. - // +unionMember=volume,optional // +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. + // +kubebuilder:validation:Required + Name string `json:"name"` + + // size is the size in GiB of the block device. + // +kubebuilder:validation:Required + Size int `json:"size"` + + // storage specifies the storage type of the block device and + // additional storage options. + // +kubebuilder:validation:Required + Storage BlockDeviceStorage `json:"storage"` +} + // 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 f59badc30a..a0874363fa 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -3771,41 +3771,56 @@ spec: description: Instance for the bastion itself properties: additionalBlockDevices: - description: AdditionalBlockDevices is a list of specifications + 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 + 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 block device. - type: integer name: - description: Name of the block device in the context - of a machine. It will be combined with the machine - name to make the actual cinder volume name, and will - be used for tagging the block device. - type: string - type: - description: Type is the type of block device to create. - This can be either "volume" or "local". - enum: - - volume - - local - 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. type: string + size: + description: size is the size in GiB of the block device. + 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. + type: string + type: + description: type is the volume type of the + volume. If omitted, the default volume type + will be used. + type: string + type: object + required: + - type + type: object required: - - diskSize - name - - type + - size + - storage type: object type: array x-kubernetes-list-map-keys: 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 1452e7832d..6ae716aedc 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -1611,47 +1611,61 @@ spec: description: Instance for the bastion itself properties: additionalBlockDevices: - description: AdditionalBlockDevices is a list of specifications + description: additionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: - description: AdditionalBlockDevice is a block device + 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 block - device. - type: integer name: - description: Name of the block device in the - context of a machine. It will be combined - with the machine name to make the actual cinder - volume name, and will be used for tagging + 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. type: string - type: - description: Type is the type of block device - to create. This can be either "volume" or - "local". - enum: - - volume - - local - type: string - volumeType: - description: VolumeType is the volume type of - the volume. If omitted, the default type will - be used. - type: string + size: + description: size is the size in GiB of the + block device. + 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. + type: string + type: + description: type is the volume type + of the volume. If omitted, the default + volume type will be used. + type: string + type: object + required: + - type + type: object required: - - diskSize - name - - type + - size + - storage type: object type: array x-kubernetes-list-map-keys: 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 fa140da9b7..bcba7f3c90 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1160,41 +1160,53 @@ spec: description: OpenStackMachineSpec defines the desired state of OpenStackMachine. properties: additionalBlockDevices: - description: AdditionalBlockDevices is a list of specifications for + 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 + 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 block device. - type: integer name: - description: Name of the block device in the context of a machine. - It will be combined with the machine name to make the actual - cinder volume name, and will be used for tagging the block - device. - type: string - type: - description: Type is the type of block device to create. This - can be either "volume" or "local". - enum: - - volume - - local - 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. type: string + size: + description: size is the size in GiB of the block device. + 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. + type: string + type: + description: type is the volume type of the volume. + If omitted, the default volume type will be used. + type: string + type: object + required: + - type + type: object required: - - diskSize - name - - type + - size + - storage type: object type: array x-kubernetes-list-map-keys: 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 1dd79af913..53b81a7505 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -959,41 +959,56 @@ spec: of the machine. properties: additionalBlockDevices: - description: AdditionalBlockDevices is a list of specifications + 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 + 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 block device. - type: integer name: - description: Name of the block device in the context - of a machine. It will be combined with the machine - name to make the actual cinder volume name, and will - be used for tagging the block device. - type: string - type: - description: Type is the type of block device to create. - This can be either "volume" or "local". - enum: - - volume - - local - 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. type: string + size: + description: size is the size in GiB of the block device. + 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. + type: string + type: + description: type is the volume type of the + volume. If omitted, the default volume type + will be used. + type: string + type: object + required: + - type + type: object required: - - diskSize - name - - type + - size + - storage type: object type: array x-kubernetes-list-map-keys: 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 c1530fb7e2..1804d016f3 100644 --- a/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md +++ b/docs/book/src/topics/crd-changes/v1alpha6-to-v1alpha7.md @@ -224,18 +224,22 @@ to the server instance: ```yaml additionalBlockDevices: - name: database - type: volume - diskSize: 50 - volumeType: my-volume-type - availabilityZone: az0 + size: 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 - type: local - diskSize: 1 + size: 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 diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index f259ea5949..2747c753c0 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -439,8 +439,8 @@ func (s *Service) waitForVolume(volumeID string, timeout time.Duration, retryInt // 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 blockDevice.AvailabilityZone != "" { - availabilityZone = blockDevice.AvailabilityZone + if blockDevice.Storage.Volume.AvailabilityZone != "" { + availabilityZone = blockDevice.Storage.Volume.AvailabilityZone } createOpts := volumes.CreateOpts{ @@ -450,7 +450,7 @@ func (s *Service) getOrCreateVolumeBuilder(eventObject runtime.Object, instanceS ImageID: imageID, Multiattach: false, AvailabilityZone: availabilityZone, - VolumeType: blockDevice.VolumeType, + VolumeType: blockDevice.Storage.Volume.Type, } return s.getOrCreateVolume(eventObject, createOpts) @@ -463,11 +463,15 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst if hasRootVolume(instanceSpec) { rootVolumeToBlockDevice := infrav1.AdditionalBlockDevice{ - Type: infrav1.VolumeBlockDevice, - Name: "root", - AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, - Size: instanceSpec.RootVolume.Size, - VolumeType: instanceSpec.RootVolume.VolumeType, + Name: "root", + Size: 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 { @@ -494,7 +498,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst var bdUUID string var sourceType bootfromvolume.SourceType var destinationType bootfromvolume.DestinationType - if blockDeviceSpec.Type == infrav1.VolumeBlockDevice { + 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 @@ -502,11 +506,11 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst bdUUID = blockDevice.ID sourceType = bootfromvolume.SourceVolume destinationType = bootfromvolume.DestinationVolume - } else if blockDeviceSpec.Type == infrav1.LocalBlockDevice { + } else if blockDeviceSpec.Storage.Type == infrav1.LocalBlockDevice { sourceType = bootfromvolume.SourceBlank destinationType = bootfromvolume.DestinationLocal } else { - return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.Type) + return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.Storage.Type) } blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 6f2152d386..ec96ddb0dc 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -658,15 +658,21 @@ func TestService_ReconcileInstance(t *testing.T) { } s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Type: "volume", - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", + Name: "etcd", + Size: 50, + Storage: infrav1.BlockDeviceStorage{ + Type: "Volume", + Volume: &infrav1.BlockDeviceVolume{ + Type: "test-volume-type", + }, + }, }, { - Type: "local", Name: "local-device", Size: 10, + Storage: infrav1.BlockDeviceStorage{ + Type: "Local", + }, }, } return s @@ -741,15 +747,21 @@ func TestService_ReconcileInstance(t *testing.T) { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Type: "volume", - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", + Name: "etcd", + Size: 50, + Storage: infrav1.BlockDeviceStorage{ + Type: "Volume", + Volume: &infrav1.BlockDeviceVolume{ + Type: "test-volume-type", + }, + }, }, { - Type: "local", Name: "data", Size: 10, + Storage: infrav1.BlockDeviceStorage{ + Type: "Local", + }, }, } return s @@ -811,11 +823,15 @@ func TestService_ReconcileInstance(t *testing.T) { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Type: "volume", - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", - AvailabilityZone: "test-alternate-az", + Name: "etcd", + Size: 50, + Storage: infrav1.BlockDeviceStorage{ + Type: "Volume", + Volume: &infrav1.BlockDeviceVolume{ + AvailabilityZone: "test-alternate-az", + Type: "test-volume-type", + }, + }, }, } return s @@ -869,9 +885,11 @@ func TestService_ReconcileInstance(t *testing.T) { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - Type: "doesnt-exist", Name: "oops", Size: 1, + Storage: infrav1.BlockDeviceStorage{ + Type: "doesnt-exist", + }, }, } return s 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 a54f78a883..86fdc55d6b 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,9 +6,11 @@ - op: add path: /spec/template/spec/additionalBlockDevices value: - - diskSize: 1 - type: volume - name: extravol - - diskSize: 1 - type: local - name: etcd + - name: extravol + size: 1 + storage: + type: Volume + - name: etcd + size: 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 b46bfc4848..72f0c8dc1e 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,11 +8,14 @@ - op: add path: /spec/template/spec/additionalBlockDevices value: - - diskSize: 1 - type: volume - name: extravol - volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} - availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} - - diskSize: 1 - type: local - name: etcd \ No newline at end of file + - name: extravol + size: 1 + storage: + type: Volume + volume: + type: ${OPENSTACK_VOLUME_TYPE_ALT} + availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + - name: etcd + size: 1 + storage: + type: Local \ No newline at end of file