diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go index bfedd8d095ca..60a94d42aa23 100644 --- a/pkg/providers/amifamily/ami.go +++ b/pkg/providers/amifamily/ami.go @@ -54,10 +54,11 @@ type DefaultProvider struct { } type AMI struct { - Name string - AmiID string - CreationDate string - Requirements scheduling.Requirements + Name string + AmiID string + CreationDate string + DeprecationTime string + Requirements scheduling.Requirements } type AMIs []AMI @@ -65,13 +66,31 @@ type AMIs []AMI // Sort orders the AMIs 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 ID, which is guaranteed to be unique, in ascending order. func (a AMIs) Sort() { + now := time.Now().Unix() sort.Slice(a, func(i, j int) bool { + itimeDep, _ := time.Parse(time.RFC3339, a[i].DeprecationTime) + jtimeDep, _ := time.Parse(time.RFC3339, a[j].DeprecationTime) itime, _ := time.Parse(time.RFC3339, a[i].CreationDate) jtime, _ := time.Parse(time.RFC3339, a[j].CreationDate) - if itime.Unix() != jtime.Unix() { + + // Compare deprecation time with current time + iIsDep := itimeDep.Unix() > 0 && itimeDep.Unix() <= now + jIsDep := jtimeDep.Unix() > 0 && jtimeDep.Unix() <= now + + switch { + // if both are deprecated and their deprecation times aren't same, else they will be sorted by ID, in ascending order + case iIsDep && jIsDep && itimeDep.Unix() != jtimeDep.Unix(): + return itimeDep.Unix() > jtimeDep.Unix() + // One is deprecated, prioritize non deprecated + case iIsDep && !jIsDep: + return iIsDep + case !iIsDep && jIsDep: + return jIsDep + case itime.Unix() != jtime.Unix(): return itime.Unix() > jtime.Unix() + default: + return a[i].AmiID < a[j].AmiID } - return a[i].AmiID < a[j].AmiID }) } @@ -213,10 +232,11 @@ func (p *DefaultProvider) getAMIs(ctx context.Context, terms []v1.AMISelectorTer } } images[reqsHash] = AMI{ - Name: lo.FromPtr(page.Images[i].Name), - AmiID: lo.FromPtr(page.Images[i].ImageId), - CreationDate: lo.FromPtr(page.Images[i].CreationDate), - Requirements: reqs, + Name: lo.FromPtr(page.Images[i].Name), + AmiID: lo.FromPtr(page.Images[i].ImageId), + CreationDate: lo.FromPtr(page.Images[i].CreationDate), + DeprecationTime: lo.FromPtr(page.Images[i].DeprecationTime), + Requirements: reqs, } } return true diff --git a/pkg/providers/amifamily/suite_test.go b/pkg/providers/amifamily/suite_test.go index 57e1f2f0c8dd..af48b4ba0a9e 100644 --- a/pkg/providers/amifamily/suite_test.go +++ b/pkg/providers/amifamily/suite_test.go @@ -564,6 +564,212 @@ var _ = Describe("AMIProvider", func() { }, )) }) + It("should sort amis with deprecation time and creation date consistently", func() { + amis := amifamily.AMIs{ + { + Name: "test-ami-5", + AmiID: "test-ami-5-id", + CreationDate: "2021-08-31T00:04:42.000Z", + DeprecationTime: "2021-09-01T00:06:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-4", + AmiID: "test-ami-4-id", + CreationDate: "2021-08-31T00:06:42.000Z", + DeprecationTime: "2021-09-01T00:08:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-3", + AmiID: "test-ami-3-id", + CreationDate: "", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-2", + AmiID: "test-ami-2-id", + CreationDate: "2021-08-31T00:08:42.000Z", + DeprecationTime: "", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-1-id", + CreationDate: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + } + + amis.Sort() + Expect(amis).To(Equal( + amifamily.AMIs{ + { + Name: "test-ami-1", + AmiID: "test-ami-1-id", + CreationDate: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-2", + AmiID: "test-ami-2-id", + CreationDate: "2021-08-31T00:08:42.000Z", + DeprecationTime: "", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-3", + AmiID: "test-ami-3-id", + CreationDate: "", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-4", + AmiID: "test-ami-4-id", + CreationDate: "2021-08-31T00:06:42.000Z", + DeprecationTime: "2021-09-01T00:08:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-5", + AmiID: "test-ami-5-id", + CreationDate: "2021-08-31T00:04:42.000Z", + DeprecationTime: "2021-09-01T00:06:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + }, + )) + }) + It("should sort non deprecated amis over deprecated amis with same creation time consistently", func() { + // If there are 2 AMIs with same creation date and one with a greater id is deprecated + // sort should priortize the non deprecated ami first + amis := amifamily.AMIs{ + { + Name: "test-ami-3", + AmiID: "test-ami-3-id", + CreationDate: "2021-08-31T00:12:42.000Z", + DeprecationTime: time.Now().Add(-1 * time.Minute).Format(time.RFC3339), + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-2", + AmiID: "test-ami-2-id", + CreationDate: "2021-08-31T00:10:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-1-id", + CreationDate: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-4", + AmiID: "test-ami-4-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: time.Now().Add(-2 * time.Minute).Format(time.RFC3339), + Requirements: scheduling.NewRequirements(), + }, + } + + amis.Sort() + Expect(amis).To(Equal( + amifamily.AMIs{ + { + Name: "test-ami-1", + AmiID: "test-ami-1-id", + CreationDate: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-2", + AmiID: "test-ami-2-id", + CreationDate: "2021-08-31T00:10:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-3", + AmiID: "test-ami-3-id", + CreationDate: "2021-08-31T00:12:42.000Z", + DeprecationTime: time.Now().Add(-1 * time.Minute).Format(time.RFC3339), + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-4", + AmiID: "test-ami-4-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: time.Now().Add(-2 * time.Minute).Format(time.RFC3339), + Requirements: scheduling.NewRequirements(), + }, + }, + )) + }) + It("should sort only deprecated amis with the same name and deprecation time consistently", func() { + amis := amifamily.AMIs{ + { + Name: "test-ami-1", + AmiID: "test-ami-4-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-3-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-2-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-1-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + } + + amis.Sort() + Expect(amis).To(Equal( + amifamily.AMIs{ + { + Name: "test-ami-1", + AmiID: "test-ami-1-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-2-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-3-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + { + Name: "test-ami-1", + AmiID: "test-ami-4-id", + CreationDate: "2021-08-31T00:10:42.000Z", + DeprecationTime: "2021-08-31T00:12:42.000Z", + Requirements: scheduling.NewRequirements(), + }, + }, + )) + }) }) })