Skip to content

Commit b7cf115

Browse files
jmdealjmdeal
authored
cherry-pick(v1.2.x): fix: only select available AMIs (aws#7672) (aws#7685)
1 parent 058c665 commit b7cf115

File tree

9 files changed

+97
-13
lines changed

9 files changed

+97
-13
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/aws/karpenter-provider-aws
22

3-
go 1.23.2
3+
go 1.23.5
44

55
require (
66
github.com/Pallinder/go-randomdata v1.2.0

‎pkg/cloudprovider/suite_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,7 @@ var _ = Describe("CloudProvider", func() {
639639
Value: aws.String("ami-value-1"),
640640
},
641641
},
642+
State: ec2types.ImageStateAvailable,
642643
},
643644
{
644645
Name: aws.String(coretest.RandomName()),
@@ -651,6 +652,7 @@ var _ = Describe("CloudProvider", func() {
651652
Value: aws.String("ami-value-2"),
652653
},
653654
},
655+
State: ec2types.ImageStateAvailable,
654656
},
655657
},
656658
})

‎pkg/controllers/nodeclass/ami_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
6868
{Key: aws.String("Name"), Value: aws.String("amd64-standard")},
6969
{Key: aws.String("foo"), Value: aws.String("bar")},
7070
},
71+
State: ec2types.ImageStateAvailable,
7172
},
7273
{
7374
Name: aws.String("amd64-standard-new"),
@@ -78,6 +79,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
7879
{Key: aws.String("Name"), Value: aws.String("amd64-standard")},
7980
{Key: aws.String("foo"), Value: aws.String("bar")},
8081
},
82+
State: ec2types.ImageStateAvailable,
8183
},
8284
{
8385
Name: aws.String("amd64-nvidia"),
@@ -88,6 +90,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
8890
{Key: aws.String("Name"), Value: aws.String("amd64-nvidia")},
8991
{Key: aws.String("foo"), Value: aws.String("bar")},
9092
},
93+
State: ec2types.ImageStateAvailable,
9194
},
9295
{
9396
Name: aws.String("amd64-neuron"),
@@ -98,6 +101,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
98101
{Key: aws.String("Name"), Value: aws.String("amd64-neuron")},
99102
{Key: aws.String("foo"), Value: aws.String("bar")},
100103
},
104+
State: ec2types.ImageStateAvailable,
101105
},
102106
{
103107
Name: aws.String("arm64-standard"),
@@ -108,6 +112,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
108112
{Key: aws.String("Name"), Value: aws.String("arm64-standard")},
109113
{Key: aws.String("foo"), Value: aws.String("bar")},
110114
},
115+
State: ec2types.ImageStateAvailable,
111116
},
112117
{
113118
Name: aws.String("arm64-nvidia"),
@@ -118,6 +123,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
118123
{Key: aws.String("Name"), Value: aws.String("arm64-nvidia")},
119124
{Key: aws.String("foo"), Value: aws.String("bar")},
120125
},
126+
State: ec2types.ImageStateAvailable,
121127
},
122128
},
123129
})
@@ -555,6 +561,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
555561
{Key: aws.String("Name"), Value: aws.String("test-ami-3")},
556562
{Key: aws.String("foo"), Value: aws.String("bar")},
557563
},
564+
State: ec2types.ImageStateAvailable,
558565
},
559566
{
560567
Name: aws.String("test-ami-2"),
@@ -565,6 +572,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
565572
{Key: aws.String("Name"), Value: aws.String("test-ami-2")},
566573
{Key: aws.String("foo"), Value: aws.String("bar")},
567574
},
575+
State: ec2types.ImageStateAvailable,
568576
},
569577
},
570578
})
@@ -659,8 +667,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
659667
Key: corev1.LabelArchStable,
660668
Operator: corev1.NodeSelectorOpIn,
661669
Values: []string{karpv1.ArchitectureArm64},
662-
},
663-
},
670+
}},
664671
},
665672
{
666673
Name: "test-ami-3",
@@ -671,8 +678,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
671678
Key: corev1.LabelArchStable,
672679
Operator: corev1.NodeSelectorOpIn,
673680
Values: []string{karpv1.ArchitectureAmd64},
674-
},
675-
},
681+
}},
676682
},
677683
},
678684
))
@@ -692,6 +698,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
692698
{Key: aws.String("Name"), Value: aws.String("test-ami-4")},
693699
{Key: aws.String("foo"), Value: aws.String("bar")},
694700
},
701+
State: ec2types.ImageStateAvailable,
695702
},
696703
{
697704
Name: aws.String("test-ami-2"),
@@ -702,6 +709,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
702709
{Key: aws.String("Name"), Value: aws.String("test-ami-2")},
703710
{Key: aws.String("foo"), Value: aws.String("bar")},
704711
},
712+
State: ec2types.ImageStateAvailable,
705713
},
706714
},
707715
})

