Skip to content

Commit

Permalink
Address comments from Matt on union & API
Browse files Browse the repository at this point in the history
  • Loading branch information
@EmilienM
EmilienM committed Oct 10, 2023
1 parent 5124402 commit 13db8c9
Show file tree
Hide file tree
Showing 9 changed files with 204 additions and 60 deletions.
40 changes: 30 additions & 10 deletions api/v1alpha7/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
24 changes: 16 additions & 8 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
26 changes: 17 additions & 9 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1615,41 +1615,49 @@ spec:
for additional block devices to attach to the server
instance
items:
description: AdditionalBlockDevice is a block device
to attach to the server.
properties:
availabilityZone:
description: AvailabilityZone is the volume
availability zone to create the volume in.
If omitted, the availability zone of the server
will be used.
type: string
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:
Expand Down
23 changes: 15 additions & 8 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
24 changes: 16 additions & 8 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
16 changes: 12 additions & 4 deletions pkg/cloud/services/compute/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -491,22 +491,29 @@ 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
}
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{
Expand All @@ -517,6 +524,7 @@ func (s *Service) getBlockDevices(eventObject runtime.Object, instanceSpec *Inst
DeleteOnTermination: true,
VolumeSize: blockDeviceSpec.Size,
Tag: blockDeviceSpec.Name,
GuestFormat: blockDeviceSpec.GuestFormat,
})
}

Expand Down
Loading

0 comments on commit 13db8c9

Please sign in to comment.