Repository: keikoproj/instance-manager
Pull request #380 (2023-04-01 00:20:30)

Fix: Not honoring gp3 volume iops and throughput values
Fixes https://github.com/keikoproj/instance-manager/issues/379




github-code-scanning[bot]: You have successfully added a new CodeQL configuration `.github/workflows/unit-test.yml:unit-test`. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository [Security tab](/keikoproj/instance-manager/security/code-scanning?query=pr%3A380+tool%3ACodeQL+is%3Aopen).
codecov[bot]: ## [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) Report
> Merging [#380](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) (91e0574) into [master](https://codecov.io/gh/keikoproj/instance-manager/commit/5eb4421047001f80177cb59b2c278a95eddeddeb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) (5eb4421) will **decrease** coverage by `0.05%`.
> The diff coverage is `25.00%`.

> :exclamation: Current head 91e0574 differs from pull request most recent head 4dde850. Consider uploading reports for the commit 4dde850 to get more accurate results

```diff
@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
- Coverage   51.41%   51.37%   -0.05%     
==========================================
  Files          33       33              
  Lines        4553     4557       +4     
==========================================
  Hits         2341     2341              
- Misses       2065     2069       +4     
  Partials      147      147              
```


| [Impacted Files](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) | Coverage Δ | |
|---|---|---|
| [controllers/providers/aws/constructors.go](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlkZXJzL2F3cy9jb25zdHJ1Y3RvcnMuZ28=) | `0.00% <0.00%> (ø)` | |
| [controllers/provisioners/eks/eks.go](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9la3MuZ28=) | `89.58% <ø> (ø)` | |
| [...ntrollers/provisioners/eks/scaling/launchconfig.go](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9zY2FsaW5nL2xhdW5jaGNvbmZpZy5nbw==) | `92.61% <100.00%> (ø)` | |
| [...rollers/provisioners/eks/scaling/launchtemplate.go](https://codecov.io/gh/keikoproj/instance-manager/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9zY2FsaW5nL2xhdW5jaHRlbXBsYXRlLmdv) | `87.93% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj)

vgunapati: Looks like [functional-test](https://github.com/keikoproj/instance-manager/actions/runs/2895189866) is broken for a while, Do we have a doc on how to set this up if the cluster is deleted 
eytan-avisror: No doc, but it pretty much needs a vanilla EKS cluster + upgrade manager
vgunapati: I don't have access to the settings page to update Access keys, I will follow up with the team 

Patch:
diff --git a/controllers/providers/aws/constructors.go b/controllers/providers/aws/constructors.go
index ccc5678..4a59d3d 100644
--- a/controllers/providers/aws/constructors.go
+++ b/controllers/providers/aws/constructors.go
@@ -16,8 +16,6 @@ limitations under the License.
 package aws
 
 import (
-	"strings"
-
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -25,7 +23,7 @@ import (
 	"github.com/keikoproj/instance-manager/controllers/common"
 )
 
-func (w *AwsWorker) GetAutoScalingBasicBlockDevice(name, volType, snapshot string, volSize, iops int64, delete, encrypt *bool) *autoscaling.BlockDeviceMapping {
+func (w *AwsWorker) GetAutoScalingBasicBlockDevice(name, volType, snapshot string, volSize, iops int64, throughput int64, delete, encrypt *bool) *autoscaling.BlockDeviceMapping {
 	device := &autoscaling.BlockDeviceMapping{
 		DeviceName: aws.String(name),
 		Ebs: &autoscaling.Ebs{
@@ -40,20 +38,22 @@ func (w *AwsWorker) GetAutoScalingBasicBlockDevice(name, volType, snapshot strin
 	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)
 	}
 	if !common.StringEmpty(snapshot) {
 		device.Ebs.SnapshotId = aws.String(snapshot)
 	}
-
 	return device
 }
 
-func (w *AwsWorker) GetLaunchTemplateBlockDeviceRequest(name, volType, snapshot string, volSize, iops int64, delete, encrypt *bool) *ec2.LaunchTemplateBlockDeviceMappingRequest {
+func (w *AwsWorker) GetLaunchTemplateBlockDeviceRequest(name, volType, snapshot string, volSize, iops int64, throughput int64, delete, encrypt *bool) *ec2.LaunchTemplateBlockDeviceMappingRequest {
 	device := &ec2.LaunchTemplateBlockDeviceMappingRequest{
 		DeviceName: aws.String(name),
 		Ebs: &ec2.LaunchTemplateEbsBlockDeviceRequest{
@@ -68,9 +68,12 @@ func (w *AwsWorker) GetLaunchTemplateBlockDeviceRequest(name, volType, snapshot
 	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/eks.go b/controllers/provisioners/eks/eks.go
index 7e20cbf..0bfe752 100644
--- a/controllers/provisioners/eks/eks.go
+++ b/controllers/provisioners/eks/eks.go
@@ -54,10 +54,10 @@ var (
 	InstanceMgrLifecycleLabel = "instancemgr.keikoproj.io/lifecycle"
 	InstanceMgrImageLabel     = "instancemgr.keikoproj.io/image"
 
-	AllowedOsFamilies         = []string{OsFamilyWindows, OsFamilyBottleRocket, OsFamilyAmazonLinux2}
-	DefaultManagedPolicies    = []string{"AmazonEKSWorkerNodePolicy", "AmazonEC2ContainerRegistryReadOnly"}
-	CNIManagedPolicy          = "AmazonEKS_CNI_Policy"
-	SupportedArchitectures    = []string{"x86_64", "arm64"}
+	AllowedOsFamilies      = []string{OsFamilyWindows, OsFamilyBottleRocket, OsFamilyAmazonLinux2}
+	DefaultManagedPolicies = []string{"AmazonEKSWorkerNodePolicy", "AmazonEC2ContainerRegistryReadOnly"}
+	CNIManagedPolicy       = "AmazonEKS_CNI_Policy"
+	SupportedArchitectures = []string{"x86_64", "arm64"}
 )
 
 // New constructs a new instance group provisioner of EKS type
diff --git a/controllers/provisioners/eks/scaling/launchconfig.go b/controllers/provisioners/eks/scaling/launchconfig.go
index 186348f..7b4df50 100644
--- a/controllers/provisioners/eks/scaling/launchconfig.go
+++ b/controllers/provisioners/eks/scaling/launchconfig.go
@@ -288,7 +288,7 @@ func (lc *LaunchConfiguration) placementTenancy(placement *v1alpha1.PlacementSpe
 func (lc *LaunchConfiguration) blockDeviceList(volumes []v1alpha1.NodeVolume) []*autoscaling.BlockDeviceMapping {
 	var devices []*autoscaling.BlockDeviceMapping
 	for _, v := range volumes {
-		devices = append(devices, lc.GetAutoScalingBasicBlockDevice(v.Name, v.Type, v.SnapshotID, v.Size, v.Iops, v.DeleteOnTermination, v.Encrypted))
+		devices = append(devices, lc.GetAutoScalingBasicBlockDevice(v.Name, v.Type, v.SnapshotID, v.Size, v.Iops, v.Throughput, v.DeleteOnTermination, v.Encrypted))
 	}
 	return sortConfigDevices(devices)
 }
diff --git a/controllers/provisioners/eks/scaling/launchtemplate.go b/controllers/provisioners/eks/scaling/launchtemplate.go
index fd5c40d..ac52a66 100644
--- a/controllers/provisioners/eks/scaling/launchtemplate.go
+++ b/controllers/provisioners/eks/scaling/launchtemplate.go
@@ -334,7 +334,7 @@ func (lt *LaunchTemplate) RotationNeeded(input *DiscoverConfigurationInput) bool
 func (lt *LaunchTemplate) blockDeviceListRequest(volumes []v1alpha1.NodeVolume) []*ec2.LaunchTemplateBlockDeviceMappingRequest {
 	var devices []*ec2.LaunchTemplateBlockDeviceMappingRequest
 	for _, v := range volumes {
-		devices = append(devices, lt.GetLaunchTemplateBlockDeviceRequest(v.Name, v.Type, v.SnapshotID, v.Size, v.Iops, v.DeleteOnTermination, v.Encrypted))
+		devices = append(devices, lt.GetLaunchTemplateBlockDeviceRequest(v.Name, v.Type, v.SnapshotID, v.Size, v.Iops, v.Throughput, v.DeleteOnTermination, v.Encrypted))
 	}
 
 	return devices
diff --git a/go.mod b/go.mod
index 5684a94..0694fe8 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,7 @@ go 1.17
 
 require (
 	github.com/Masterminds/semver v1.5.0
-	github.com/aws/aws-sdk-go v1.38.24
+	github.com/aws/aws-sdk-go v1.38.71
 	github.com/cucumber/godog v0.8.1
 	github.com/evanphx/json-patch v4.11.0+incompatible
 	github.com/ghodss/yaml v1.0.0
diff --git a/go.sum b/go.sum
index b20ade0..f585f0a 100644
--- a/go.sum
+++ b/go.sum
@@ -51,8 +51,8 @@ github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmV
 github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
 github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
 github.com/aws/aws-sdk-go v1.35.7/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
-github.com/aws/aws-sdk-go v1.38.24 h1:zbKHDxFepE77ihVMZ+wZ62Ci646zkorN8rB5s4fj4kU=
-github.com/aws/aws-sdk-go v1.38.24/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
+github.com/aws/aws-sdk-go v1.38.71 h1:aWhtgoOiDhBCfaAj9XbxzcyvjEAKovbtv7d5mCVBZXw=
+github.com/aws/aws-sdk-go v1.38.71/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
 github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
 github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=


Test patch:
diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go
index 7581aec..6b6e1be 100644
--- a/api/v1alpha1/instancegroup_types_test.go
+++ b/api/v1alpha1/instancegroup_types_test.go
@@ -1,10 +1,9 @@
 /*
-
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
-    http://www.apache.org/licenses/LICENSE-2.0
+	http://www.apache.org/licenses/LICENSE-2.0
 
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
diff --git a/controllers/provisioners/eks/helpers_test.go b/controllers/provisioners/eks/helpers_test.go
index 1e4b394..90af0b2 100644
--- a/controllers/provisioners/eks/helpers_test.go
+++ b/controllers/provisioners/eks/helpers_test.go
@@ -189,7 +189,7 @@ func TestGetBasicUserDataWindows(t *testing.T) {
 	ctx := MockContext(ig, k, w)
 
 	configuration.BootstrapOptions = &v1alpha1.BootstrapOptions{
-		MaxPods: 4,
+		MaxPods:          4,
 		ContainerRuntime: "containerd",
 	}
 	configuration.Labels = map[string]string{
diff --git a/controllers/provisioners/eks/update_test.go b/controllers/provisioners/eks/update_test.go
index 650f44f..c930cb7 100644
--- a/controllers/provisioners/eks/update_test.go
+++ b/controllers/provisioners/eks/update_test.go
@@ -408,7 +408,7 @@ func TestLaunchConfigurationDrifted(t *testing.T) {
 		SecurityGroups:          aws.StringSlice([]string{"sg-1", "sg-2"}),
 		KeyName:                 aws.String("somekey"),
 		UserData:                aws.String("userdata"),
-		BlockDeviceMappings:     []*autoscaling.BlockDeviceMapping{w.GetAutoScalingBasicBlockDevice("/dev/xvda", "gp2", "", 40, 100, nil, nil)},
+		BlockDeviceMappings:     []*autoscaling.BlockDeviceMapping{w.GetAutoScalingBasicBlockDevice("/dev/xvda", "gp2", "", 40, 100, 200, nil, nil)},
 	}
 
 	existingConfig := &scaling.CreateConfigurationInput{
@@ -422,10 +422,11 @@ func TestLaunchConfigurationDrifted(t *testing.T) {
 		UserData:              "userdata",
 		Volumes: []v1alpha1.NodeVolume{
 			{
-				Name: "/dev/xvda",
-				Type: "gp2",
-				Size: 40,
-				Iops: 100,
+				Name:       "/dev/xvda",
+				Type:       "gp2",
+				Size:       40,
+				Iops:       100,
+				Throughput: 200,
 			},
 		},
 	}
@@ -448,7 +449,7 @@ func TestLaunchConfigurationDrifted(t *testing.T) {
 	keyDrift.KeyName = aws.String("some-key")
 	usrDrift.UserData = aws.String("some-userdata")
 	devDrift.BlockDeviceMappings = []*autoscaling.BlockDeviceMapping{
-		w.GetAutoScalingBasicBlockDevice("some-device", "some-type", "", 32, 0, nil, nil),
+		w.GetAutoScalingBasicBlockDevice("some-device", "some-type", "", 32, 0, 0, nil, nil),
 	}
 
 	tests := []struct {