‎pkg/controllers/providers/ssm/invalidation/suite_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ func deprecateAMIs(amiIDs ...string) {
158158
CreationDate: lo.ToPtr(awsEnv.Clock.Now().Add(-24 * time.Hour).Format(time.RFC3339)),
159159
Architecture: "x86_64",
160160
DeprecationTime: lo.ToPtr(awsEnv.Clock.Now().Add(-12 * time.Hour).Format(time.RFC3339)),
161+
State: ec2types.ImageStateAvailable,
161162
}
162163
}),
163164
})

‎pkg/fake/ec2api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ func (e *EC2API) DescribeImages(_ context.Context, input *ec2.DescribeImagesInpu
338338
ImageId: aws.String(test.RandomName()),
339339
CreationDate: aws.String(time.Now().Format(time.UnixDate)),
340340
Architecture: "x86_64",
341+
State: ec2types.ImageStateAvailable,
341342
},
342343
},
343344
}, nil

‎pkg/fake/utils.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,16 @@ func FilterDescribeSubnets(subnets []ec2types.Subnet, filters []ec2types.Filter)
104104

105105
func FilterDescribeImages(images []ec2types.Image, filters []ec2types.Filter) []ec2types.Image {
106106
return lo.Filter(images, func(image ec2types.Image, _ int) bool {
107-
return Filter(filters, *image.ImageId, *image.Name, image.Tags)
107+
if stateFilter, ok := lo.Find(filters, func(f ec2types.Filter) bool {
108+
return lo.FromPtr(f.Name) == "state"
109+
}); ok {
110+
if !lo.Contains(stateFilter.Values, string(image.State)) {
111+
return false
112+
}
113+
}
114+
return Filter(lo.Reject(filters, func(f ec2types.Filter, _ int) bool {
115+
return lo.FromPtr(f.Name) == "state"
116+
}), *image.ImageId, *image.Name, image.Tags)
108117
})
109118
}
110119

