Skip to content

Commit

Permalink
feat: convert unpinned Ubuntu EC2NodeClasses (#6699)
Browse files Browse the repository at this point in the history
Co-authored-by: Nick Tran <[email protected]>
Co-authored-by: Nick Tran <[email protected]>
  • Loading branch information
3 people committed Aug 12, 2024
1 parent 343e0c9 commit ebe924e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
6 changes: 6 additions & 0 deletions pkg/apis/v1/ec2nodeclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,12 @@ func (in *EC2NodeClass) InstanceProfileTags(clusterName string) map[string]strin
})
}

// UbuntuIncompatible returns true if the NodeClass has the ubuntu compatibility annotation. This will cause the NodeClass to show
// as NotReady in its status conditions, opting its referencing NodePools out of provisioning and drift.
func (in *EC2NodeClass) UbuntuIncompatible() bool {
return lo.Contains(strings.Split(in.Annotations[AnnotationUbuntuCompatibilityKey], ","), AnnotationUbuntuCompatibilityIncompatible)
}

// AMIFamily returns the family for a NodePool based on the following items, in order of precdence:
// - ec2nodeclass.spec.amiFamily
// - ec2nodeclass.spec.amiSelectorTerms[].alias
Expand Down
22 changes: 15 additions & 7 deletions pkg/apis/v1/ec2nodeclass_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ func (in *EC2NodeClass) ConvertTo(ctx context.Context, to apis.Convertible) erro

if value, ok := in.Annotations[AnnotationUbuntuCompatibilityKey]; ok {
compatSpecifiers := strings.Split(value, ",")
// Remove the `id: ami-placeholder` AMISelectorTerms that are injected to pass CRD validation at v1
// we don't need these in v1beta1, and should be dropped
if lo.Contains(compatSpecifiers, AnnotationUbuntuCompatibilityIncompatible) {
in.Spec.AMISelectorTerms = nil
}
// The only blockDeviceMappings present on the v1 EC2NodeClass are those that we injected during conversion.
// These should be dropped.
if lo.Contains(compatSpecifiers, AnnotationUbuntuCompatibilityBlockDeviceMappings) {
Expand Down Expand Up @@ -140,13 +145,6 @@ func (in *EC2NodeClass) ConvertFrom(ctx context.Context, from apis.Convertible)
in.Spec.AMIFamily = v1beta1enc.Spec.AMIFamily
}
case AMIFamilyUbuntu:
// If there are no AMISelectorTerms specified, we will fail closed when converting the NodeClass. Users must
// pin their AMIs **before** upgrading to Karpenter v1.0.0 if they were using the Ubuntu AMIFamily.
// TODO: jmdeal@ verify doc link to the upgrade guide once available
if len(v1beta1enc.Spec.AMISelectorTerms) == 0 {
return fmt.Errorf("converting EC2NodeClass %q from v1beta1 to v1, automatic Ubuntu AMI discovery is not supported (https://karpenter.sh/v1.0/upgrading/upgrade-guide/)", v1beta1enc.Name)
}

// If AMISelectorTerms were specified, we can continue to use them to discover Ubuntu AMIs and use the AL2 AMI
// family for bootstrapping. AL2 and Ubuntu have an identical UserData format, but do have different default
// BlockDeviceMappings. We'll set the BlockDeviceMappings to Ubuntu's default if no user specified
Expand All @@ -165,6 +163,16 @@ func (in *EC2NodeClass) ConvertFrom(ctx context.Context, from apis.Convertible)
},
}}
}

// If there are no AMISelectorTerms specified, we mark the ec2nodeclass as incompatible.
// Karpenter will ignore incompatible ec2nodeclasses for provisioning and computing drift.
if len(v1beta1enc.Spec.AMISelectorTerms) == 0 {
compatSpecifiers = append(compatSpecifiers, AnnotationUbuntuCompatibilityIncompatible)
in.Spec.AMISelectorTerms = []AMISelectorTerm{{
ID: "ami-placeholder",
}}
}

// This compatibility annotation will be used to determine if the amiFamily was mutated from Ubuntu to AL2, and
// if we needed to inject any blockDeviceMappings. This is required to enable a round-trip conversion.
in.Annotations = lo.Assign(in.Annotations, map[string]string{
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/v1/ec2nodeclass_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,10 +447,11 @@ var _ = Describe("Convert v1beta1 to v1 EC2NodeClass API", func() {
},
}}))
})
It("should fail to convert v1beta1 ec2nodeclass when amiFamily is Ubuntu (without amiSelectorTerms)", func() {
It("should convert v1beta1 ec2nodeclass when amiFamily is Ubuntu (without amiSelectorTerms) but mark incompatible", func() {
v1beta1ec2nodeclass.Spec.AMIFamily = lo.ToPtr(v1beta1.AMIFamilyUbuntu)
v1beta1ec2nodeclass.Spec.AMISelectorTerms = nil
Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).ToNot(Succeed())
Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).To(Succeed())
Expect(v1ec2nodeclass.UbuntuIncompatible()).To(BeTrue())
})
It("should convert v1beta1 ec2nodeclass user data", func() {
v1beta1ec2nodeclass.Spec.UserData = lo.ToPtr("test user data")
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/v1/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ var (
AnnotationInstanceTagged = Group + "/tagged"

AnnotationUbuntuCompatibilityKey = CompatibilityGroup + "/v1beta1-ubuntu"
AnnotationUbuntuCompatibilityIncompatible = "incompatible"
AnnotationUbuntuCompatibilityAMIFamily = "amiFamily"
AnnotationUbuntuCompatibilityBlockDeviceMappings = "blockDeviceMappings"

Expand Down

0 comments on commit ebe924e

Please sign in to comment.