Skip to content

Commit

Permalink
fix: ami conversion
Browse files Browse the repository at this point in the history
Fixes the following issues with AMI conversion:
- Fail closed when the AMI family we're convering from is Ubuntu
- Don't create an alias when the AMI family is non-custom but selector
  terms are specified
  • Loading branch information
jmdeal committed Jul 16, 2024
1 parent 2958854 commit 689d937
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
17 changes: 12 additions & 5 deletions pkg/apis/v1/ec2nodeclass_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,21 @@ func (in *EC2NodeClass) ConvertFrom(ctx context.Context, from apis.Convertible)
v1beta1enc := from.(*v1beta1.EC2NodeClass)
in.ObjectMeta = v1beta1enc.ObjectMeta

// If the v1beta1 AMI family is supported on v1, construct an alias. Otherwise, use the compatibility annotation.
// In practice, this is only used to support the Ubuntu AMI family during conversion.
switch lo.FromPtr(v1beta1enc.Spec.AMIFamily) {
case AMIFamilyAL2, AMIFamilyAL2023, AMIFamilyBottlerocket, Windows2019, Windows2022:
// Temporarily fail closed when trying to convert EC2NodeClasses with the Ubuntu AMI family since compatibility support isn't yet integrated.
// This check can be removed once it's added.
if lo.FromPtr(v1beta1enc.Spec.AMIFamily) == v1beta1.AMIFamilyUbuntu {
return fmt.Errorf("failed to convert v1beta1 EC2NodeClass to v1, conversion for Ubuntu AMIFamily is currently unsupported")
}

// If the AMIFamily is still supported by the v1 APIs, and there are no AMISelectorTerms defined, create an alias.
// Otherwise, don't modify the AMISelectorTerms and add the compatibility annotation.
if lo.Contains([]string{
AMIFamilyAL2, AMIFamilyAL2023, AMIFamilyBottlerocket, AMIFamilyWindows2019, AMIFamilyWindows2022,
}, lo.FromPtr(v1beta1enc.Spec.AMIFamily)) && len(v1beta1enc.Spec.AMISelectorTerms) == 0 {
in.Spec.AMISelectorTerms = []AMISelectorTerm{{
Alias: fmt.Sprintf("%s@latest", strings.ToLower(lo.FromPtr(v1beta1enc.Spec.AMIFamily))),
}}
default:
} else {
in.Annotations = lo.Assign(in.Annotations, map[string]string{
AnnotationAMIFamilyCompatibility: lo.FromPtr(v1beta1enc.Spec.AMIFamily),
})
Expand Down
16 changes: 13 additions & 3 deletions pkg/apis/v1/ec2nodeclass_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,20 @@ var _ = Describe("Convert v1beta1 to v1 EC2NodeClass API", func() {
Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).To(Succeed())
Expect(v1ec2nodeclass.Spec.AMISelectorTerms).To(ContainElement(AMISelectorTerm{Alias: "al2023@latest"}))
})
It("should convert v1beta1 ec2nodeclass ami family (ubuntu compat)", func() {
v1beta1ec2nodeclass.Spec.AMIFamily = &v1beta1.AMIFamilyUbuntu
It("should convert v1beta1 ec2nodeclass ami family with non-custom ami family and ami selector terms", func() {
v1beta1ec2nodeclass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2023
v1beta1ec2nodeclass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{
ID: "ami-0123456789abcdef",
}}
Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).To(Succeed())
Expect(v1ec2nodeclass.Annotations).To(HaveKeyWithValue(AnnotationAMIFamilyCompatibility, "Ubuntu"))
Expect(v1ec2nodeclass.Annotations).To(HaveKeyWithValue(AnnotationAMIFamilyCompatibility, AMIFamilyAL2023))
Expect(v1ec2nodeclass.Spec.AMISelectorTerms).To(Equal([]AMISelectorTerm{{
ID: "ami-0123456789abcdef",
}}))
})
It("should fail to convert v1beta1 ec2nodeclass when ami family is Ubuntu", func() {
v1beta1ec2nodeclass.Spec.AMIFamily = &v1beta1.AMIFamilyUbuntu
Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).ToNot(Succeed())
})
It("should convert v1beta1 ec2nodeclass user data", func() {
v1beta1ec2nodeclass.Spec.UserData = lo.ToPtr("test user data")
Expand Down

0 comments on commit 689d937

Please sign in to comment.