Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add ebs volume throughput field to v1alpha4 Volume #2468

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/v1alpha3/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ func (r *AWSClusterControllerIdentityList) ConvertFrom(srcRaw conversion.Hub) er
return Convert_v1alpha4_AWSClusterControllerIdentityList_To_v1alpha3_AWSClusterControllerIdentityList(src, r, nil)
}

// Convert_v1alpha4_Volume_To_v1alpha3_Volume .
func Convert_v1alpha4_Volume_To_v1alpha3_Volume(in *v1alpha4.Volume, out *Volume, s apiconversion.Scope) error {
return autoConvert_v1alpha4_Volume_To_v1alpha3_Volume(in, out, s)
}

// Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint .
func Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint(in *apiv1alpha3.APIEndpoint, out *apiv1alpha4.APIEndpoint, s apiconversion.Scope) error {
return apiv1alpha3.Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint(in, out, s)
Expand Down Expand Up @@ -320,6 +325,8 @@ func restoreInstance(restored, dst *v1alpha4.Instance) {
return
}
dst.VolumeIDs = restored.VolumeIDs
RestoreRootVolume(restored.RootVolume, dst.RootVolume)
restoreNonRootVolumes(restored.NonRootVolumes, dst.NonRootVolumes)
}

// Convert_v1alpha3_AWSResourceReference_To_v1alpha4_AMIReference is a conversion function.
Expand Down Expand Up @@ -360,6 +367,7 @@ func restoreNonRootVolumes(restoredVolumes, dstVolumes []v1alpha4.Volume) {
dstVolumes[i].Encrypted = nil
}
}
dstVolumes[i].Throughput = restoredVolumes[i].Throughput
}
}

Expand All @@ -377,5 +385,6 @@ func RestoreRootVolume(restored, dst *v1alpha4.Volume) {
if restored.Encrypted == nil {
dst.Encrypted = nil
}
dst.Throughput = restored.Throughput
return
}
20 changes: 8 additions & 12 deletions api/v1alpha3/zz_generated.conversion.go

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

34 changes: 24 additions & 10 deletions api/v1alpha4/awsmachine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,21 @@ func (r *AWSMachine) validateRootVolume() field.ErrorList {
return allErrs
}

if (r.Spec.RootVolume.Type == "io1" || r.Spec.RootVolume.Type == "io2") && r.Spec.RootVolume.IOPS == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.rootVolumeOptions.iops"), "iops required if type is 'io1' or 'io2'"))
if VolumeTypesProvisioned.Has(string(r.Spec.RootVolume.Type)) && r.Spec.RootVolume.IOPS == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.rootVolume.iops"), "iops required if type is 'io1' or 'io2'"))
}

if r.Spec.RootVolume.Throughput != nil {
if r.Spec.RootVolume.Type != VolumeTypeGP3 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.rootVolume.throughput"), "throughput is valid only for type 'gp3'"))
}
if *r.Spec.RootVolume.Throughput < 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.rootVolume.throughput"), "throughput must be nonnegative"))
}
}

if r.Spec.RootVolume.DeviceName != "" {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.rootVolumeOptions.deviceName"), "root volume shouldn't have device name"))
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.rootVolume.deviceName"), "root volume shouldn't have device name"))
}

return allErrs
Expand All @@ -159,17 +168,22 @@ func (r *AWSMachine) validateRootVolume() field.ErrorList {
func (r *AWSMachine) validateNonRootVolumes() field.ErrorList {
var allErrs field.ErrorList

if r.Spec.NonRootVolumes == nil {
return allErrs
}

for _, volume := range r.Spec.NonRootVolumes {
if (volume.Type == "io1" || volume.Type == "io2") && volume.IOPS == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.volumeOptions.iops"), "iops required if type is 'io1' or 'io2'"))
if VolumeTypesProvisioned.Has(string(r.Spec.RootVolume.Type)) && volume.IOPS == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.iops"), "iops required if type is 'io1' or 'io2'"))
}

if volume.Throughput != nil {
if volume.Type != VolumeTypeGP3 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.throughput"), "throughput is valid only for type 'gp3'"))
}
if *volume.Throughput < 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.throughput"), "throughput must be nonnegative"))
}
}

if volume.DeviceName == "" {
allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.volumeOptions.deviceName"), "non root volume should have device name"))
allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.deviceName"), "non root volume should have device name"))
}
}

Expand Down
24 changes: 24 additions & 0 deletions api/v1alpha4/awsmachine_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,17 @@ func TestAWSMachine_Create(t *testing.T) {
},
wantErr: true,
},
{
name: "ensure root volume throughput is nonnegative",
machine: &AWSMachine{
Spec: AWSMachineSpec{
RootVolume: &Volume{
Throughput: aws.Int64(-125),
},
},
},
wantErr: true,
},
{
name: "ensure root volume has no device name",
machine: &AWSMachine{
Expand Down Expand Up @@ -114,6 +125,19 @@ func TestAWSMachine_Create(t *testing.T) {
},
wantErr: true,
},
{
name: "ensure non root volume throughput is nonnegative",
machine: &AWSMachine{
Spec: AWSMachineSpec{
NonRootVolumes: []Volume{
{
Throughput: aws.Int64(-125),
},
},
},
},
wantErr: true,
},
{
name: "additional security groups may have id",
machine: &AWSMachine{
Expand Down
58 changes: 58 additions & 0 deletions api/v1alpha4/awsmachinetemplate_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,61 @@ var (
_ webhook.Validator = &AWSMachineTemplate{}
)

func (r *AWSMachineTemplate) validateRootVolume() field.ErrorList {
var allErrs field.ErrorList

spec := r.Spec.Template.Spec
if spec.RootVolume == nil {
return allErrs
}

if VolumeTypesProvisioned.Has(string(spec.RootVolume.Type)) && spec.RootVolume.IOPS == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.rootVolume.iops"), "iops required if type is 'io1' or 'io2'"))
}

if spec.RootVolume.Throughput != nil {
if spec.RootVolume.Type != VolumeTypeGP3 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.rootVolume.throughput"), "throughput is valid only for type 'gp3'"))
}
if *spec.RootVolume.Throughput < 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.rootVolume.throughput"), "throughput must be nonnegative"))
}
}

if spec.RootVolume.DeviceName != "" {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.template.spec.rootVolume.deviceName"), "root volume shouldn't have device name"))
}

return allErrs
}

