fix: only warn for ssm query errors (#4128)

Co-authored-by: Jonathan Innis <joinnis@amazon.com>
aws · Jun 29, 2023 · 16faeda · 16faeda
1 parent 9dedcf9
commit 16faeda
Show file tree

Hide file tree

Showing 4 changed files with 420 additions and 141 deletions.
diff --git a/pkg/controllers/nodetemplate/suite_test.go b/pkg/controllers/nodetemplate/suite_test.go
@@ -573,8 +573,85 @@ var _ = Describe("AWSNodeTemplateController", func() {
 						},
 					},
 				},
-			},
-			))
+			}))
+		})
+		It("should resolve amiSelector AMis and requirements into status when all SSM aliases don't resolve", func() {
+			version := lo.Must(awsEnv.AMIProvider.KubeServerVersion(ctx))
+			// This parameter set doesn't include any of the Nvidia AMIs
+			awsEnv.SSMAPI.Parameters = map[string]string{
+				fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): "ami-id-123",
+				fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/arm64/latest/image_id", version):  "ami-id-456",
+			}
+			nodeTemplate.Spec.AMIFamily = &v1alpha1.AMIFamilyBottlerocket
+			nodeTemplate.Spec.AMISelector = nil
+			awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{
+				Images: []*ec2.Image{
+					{
+						Name:         aws.String("test-ami-1"),
+						ImageId:      aws.String("ami-id-123"),
+						CreationDate: aws.String(time.Now().Format(time.RFC3339)),
+						Architecture: aws.String("x86_64"),
+						Tags: []*ec2.Tag{
+							{Key: aws.String("Name"), Value: aws.String("test-ami-1")},
+							{Key: aws.String("foo"), Value: aws.String("bar")},
+						},
+					},
+					{
+						Name:         aws.String("test-ami-2"),
+						ImageId:      aws.String("ami-id-456"),
+						CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)),
+						Architecture: aws.String("arm64"),
+						Tags: []*ec2.Tag{
+							{Key: aws.String("Name"), Value: aws.String("test-ami-2")},
+							{Key: aws.String("foo"), Value: aws.String("bar")},
+						},
+					},
+				},
+			})
+			ExpectApplied(ctx, env.Client, nodeTemplate)
+			ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate))
+			nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate)
+			sortRequirements(nodeTemplate.Status.AMIs)
+			Expect(nodeTemplate.Status.AMIs).To(ContainElements([]v1alpha1.AMI{
+				{
+					Name: "test-ami-1",
+					ID:   "ami-id-123",
+					Requirements: []v1.NodeSelectorRequirement{
+						{
+							Key:      v1.LabelArchStable,
+							Operator: v1.NodeSelectorOpIn,
+							Values:   []string{v1alpha5.ArchitectureAmd64},
+						},
+						{
+							Key:      v1alpha1.LabelInstanceGPUCount,
+							Operator: v1.NodeSelectorOpDoesNotExist,
+						},
+						{
+							Key:      v1alpha1.LabelInstanceAcceleratorCount,
+							Operator: v1.NodeSelectorOpDoesNotExist,
+						},
+					},
+				},
+				{
+					Name: "test-ami-2",
+					ID:   "ami-id-456",
+					Requirements: []v1.NodeSelectorRequirement{
+						{
+							Key:      v1.LabelArchStable,
+							Operator: v1.NodeSelectorOpIn,
+							Values:   []string{v1alpha5.ArchitectureArm64},
+						},
+						{
+							Key:      v1alpha1.LabelInstanceGPUCount,
+							Operator: v1.NodeSelectorOpDoesNotExist,
+						},
+						{
+							Key:      v1alpha1.LabelInstanceAcceleratorCount,
+							Operator: v1.NodeSelectorOpDoesNotExist,
+						},
+					},
+				},
+			}))
 		})
 		It("Should resolve a valid AMI selector", func() {
 			ExpectApplied(ctx, env.Client, nodeTemplate)

diff --git a/pkg/fake/ssmapi.go b/pkg/fake/ssmapi.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/mitchellh/hashstructure/v2"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -37,10 +38,13 @@ func (a SSMAPI) GetParameterWithContext(_ context.Context, input *ssm.GetParamet
 	if a.WantErr != nil {
 		return nil, a.WantErr
 	}
-	if amiID, ok := a.Parameters[*input.Name]; ok {
-		return &ssm.GetParameterOutput{
-			Parameter: &ssm.Parameter{Value: aws.String(amiID)},
-		}, nil
+	if len(a.Parameters) > 0 {
+		if amiID, ok := a.Parameters[*input.Name]; ok {
+			return &ssm.GetParameterOutput{
+				Parameter: &ssm.Parameter{Value: aws.String(amiID)},
+			}, nil
+		}
+		return nil, awserr.New(ssm.ErrCodeParameterNotFound, fmt.Sprintf("%s couldn't be found", *input.Name), nil)
 	}
 	hc, _ := hashstructure.Hash(input.Name, hashstructure.FormatV2, nil)
 	if a.GetParameterOutput != nil {
@@ -54,5 +58,6 @@ func (a SSMAPI) GetParameterWithContext(_ context.Context, input *ssm.GetParamet
 
 func (a *SSMAPI) Reset() {
 	a.GetParameterOutput = nil
+	a.Parameters = nil
 	a.WantErr = nil
 }
diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go
@@ -127,7 +127,7 @@ func (p *Provider) Get(ctx context.Context, nodeTemplate *v1alpha1.AWSNodeTempla
 			return nil, err
 		}
 	}
-	amis = groupAMIsByRequirements(sortAMIsByCreationDate(amis))
+	amis = groupAMIsByRequirements(SortAMIsByCreationDate(amis))
 	if p.cm.HasChanged(fmt.Sprintf("amis/%s", nodeTemplate.Name), amis) {
 		logging.FromContext(ctx).With("ids", amiList(amis), "count", len(amis)).Debugf("discovered amis")
 	}
@@ -160,15 +160,15 @@ func (p *Provider) getDefaultAMIFromSSM(ctx context.Context, nodeTemplate *v1alp
 	for _, ssmOutput := range ssmRequirements {
 		amiID, err := p.fetchAMIsFromSSM(ctx, ssmOutput.Query)
 		if err != nil {
-			return nil, err
+			logging.FromContext(ctx).With("query", ssmOutput.Query).Errorf("discovering amis from ssm, %s", err)
+			continue
 		}
 		amis = append(amis, AMI{AmiID: amiID, Requirements: ssmOutput.Requirements})
 	}
 	amis, err = p.findAMINames(ctx, amis)
 	if err != nil {
 		return nil, err
 	}
-
 	return amis, nil
 }
 
@@ -229,7 +229,7 @@ func (p *Provider) getAMIsFromSelector(ctx context.Context, selector map[string]
 }
 
 func (p *Provider) fetchAMIsFromEC2(ctx context.Context, amiSelector map[string]string) ([]*ec2.Image, error) {
-	filters, owners := getFiltersAndOwners(amiSelector)
+	filters, owners := GetFiltersAndOwners(amiSelector)
 	hash, err := hashstructure.Hash(filters, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
 	if err != nil {
 		return nil, err
@@ -263,8 +263,8 @@ func amiList(amis []AMI) string {
 	return sb.String()
 }
 
-func getFiltersAndOwners(amiSelector map[string]string) ([]*ec2.Filter, []*string) {
-	filters := []*ec2.Filter{}
+func GetFiltersAndOwners(amiSelector map[string]string) ([]*ec2.Filter, []*string) {
+	var filters []*ec2.Filter
 	var owners []*string
 	imagesSet := false
 	for key, value := range amiSelector {
@@ -298,9 +298,9 @@ func getFiltersAndOwners(amiSelector map[string]string) ([]*ec2.Filter, []*strin
 	return filters, owners
 }
 
-// sortAMIsByCreationDate the AMIs are sorted by creation date in descending order.
+// SortAMIsByCreationDate the AMIs are sorted by creation date in descending order.
 // If creation date is nil or two AMIs have the same creation date, the AMIs will be sorted by name in ascending order.
-func sortAMIsByCreationDate(amis []AMI) []AMI {
+func SortAMIsByCreationDate(amis []AMI) []AMI {
 	sort.Slice(amis, func(i, j int) bool {
 		if amis[i].CreationDate != "" || amis[j].CreationDate != "" {
 			itime, _ := time.Parse(time.RFC3339, amis[i].CreationDate)