diff --git a/api/v1alpha7/types.go b/api/v1alpha7/types.go index 73ae0fcbd4..d738bdc7bc 100644 --- a/api/v1alpha7/types.go +++ b/api/v1alpha7/types.go @@ -16,6 +16,17 @@ limitations under the License. package v1alpha7 +// BlockDeviceType defines the type of block device to create. +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,33 +174,42 @@ type RootVolume struct { AvailabilityZone string `json:"availabilityZone,omitempty"` } +// AdditionalBlockDevice is a block device to attach to the server. type AdditionalBlockDevice struct { - // BlockDeviceType is the type of block device to create. + // Type is the type of block device to create. // This can be either "volume" or "local". // +kubebuilder:validation:Required // +kubebuilder:validation:Enum=volume;local - BlockDeviceType string `json:"blockDeviceType"` - - // Here I'm hesitant to add GuestFormat. Not to let users choose the format of the block device - // (ie ext4, etc) because I think this should be handled by the user when the instane is started, - // but rather to let the user create swap partitions. - // I see 3 options: - // 1. Add GuestFormat here and let the user create swap partitions, among other filesystems (ext4, etc) - // 2. Add a new field for swap partitions - // 3. Don't add fields, the block device will be blank and can't be used for swap partitions. + // +unionDiscriminator + // +required + 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"` + // 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"` + + // GuestFormat tells Nova how/if to format the device prior to attaching, should be only used with + // blank local images. Denotes a swap disk if the value is swap. + // We can't have more than one AdditionalBlockDevice with GuestFormat=swap. + // +unionMember=local,optional + // +optional + GuestFormat string `json:"guestFormat,omitempty"` } // NetworkStatus contains basic information about an existing neutron network. 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 132b109781..e049436ffc 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -3774,36 +3774,44 @@ spec: description: AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device to + attach to the server. properties: availabilityZone: description: AvailabilityZone is the volume availability zone to create the volume in. If omitted, the availability zone of the server will be used. type: string - blockDeviceType: - description: BlockDeviceType is the type of block device - to create. This can be either "volume" or "local". - enum: - - volume - - local - type: string diskSize: description: Size is the size in GB of the block device. type: integer + guestFormat: + description: GuestFormat tells Nova how/if to format + the device prior to attaching, should be only used + with blank local images. Denotes a swap disk if the + value is swap. + type: string 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. type: string required: - - blockDeviceType - 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 810f4c89b1..832fdd5572 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 @@ -1622,18 +1624,16 @@ spec: If omitted, the availability zone of the server will be used. type: string - blockDeviceType: - description: BlockDeviceType is the type of - block device to create. This can be either - "volume" or "local". - enum: - - volume - - local - type: string diskSize: description: Size is the size in GB of the block device. type: integer + guestFormat: + description: GuestFormat tells Nova how/if to + format the device prior to attaching, should + be only used with blank local images. Denotes + a swap disk if the value is swap. + type: string name: description: Name of the block device in the context of a machine. It will be combined @@ -1641,15 +1641,23 @@ spec: 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. type: string required: - - blockDeviceType - 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 25e38b9015..cc1105345e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1163,36 +1163,43 @@ spec: description: AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device to attach to + the server. properties: availabilityZone: description: AvailabilityZone is the volume availability zone to create the volume in. If omitted, the availability zone of the server will be used. type: string - blockDeviceType: - description: BlockDeviceType is the type of block device to - create. This can be either "volume" or "local". - enum: - - volume - - local - type: string diskSize: description: Size is the size in GB of the block device. type: integer + guestFormat: + description: GuestFormat tells Nova how/if to format the device + prior to attaching, should be only used with blank local images. + Denotes a swap disk if the value is swap. + type: string 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. type: string required: - - blockDeviceType - 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 6a3f2ce2ef..86e2a1e128 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -962,36 +962,44 @@ spec: description: AdditionalBlockDevices is a list of specifications for additional block devices to attach to the server instance items: + description: AdditionalBlockDevice is a block device to + attach to the server. properties: availabilityZone: description: AvailabilityZone is the volume availability zone to create the volume in. If omitted, the availability zone of the server will be used. type: string - blockDeviceType: - description: BlockDeviceType is the type of block device - to create. This can be either "volume" or "local". - enum: - - volume - - local - type: string diskSize: description: Size is the size in GB of the block device. type: integer + guestFormat: + description: GuestFormat tells Nova how/if to format + the device prior to attaching, should be only used + with blank local images. Denotes a swap disk if the + value is swap. + type: string 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. type: string required: - - blockDeviceType - diskSize - name + - type type: object type: array x-kubernetes-list-map-keys: diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index d65c8d9be3..1b6a740346 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -463,7 +463,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst if hasRootVolume(instanceSpec) { rootVolumeToBlockDevice := infrav1.AdditionalBlockDevice{ - BlockDeviceType: "volume", + Type: "volume", Name: "root", AvailabilityZone: instanceSpec.RootVolume.AvailabilityZone, Size: instanceSpec.RootVolume.Size, @@ -491,10 +491,11 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst } for _, blockDeviceSpec := range instanceSpec.AdditionalBlockDevices { + var swapFound bool var bdUUID string var sourceType bootfromvolume.SourceType var destinationType bootfromvolume.DestinationType - if blockDeviceSpec.BlockDeviceType == "volume" { + if blockDeviceSpec.Type == "volume" { 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 +503,17 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst bdUUID = blockDevice.ID sourceType = bootfromvolume.SourceVolume destinationType = bootfromvolume.DestinationVolume - } else if blockDeviceSpec.BlockDeviceType == "local" { + } else if blockDeviceSpec.Type == "local" { sourceType = bootfromvolume.SourceBlank destinationType = bootfromvolume.DestinationLocal + if blockDeviceSpec.GuestFormat == "swap" { + if swapFound { + return []bootfromvolume.BlockDevice{}, fmt.Errorf("only one swap block device is allowed for instance %s", instanceSpec.Name) + } + swapFound = true + } } else { - return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.BlockDeviceType) + return []bootfromvolume.BlockDevice{}, fmt.Errorf("invalid block device type %s", blockDeviceSpec.Type) } blockDevices = append(blockDevices, bootfromvolume.BlockDevice{ @@ -517,6 +524,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst DeleteOnTermination: true, VolumeSize: blockDeviceSpec.Size, Tag: blockDeviceSpec.Name, + GuestFormat: blockDeviceSpec.GuestFormat, }) } diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 74788bed3e..8213ebd439 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -658,15 +658,15 @@ func TestService_ReconcileInstance(t *testing.T) { } s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - BlockDeviceType: "volume", - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", + Type: "volume", + Name: "etcd", + Size: 50, + VolumeType: "test-volume-type", }, { - BlockDeviceType: "local", - Name: "local-device", - Size: 10, + Type: "local", + Name: "local-device", + Size: 10, }, } return s @@ -736,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{ { - BlockDeviceType: "volume", - Name: "etcd", - Size: 50, - VolumeType: "test-volume-type", + Type: "volume", + Name: "etcd", + Size: 50, + VolumeType: "test-volume-type", + }, + { + Type: "local", + Name: "data", + Size: 10, + GuestFormat: "ext4", }, } return s @@ -784,6 +790,76 @@ func TestService_ReconcileInstance(t *testing.T) { "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", + "guest_format": "ext4", + }, + } + expectCreateServer(r.compute, createMap, false) + expectServerPollSuccess(r.compute) + + // Don't delete ports because the server is created: DeleteInstance will do it + }, + wantErr: false, + }, + // This tests is currently passing but it shouldn't, we need to find out why. + { + name: "Additional block devices error when creating more than one swap device", + getInstanceSpec: func() *InstanceSpec { + s := getDefaultInstanceSpec() + s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ + { + Type: "local", + Name: "swap1", + Size: 1, + GuestFormat: "swap", + }, + { + Type: "local", + Name: "swap2", + Size: 1, + GuestFormat: "swap", + }, + } + return s + }, + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) + + 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": "blank", + "destination_type": "local", + "boot_index": float64(-1), + "delete_on_termination": true, + "volume_size": float64(1), + "tag": "swap1", + "guest_format": "swap", + }, + { + "source_type": "blank", + "destination_type": "local", + "boot_index": float64(-1), + "delete_on_termination": true, + "volume_size": float64(1), + "tag": "swap2", + "guest_format": "swap", + }, } expectCreateServer(r.compute, createMap, false) expectServerPollSuccess(r.compute) @@ -798,7 +874,7 @@ func TestService_ReconcileInstance(t *testing.T) { s := getDefaultInstanceSpec() s.AdditionalBlockDevices = []infrav1.AdditionalBlockDevice{ { - BlockDeviceType: "volume", + Type: "volume", Name: "etcd", Size: 50, VolumeType: "test-volume-type", 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 70c777c1f5..5a4f01d80c 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 @@ -9,6 +9,10 @@ - diskSize: 1 blockDeviceType: volume name: extravol + - diskSize: 1 + blockDeviceType: local + name: swap + guestFormat: swap - diskSize: 1 blockDeviceType: 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 6ac8e78e10..2a33be6662 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 @@ -13,6 +13,11 @@ name: extravol volumeType: ${OPENSTACK_VOLUME_TYPE_ALT} availabilityZone: ${OPENSTACK_FAILURE_DOMAIN} + - diskSize: 1 + blockDeviceType: local + name: swap + guestFormat: swap - diskSize: 1 blockDeviceType: local name: etcd + guestFormat: ext4