func (r *AWSMachineTemplate) validateNonRootVolumes() field.ErrorList {
var allErrs field.ErrorList

spec := r.Spec.Template.Spec

for _, volume := range spec.NonRootVolumes {
if VolumeTypesProvisioned.Has(string(volume.Type)) && volume.IOPS == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.nonRootVolumes.iops"), "iops required if type is 'io1' or 'io2'"))
}

if volume.Throughput != nil {
if volume.Type != VolumeTypeGP3 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.nonRootVolumes.throughput"), "throughput is valid only for type 'gp3'"))
}
if *volume.Throughput < 0 {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.nonRootVolumes.throughput"), "throughput must be nonnegative"))
}
}

if volume.DeviceName == "" {
allErrs = append(allErrs, field.Required(field.NewPath("spec.template.spec.nonRootVolumes.deviceName"), "non root volume should have device name"))
}
}

return allErrs
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
func (r *AWSMachineTemplate) ValidateCreate() error {
var allErrs field.ErrorList
Expand All @@ -55,6 +110,9 @@ func (r *AWSMachineTemplate) ValidateCreate() error {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "providerID"), "cannot be set in templates"))
}

allErrs = append(allErrs, r.validateRootVolume()...)
allErrs = append(allErrs, r.validateNonRootVolumes()...)

return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
}

Expand Down
36 changes: 35 additions & 1 deletion api/v1alpha4/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -712,12 +712,16 @@ type Volume struct {

// Type is the type of the volume (e.g. gp2, io1, etc...).
// +optional
Type string `json:"type,omitempty"`
Type VolumeType `json:"type,omitempty"`

// IOPS is the number of IOPS requested for the disk. Not applicable to all types.
// +optional
IOPS int64 `json:"iops,omitempty"`

// Throughput to provision in MiB/s supported for the volume type. Not applicable to all types.
richardcase marked this conversation as resolved.
Show resolved Hide resolved
// +optional
Throughput *int64 `json:"throughput,omitempty"`

// Encrypted is whether the volume should be encrypted or not.
// +optional
Encrypted *bool `json:"encrypted,omitempty"`
Expand All @@ -729,6 +733,36 @@ type Volume struct {
EncryptionKey string `json:"encryptionKey,omitempty"`
}

// VolumeType describes the EBS volume type.
// See: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html
type VolumeType string

var (
// VolumeTypeIO1 is the string representing a provisioned iops ssd io1 volume
VolumeTypeIO1 = VolumeType("io1")

// VolumeTypeIO2 is the string representing a provisioned iops ssd io2 volume
VolumeTypeIO2 = VolumeType("io2")

// VolumeTypeGP2 is the string representing a general purpose ssd gp2 volume
VolumeTypeGP2 = VolumeType("gp2")

// VolumeTypeGP3 is the string representing a general purpose ssd gp3 volume
VolumeTypeGP3 = VolumeType("gp3")

// VolumeTypesGP are volume types provisioned for general purpose io
VolumeTypesGP = sets.NewString(
string(VolumeTypeIO1),
string(VolumeTypeIO2),
)

// VolumeTypesProvisioned are volume types provisioned for high performance io
VolumeTypesProvisioned = sets.NewString(
string(VolumeTypeIO1),
string(VolumeTypeIO2),
)
)

// SpotMarketOptions defines the options available to a user when configuring
// Machines to run on Spot instances.
// Most users should provide an empty struct.
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha4/zz_generated.deepcopy.go

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

10 changes: 10 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1166,6 +1166,11 @@ spec:
format: int64
minimum: 8
type: integer
throughput:
description: Throughput to provision in MiB/s supported
for the volume type. Not applicable to all types.
format: int64
type: integer
type:
description: Type is the type of the volume (e.g. gp2, io1,
etc...).
Expand Down Expand Up @@ -1210,6 +1215,11 @@ spec:
format: int64
minimum: 8
type: integer
throughput:
description: Throughput to provision in MiB/s supported for
the volume type. Not applicable to all types.
format: int64
type: integer
type:
description: Type is the type of the volume (e.g. gp2, io1,
etc...).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,11 @@ spec:
format: int64
minimum: 8
type: integer
throughput:
description: Throughput to provision in MiB/s supported for
the volume type. Not applicable to all types.
format: int64
type: integer
type:
description: Type is the type of the volume (e.g. gp2, io1,
etc...).
Expand Down
10 changes: 10 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,11 @@ spec:
format: int64
minimum: 8
type: integer
throughput:
description: Throughput to provision in MiB/s supported for
the volume type. Not applicable to all types.
format: int64
type: integer
type:
description: Type is the type of the volume (e.g. gp2, io1,
etc...).
Expand Down Expand Up @@ -741,6 +746,11 @@ spec:
format: int64
minimum: 8
type: integer
throughput:
description: Throughput to provision in MiB/s supported for the
volume type. Not applicable to all types.
format: int64
type: integer
type:
description: Type is the type of the volume (e.g. gp2, io1, etc...).
type: string
Expand Down
Loading