Repository: keikoproj/instance-manager
Pull request #351 (2022-03-03 05:09:08)

feat: allow resolving EKS AMIs by version/slug for better readability…
… and portability

codecov[bot]: # [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) Report
> Merging [#351](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) (f93b07c) into [master](https://codecov.io/gh/keikoproj/instance-manager/commit/798d06d2b53f6e628892124e5e8475c6535f3423?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) (798d06d) will **increase** coverage by `0.05%`.
> The diff coverage is `75.86%`.

> :exclamation: Current head f93b07c differs from pull request most recent head 8f03dfb. Consider uploading reports for the commit 8f03dfb to get more accurate results

[![Impacted file tree graph](https://codecov.io/gh/keikoproj/instance-manager/pull/351/graphs/tree.svg?width=650&height=150&src=pr&token=IJbjmSBliL&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj)](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj)

```diff
@@            Coverage Diff             @@
##           master     #351      +/-   ##
==========================================
+ Coverage   51.29%   51.34%   +0.05%     
==========================================
  Files          33       33              
  Lines        4533     4561      +28     
==========================================
+ Hits         2325     2342      +17     
- Misses       2064     2071       +7     
- Partials      144      148       +4     
```


| [Impacted Files](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) | Coverage Δ | |
|---|---|---|
| [api/v1alpha1/instancegroup\_types.go](https://codecov.io/gh/keikoproj/instance-manager/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-YXBpL3YxYWxwaGExL2luc3RhbmNlZ3JvdXBfdHlwZXMuZ28=) | `18.69% <ø> (ø)` | |
| [api/v1alpha1/zz\_generated.deepcopy.go](https://codecov.io/gh/keikoproj/instance-manager/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-YXBpL3YxYWxwaGExL3p6X2dlbmVyYXRlZC5kZWVwY29weS5nbw==) | `0.00% <ø> (ø)` | |
| [controllers/providers/aws/ssm.go](https://codecov.io/gh/keikoproj/instance-manager/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlkZXJzL2F3cy9zc20uZ28=) | `0.00% <0.00%> (ø)` | |
| [controllers/provisioners/eks/cloud.go](https://codecov.io/gh/keikoproj/instance-manager/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9jbG91ZC5nbw==) | `85.37% <71.42%> (-0.40%)` | :arrow_down: |
| [controllers/provisioners/eks/helpers.go](https://codecov.io/gh/keikoproj/instance-manager/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9oZWxwZXJzLmdv) | `90.85% <100.00%> (-0.46%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj). Last update [798d06d...8f03dfb](https://codecov.io/gh/keikoproj/instance-manager/pull/351?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj).

eytan-avisror: @backjo this is awesome, love this abstraction, will review shortly

Patch:
diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
index 8334002..70a8af3 100644
--- a/api/v1alpha1/instancegroup_types.go
+++ b/api/v1alpha1/instancegroup_types.go
@@ -79,6 +79,7 @@ const (
 	DedicatedPlacementTenancyType = "dedicated"
 
 	ImageLatestValue = "latest"
+	ImageSSMPrefix   = "ssm://"
 )
 
 type ContainerRuntime string
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 90f591c..83dce64 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -1,3 +1,4 @@
+//go:build !ignore_autogenerated
 // +build !ignore_autogenerated
 
 /*
diff --git a/controllers/providers/aws/ssm.go b/controllers/providers/aws/ssm.go
index f797beb..42f44fb 100644
--- a/controllers/providers/aws/ssm.go
+++ b/controllers/providers/aws/ssm.go
@@ -15,12 +15,12 @@ import (
 type architectureMap map[string]string
 
 const (
-	EksOptimisedAmiPath           = "/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id"
-	EksOptimisedAmazonLinux2Arm64 = "/aws/service/eks/optimized-ami/%s/amazon-linux-2-arm64/recommended/image_id"
-	EksOptimisedBottlerocket      = "/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id"
-	EksOptimisedBottlerocketArm64 = "/aws/service/bottlerocket/aws-k8s-%s/arm64/latest/image_id"
-	EksOptimisedWindowsCore       = "/aws/service/ami-windows-latest/Windows_Server-2019-English-Core-EKS_Optimized-%s/image_id"
-	EksOptimisedWindowsFull       = "/aws/service/ami-windows-latest/Windows_Server-2019-English-Full-EKS_Optimized-%s/image_id"
+	EksOptimisedAmiPath           = "/aws/service/eks/optimized-ami/%s/amazon-linux-2/%s/image_id"
+	EksOptimisedAmazonLinux2Arm64 = "/aws/service/eks/optimized-ami/%s/amazon-linux-2-arm64/%s/image_id"
+	EksOptimisedBottlerocket      = "/aws/service/bottlerocket/aws-k8s-%s/x86_64/%s/image_id"
+	EksOptimisedBottlerocketArm64 = "/aws/service/bottlerocket/aws-k8s-%s/arm64/%s/image_id"
+	EksOptimisedWindowsCore       = "/aws/service/ami-windows-%s/Windows_Server-2019-English-Core-EKS_Optimized-%s/image_id"
+	EksOptimisedWindowsFull       = "/aws/service/ami-windows-%s/Windows_Server-2019-English-Full-EKS_Optimized-%s/image_id"
 )
 
 var (
@@ -37,6 +37,11 @@ var (
 			"x86_64": EksOptimisedWindowsCore,
 		},
 	}
+	LatestIdentifiers = map[string]string{
+		"bottlerocket": "latest",
+		"amazonlinux2": "recommended",
+		"windows":      "latest",
+	}
 )
 
 func GetAwsSsmClient(region string, cacheCfg *cache.Config, maxRetries int, collector *common.MetricsCollector) ssmiface.SSMAPI {
@@ -60,8 +65,16 @@ func GetAwsSsmClient(region string, cacheCfg *cache.Config, maxRetries int, coll
 }
 
 func (w *AwsWorker) GetEksLatestAmi(OSFamily string, arch string, kubernetesVersion string) (string, error) {
+	return w.GetEksSsmAmi(OSFamily, arch, kubernetesVersion, LatestIdentifiers[OSFamily])
+}
+
+func (w *AwsWorker) GetEksSsmAmi(OSFamily string, arch string, kubernetesVersion string, ssmId string) (string, error) {
+	var inputString = aws.String(fmt.Sprintf(EksAmis[OSFamily][arch], kubernetesVersion, ssmId))
+	if OSFamily == "windows" {
+		inputString = aws.String(fmt.Sprintf(EksAmis[OSFamily][arch], ssmId, kubernetesVersion))
+	}
 	input := &ssm.GetParameterInput{
-		Name: aws.String(fmt.Sprintf(EksAmis[OSFamily][arch], kubernetesVersion)),
+		Name: inputString,
 	}
 
 	output, err := w.SsmClient.GetParameter(input)
diff --git a/controllers/provisioners/eks/cloud.go b/controllers/provisioners/eks/cloud.go
index 5181a5d..1ee2230 100644
--- a/controllers/provisioners/eks/cloud.go
+++ b/controllers/provisioners/eks/cloud.go
@@ -186,6 +186,16 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 		ctx.Log.V(4).Info("Updating Image ID with latest", "ami_id", latestAmiId)
 	}
 
+	if strings.HasPrefix(configuration.Image, v1alpha1.ImageSSMPrefix) {
+		ssmKey := strings.TrimPrefix(configuration.Image, v1alpha1.ImageSSMPrefix)
+		amiId, err := ctx.GetEksSsmAmi(ssmKey)
+		if err != nil {
+			return errors.Wrap(err, "failed to discover ami")
+		}
+		configuration.Image = amiId
+		ctx.Log.V(4).Info("Updating Image ID with ami", "ami_id", amiId)
+	}
+
 	// All information needed to creating the scaling group must happen before this line.
 	// find all owned scaling groups
 	ownedScalingGroups := ctx.findOwnedScalingGroups(scalingGroups)
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index 32bfbee..9871b32 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -1240,3 +1240,21 @@ func (ctx *EksInstanceGroupContext) GetEksLatestAmi() (string, error) {
 
 	return ctx.AwsWorker.GetEksLatestAmi(OSFamily, arch, clusterVersion)
 }
+
+func (ctx *EksInstanceGroupContext) GetEksSsmAmi(id string) (string, error) {
+	var (
+		instanceGroup = ctx.GetInstanceGroup()
+		state         = ctx.GetDiscoveredState()
+		osFamily      = ctx.GetOsFamily()
+		configuration = instanceGroup.GetEKSConfiguration()
+	)
+	clusterVersion := state.GetClusterVersion()
+
+	supportedArchitectures := awsprovider.GetInstanceTypeArchitectures(state.GetInstanceTypeInfo(), configuration.InstanceType)
+	arch := FilterSupportedArch(supportedArchitectures)
+	if arch == "" {
+		return "", fmt.Errorf("No supported CPU architecture found for instance type %s", configuration.InstanceType)
+	}
+
+	return ctx.AwsWorker.GetEksSsmAmi(osFamily, arch, clusterVersion, id)
+}
diff --git a/docs/examples/EKS.md b/docs/examples/EKS.md
index 5c14efb..c4b6e0f 100644
--- a/docs/examples/EKS.md
+++ b/docs/examples/EKS.md
@@ -202,4 +202,30 @@ spec:
   eks:
     configuration:
       image: latest
-```
\ No newline at end of file
+```
+
+
+## SSM-based versioned AMI
+
+It might not be acceptable to pin your `InstanceGroup` to the latest AMI. If you want to pin to a specific version,
+you can specify the relevant version information using syntax like `ssm://version`. An example for the EKS-optimized Linux AMI might look like:
+
+```yaml
+apiVersion: instancemgr.keikoproj.io/v1alpha1
+kind: InstanceGroup
+metadata:
+  name: hello-world
+  namespace: instance-manager
+  annotations:
+    instancemgr.keikoproj.io/os-family: "amazonlinux2" #Default if not provided
+spec:
+  strategy: <...>
+  provisioner: eks
+  eks:
+    configuration:
+      image: ssm://amazon-eks-node-1.18-v20220226 # resolves to value in /aws/service/eks/optimized-ami/1.18/amazon-linux-2/amazon-eks-node-1.18-v20220226/image_id
+```
+
+Using this type of image reference over a raw ami ID has a few benefits:
+- The same image reference can be used across regions, since it's not a direct reference to the AMI.
+- The image is more readable - for users leveraging GitOps, the changes will be much easier to review.
\ No newline at end of file


Test patch:
diff --git a/controllers/provisioners/eks/create_test.go b/controllers/provisioners/eks/create_test.go
index cd37181..56a680e 100644
--- a/controllers/provisioners/eks/create_test.go
+++ b/controllers/provisioners/eks/create_test.go
@@ -342,6 +342,11 @@ func TestCreateAutoScalingGroupNegative(t *testing.T) {
 	g.Expect(ctx.GetState()).To(gomega.Equal(v1alpha1.ReconcileModifying))
 }
 
+type AMITest struct {
+	igImage     string
+	expectedAmi string
+}
+
 func TestCreateLatestAMI(t *testing.T) {
 	var (
 		g       = gomega.NewGomegaWithT(t)
@@ -355,6 +360,7 @@ func TestCreateLatestAMI(t *testing.T) {
 	)
 
 	testLatestAmiID := "ami-12345678"
+	testCustomAmi := "ami-98765432"
 	w := MockAwsWorker(asgMock, iamMock, eksMock, ec2Mock, ssmMock)
 	ctx := MockContext(ig, k, w)
 
@@ -366,10 +372,11 @@ func TestCreateLatestAMI(t *testing.T) {
 		RoleName: aws.String("some-role"),
 	}
 
-	// Setup Latest AMI
-	ig.GetEKSConfiguration().Image = "latest"
-	ssmMock.latestAMI = testLatestAmiID
-
+	// Setup mock SSM parameters
+	ssmMock.parameterMap = map[string]string{
+		"/aws/service/eks/optimized-ami/1.18/amazon-linux-2/recommended/image_id":                    testLatestAmiID,
+		"/aws/service/eks/optimized-ami/1.18/amazon-linux-2/amazon-eks-node-1.18-v20220226/image_id": testCustomAmi,
+	}
 	ec2Mock.InstanceTypes = []*ec2.InstanceTypeInfo{
 		&ec2.InstanceTypeInfo{
 			InstanceType: aws.String("m5.large"),
@@ -379,19 +386,33 @@ func TestCreateLatestAMI(t *testing.T) {
 		},
 	}
 
-	err := ctx.CloudDiscovery()
-	g.Expect(err).NotTo(gomega.HaveOccurred())
-	// Must happen after ctx.CloudDiscover()
-	ctx.GetDiscoveredState().SetInstanceTypeInfo([]*ec2.InstanceTypeInfo{
+	testAmis := []AMITest{
 		{
-			InstanceType: aws.String("m5.large"),
-			ProcessorInfo: &ec2.ProcessorInfo{
-				SupportedArchitectures: []*string{aws.String("x86_64")},
-			},
+			igImage:     "latest",
+			expectedAmi: testLatestAmiID,
 		},
-	})
+		{
+			igImage:     "ssm://amazon-eks-node-1.18-v20220226",
+			expectedAmi: testCustomAmi,
+		},
+	}
 
-	err = ctx.Create()
-	g.Expect(err).NotTo(gomega.HaveOccurred())
-	g.Expect(ctx.GetInstanceGroup().Spec.EKSSpec.EKSConfiguration.Image).To(gomega.Equal(testLatestAmiID))
+	for _, tc := range testAmis {
+		ig.GetEKSConfiguration().Image = tc.igImage
+		err := ctx.CloudDiscovery()
+		g.Expect(err).NotTo(gomega.HaveOccurred())
+		// Must happen after ctx.CloudDiscover()
+		ctx.GetDiscoveredState().SetInstanceTypeInfo([]*ec2.InstanceTypeInfo{
+			{
+				InstanceType: aws.String("m5.large"),
+				ProcessorInfo: &ec2.ProcessorInfo{
+					SupportedArchitectures: []*string{aws.String("x86_64")},
+				},
+			},
+		})
+
+		err = ctx.Create()
+		g.Expect(err).NotTo(gomega.HaveOccurred())
+		g.Expect(ctx.GetInstanceGroup().Spec.EKSSpec.EKSConfiguration.Image).To(gomega.Equal(tc.expectedAmi))
+	}
 }
diff --git a/controllers/provisioners/eks/eks_test.go b/controllers/provisioners/eks/eks_test.go
index 2fd2f1d..7c13439 100644
--- a/controllers/provisioners/eks/eks_test.go
+++ b/controllers/provisioners/eks/eks_test.go
@@ -865,13 +865,13 @@ func (i *MockIamClient) WaitUntilInstanceProfileExists(input *iam.GetInstancePro
 
 type MockSsmClient struct {
 	ssmiface.SSMAPI
-	latestAMI string
+	parameterMap map[string]string
 }
 
 func (i *MockSsmClient) GetParameter(input *ssm.GetParameterInput) (*ssm.GetParameterOutput, error) {
 	return &ssm.GetParameterOutput{
 		Parameter: &ssm.Parameter{
-			Value: aws.String(i.latestAMI),
+			Value: aws.String(i.parameterMap[*input.Name]),
 		},
 	}, nil
 }
diff --git a/controllers/provisioners/eks/update_test.go b/controllers/provisioners/eks/update_test.go
index 3be098c..7e28a3c 100644
--- a/controllers/provisioners/eks/update_test.go
+++ b/controllers/provisioners/eks/update_test.go
@@ -701,6 +701,12 @@ func TestUpdateManagedPolicies(t *testing.T) {
 	}
 }
 
+type amiTest struct {
+	igImage     string
+	expectedAmi string
+	osFamily    string
+}
+
 func TestUpdateWithLatestAmiID(t *testing.T) {
 	var (
 		g       = gomega.NewGomegaWithT(t)
@@ -714,6 +720,10 @@ func TestUpdateWithLatestAmiID(t *testing.T) {
 	)
 
 	testLatestAmiID := "ami-12345678"
+	testCustomAmi := "ami-98765432"
+	testBottlerocketAmi := "ami-56789101"
+	testWindowsAmi := "ami-56789201"
+
 	w := MockAwsWorker(asgMock, iamMock, eksMock, ec2Mock, ssmMock)
 	ctx := MockContext(ig, k, w)
 
@@ -738,9 +748,12 @@ func TestUpdateWithLatestAmiID(t *testing.T) {
 	}
 
 	// Setup Latest AMI
-	ig.GetEKSConfiguration().Image = "latest"
-	ssmMock.latestAMI = testLatestAmiID
-
+	ssmMock.parameterMap = map[string]string{
+		"/aws/service/eks/optimized-ami/1.18/amazon-linux-2/recommended/image_id":                      testLatestAmiID,
+		"/aws/service/eks/optimized-ami/1.18/amazon-linux-2/amazon-eks-node-1.18-v20220226/image_id":   testCustomAmi,
+		"/aws/service/bottlerocket/aws-k8s-1.18/x86_64/1.6.1/image_id":                                 testBottlerocketAmi,
+		"/aws/service/ami-windows-latest/Windows_Server-2019-English-Core-EKS_Optimized-1.18/image_id": testWindowsAmi,
+	}
 	ec2Mock.InstanceTypes = []*ec2.InstanceTypeInfo{
 		&ec2.InstanceTypeInfo{
 			InstanceType: aws.String("m5.large"),
@@ -750,33 +763,61 @@ func TestUpdateWithLatestAmiID(t *testing.T) {
 		},
 	}
 
-	err := ctx.CloudDiscovery()
-	g.Expect(err).NotTo(gomega.HaveOccurred())
-
-	ctx.SetDiscoveredState(&DiscoveredState{
-		Publisher: kubeprovider.EventPublisher{
-			Client: k.Kubernetes,
+	testAmis := []amiTest{
+		{
+			igImage:     "latest",
+			expectedAmi: testLatestAmiID,
+			osFamily:    "amazonlinux2",
 		},
-		ScalingGroup: mockScalingGroup,
-		ScalingConfiguration: &scaling.LaunchConfiguration{
-			AwsWorker: w,
+		{
+			igImage:     "ssm://amazon-eks-node-1.18-v20220226",
+			expectedAmi: testCustomAmi,
+			osFamily:    "amazonlinux2",
 		},
-		InstanceProfile: &iam.InstanceProfile{
-			Arn: aws.String("some-instance-arn"),
+		{
+			igImage:     "ssm://1.6.1",
+			expectedAmi: testBottlerocketAmi,
+			osFamily:    "bottlerocket",
 		},
-		ClusterNodes: nil,
-		Cluster:      nil,
-		InstanceTypeInfo: []*ec2.InstanceTypeInfo{
-			{
-				InstanceType: aws.String("m5.large"),
-				ProcessorInfo: &ec2.ProcessorInfo{
-					SupportedArchitectures: []*string{aws.String("x86_64")},
+		{
+			igImage:     "ssm://latest",
+			expectedAmi: testWindowsAmi,
+			osFamily:    "windows",
+		},
+	}
+
+	for _, tc := range testAmis {
+		ig.GetEKSConfiguration().Image = tc.igImage
+		ig.Annotations[OsFamilyAnnotation] = tc.osFamily
+		err := ctx.CloudDiscovery()
+		g.Expect(err).NotTo(gomega.HaveOccurred())
+
+		ctx.SetDiscoveredState(&DiscoveredState{
+			Publisher: kubeprovider.EventPublisher{
+				Client: k.Kubernetes,
+			},
+			ScalingGroup: mockScalingGroup,
+			ScalingConfiguration: &scaling.LaunchConfiguration{
+				AwsWorker: w,
+			},
+			InstanceProfile: &iam.InstanceProfile{
+				Arn: aws.String("some-instance-arn"),
+			},
+			ClusterNodes: nil,
+			Cluster:      nil,
+			InstanceTypeInfo: []*ec2.InstanceTypeInfo{
+				{
+					InstanceType: aws.String("m5.large"),
+					ProcessorInfo: &ec2.ProcessorInfo{
+						SupportedArchitectures: []*string{aws.String("x86_64")},
+					},
 				},
 			},
-		},
-	})
+		})
+
+		err = ctx.Update()
+		g.Expect(err).NotTo(gomega.HaveOccurred())
+		g.Expect(ctx.GetInstanceGroup().Spec.EKSSpec.EKSConfiguration.Image).To(gomega.Equal(tc.expectedAmi))
+	}
 
-	err = ctx.Update()
-	g.Expect(err).NotTo(gomega.HaveOccurred())
-	g.Expect(ctx.GetInstanceGroup().Spec.EKSSpec.EKSConfiguration.Image).To(gomega.Equal(testLatestAmiID))
 }