‎pkg/providers/amifamily/suite_test.go

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ var _ = BeforeEach(func() {
8585
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
8686
{Key: aws.String("foo"), Value: aws.String("bar")},
8787
},
88+
State: ec2types.ImageStateAvailable,
8889
},
8990
{
9091
Name: aws.String(arm64AMI),
@@ -95,6 +96,7 @@ var _ = BeforeEach(func() {
9596
{Key: aws.String("Name"), Value: aws.String(arm64AMI)},
9697
{Key: aws.String("foo"), Value: aws.String("bar")},
9798
},
99+
State: ec2types.ImageStateAvailable,
98100
},
99101
{
100102
Name: aws.String(amd64NvidiaAMI),
@@ -105,6 +107,7 @@ var _ = BeforeEach(func() {
105107
{Key: aws.String("Name"), Value: aws.String(amd64NvidiaAMI)},
106108
{Key: aws.String("foo"), Value: aws.String("bar")},
107109
},
110+
State: ec2types.ImageStateAvailable,
108111
},
109112
{
110113
Name: aws.String(arm64NvidiaAMI),
@@ -115,6 +118,7 @@ var _ = BeforeEach(func() {
115118
{Key: aws.String("Name"), Value: aws.String(arm64NvidiaAMI)},
116119
{Key: aws.String("foo"), Value: aws.String("bar")},
117120
},
121+
State: ec2types.ImageStateAvailable,
118122
},
119123
},
120124
})
@@ -228,6 +232,45 @@ var _ = Describe("AMIProvider", func() {
228232
}
229233
wg.Wait()
230234
})
235+
DescribeTable(
236+
"should ignore images when image.state != available",
237+
func(state ec2types.ImageState) {
238+
awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{Images: []ec2types.Image{
239+
{
240+
Name: aws.String(coretest.RandomName()),
241+
ImageId: aws.String("ami-123"),
242+
Architecture: "x86_64",
243+
Tags: []ec2types.Tag{{Key: lo.ToPtr("test"), Value: lo.ToPtr("test")}},
244+
CreationDate: aws.String("2022-08-15T12:00:00Z"),
245+
State: ec2types.ImageStateAvailable,
246+
},
247+
{
248+
Name: aws.String(coretest.RandomName()),
249+
ImageId: aws.String("ami-456"),
250+
Architecture: "arm64",
251+
Tags: []ec2types.Tag{{Key: lo.ToPtr("test"), Value: lo.ToPtr("test")}},
252+
CreationDate: aws.String("2022-08-15T12:00:00Z"),
253+
State: state,
254+
},
255+
}})
256+
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{
257+
Tags: map[string]string{
258+
"test": "test",
259+
},
260+
}}
261+
amis, err := awsEnv.AMIProvider.List(ctx, nodeClass)
262+
Expect(err).ToNot(HaveOccurred())
263+
Expect(amis).To(HaveLen(1))
264+
Expect(amis[0].AmiID).To(Equal("ami-123"))
265+
},
266+
lo.FilterMap(ec2types.ImageState("").Values(), func(state ec2types.ImageState, _ int) (TableEntry, bool) {
267+
if state == ec2types.ImageStateAvailable {
268+
return TableEntry{}, false
269+
}
270+
return Entry(string(state), state), true
271+
}),
272+
)
273+
231274
Context("SSM Alias Missing", func() {
232275
It("should succeed to partially resolve AMIs if all SSM aliases don't exist (Al2)", func() {
233276
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}}
@@ -278,6 +321,7 @@ var _ = Describe("AMIProvider", func() {
278321
{Key: aws.String(corev1.LabelInstanceTypeStable), Value: aws.String("m5.large")},
279322
{Key: aws.String(corev1.LabelTopologyZone), Value: aws.String("test-zone-1a")},
280323
},
324+
State: ec2types.ImageStateAvailable,
281325
}
282326
awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{
283327
Images: []ec2types.Image{
@@ -329,6 +373,7 @@ var _ = Describe("AMIProvider", func() {
329373
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
330374
{Key: aws.String("foo"), Value: aws.String("bar")},
331375
},
376+
State: ec2types.ImageStateAvailable,
332377
},
333378
{
334379
Name: aws.String(amd64AMI),
@@ -339,6 +384,7 @@ var _ = Describe("AMIProvider", func() {
339384
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
340385
{Key: aws.String("foo"), Value: aws.String("bar")},
341386
},
387+
State: ec2types.ImageStateAvailable,
342388
},
343389
},
344390
})
@@ -369,6 +415,7 @@ var _ = Describe("AMIProvider", func() {
369415
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
370416
{Key: aws.String("foo"), Value: aws.String("bar")},
371417
},
418+
State: ec2types.ImageStateAvailable,
372419
},
373420
{
374421
Name: aws.String(amd64AMI),
@@ -380,6 +427,7 @@ var _ = Describe("AMIProvider", func() {
380427
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
381428
{Key: aws.String("foo"), Value: aws.String("bar")},
382429
},
430+
State: ec2types.ImageStateAvailable,
383431
},
384432
},
385433
})
@@ -411,6 +459,7 @@ var _ = Describe("AMIProvider", func() {
411459
{Key: aws.String("Name"), Value: aws.String("test-ami-2")},
412460
{Key: aws.String("foo"), Value: aws.String("bar")},
413461
},
462+
State: ec2types.ImageStateAvailable,
414463
},
415464
{
416465
Name: aws.String("test-ami-1"),
@@ -422,6 +471,7 @@ var _ = Describe("AMIProvider", func() {
422471
{Key: aws.String("Name"), Value: aws.String("test-ami-1")},
423472
{Key: aws.String("foo"), Value: aws.String("bar")},
424473
},
474+
State: ec2types.ImageStateAvailable,
425475
},
426476
},
427477
})
@@ -452,6 +502,7 @@ var _ = Describe("AMIProvider", func() {
452502
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
453503
{Key: aws.String("foo"), Value: aws.String("bar")},
454504
},
505+
State: ec2types.ImageStateAvailable,
455506
},
456507
{
457508
Name: aws.String(amd64AMI),
@@ -463,6 +514,7 @@ var _ = Describe("AMIProvider", func() {
463514
{Key: aws.String("Name"), Value: aws.String(amd64AMI)},
464515
{Key: aws.String("foo"), Value: aws.String("bar")},
465516
},
517+
State: ec2types.ImageStateAvailable,
466518
},
467519
},
468520
})
@@ -498,7 +550,7 @@ var _ = Describe("AMIProvider", func() {
498550
{
499551
Filters: []ec2types.Filter{
500552
{
501-
Name: aws.String("tag:Name"),
553+
Name: lo.ToPtr("tag:Name"),
502554
Values: []string{"my-ami"},
503555
},
504556
},
@@ -519,7 +571,7 @@ var _ = Describe("AMIProvider", func() {
519571
{
520572
Filters: []ec2types.Filter{
521573
{
522-
Name: aws.String("name"),
574+
Name: lo.ToPtr("name"),
523575
Values: []string{"my-ami"},
524576
},
525577
},
@@ -548,7 +600,7 @@ var _ = Describe("AMIProvider", func() {
548600
{
549601
Filters: []ec2types.Filter{
550602
{
551-
Name: aws.String("image-id"),
603+
Name: lo.ToPtr("image-id"),
552604
Values: []string{"ami-abcd1234", "ami-cafeaced"},
553605
},
554606
},
@@ -599,7 +651,7 @@ var _ = Describe("AMIProvider", func() {
599651
Owners: []string{"0123456789"},
600652
Filters: []ec2types.Filter{
601653
{
602-
Name: aws.String("name"),
654+
Name: lo.ToPtr("name"),
603655
Values: []string{"my-name"},
604656
},
605657
},
@@ -608,7 +660,7 @@ var _ = Describe("AMIProvider", func() {
608660
Owners: []string{"self"},
609661
Filters: []ec2types.Filter{
610662
{
611-
Name: aws.String("name"),
663+
Name: lo.ToPtr("name"),
612664
Values: []string{"my-name"},
613665
},
614666
},

‎pkg/providers/amifamily/types.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,10 @@ type DescribeImageQuery struct {
108108

109109
func (q DescribeImageQuery) DescribeImagesInput() *ec2.DescribeImagesInput {
110110
return &ec2.DescribeImagesInput{
111-
// Don't include filters in the Describe Images call as EC2 API doesn't allow empty filters.
112-
Filters: lo.Ternary(len(q.Filters) > 0, q.Filters, nil),
111+
Filters: append(q.Filters, ec2types.Filter{
112+
Name: lo.ToPtr("state"),
113+
Values: []string{string(ec2types.ImageStateAvailable)},
114+
}),
113115
Owners: lo.Ternary(len(q.Owners) > 0, q.Owners, nil),
114116
IncludeDeprecated: aws.Bool(true),
115117
MaxResults: aws.Int32(1000),

0 commit comments

Comments
 (0)