Repository: keikoproj/instance-manager
Pull request #316 (2021-06-09 18:35:20)

fix: add throughput parameter for volumes, allow iops to be set for a…
…ny provisioned iops-supported volume

Signed-off-by: Jonah Back <jonah@jonahback.com>


Patch:
diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
index 2dcfb1e..0b7be0d 100644
--- a/api/v1alpha1/instancegroup_types.go
+++ b/api/v1alpha1/instancegroup_types.go
@@ -291,6 +291,7 @@ type NodeVolume struct {
 	Type                string                  `json:"type"`
 	Size                int64                   `json:"size"`
 	Iops                int64                   `json:"iops,omitempty"`
+	Throughput          int64                   `json:"throughput,omitempty"`
 	DeleteOnTermination *bool                   `json:"deleteOnTermination,omitempty"`
 	Encrypted           *bool                   `json:"encrypted,omitempty"`
 	SnapshotID          string                  `json:"snapshotId,omitempty"`
@@ -430,6 +431,14 @@ func (s *EKSSpec) Validate() error {
 				return errors.Errorf("validation failed, volume type '%v' is unsupported", v.Type)
 			}
 		}
+
+		if v.Iops != 0 && !common.ContainsEqualFold(awsprovider.AllowedVolumeTypesWithProvisionedIOPS, v.Type) {
+			return errors.Errorf("validation failed, volume type '%v' does not support provisioned iops", v.Type)
+		}
+
+		if v.Throughput != 0 && !common.ContainsEqualFold(awsprovider.AllowedVolumeTypesWithProvisionedThroughput, v.Type) {
+			return errors.Errorf("validation failed, volume type '%v' does not support provisioned throughput", v.Type)
+		}
 	}
 
 	if s.HasWarmPool() {
diff --git a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
index 3021d8e..be7db4a 100644
--- a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
+++ b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
@@ -261,6 +261,9 @@ spec:
                               type: integer
                             snapshotId:
                               type: string
+                            throughput:
+                              format: int64
+                              type: integer
                             type:
                               type: string
                           required:
diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go
index 6701362..b558cb4 100644
--- a/controllers/providers/aws/aws.go
+++ b/controllers/providers/aws/aws.go
@@ -103,11 +103,12 @@ var (
 		"GroupTotalCapacity",
 	}
 
-	ConfigurationAllowedVolumeTypes       = []string{"gp2", "io1", "sc1", "st1"}
-	TemplateAllowedVolumeTypes            = []string{"gp2", "gp3", "io1", "io2", "sc1", "st1"}
-	AllowedVolumeTypesWithProvisionedIOPS = []string{"io1", "io2", "gp3"}
-	LifecycleHookTransitionLaunch         = "autoscaling:EC2_INSTANCE_LAUNCHING"
-	LifecycleHookTransitionTerminate      = "autoscaling:EC2_INSTANCE_TERMINATING"
+	ConfigurationAllowedVolumeTypes             = []string{"gp2", "io1", "sc1", "st1"}
+	TemplateAllowedVolumeTypes                  = []string{"gp2", "gp3", "io1", "io2", "sc1", "st1"}
+	AllowedVolumeTypesWithProvisionedIOPS       = []string{"io1", "io2", "gp3"}
+	AllowedVolumeTypesWithProvisionedThroughput = []string{"gp3"}
+	LifecycleHookTransitionLaunch               = "autoscaling:EC2_INSTANCE_LAUNCHING"
+	LifecycleHookTransitionTerminate            = "autoscaling:EC2_INSTANCE_TERMINATING"
 )
 
 type AwsWorker struct {
diff --git a/controllers/providers/aws/constructors.go b/controllers/providers/aws/constructors.go
index ee1e543..ccc5678 100644
--- a/controllers/providers/aws/constructors.go
+++ b/controllers/providers/aws/constructors.go
@@ -81,7 +81,7 @@ func (w *AwsWorker) GetLaunchTemplateBlockDeviceRequest(name, volType, snapshot
 	return device
 }
 
-func (w *AwsWorker) GetLaunchTemplateBlockDevice(name, volType, snapshot string, volSize, iops int64, delete, encrypt *bool) *ec2.LaunchTemplateBlockDeviceMapping {
+func (w *AwsWorker) GetLaunchTemplateBlockDevice(name, volType, snapshot string, volSize, iops int64, throughput int64, delete, encrypt *bool) *ec2.LaunchTemplateBlockDeviceMapping {
 	device := &ec2.LaunchTemplateBlockDeviceMapping{
 		DeviceName: aws.String(name),
 		Ebs: &ec2.LaunchTemplateEbsBlockDevice{
@@ -96,9 +96,12 @@ func (w *AwsWorker) GetLaunchTemplateBlockDevice(name, volType, snapshot string,
 	if encrypt != nil {
 		device.Ebs.Encrypted = encrypt
 	}
-	if iops != 0 && strings.EqualFold(volType, "io1") {
+	if iops != 0 && common.ContainsEqualFold(AllowedVolumeTypesWithProvisionedIOPS, volType) {
 		device.Ebs.Iops = aws.Int64(iops)
 	}
+	if throughput != 0 && common.ContainsEqualFold(AllowedVolumeTypesWithProvisionedThroughput, volType) {
+		device.Ebs.Throughput = aws.Int64(throughput)
+	}
 	if volSize != 0 {
 		device.Ebs.VolumeSize = aws.Int64(volSize)
 	}
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index 7832dbf..a89c50a 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -279,15 +279,15 @@ func (ctx *EksInstanceGroupContext) GetMountOpts() []MountOpts {
 
 func (ctx *EksInstanceGroupContext) GetAddedTags(asgName string) []*autoscaling.Tag {
 	var (
-		tags          []*autoscaling.Tag
-		instanceGroup = ctx.GetInstanceGroup()
-		configuration = instanceGroup.GetEKSConfiguration()
-		clusterName   = configuration.GetClusterName()
-		annotations   = instanceGroup.GetAnnotations()
-		labels        = ctx.GetComputedLabels()
-		taints        = configuration.GetTaints()
-		osFamily      = ctx.GetOsFamily()
-		state         = ctx.GetDiscoveredState()
+		tags             []*autoscaling.Tag
+		instanceGroup    = ctx.GetInstanceGroup()
+		configuration    = instanceGroup.GetEKSConfiguration()
+		clusterName      = configuration.GetClusterName()
+		annotations      = instanceGroup.GetAnnotations()
+		labels           = ctx.GetComputedLabels()
+		taints           = configuration.GetTaints()
+		osFamily         = ctx.GetOsFamily()
+		state            = ctx.GetDiscoveredState()
 		instanceTypeInfo = state.GetInstanceTypeInfo()
 	)
 
diff --git a/controllers/provisioners/eks/scaling/launchtemplate.go b/controllers/provisioners/eks/scaling/launchtemplate.go
index 6a1d269..fd5c40d 100644
--- a/controllers/provisioners/eks/scaling/launchtemplate.go
+++ b/controllers/provisioners/eks/scaling/launchtemplate.go
@@ -343,7 +343,7 @@ func (lt *LaunchTemplate) blockDeviceListRequest(volumes []v1alpha1.NodeVolume)
 func (lt *LaunchTemplate) blockDeviceList(volumes []v1alpha1.NodeVolume) []*ec2.LaunchTemplateBlockDeviceMapping {
 	var devices []*ec2.LaunchTemplateBlockDeviceMapping
 	for _, v := range volumes {
-		devices = append(devices, lt.GetLaunchTemplateBlockDevice(v.Name, v.Type, v.SnapshotID, v.Size, v.Iops, v.DeleteOnTermination, v.Encrypted))
+		devices = append(devices, lt.GetLaunchTemplateBlockDevice(v.Name, v.Type, v.SnapshotID, v.Size, v.Iops, v.Throughput, v.DeleteOnTermination, v.Encrypted))
 	}
 
 	return sortTemplateDevices(devices)


Test patch:
diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go
index 3160dce..4780b13 100644
--- a/api/v1alpha1/instancegroup_types_test.go
+++ b/api/v1alpha1/instancegroup_types_test.go
@@ -222,8 +222,9 @@ func TestInstanceGroupSpecValidate(t *testing.T) {
 						},
 						Volumes: []NodeVolume{
 							{
-								Type: "gp3",
-								Iops: 230,
+								Type:       "gp3",
+								Iops:       230,
+								Throughput: 1000,
 							},
 						},
 					},
@@ -231,6 +232,66 @@ func TestInstanceGroupSpecValidate(t *testing.T) {
 			},
 			want: "",
 		},
+		{
+			name: "eks with gp2 volume with provisioned throughput fails",
+			args: args{
+				instancegroup: MockInstanceGroup("eks", "rollingUpdate", &EKSSpec{
+					MaxSize: 1,
+					MinSize: 1,
+					Type:    "LaunchTemplate",
+					EKSConfiguration: &EKSConfiguration{
+						EksClusterName:     "my-eks-cluster",
+						NodeSecurityGroups: []string{"sg-123456789"},
+						Image:              "ami-12345",
+						InstanceType:       "m5.large",
+						KeyPairName:        "thisShouldBeOptional",
+						Subnets:            []string{"subnet-1111111", "subnet-222222"},
+						Placement: &PlacementSpec{
+							AvailabilityZone:     "us-west-2a",
+							HostResourceGroupArn: "arn:aws:resource-groups:us-west-2:1122334455:group/resourceName",
+							Tenancy:              "host",
+						},
+						Volumes: []NodeVolume{
+							{
+								Type:       "gp2",
+								Throughput: 1000,
+							},
+						},
+					},
+				}, nil, nil),
+			},
+			want: "validation failed, volume type 'gp2' does not support provisioned throughput",
+		},
+		{
+			name: "eks with gp2 volume with provisioned iops fails",
+			args: args{
+				instancegroup: MockInstanceGroup("eks", "rollingUpdate", &EKSSpec{
+					MaxSize: 1,
+					MinSize: 1,
+					Type:    "LaunchTemplate",
+					EKSConfiguration: &EKSConfiguration{
+						EksClusterName:     "my-eks-cluster",
+						NodeSecurityGroups: []string{"sg-123456789"},
+						Image:              "ami-12345",
+						InstanceType:       "m5.large",
+						KeyPairName:        "thisShouldBeOptional",
+						Subnets:            []string{"subnet-1111111", "subnet-222222"},
+						Placement: &PlacementSpec{
+							AvailabilityZone:     "us-west-2a",
+							HostResourceGroupArn: "arn:aws:resource-groups:us-west-2:1122334455:group/resourceName",
+							Tenancy:              "host",
+						},
+						Volumes: []NodeVolume{
+							{
+								Type:       "gp2",
+								Iops: 1000,
+							},
+						},
+					},
+				}, nil, nil),
+			},
+			want: "validation failed, volume type 'gp2' does not support provisioned iops",
+		},
 		{
 			name: "eks with metadataoptions validates",
 			args: args{