diff --git a/pkg/ami/api.go b/pkg/ami/api.go index eb2ba68237..60fee7aa8a 100644 --- a/pkg/ami/api.go +++ b/pkg/ami/api.go @@ -1,6 +1,9 @@ package ami import ( + "sort" + "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" @@ -42,7 +45,9 @@ func IsAvailable(api ec2iface.EC2API, id string) (bool, error) { return *output.Images[0].State == "available", nil } -// FindImage will get the AMI to use for the EKS nodes by querying AWS EC2 API +// FindImage will get the AMI to use for the EKS nodes by querying AWS EC2 API. +// It will only look for images with a status of available and it will pick the +// image with the newest creation date. func FindImage(api ec2iface.EC2API, namePattern string) (string, error) { input := &ec2.DescribeImagesInput{ Filters: []*ec2.Filter{ @@ -62,21 +67,32 @@ func FindImage(api ec2iface.EC2API, namePattern string) (string, error) { Name: aws.String("is-public"), Values: []*string{aws.String("true")}, }, + &ec2.Filter{ + Name: aws.String("state"), + Values: []*string{aws.String("available")}, + }, }, } output, err := api.DescribeImages(input) if err != nil { - return "", errors.Wrapf(err, "cannot find image") + return "", errors.Wrapf(err, "error querying AWS for images") } if len(output.Images) < 1 { return "", nil } - if *output.Images[0].State == "available" { + if len(output.Images) == 1 { return *output.Images[0].ImageId, nil } - return "", nil + // Sort images so newest is first + sort.Slice(output.Images, func(i, j int) bool { + creationLeft, _ := time.Parse(time.RFC3339, *output.Images[i].CreationDate) + creationRight, _ := time.Parse(time.RFC3339, *output.Images[j].CreationDate) + return creationLeft.After(creationRight) + }) + + return *output.Images[0].ImageId, nil } diff --git a/pkg/ami/auto_resolver_test.go b/pkg/ami/auto_resolver_test.go index ec0347b09b..0e2924fab9 100644 --- a/pkg/ami/auto_resolver_test.go +++ b/pkg/ami/auto_resolver_test.go @@ -12,9 +12,16 @@ import ( "github.com/weaveworks/eksctl/pkg/testutils" ) +type returnAmi struct { + imageId string + state string + createdDate string +} + var _ = Describe("AMI Auto Resolution", func() { Describe("When resolving an AMI to use", func() { + var ( p *testutils.MockProvider err error @@ -41,7 +48,7 @@ var _ = Describe("AMI Auto Resolution", func() { imageState = "available" _, p = createProviders() - addMockDescribeImages(p, "amazon-eks-node-*", expectedAmi, imageState) + addMockDescribeImages(p, "amazon-eks-node-*", expectedAmi, imageState, "2018-08-20T23:25:53.000Z") resolver := NewAutoResolver(p.MockEC2()) resolvedAmi, err = resolver.Resolve(region, instanceType) @@ -60,12 +67,12 @@ var _ = Describe("AMI Auto Resolution", func() { }) }) - Context("and ami is pending", func() { + Context("and ami is NOT available", func() { BeforeEach(func() { imageState = "pending" _, p = createProviders() - addMockDescribeImages(p, "amazon-eks-node-*", expectedAmi, imageState) + addMockDescribeImagesMultiple(p, "amazon-eks-node-*", []returnAmi{}) resolver := NewAutoResolver(p.MockEC2()) resolvedAmi, err = resolver.Resolve(region, instanceType) @@ -83,6 +90,44 @@ var _ = Describe("AMI Auto Resolution", func() { Expect(resolvedAmi).To(BeEquivalentTo("")) }) }) + + Context("and there are 2 ami's available", func() { + BeforeEach(func() { + imageState = "available" + expectedAmi = "ami-5678" + + _, p = createProviders() + images := []returnAmi{ + returnAmi{ + createdDate: "2018-08-20T23:25:53.000Z", + imageId: "ami-1234", + state: "available", + }, + returnAmi{ + createdDate: "2018-09-12T22:21:11.000Z", + imageId: expectedAmi, + state: "available", + }, + } + + addMockDescribeImagesMultiple(p, "amazon-eks-node-*", images) + + resolver := NewAutoResolver(p.MockEC2()) + resolvedAmi, err = resolver.Resolve(region, instanceType) + }) + + It("should not error", func() { + Expect(err).NotTo(HaveOccurred()) + }) + + It("should have called AWS EC2 DescribeImages", func() { + Expect(p.MockEC2().AssertNumberOfCalls(GinkgoT(), "DescribeImages", 1)).To(BeTrue()) + }) + + It("should have returned an ami id", func() { + Expect(resolvedAmi).To(BeEquivalentTo(expectedAmi)) + }) + }) }) Context("and gpu instance type", func() { @@ -95,7 +140,7 @@ var _ = Describe("AMI Auto Resolution", func() { imageState = "available" _, p = createProviders() - addMockDescribeImages(p, "amazon-eks-gpu-node-*", expectedAmi, imageState) + addMockDescribeImages(p, "amazon-eks-gpu-node-*", expectedAmi, imageState, "2018-08-20T23:25:53.000Z") resolver := NewAutoResolver(p.MockEC2()) resolvedAmi, err = resolver.Resolve(region, instanceType) @@ -131,7 +176,7 @@ func createProviders() (*eks.ClusterProvider, *testutils.MockProvider) { return c, p } -func addMockDescribeImages(p *testutils.MockProvider, expectedNamePattern string, amiId string, amiState string) { +func addMockDescribeImages(p *testutils.MockProvider, expectedNamePattern string, amiId string, amiState string, createdDate string) { p.MockEC2().On("DescribeImages", mock.MatchedBy(func(input *ec2.DescribeImagesInput) bool { for _, filter := range input.Filters { @@ -154,3 +199,31 @@ func addMockDescribeImages(p *testutils.MockProvider, expectedNamePattern string }, }, nil) } + +func addMockDescribeImagesMultiple(p *testutils.MockProvider, expectedNamePattern string, returnAmis []returnAmi) { + images := make([]*ec2.Image, len(returnAmis)) + for index, ami := range returnAmis { + images[index] = &ec2.Image{ + ImageId: aws.String(ami.imageId), + State: aws.String(ami.state), + CreationDate: aws.String(ami.createdDate), + } + } + + p.MockEC2().On("DescribeImages", + mock.MatchedBy(func(input *ec2.DescribeImagesInput) bool { + for _, filter := range input.Filters { + if *filter.Name == "name" { + if len(filter.Values) > 0 { + if *filter.Values[0] == expectedNamePattern { + return true + } + } + } + } + return false + }), + ).Return(&ec2.DescribeImagesOutput{ + Images: images, + }, nil) +} diff --git a/pkg/ami/static_resolver.go b/pkg/ami/static_resolver.go index e695db8470..23ec03b1a6 100644 --- a/pkg/ami/static_resolver.go +++ b/pkg/ami/static_resolver.go @@ -16,14 +16,14 @@ import ( var StaticImages = map[string]map[int]map[string]string{ ImageFamilyAmazonLinux2: { ImageClassGeneral: { - api.EKS_REGION_US_WEST_2: "ami-08cab282f9979fc7a", - api.EKS_REGION_US_EAST_1: "ami-0b2ae3c6bda8b5c06", - api.EKS_REGION_EU_WEST_1: "ami-066110c1a7466949e", + api.EKS_REGION_US_WEST_2: "ami-0a54c984b9f908c81", + api.EKS_REGION_US_EAST_1: "ami-0440e4f6b9713faf6", + api.EKS_REGION_EU_WEST_1: "ami-0c7a4976cb6fafd3a", }, ImageClassGPU: { - api.EKS_REGION_US_WEST_2: "ami-0d20f2404b9a1c4d1", - api.EKS_REGION_US_EAST_1: "ami-09fe6fc9106bda972", - api.EKS_REGION_EU_WEST_1: "ami-09e0c6b3d3cf906f1", + api.EKS_REGION_US_WEST_2: "ami-0731694d53ef9604b", + api.EKS_REGION_US_EAST_1: "ami-058bfb8c236caae89", + api.EKS_REGION_EU_WEST_1: "ami-0706dc8a5eed2eed9", }, }, } diff --git a/pkg/ami/static_resolver_test.go b/pkg/ami/static_resolver_test.go index a1386a9dab..f79ab0b51e 100644 --- a/pkg/ami/static_resolver_test.go +++ b/pkg/ami/static_resolver_test.go @@ -33,19 +33,19 @@ var _ = Describe("AMI Static Resolution", func() { Entry("with non-gpu instance and us-west-2", ResolveCase{ Region: "us-west-2", InstanceType: "t2.medium", - ExpectedAMI: "ami-08cab282f9979fc7a", + ExpectedAMI: "ami-0a54c984b9f908c81", ExpectError: false, }), Entry("with non-gpu instance and us-east-1", ResolveCase{ Region: "us-east-1", InstanceType: "t2.medium", - ExpectedAMI: "ami-0b2ae3c6bda8b5c06", + ExpectedAMI: "ami-0440e4f6b9713faf6", ExpectError: false, }), Entry("with non-gpu instance and eu-west-1", ResolveCase{ Region: "eu-west-1", InstanceType: "t2.medium", - ExpectedAMI: "ami-066110c1a7466949e", + ExpectedAMI: "ami-0c7a4976cb6fafd3a", ExpectError: false, }), Entry("with non-gpu instance and non-eks enabled region", ResolveCase{ @@ -57,19 +57,19 @@ var _ = Describe("AMI Static Resolution", func() { Entry("with gpu (p2) instance and us-west-2", ResolveCase{ Region: "us-west-2", InstanceType: "p2.xlarge", - ExpectedAMI: "ami-0d20f2404b9a1c4d1", + ExpectedAMI: "ami-0731694d53ef9604b", ExpectError: false, }), Entry("with gpu (p3) instance and us-east-1", ResolveCase{ Region: "us-east-1", InstanceType: "p3.2xlarge", - ExpectedAMI: "ami-09fe6fc9106bda972", + ExpectedAMI: "ami-058bfb8c236caae89", ExpectError: false, }), Entry("with gpu (p2) instance and eu-west-1", ResolveCase{ Region: "eu-west-1", InstanceType: "p2.xlarge", - ExpectedAMI: "ami-09e0c6b3d3cf906f1", + ExpectedAMI: "ami-0706dc8a5eed2eed9", ExpectError: false, }), Entry("with gpu (p3) instance and non-eks enabled region", ResolveCase{