diff --git a/hack/docs/instancetypes_gen/main.go b/hack/docs/instancetypes_gen/main.go index 87b4deb1f30d..e5cafcde2803 100644 --- a/hack/docs/instancetypes_gen/main.go +++ b/hack/docs/instancetypes_gen/main.go @@ -105,7 +105,9 @@ func main() { // Fake a NodeClass so we can use it to get InstanceTypes nodeClass := &v1.EC2NodeClass{ Spec: v1.EC2NodeClassSpec{ - AMIFamily: &v1.AMIFamilyAL2023, + AMISelectorTerms: []v1.AMISelectorTerm{{ + Alias: "al2023@latest", + }}, SubnetSelectorTerms: []v1.SubnetSelectorTerm{ { Tags: map[string]string{ diff --git a/pkg/apis/apis.go b/pkg/apis/apis.go index 294b5983c8bd..3e255ad9275d 100644 --- a/pkg/apis/apis.go +++ b/pkg/apis/apis.go @@ -27,7 +27,7 @@ import ( //go:generate controller-gen crd object:headerFile="../../hack/boilerplate.go.txt" paths="./..." output:crd:artifacts:config=crds var ( Group = "karpenter.k8s.aws" - CompatabilityGroup = "compatibility." + Group + CompatibilityGroup = "compatibility." + Group //go:embed crds/karpenter.k8s.aws_ec2nodeclasses.yaml EC2NodeClassCRD []byte CRDs = append(apis.CRDs, diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index cc061eb57cf9..0753b8454e9b 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -57,17 +57,6 @@ spec: EC2NodeClassSpec is the top level specification for the AWS Karpenter Provider. This will contain configuration necessary to launch instances in AWS. properties: - amiFamily: - description: AMIFamily is the AMI family that instances use. - enum: - - AL2 - - AL2023 - - Bottlerocket - - Ubuntu - - Custom - - Windows2019 - - Windows2022 - type: string amiSelectorTerms: description: AMISelectorTerms is a list of or ami selector terms. The terms are ORed. items: @@ -75,6 +64,21 @@ spec: AMISelectorTerm defines selection logic for an ami used by Karpenter to launch nodes. If multiple fields are used for selection, the requirements are ANDed. properties: + alias: + description: |- + Alias specifies which EKS optimized AMI to select. + Each alias consists of a family and an AMI version, specified as "family@version". + Valid families include: al2, al2023, bottlerocket, windows2019, and windows2022. + The version can either be pinned to a specific AMI release, with that AMIs version format (ex: "al2023@v20240625" or "bottlerocket@v1.10.0"). + The version can also be set to "latest" for any family. Setting the version to latest will result in drift when a new AMI is released. This is **not** recommended for production environments. + Note: The Windows families do **not** support version pinning, and only latest may be used. + maxLength: 30 + type: string + x-kubernetes-validations: + - message: '''alias'' is improperly formatted, must match the format ''family@version''' + rule: self.matches('^[a-zA-Z0-9]*@.*$') + - message: 'family is not supported, must be one of the following: ''al2'', ''al2023'', ''bottlerocket'', ''windows2019'', ''windows2022''' + rule: self.find('^[^@]+') in ['al2','al2023','bottlerocket','windows2019','windows2022'] id: description: ID is the ami id in EC2 pattern: ami-[0-9a-z]+ @@ -102,12 +106,17 @@ spec: rule: self.all(k, k != '' && self[k] != '') type: object maxItems: 30 + minItems: 1 type: array x-kubernetes-validations: - - message: expected at least one, got none, ['tags', 'id', 'name'] - rule: self.all(x, has(x.tags) || has(x.id) || has(x.name)) + - message: expected at least one, got none, ['tags', 'id', 'name', 'alias'] + rule: self.all(x, has(x.tags) || has(x.id) || has(x.name) || has(x.alias)) - message: '''id'' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms' - rule: '!self.all(x, has(x.id) && (has(x.tags) || has(x.name) || has(x.owner)))' + rule: '!self.exists(x, has(x.id) && (has(x.alias) || has(x.tags) || has(x.name) || has(x.owner)))' + - message: '''alias'' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms' + rule: '!self.exists(x, has(x.alias) && (has(x.id) || has(x.tags) || has(x.name) || has(x.owner)))' + - message: '''alias'' is mutually exclusive, cannot be set with a combination of other amiSelectorTerms' + rule: '!(self.exists(x, has(x.alias)) && self.size() != 1)' associatePublicIPAddress: description: AssociatePublicIPAddress controls if public IP addresses are assigned to instances that are launched with the nodeclass. type: boolean @@ -549,13 +558,10 @@ spec: this UserData to ensure nodes are being provisioned with the correct configuration. type: string required: - - amiFamily - securityGroupSelectorTerms - subnetSelectorTerms type: object x-kubernetes-validations: - - message: amiSelectorTerms is required when amiFamily == 'Custom' - rule: 'self.amiFamily == ''Custom'' ? self.amiSelectorTerms.size() != 0 : true' - message: must specify exactly one of ['role', 'instanceProfile'] rule: (has(self.role) && !has(self.instanceProfile)) || (!has(self.role) && has(self.instanceProfile)) - message: changing from 'instanceProfile' to 'role' is not supported. You must delete and recreate this node class if you want to change this. diff --git a/pkg/apis/v1/ec2nodeclass.go b/pkg/apis/v1/ec2nodeclass.go index 1c43db9f9607..ec468f50c3a1 100644 --- a/pkg/apis/v1/ec2nodeclass.go +++ b/pkg/apis/v1/ec2nodeclass.go @@ -16,6 +16,8 @@ package v1 import ( "fmt" + "log" + "strings" "github.com/mitchellh/hashstructure/v2" "github.com/samber/lo" @@ -46,15 +48,14 @@ type EC2NodeClassSpec struct { // +optional AssociatePublicIPAddress *bool `json:"associatePublicIPAddress,omitempty"` // AMISelectorTerms is a list of or ami selector terms. The terms are ORed. - // +kubebuilder:validation:XValidation:message="expected at least one, got none, ['tags', 'id', 'name']",rule="self.all(x, has(x.tags) || has(x.id) || has(x.name))" - // +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.all(x, has(x.id) && (has(x.tags) || has(x.name) || has(x.owner)))" + // +kubebuilder:validation:XValidation:message="expected at least one, got none, ['tags', 'id', 'name', 'alias']",rule="self.all(x, has(x.tags) || has(x.id) || has(x.name) || has(x.alias))" + // +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.exists(x, has(x.id) && (has(x.alias) || has(x.tags) || has(x.name) || has(x.owner)))" + // +kubebuilder:validation:XValidation:message="'alias' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.exists(x, has(x.alias) && (has(x.id) || has(x.tags) || has(x.name) || has(x.owner)))" + // +kubebuilder:validation:XValidation:message="'alias' is mutually exclusive, cannot be set with a combination of other amiSelectorTerms",rule="!(self.exists(x, has(x.alias)) && self.size() != 1)" + // +kubebuilder:validation:MinItems:=1 // +kubebuilder:validation:MaxItems:=30 // +optional - AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms,omitempty" hash:"ignore"` - // AMIFamily is the AMI family that instances use. - // +kubebuilder:validation:Enum:={AL2,AL2023,Bottlerocket,Ubuntu,Custom,Windows2019,Windows2022} - // +required - AMIFamily *string `json:"amiFamily"` + AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms" hash:"ignore"` // UserData to be applied to the provisioned nodes. // It must be in the appropriate format based on the AMIFamily in use. Karpenter will merge certain fields into // this UserData to ensure nodes are being provisioned with the correct configuration. @@ -163,6 +164,17 @@ type SecurityGroupSelectorTerm struct { // AMISelectorTerm defines selection logic for an ami used by Karpenter to launch nodes. // If multiple fields are used for selection, the requirements are ANDed. type AMISelectorTerm struct { + // Alias specifies which EKS optimized AMI to select. + // Each alias consists of a family and an AMI version, specified as "family@version". + // Valid families include: al2, al2023, bottlerocket, windows2019, and windows2022. + // The version can either be pinned to a specific AMI release, with that AMIs version format (ex: "al2023@v20240625" or "bottlerocket@v1.10.0"). + // The version can also be set to "latest" for any family. Setting the version to latest will result in drift when a new AMI is released. This is **not** recommended for production environments. + // Note: The Windows families do **not** support version pinning, and only latest may be used. + // +kubebuilder:validation:XValidation:message="'alias' is improperly formatted, must match the format 'family@version'",rule="self.matches('^[a-zA-Z0-9]*@.*$')" + // +kubebuilder:validation:XValidation:message="family is not supported, must be one of the following: 'al2', 'al2023', 'bottlerocket', 'windows2019', 'windows2022'",rule="self.find('^[^@]+') in ['al2','al2023','bottlerocket','windows2019','windows2022']" + // +kubebuilder:validation:MaxLength=30 + // +optional + Alias string `json:"alias,omitempty"` // Tags is a map of key/value tags used to select subnets // Specifying '*' for a value selects all values for a given tag key. // +kubebuilder:validation:XValidation:message="empty tag keys or values aren't supported",rule="self.all(k, k != '' && self[k] != '')" @@ -405,7 +417,6 @@ type EC2NodeClass struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // +kubebuilder:validation:XValidation:message="amiSelectorTerms is required when amiFamily == 'Custom'",rule="self.amiFamily == 'Custom' ? self.amiSelectorTerms.size() != 0 : true" // +kubebuilder:validation:XValidation:message="must specify exactly one of ['role', 'instanceProfile']",rule="(has(self.role) && !has(self.instanceProfile)) || (!has(self.role) && has(self.instanceProfile))" // +kubebuilder:validation:XValidation:message="changing from 'instanceProfile' to 'role' is not supported. You must delete and recreate this node class if you want to change this.",rule="(has(oldSelf.role) && has(self.role)) || (has(oldSelf.instanceProfile) && has(self.instanceProfile))" Spec EC2NodeClassSpec `json:"spec,omitempty"` @@ -442,6 +453,45 @@ func (in *EC2NodeClass) InstanceProfileTags(clusterName string) map[string]strin }) } +func (in *EC2NodeClass) AMIFamily() string { + if family, ok := in.Annotations[AnnotationAMIFamilyCompatibility]; ok { + return family + } + if term, ok := lo.Find(in.Spec.AMISelectorTerms, func(t AMISelectorTerm) bool { + return t.Alias != "" + }); ok { + switch strings.Split(term.Alias, "@")[0] { + case "al2": + return AMIFamilyAL2 + case "al2023": + return AMIFamilyAL2023 + case "bottlerocket": + return AMIFamilyBottlerocket + case "windows2019": + return AMIFamilyWindows2019 + case "windows2022": + return AMIFamilyWindows2022 + } + } + return AMIFamilyCustom +} + +func (in *EC2NodeClass) AMIVersion() string { + if _, ok := in.Annotations[AnnotationAMIFamilyCompatibility]; ok { + return "latest" + } + if term, ok := lo.Find(in.Spec.AMISelectorTerms, func(t AMISelectorTerm) bool { + return t.Alias != "" + }); ok { + parts := strings.Split(term.Alias, "@") + if len(parts) != 2 { + log.Fatalf("failed to parse AMI alias %q, invalid format", term.Alias) + } + return parts[1] + } + return "latest" +} + // EC2NodeClassList contains a list of EC2NodeClass // +kubebuilder:object:root=true type EC2NodeClassList struct { diff --git a/pkg/apis/v1/ec2nodeclass_conversion.go b/pkg/apis/v1/ec2nodeclass_conversion.go index 15518f8ed381..18ec075542d5 100644 --- a/pkg/apis/v1/ec2nodeclass_conversion.go +++ b/pkg/apis/v1/ec2nodeclass_conversion.go @@ -16,6 +16,8 @@ package v1 import ( "context" + "fmt" + "strings" "github.com/samber/lo" "knative.dev/pkg/apis" @@ -27,6 +29,7 @@ func (in *EC2NodeClass) ConvertTo(ctx context.Context, to apis.Convertible) erro v1beta1enc := to.(*v1beta1.EC2NodeClass) v1beta1enc.ObjectMeta = in.ObjectMeta + v1beta1enc.Spec.AMIFamily = lo.ToPtr(in.AMIFamily()) in.Spec.convertTo(&v1beta1enc.Spec) in.Status.convertTo((&v1beta1enc.Status)) return nil @@ -54,7 +57,6 @@ func (in *EC2NodeClassSpec) convertTo(v1beta1enc *v1beta1.EC2NodeClassSpec) { Tags: ami.Tags, } }) - v1beta1enc.AMIFamily = in.AMIFamily v1beta1enc.AssociatePublicIPAddress = in.AssociatePublicIPAddress v1beta1enc.Context = in.Context v1beta1enc.DetailedMonitoring = in.DetailedMonitoring @@ -102,6 +104,19 @@ 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: + in.Spec.AMISelectorTerms = []AMISelectorTerm{{ + Alias: fmt.Sprintf("%s@latest", strings.ToLower(lo.FromPtr(v1beta1enc.Spec.AMIFamily))), + }} + default: + in.Annotations = lo.Assign(in.Annotations, map[string]string{ + AnnotationAMIFamilyCompatibility: lo.FromPtr(v1beta1enc.Spec.AMIFamily), + }) + } + in.Spec.convertFrom(&v1beta1enc.Spec) in.Status.convertFrom((&v1beta1enc.Status)) return nil @@ -121,15 +136,14 @@ func (in *EC2NodeClassSpec) convertFrom(v1beta1enc *v1beta1.EC2NodeClassSpec) { Tags: sg.Tags, } }) - in.AMISelectorTerms = lo.Map(v1beta1enc.AMISelectorTerms, func(ami v1beta1.AMISelectorTerm, _ int) AMISelectorTerm { + in.AMISelectorTerms = append(in.AMISelectorTerms, lo.Map(v1beta1enc.AMISelectorTerms, func(ami v1beta1.AMISelectorTerm, _ int) AMISelectorTerm { return AMISelectorTerm{ ID: ami.ID, Name: ami.Name, Owner: ami.Owner, Tags: ami.Tags, } - }) - in.AMIFamily = v1beta1enc.AMIFamily + })...) in.AssociatePublicIPAddress = v1beta1enc.AssociatePublicIPAddress in.Context = v1beta1enc.Context in.DetailedMonitoring = v1beta1enc.DetailedMonitoring diff --git a/pkg/apis/v1/ec2nodeclass_conversion_test.go b/pkg/apis/v1/ec2nodeclass_conversion_test.go index 70a60fcf1e52..38b6235e9891 100644 --- a/pkg/apis/v1/ec2nodeclass_conversion_test.go +++ b/pkg/apis/v1/ec2nodeclass_conversion_test.go @@ -22,6 +22,8 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "sigs.k8s.io/karpenter/pkg/test" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + . "github.com/aws/karpenter-provider-aws/pkg/apis/v1" "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" ) @@ -38,8 +40,10 @@ var _ = Describe("Convert v1 to v1beta1 EC2NodeClass API", func() { }) It("should convert v1 ec2nodeclass metadata", func() { - v1ec2nodeclass.ObjectMeta = test.ObjectMeta() - Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).To(Succeed()) + v1ec2nodeclass.ObjectMeta = test.ObjectMeta(metav1.ObjectMeta{ + Annotations: map[string]string{"foo": "bar"}, + }) + Expect(v1ec2nodeclass.ConvertTo(ctx, v1beta1ec2nodeclass)).To(Succeed()) Expect(v1beta1ec2nodeclass.ObjectMeta).To(BeEquivalentTo(v1ec2nodeclass.ObjectMeta)) }) Context("EC2NodeClass Spec", func() { @@ -108,10 +112,17 @@ var _ = Describe("Convert v1 to v1beta1 EC2NodeClass API", func() { Expect(v1ec2nodeclass.ConvertTo(ctx, v1beta1ec2nodeclass)).To(Succeed()) Expect(lo.FromPtr(v1beta1ec2nodeclass.Spec.AssociatePublicIPAddress)).To(BeTrue()) }) - It("should convert v1 ec2nodeclass ami family", func() { - v1ec2nodeclass.Spec.AMIFamily = &AMIFamilyUbuntu + It("should convert v1 ec2nodeclass alias", func() { + v1ec2nodeclass.Spec.AMISelectorTerms = []AMISelectorTerm{{Alias: "al2023@latest"}} + Expect(v1ec2nodeclass.ConvertTo(ctx, v1beta1ec2nodeclass)).To(Succeed()) + Expect(lo.FromPtr(v1beta1ec2nodeclass.Spec.AMIFamily)).To(Equal(v1beta1.AMIFamilyAL2023)) + }) + It("should convert v1 ec2nodeclass with AMIFamily compat annotation", func() { + v1ec2nodeclass.Annotations = lo.Assign(v1ec2nodeclass.Annotations, map[string]string{ + AnnotationAMIFamilyCompatibility: v1beta1.AMIFamilyAL2023, + }) Expect(v1ec2nodeclass.ConvertTo(ctx, v1beta1ec2nodeclass)).To(Succeed()) - Expect(lo.FromPtr(v1beta1ec2nodeclass.Spec.AMIFamily)).To(Equal(v1beta1.AMIFamilyUbuntu)) + Expect(lo.FromPtr(v1beta1ec2nodeclass.Spec.AMIFamily)).To(Equal(v1beta1.AMIFamilyAL2023)) }) It("should convert v1 ec2nodeclass user data", func() { v1ec2nodeclass.Spec.UserData = lo.ToPtr("test user data") @@ -269,8 +280,18 @@ var _ = Describe("Convert v1beta1 to v1 EC2NodeClass API", func() { }) It("should convert v1beta1 ec2nodeclass metadata", func() { - v1beta1ec2nodeclass.ObjectMeta = test.ObjectMeta() + v1beta1ec2nodeclass.ObjectMeta = test.ObjectMeta(metav1.ObjectMeta{ + Annotations: map[string]string{"foo": "bar"}, + }) Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).To(Succeed()) + + // Remove the compatibility annotations from the EC2NodeClass + v1ec2nodeclass.ObjectMeta.Annotations = lo.OmitByKeys(v1ec2nodeclass.ObjectMeta.Annotations, []string{ + AnnotationAMIFamilyCompatibility, + }) + if len(v1ec2nodeclass.ObjectMeta.Annotations) == 0 { + v1ec2nodeclass.ObjectMeta.Annotations = nil + } Expect(v1ec2nodeclass.ObjectMeta).To(BeEquivalentTo(v1beta1ec2nodeclass.ObjectMeta)) }) Context("EC2NodeClass Spec", func() { @@ -340,9 +361,14 @@ var _ = Describe("Convert v1beta1 to v1 EC2NodeClass API", func() { Expect(lo.FromPtr(v1ec2nodeclass.Spec.AssociatePublicIPAddress)).To(BeTrue()) }) It("should convert v1beta1 ec2nodeclass ami family", func() { - v1beta1ec2nodeclass.Spec.AMIFamily = &AMIFamilyUbuntu + v1beta1ec2nodeclass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2023 + 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 Expect(v1ec2nodeclass.ConvertFrom(ctx, v1beta1ec2nodeclass)).To(Succeed()) - Expect(lo.FromPtr(v1ec2nodeclass.Spec.AMIFamily)).To(Equal(v1beta1.AMIFamilyUbuntu)) + Expect(v1ec2nodeclass.Annotations).To(HaveKeyWithValue(AnnotationAMIFamilyCompatibility, "Ubuntu")) }) It("should convert v1beta1 ec2nodeclass user data", func() { v1beta1ec2nodeclass.Spec.UserData = lo.ToPtr("test user data") diff --git a/pkg/apis/v1/ec2nodeclass_hash_test.go b/pkg/apis/v1/ec2nodeclass_hash_test.go index afed0631aaed..a4ca5dd09f8c 100644 --- a/pkg/apis/v1/ec2nodeclass_hash_test.go +++ b/pkg/apis/v1/ec2nodeclass_hash_test.go @@ -29,14 +29,13 @@ import ( ) var _ = Describe("Hash", func() { - const staticHash = "10790156025840984195" + const staticHash = "12469392724194263290" var nodeClass *v1.EC2NodeClass BeforeEach(func() { nodeClass = &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: lo.ToPtr(v1.AMIFamilyAL2023), - Role: "role-1", + Role: "role-1", Tags: map[string]string{ "keyTag-1": "valueTag-1", "keyTag-2": "valueTag-2", @@ -81,26 +80,26 @@ var _ = Describe("Hash", func() { }, Entry("Base EC2NodeClass", staticHash, v1.EC2NodeClass{}), // Static fields, expect changed hash from base - Entry("UserData", "18317182711135792962", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), - Entry("Tags", "7254882043893135054", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), - Entry("Context", "17271601354348855032", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Context: aws.String("context-2")}}), - Entry("DetailedMonitoring", "3320998103335094348", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), - Entry("AMIFamily", "11029247967399146065", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AMIFamily: aws.String(v1.AMIFamilyBottlerocket)}}), - Entry("InstanceStorePolicy", "15591048753403695860", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{InstanceStorePolicy: lo.ToPtr(v1.InstanceStorePolicyRAID0)}}), - Entry("AssociatePublicIPAddress", "8788624850560996180", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AssociatePublicIPAddress: lo.ToPtr(true)}}), - Entry("MetadataOptions HTTPEndpoint", "12130088184516131939", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPEndpoint: lo.ToPtr("enabled")}}}), - Entry("MetadataOptions HTTPProtocolIPv6", "9851778617676567202", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPProtocolIPv6: lo.ToPtr("enabled")}}}), - Entry("MetadataOptions HTTPPutResponseHopLimit", "10114972825726256442", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPPutResponseHopLimit: lo.ToPtr(int64(10))}}}), - Entry("MetadataOptions HTTPTokens", "15328515228245883488", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPTokens: lo.ToPtr("required")}}}), - Entry("BlockDeviceMapping DeviceName", "14855383487702710824", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{DeviceName: lo.ToPtr("map-device-test-3")}}}}), - Entry("BlockDeviceMapping RootVolume", "9591488558660758449", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{RootVolume: true}}}}), - Entry("BlockDeviceMapping DeleteOnTermination", "2802222466202766732", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{DeleteOnTermination: lo.ToPtr(true)}}}}}), - Entry("BlockDeviceMapping Encrypted", "16743053872042184219", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{Encrypted: lo.ToPtr(true)}}}}}), - Entry("BlockDeviceMapping IOPS", "17284705682110195253", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{IOPS: lo.ToPtr(int64(10))}}}}}), - Entry("BlockDeviceMapping KMSKeyID", "9151019926310241707", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{KMSKeyID: lo.ToPtr("test")}}}}}), - Entry("BlockDeviceMapping SnapshotID", "5250341140179985875", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{SnapshotID: lo.ToPtr("test")}}}}}), - Entry("BlockDeviceMapping Throughput", "16711481758638864953", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{Throughput: lo.ToPtr(int64(10))}}}}}), - Entry("BlockDeviceMapping VolumeType", "488614640133725370", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{VolumeType: lo.ToPtr("io1")}}}}}), + + Entry("UserData", "10726797622220086701", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), + Entry("Tags", "13171706991797150099", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), + Entry("Context", "2889419402034926781", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Context: aws.String("context-2")}}), + Entry("DetailedMonitoring", "3268207623472999947", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), + Entry("InstanceStorePolicy", "14988327748406934174", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{InstanceStorePolicy: lo.ToPtr(v1.InstanceStorePolicyRAID0)}}), + Entry("AssociatePublicIPAddress", "15544493229975292346", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AssociatePublicIPAddress: lo.ToPtr(true)}}), + Entry("MetadataOptions HTTPEndpoint", "17871731458970668774", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPEndpoint: lo.ToPtr("enabled")}}}), + Entry("MetadataOptions HTTPProtocolIPv6", "2470633037813609088", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPProtocolIPv6: lo.ToPtr("enabled")}}}), + Entry("MetadataOptions HTTPPutResponseHopLimit", "17675179355294901357", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPPutResponseHopLimit: lo.ToPtr(int64(10))}}}), + Entry("MetadataOptions HTTPTokens", "2669105690505918645", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPTokens: lo.ToPtr("required")}}}), + Entry("BlockDeviceMapping DeviceName", "5415148539492750790", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{DeviceName: lo.ToPtr("map-device-test-3")}}}}), + Entry("BlockDeviceMapping RootVolume", "5518942571120617117", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{RootVolume: true}}}}), + Entry("BlockDeviceMapping DeleteOnTermination", "2581652380077676602", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{DeleteOnTermination: lo.ToPtr(true)}}}}}), + Entry("BlockDeviceMapping Encrypted", "9177809208865678872", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{Encrypted: lo.ToPtr(true)}}}}}), + Entry("BlockDeviceMapping IOPS", "10810952692173241494", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{IOPS: lo.ToPtr(int64(10))}}}}}), + Entry("BlockDeviceMapping KMSKeyID", "2530423540851551058", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{KMSKeyID: lo.ToPtr("test")}}}}}), + Entry("BlockDeviceMapping SnapshotID", "9712886753345517947", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{SnapshotID: lo.ToPtr("test")}}}}}), + Entry("BlockDeviceMapping Throughput", "3334301275630239638", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{Throughput: lo.ToPtr(int64(10))}}}}}), + Entry("BlockDeviceMapping VolumeType", "7651488174200364927", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{EBS: &v1.BlockDevice{VolumeType: lo.ToPtr("io1")}}}}}), // Behavior / Dynamic fields, expect same hash as base Entry("Modified AMISelector", staticHash, v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AMISelectorTerms: []v1.AMISelectorTerm{{Tags: map[string]string{"ami-test-key": "ami-test-value"}}}}}), @@ -111,12 +110,12 @@ var _ = Describe("Hash", func() { // doesn't work well with unexported fields, like the ones that are present in resource.Quantity It("should match static hash when updating blockDeviceMapping volumeSize", func() { nodeClass.Spec.BlockDeviceMappings[0].EBS.VolumeSize = resource.NewScaledQuantity(10, resource.Giga) - Expect(nodeClass.Hash()).To(Equal("4802236799448001710")) + Expect(nodeClass.Hash()).To(Equal("13279394903563315705")) }) It("should match static hash for instanceProfile", func() { nodeClass.Spec.Role = "" nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nodeClass.Hash()).To(Equal("7914642030762404205")) + Expect(nodeClass.Hash()).To(Equal("13329599275048356421")) }) It("should match static hash when reordering tags", func() { nodeClass.Spec.Tags = map[string]string{"keyTag-2": "valueTag-2", "keyTag-1": "valueTag-1"} @@ -136,7 +135,6 @@ var _ = Describe("Hash", func() { Entry("Tags", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), Entry("Context", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Context: aws.String("context-2")}}), Entry("DetailedMonitoring", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), - Entry("AMIFamily", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AMIFamily: aws.String(v1.AMIFamilyBottlerocket)}}), Entry("InstanceStorePolicy", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{InstanceStorePolicy: lo.ToPtr(v1.InstanceStorePolicyRAID0)}}), Entry("AssociatePublicIPAddress", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AssociatePublicIPAddress: lo.ToPtr(true)}}), Entry("MetadataOptions HTTPEndpoint", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPEndpoint: lo.ToPtr("enabled")}}}), diff --git a/pkg/apis/v1/ec2nodeclass_validation_cel_test.go b/pkg/apis/v1/ec2nodeclass_validation_cel_test.go index d862d62e9b8f..fb9823e355bf 100644 --- a/pkg/apis/v1/ec2nodeclass_validation_cel_test.go +++ b/pkg/apis/v1/ec2nodeclass_validation_cel_test.go @@ -18,6 +18,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/imdario/mergo" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -42,8 +43,8 @@ var _ = Describe("CEL/Validation", func() { nc = &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: lo.ToPtr(v1.AMIFamilyAL2023), - Role: "role-1", + AMISelectorTerms: []v1.AMISelectorTerm{{Alias: "al2023@latest"}}, + Role: "role-1", SecurityGroupSelectorTerms: []v1.SecurityGroupSelectorTerm{ { Tags: map[string]string{ @@ -343,6 +344,12 @@ var _ = Describe("CEL/Validation", func() { }) }) Context("AMISelectorTerms", func() { + It("should succeed with a valid ami selector on alias", func() { + nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ + Alias: "al2023@latest", + }} + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) It("should succeed with a valid ami selector on tags", func() { nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ { @@ -448,37 +455,60 @@ var _ = Describe("CEL/Validation", func() { } Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) }) - It("should fail when specifying id with tags", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "ami-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "ami-12345749", - Name: "my-custom-ami", - }, - } - Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) - }) - It("should fail when specifying id with owner", func() { + DescribeTable( + "should fail when specifying id with other fields", + func(mutation v1.AMISelectorTerm) { + term := v1.AMISelectorTerm{ID: "ami-1234749"} + Expect(mergo.Merge(&term, &mutation)).To(Succeed()) + nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{term} + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }, + Entry("alias", v1.AMISelectorTerm{Alias: "al2023@latest"}), + Entry("tags", v1.AMISelectorTerm{ + Tags: map[string]string{"test": "testvalue"}, + }), + Entry("name", v1.AMISelectorTerm{Name: "my-custom-ami"}), + Entry("owner", v1.AMISelectorTerm{Owner: "123456789"}), + ) + DescribeTable( + "should fail when specifying alias with other fields", + func(mutation v1.AMISelectorTerm) { + term := v1.AMISelectorTerm{Alias: "al2023@latest"} + Expect(mergo.Merge(&term, &mutation)).To(Succeed()) + nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{term} + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }, + Entry("id", v1.AMISelectorTerm{ID: "ami-1234749"}), + Entry("tags", v1.AMISelectorTerm{ + Tags: map[string]string{"test": "testvalue"}, + }), + Entry("name", v1.AMISelectorTerm{Name: "my-custom-ami"}), + Entry("owner", v1.AMISelectorTerm{Owner: "123456789"}), + ) + It("should fail when specifying alias with other terms", func() { nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "ami-12345749", - Owner: "123456789", - }, + {Alias: "al2023@latest"}, + {ID: "ami-1234749"}, } Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) }) - It("should fail when AMIFamily is Custom and not AMISelectorTerms", func() { - nc.Spec.AMIFamily = &v1.AMIFamilyCustom + DescribeTable( + "should succeed for valid aliases", + func(alias string) { + nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: alias}} + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }, + Entry("al2 (latest)", "al2@latest"), + Entry("al2 (pinned)", "al2@v20240625"), + Entry("al2023 (latest)", "al2023@latest"), + Entry("al2023 (pinned)", "al2023@v20240625"), + Entry("bottlerocket (latest)", "bottlerocket@latest"), + Entry("bottlerocket (pinned)", "bottlerocket@1.10.0"), + Entry("windows2019 (latest)", "windows2019@latest"), + Entry("windows2022 (latest)", "windows2022@latest"), + ) + It("should fail for an alias with an invalid family", func() { + nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "ubuntu@latest"}} Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) }) }) @@ -725,7 +755,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, @@ -755,7 +785,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, @@ -776,7 +806,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, @@ -797,7 +827,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, @@ -825,7 +855,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, @@ -846,7 +876,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, @@ -867,7 +897,7 @@ var _ = Describe("CEL/Validation", func() { nodeClass := &v1.EC2NodeClass{ ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), Spec: v1.EC2NodeClassSpec{ - AMIFamily: nc.Spec.AMIFamily, + AMISelectorTerms: nc.Spec.AMISelectorTerms, SubnetSelectorTerms: nc.Spec.SubnetSelectorTerms, SecurityGroupSelectorTerms: nc.Spec.SecurityGroupSelectorTerms, Role: nc.Spec.Role, diff --git a/pkg/apis/v1/labels.go b/pkg/apis/v1/labels.go index 40bc9b2ca13e..1c96373a8b65 100644 --- a/pkg/apis/v1/labels.go +++ b/pkg/apis/v1/labels.go @@ -119,9 +119,10 @@ var ( LabelInstanceAcceleratorManufacturer = apis.Group + "/instance-accelerator-manufacturer" LabelInstanceAcceleratorCount = apis.Group + "/instance-accelerator-count" AnnotationEC2NodeClassHash = apis.Group + "/ec2nodeclass-hash" - AnnotationKubeletCompatibilityHash = apis.CompatabilityGroup + "/kubelet-drift-hash" + AnnotationKubeletCompatibilityHash = apis.CompatibilityGroup + "/kubelet-drift-hash" AnnotationEC2NodeClassHashVersion = apis.Group + "/ec2nodeclass-hash-version" AnnotationInstanceTagged = apis.Group + "/tagged" + AnnotationAMIFamilyCompatibility = apis.CompatibilityGroup + "/v1beta1-ami-family-conversion" TagNodeClaim = coreapis.Group + "/nodeclaim" TagManagedLaunchTemplate = apis.Group + "/cluster" diff --git a/pkg/apis/v1/zz_generated.deepcopy.go b/pkg/apis/v1/zz_generated.deepcopy.go index 627c14cfad13..c7732b18e478 100644 --- a/pkg/apis/v1/zz_generated.deepcopy.go +++ b/pkg/apis/v1/zz_generated.deepcopy.go @@ -237,11 +237,6 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.AMIFamily != nil { - in, out := &in.AMIFamily, &out.AMIFamily - *out = new(string) - **out = **in - } if in.UserData != nil { in, out := &in.UserData, &out.UserData *out = new(string) diff --git a/pkg/cloudprovider/suite_test.go b/pkg/cloudprovider/suite_test.go index f28739327e15..9fbe90a7bae0 100644 --- a/pkg/cloudprovider/suite_test.go +++ b/pkg/cloudprovider/suite_test.go @@ -34,7 +34,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/aws/aws-sdk-go/service/ssm" opstatus "github.com/awslabs/operatorpkg/status" "github.com/imdario/mergo" "github.com/samber/lo" @@ -601,9 +600,6 @@ var _ = Describe("CloudProvider", func() { validSecurityGroup = fake.SecurityGroupID() validSubnet1 = fake.SubnetID() validSubnet2 = fake.SubnetID() - awsEnv.SSMAPI.GetParameterOutput = &ssm.GetParameterOutput{ - Parameter: &ssm.Parameter{Value: aws.String(armAMIID)}, - } awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ Images: []*ec2.Image{ { @@ -888,9 +884,11 @@ var _ = Describe("CloudProvider", func() { Tags: map[string]string{ "fakeKey": "fakeValue", }, - Context: lo.ToPtr("fake-context"), - DetailedMonitoring: lo.ToPtr(false), - AMIFamily: lo.ToPtr(v1.AMIFamilyAL2023), + Context: lo.ToPtr("fake-context"), + DetailedMonitoring: lo.ToPtr(false), + AMISelectorTerms: []v1.AMISelectorTerm{{ + Alias: "al2023@latest", + }}, AssociatePublicIPAddress: lo.ToPtr(false), MetadataOptions: &v1.MetadataOptions{ HTTPEndpoint: lo.ToPtr("disabled"), @@ -966,7 +964,6 @@ var _ = Describe("CloudProvider", func() { Entry("Tags", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), Entry("Context", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Context: lo.ToPtr("context-2")}}), Entry("DetailedMonitoring", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), - Entry("AMIFamily", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AMIFamily: lo.ToPtr(v1.AMIFamilyBottlerocket)}}), Entry("InstanceStorePolicy", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{InstanceStorePolicy: lo.ToPtr(v1.InstanceStorePolicyRAID0)}}), Entry("AssociatePublicIPAddress", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AssociatePublicIPAddress: lo.ToPtr(true)}}), Entry("MetadataOptions HTTPEndpoint", v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{MetadataOptions: &v1.MetadataOptions{HTTPEndpoint: lo.ToPtr("enabled")}}}), diff --git a/pkg/controllers/nodeclass/hash/suite_test.go b/pkg/controllers/nodeclass/hash/suite_test.go index d9090dcfeaa4..6470dec982f9 100644 --- a/pkg/controllers/nodeclass/hash/suite_test.go +++ b/pkg/controllers/nodeclass/hash/suite_test.go @@ -135,7 +135,6 @@ var _ = Describe("NodeClass Hash Controller", func() { Expect(expectedHash).ToNot(Equal(expectedHashTwo)) }, - Entry("AMIFamily Drift", &v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{AMIFamily: aws.String(v1.AMIFamilyBottlerocket)}}), Entry("UserData Drift", &v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), Entry("Tags Drift", &v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), Entry("BlockDeviceMappings Drift", &v1.EC2NodeClass{Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), diff --git a/pkg/controllers/nodeclass/status/ami_test.go b/pkg/controllers/nodeclass/status/ami_test.go index 0ab001dd7d4c..6bd726710eb1 100644 --- a/pkg/controllers/nodeclass/status/ami_test.go +++ b/pkg/controllers/nodeclass/status/ami_test.go @@ -123,7 +123,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() { Name: aws.String("test-ami-3"), ImageId: aws.String("ami-id-789"), CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), - Architecture: aws.String("x86_64"), + Architecture: aws.String("arm64"), Tags: []*ec2.Tag{ {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, {Key: aws.String("foo"), Value: aws.String("bar")}, @@ -131,11 +131,12 @@ var _ = Describe("NodeClass AMI Status Controller", func() { }, }, }) - nodeClass.Spec.AMISelectorTerms = nil + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} ExpectApplied(ctx, env.Client, nodeClass) ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.AMIs).To(Equal([]v1.AMI{ + Expect(len(nodeClass.Status.AMIs)).To(Equal(4)) + Expect(nodeClass.Status.AMIs).To(ContainElements([]v1.AMI{ { Name: "test-ami-3", ID: "ami-id-789", @@ -214,8 +215,9 @@ var _ = Describe("NodeClass AMI Status Controller", func() { fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): "ami-id-123", fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/arm64/latest/image_id", version): "ami-id-456", } - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket - nodeClass.Spec.AMISelectorTerms = nil + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ + Alias: "bottlerocket@latest", + }} awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ Images: []*ec2.Image{ { @@ -244,7 +246,8 @@ var _ = Describe("NodeClass AMI Status Controller", func() { ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.AMIs).To(Equal([]v1.AMI{ + Expect(len(nodeClass.Status.AMIs)).To(Equal(2)) + Expect(nodeClass.Status.AMIs).To(ContainElements([]v1.AMI{ { Name: "test-ami-2", ID: "ami-id-456", diff --git a/pkg/controllers/nodeclass/status/launchtemplate_test.go b/pkg/controllers/nodeclass/status/launchtemplate_test.go index 5035d03eba5f..f69f9a82c799 100644 --- a/pkg/controllers/nodeclass/status/launchtemplate_test.go +++ b/pkg/controllers/nodeclass/status/launchtemplate_test.go @@ -52,22 +52,21 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func() awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(nil) }) It("shouldn't resolve cluster CIDR for non-AL2023 NodeClasses", func() { - for _, family := range []string{ - v1.AMIFamilyAL2, - v1.AMIFamilyBottlerocket, - v1.AMIFamilyUbuntu, - v1.AMIFamilyWindows2019, - v1.AMIFamilyWindows2022, - v1.AMIFamilyCustom, + for _, term := range []v1.AMISelectorTerm{ + {Alias: "al2@latest"}, + {Alias: "bottlerocket@latest"}, + {Alias: "windows2019@latest"}, + {Alias: "windows2022@latest"}, + {ID: "ami-12345"}, } { - nodeClass.Spec.AMIFamily = lo.ToPtr(family) + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{term} ExpectApplied(ctx, env.Client, nodeClass) ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) Expect(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load()).To(BeNil()) } }) It("should resolve cluster CIDR for IPv4 clusters", func() { - nodeClass.Spec.AMIFamily = lo.ToPtr(v1.AMIFamilyAL2023) + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} ExpectApplied(ctx, env.Client, nodeClass) ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("10.100.0.0/16")) @@ -82,7 +81,7 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func() }, }, }) - nodeClass.Spec.AMIFamily = lo.ToPtr(v1.AMIFamilyAL2023) + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} ExpectApplied(ctx, env.Client, nodeClass) ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("2001:db8::/64")) diff --git a/pkg/controllers/nodeclass/status/readiness.go b/pkg/controllers/nodeclass/status/readiness.go index 1905267fbeae..ea16052b81de 100644 --- a/pkg/controllers/nodeclass/status/readiness.go +++ b/pkg/controllers/nodeclass/status/readiness.go @@ -19,7 +19,6 @@ import ( "fmt" "github.com/awslabs/operatorpkg/status" - "github.com/samber/lo" "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" @@ -33,10 +32,15 @@ type Readiness struct { } func (n Readiness) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (reconcile.Result, error) { + // TODO: Drop runtime check once support for conversion is dropped, and make AMISelectorTerms required via CEL. + if _, ok := nodeClass.Annotations[v1.AnnotationAMIFamilyCompatibility]; !ok && len(nodeClass.Spec.AMISelectorTerms) == 0 { + nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Invalid AMI configuration") + return reconcile.Result{}, fmt.Errorf("invalid configuration, AMISelectorTerms or 'karpenter.sh/v1beta1-amifamily' compatibility annotation must be specified") + } // A NodeClass that uses AL2023 requires the cluster CIDR for launching nodes. // To allow Karpenter to be used for Non-EKS clusters, resolving the Cluster CIDR // will not be done at startup but instead in a reconcile loop. - if lo.FromPtr(nodeClass.Spec.AMIFamily) == v1.AMIFamilyAL2023 { + if nodeClass.AMIFamily() == v1.AMIFamilyAL2023 { if err := n.launchTemplateProvider.ResolveClusterCIDR(ctx); err != nil { nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Failed to detect the cluster CIDR") return reconcile.Result{}, fmt.Errorf("failed to detect the cluster CIDR, %w", err) diff --git a/pkg/fake/ssmapi.go b/pkg/fake/ssmapi.go index b9d0e1711d84..7d6e6403833e 100644 --- a/pkg/fake/ssmapi.go +++ b/pkg/fake/ssmapi.go @@ -17,11 +17,15 @@ package fake import ( "context" "fmt" + "log" + "regexp" + "strings" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/mitchellh/hashstructure/v2" + "github.com/Pallinder/go-randomdata" + "github.com/samber/lo" + + "github.com/aws/karpenter-provider-aws/pkg/providers/version" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/aws-sdk-go/service/ssm/ssmiface" @@ -29,39 +33,104 @@ import ( type SSMAPI struct { ssmiface.SSMAPI - Parameters map[string]string - GetParameterOutput *ssm.GetParameterOutput - WantErr error + Parameters map[string]string + GetParametersByPathOutput *ssm.GetParametersByPathOutput + WantErr error + + defaultParametersForPath map[string][]*ssm.Parameter } func NewSSMAPI() *SSMAPI { - return &SSMAPI{} + return &SSMAPI{ + defaultParametersForPath: map[string][]*ssm.Parameter{}, + } } -func (a SSMAPI) GetParameterWithContext(_ context.Context, input *ssm.GetParameterInput, _ ...request.Option) (*ssm.GetParameterOutput, error) { +func (a SSMAPI) GetParametersByPathPagesWithContext(_ context.Context, input *ssm.GetParametersByPathInput, f func(*ssm.GetParametersByPathOutput, bool) bool, _ ...request.Option) error { + if !lo.FromPtr(input.Recursive) { + log.Fatalf("fake SSM API currently only supports GetParametersByPathPagesWithContext when recursive is true") + } if a.WantErr != nil { - return nil, a.WantErr + return a.WantErr } - if len(a.Parameters) > 0 { - if amiID, ok := a.Parameters[*input.Name]; ok { - return &ssm.GetParameterOutput{ - Parameter: &ssm.Parameter{Value: aws.String(amiID)}, - }, nil - } - return nil, awserr.New(ssm.ErrCodeParameterNotFound, fmt.Sprintf("%s couldn't be found", *input.Name), nil) + if a.GetParametersByPathOutput != nil { + f(a.GetParametersByPathOutput, true) + return nil + } + if len(a.Parameters) != 0 { + f(&ssm.GetParametersByPathOutput{ + Parameters: lo.FilterMap(lo.Entries(a.Parameters), func(p lo.Entry[string, string], _ int) (*ssm.Parameter, bool) { + // The parameter does not start with the path + if !strings.HasPrefix(p.Key, lo.FromPtr(input.Path)) { + return nil, false + } + // The parameter starts with the input path, but the last segment of the input path is only a subset of the matching segment of the parameters path. + // Ex: "/aws/service/eks-optimized-ami/amazon-linux-2" is a prefix for "/aws/service/eks-optimized-ami/amazon-linux-2-gpu/..." but we shouldn't match + if strings.TrimPrefix(p.Key, lo.FromPtr(input.Path))[0] != '/' { + return nil, false + } + return &ssm.Parameter{ + Name: lo.ToPtr(p.Key), + Value: lo.ToPtr(p.Value), + }, true + }), + }, true) + return nil } - hc, _ := hashstructure.Hash(input.Name, hashstructure.FormatV2, nil) - if a.GetParameterOutput != nil { - return a.GetParameterOutput, nil + if params := a.getDefaultParametersForPath(lo.FromPtr(input.Path)); params != nil { + f(&ssm.GetParametersByPathOutput{Parameters: params}, true) + return nil } + return fmt.Errorf("path %q does not exist", lo.FromPtr(input.Path)) +} - return &ssm.GetParameterOutput{ - Parameter: &ssm.Parameter{Value: aws.String(fmt.Sprintf("test-ami-id-%x", hc))}, - }, nil +func (a SSMAPI) getDefaultParametersForPath(path string) []*ssm.Parameter { + // If we've already generated default parameters, return the same parameters across calls. This ensures we don't + // drift due to different results from one call to the next. + if params, ok := a.defaultParametersForPath[path]; ok { + return params + } + suffixes := map[string][]string{ + `^\/aws\/service\/eks/optimized-ami\/.*\/amazon-linux-2$`: []string{"recommended/image_id"}, + `^\/aws\/service\/eks/optimized-ami\/.*\/amazon-linux-2-arm64$`: []string{"recommended/image_id"}, + `^\/aws\/service\/eks/optimized-ami\/.*\/amazon-linux-2-gpu$`: []string{"recommended/image_id"}, + `^\/aws\/service\/eks/optimized-ami\/.*\/amazon-linux-2023$`: []string{ + "x86_64/standard/recommended/image_id", + "arm64/standard/recommended/image_id", + "x86_64/nvidia/recommended/image_id", + "arm64/nvidia/recommended/image_id", + "x86_64/neuron/recommended/image_id", + "arm64/neuron/recommended/image_id", + }, + `\/aws\/service\/bottlerocket\/aws-k8s-.*`: []string{ + "x86_64/latest/image_id", + "arm64/latest/image_id", + }, + `\/aws\/service\/ami-windows-latest`: lo.FlatMap(version.SupportedK8sVersions(), func(version string, _ int) []string { + return []string{ + fmt.Sprintf("Windows_Server-2019-English-Core-EKS_Optimized-%s/image_id", version), + fmt.Sprintf("Windows_Server-2022-English-Core-EKS_Optimized-%s/image_id", version), + } + }), + } + for matchStr, suffixes := range suffixes { + if !regexp.MustCompile(matchStr).MatchString(path) { + continue + } + params := lo.Map(suffixes, func(suffix string, _ int) *ssm.Parameter { + return &ssm.Parameter{ + Name: lo.ToPtr(fmt.Sprintf("%s/%s", path, suffix)), + Value: lo.ToPtr(fmt.Sprintf("ami-%s", randomdata.Alphanumeric(16))), + } + }) + a.defaultParametersForPath[path] = params + return params + } + return nil } func (a *SSMAPI) Reset() { - a.GetParameterOutput = nil + a.GetParametersByPathOutput = nil a.Parameters = nil a.WantErr = nil } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 1158569fe44c..2f56094a2465 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -61,6 +61,7 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" "github.com/aws/karpenter-provider-aws/pkg/providers/pricing" "github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup" + ssmp "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" "github.com/aws/karpenter-provider-aws/pkg/providers/subnet" "github.com/aws/karpenter-provider-aws/pkg/providers/version" ) @@ -87,6 +88,7 @@ type Operator struct { VersionProvider version.Provider InstanceTypesProvider instancetype.Provider InstanceProvider instance.Provider + SSMProvider ssmp.Provider } func NewOperator(ctx context.Context, operator *operator.Operator) (context.Context, *Operator) { @@ -148,7 +150,8 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont *sess.Config.Region, ) versionProvider := version.NewDefaultProvider(operator.KubernetesInterface, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - amiProvider := amifamily.NewDefaultProvider(versionProvider, ssm.New(sess), ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + ssmProvider := ssmp.NewDefaultProvider(ssm.New(sess), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + amiProvider := amifamily.NewDefaultProvider(versionProvider, ssmProvider, ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) amiResolver := amifamily.NewResolver(amiProvider) launchTemplateProvider := launchtemplate.NewDefaultProvider( ctx, @@ -196,6 +199,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont PricingProvider: pricingProvider, InstanceTypesProvider: instanceTypeProvider, InstanceProvider: instanceProvider, + SSMProvider: ssmProvider, } } diff --git a/pkg/providers/amifamily/al2.go b/pkg/providers/amifamily/al2.go index 078d8e0ddae9..493c4ca449d2 100644 --- a/pkg/providers/amifamily/al2.go +++ b/pkg/providers/amifamily/al2.go @@ -15,12 +15,18 @@ limitations under the License. package amifamily import ( + "context" "fmt" + "regexp" + "strings" "github.com/aws/aws-sdk-go/aws" corev1 "k8s.io/api/core/v1" - karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/samber/lo" + + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/karpenter/pkg/scheduling" v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" @@ -28,6 +34,7 @@ import ( "sigs.k8s.io/karpenter/pkg/cloudprovider" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" ) type AL2 struct { @@ -35,40 +42,58 @@ type AL2 struct { *Options } -// DefaultAMIs returns the AMI name, and Requirements, with an SSM query -func (a AL2) DefaultAMIs(version string) []DefaultAMIOutput { - return []DefaultAMIOutput{ - { - Query: fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpDoesNotExist), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpDoesNotExist), - ), - }, - { - Query: fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-gpu/recommended/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpExists), - ), - }, - { - Query: fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-gpu/recommended/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpExists), - ), - }, - { - Query: fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-%s/recommended/image_id", version, karpv1.ArchitectureArm64), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureArm64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpDoesNotExist), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpDoesNotExist), - ), - }, +func (a AL2) DescribeImageQuery(ctx context.Context, ssmProvider ssm.Provider, k8sVersion string, amiVersion string) (DescribeImageQuery, error) { + imageIDs := make([]*string, 0, 5) + requirements := make(map[string][]scheduling.Requirements) + // Example Paths: + // - Latest EKS 1.30 Standard Image: /aws/service/eks/optimized-ami/1.30/amazon-linux-2/recommended/image_id + // - Specific EKS 1.30 GPU Image: /aws/service/eks/optimized-ami/1.30/amazon-linux-2-gpu/amazon-eks-node-1.30-v20240625/image_id + for rootPath, variants := range map[string][]Variant{ + fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2", k8sVersion): []Variant{VariantStandard}, + fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-arm64", k8sVersion): []Variant{VariantStandard}, + fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-gpu", k8sVersion): []Variant{VariantNeuron, VariantNvidia}, + } { + results, err := ssmProvider.List(ctx, rootPath) + if err != nil { + log.FromContext(ctx).WithValues("path", rootPath, "family", "al2").Error(err, "discovering AMIs from ssm") + continue + } + for path, value := range results { + pathComponents := strings.Split(path, "/") + // Only select image_id paths which match the desired AMI version + if len(pathComponents) != 9 || pathComponents[8] != "image_id" { + continue + } + if av, err := a.extractAMIVersion(pathComponents[7]); err != nil || av != amiVersion { + continue + } + imageIDs = append(imageIDs, lo.ToPtr(value)) + requirements[value] = lo.Map(variants, func(v Variant, _ int) scheduling.Requirements { return v.Requirements() }) + } + } + // Failed to discover any AMIs, we should short circuit AMI discovery + if len(imageIDs) == 0 { + return DescribeImageQuery{}, fmt.Errorf(`failed to discover any AMIs for alias "al2@%s"`, amiVersion) + } + return DescribeImageQuery{ + Filters: []*ec2.Filter{{ + Name: lo.ToPtr("image-id"), + Values: imageIDs, + }}, + KnownRequirements: requirements, + }, nil +} + +func (a AL2) extractAMIVersion(versionStr string) (string, error) { + if versionStr == "recommended" { + return AMIVersionLatest, nil + } + rgx := regexp.MustCompile(`^.*(v\d{8})$`) + matches := rgx.FindStringSubmatch(versionStr) + if len(matches) != 2 { + return "", fmt.Errorf("failed to extract AMI version") } + return matches[1], nil } // UserData returns the exact same string for equivalent input, diff --git a/pkg/providers/amifamily/al2023.go b/pkg/providers/amifamily/al2023.go index 94f95321e23b..535074db3bd9 100644 --- a/pkg/providers/amifamily/al2023.go +++ b/pkg/providers/amifamily/al2023.go @@ -15,16 +15,22 @@ limitations under the License. package amifamily import ( + "context" "fmt" + "regexp" + "strings" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" - karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/scheduling" + "github.com/aws/aws-sdk-go/service/ec2" + v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" ) type AL2023 struct { @@ -32,21 +38,72 @@ type AL2023 struct { *Options } -func (a AL2023) DefaultAMIs(version string) []DefaultAMIOutput { - return []DefaultAMIOutput{ - { - Query: fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - ), - }, - { - Query: fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/arm64/standard/recommended/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureArm64), - ), - }, +func (a AL2023) DescribeImageQuery(ctx context.Context, ssmProvider ssm.Provider, k8sVersion string, amiVersion string) (DescribeImageQuery, error) { + requirements := make(map[string][]scheduling.Requirements) + imageIDs := make([]*string, 0, 5) + // Example Paths: + // - Latest EKS 1.30 arm64 Standard Image: /aws/service/eks/optimized-ami/1.30/amazon-linux-2023/arm64/standard/recommended/image_id + // - Specific EKS 1.30 amd64 Nvidia Image: /aws/service/eks/optimized-ami/1.30/amazon-linux-2023/x86_64/nvidia/amazon-eks-node-al2023-x86_64-nvidia-1.30-v20240625/image_id + rootPath := fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023", k8sVersion) + results, err := ssmProvider.List(ctx, rootPath) + if err != nil { + log.FromContext(ctx).WithValues("path", rootPath, "family", "al2023").Error(err, "discovering AMIs from ssm") + return DescribeImageQuery{}, fmt.Errorf(`failed to discover any AMIs for alias "al2023@%s"`, amiVersion) + } + + ids := map[string]Variant{} + for path, value := range results { + pathComponents := strings.Split(path, "/") + if len(pathComponents) != 11 || pathComponents[10] != "image_id" { + continue + } + if av, err := a.extractAMIVersion(pathComponents[9]); err != nil || av != amiVersion { + continue + } + variant, err := NewVariant(pathComponents[8]) + if err != nil { + continue + } + ids[value] = variant + } + + // EKS doesn't currently vend any accelerated AL2023 AMIs. We should schedule all workloads to + // these standard AMIs until accelerated AMIs are available. This approach ensures Karpenter is + // forwards compatible with acclerated AMIs once they become available. + hasAcceleratedAMIs := lo.ContainsBy(lo.Values(ids), func(v Variant) bool { + return v != VariantStandard + }) + for id, variant := range ids { + imageIDs = append(imageIDs, lo.ToPtr(id)) + if hasAcceleratedAMIs { + requirements[id] = []scheduling.Requirements{variant.Requirements()} + } + } + + // Failed to discover any AMIs, we should short circuit AMI discovery + if len(imageIDs) == 0 { + return DescribeImageQuery{}, fmt.Errorf(`failed to discover AMIs for alias "al2023@%s"`, amiVersion) + } + + return DescribeImageQuery{ + Filters: []*ec2.Filter{{ + Name: lo.ToPtr("image-id"), + Values: imageIDs, + }}, + KnownRequirements: requirements, + }, nil +} + +func (a AL2023) extractAMIVersion(versionStr string) (string, error) { + if versionStr == "recommended" { + return AMIVersionLatest, nil + } + rgx := regexp.MustCompile(`^.*(v\d{8})$`) + matches := rgx.FindStringSubmatch(versionStr) + if len(matches) != 2 { + return "", fmt.Errorf("failed to extract AMI version") } + return matches[1], nil } func (a AL2023) UserData(kubeletConfig *v1.KubeletConfiguration, taints []corev1.Taint, labels map[string]string, caBundle *string, _ []*cloudprovider.InstanceType, customUserData *string, instanceStorePolicy *v1.InstanceStorePolicy) bootstrap.Bootstrapper { diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go index 90ba3a30de8b..723c18a9071c 100644 --- a/pkg/providers/amifamily/ami.go +++ b/pkg/providers/amifamily/ami.go @@ -17,19 +17,15 @@ package amifamily import ( "context" "fmt" - "sort" "sync" "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" - "github.com/aws/aws-sdk-go/service/ssm" - "github.com/aws/aws-sdk-go/service/ssm/ssmiface" "github.com/mitchellh/hashstructure/v2" "github.com/patrickmn/go-cache" "github.com/samber/lo" - corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/log" v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" @@ -38,6 +34,8 @@ import ( "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/scheduling" "sigs.k8s.io/karpenter/pkg/utils/pretty" + + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" ) type Provider interface { @@ -47,55 +45,19 @@ type Provider interface { type DefaultProvider struct { sync.Mutex cache *cache.Cache - ssm ssmiface.SSMAPI ec2api ec2iface.EC2API cm *pretty.ChangeMonitor versionProvider version.Provider + ssmProvider ssm.Provider } -type AMI struct { - Name string - AmiID string - CreationDate string - Requirements scheduling.Requirements -} - -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() { - sort.Slice(a, func(i, j int) bool { - itime, _ := time.Parse(time.RFC3339, a[i].CreationDate) - jtime, _ := time.Parse(time.RFC3339, a[j].CreationDate) - if itime.Unix() != jtime.Unix() { - return itime.Unix() > jtime.Unix() - } - return a[i].AmiID < a[j].AmiID - }) -} - -// MapToInstanceTypes returns a map of AMIIDs that are the most recent on creationDate to compatible instancetypes -func MapToInstanceTypes(instanceTypes []*cloudprovider.InstanceType, amis []v1.AMI) map[string][]*cloudprovider.InstanceType { - amiIDs := map[string][]*cloudprovider.InstanceType{} - for _, instanceType := range instanceTypes { - for _, ami := range amis { - if err := instanceType.Requirements.Compatible(scheduling.NewNodeSelectorRequirements(ami.Requirements...), scheduling.AllowUndefinedWellKnownLabels); err == nil { - amiIDs[ami.ID] = append(amiIDs[ami.ID], instanceType) - break - } - } - } - return amiIDs -} - -func NewDefaultProvider(versionProvider version.Provider, ssm ssmiface.SSMAPI, ec2api ec2iface.EC2API, cache *cache.Cache) *DefaultProvider { +func NewDefaultProvider(versionProvider version.Provider, ssmProvider ssm.Provider, ec2api ec2iface.EC2API, cache *cache.Cache) *DefaultProvider { return &DefaultProvider{ cache: cache, - ssm: ssm, ec2api: ec2api, cm: pretty.NewChangeMonitor(), versionProvider: versionProvider, + ssmProvider: ssmProvider, } } @@ -103,19 +65,13 @@ func NewDefaultProvider(versionProvider version.Provider, ssm ssmiface.SSMAPI, e func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1.EC2NodeClass) (AMIs, error) { p.Lock() defer p.Unlock() - - var err error - var amis AMIs - if len(nodeClass.Spec.AMISelectorTerms) == 0 { - amis, err = p.getDefaultAMIs(ctx, nodeClass) - if err != nil { - return nil, err - } - } else { - amis, err = p.getAMIs(ctx, nodeClass.Spec.AMISelectorTerms) - if err != nil { - return nil, err - } + queries, err := p.DescribeImageQueries(ctx, nodeClass) + if err != nil { + return nil, fmt.Errorf("getting AMI queries, %w", err) + } + amis, err := p.amis(ctx, queries) + if err != nil { + return nil, err } amis.Sort() uniqueAMIs := lo.Uniq(lo.Map(amis, func(a AMI, _ int) string { return a.AmiID })) @@ -126,129 +82,40 @@ func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1.EC2NodeClass) return amis, nil } -func (p *DefaultProvider) getDefaultAMIs(ctx context.Context, nodeClass *v1.EC2NodeClass) (res AMIs, err error) { - if images, ok := p.cache.Get(lo.FromPtr(nodeClass.Spec.AMIFamily)); ok { - // Ensure what's returned from this function is a deep-copy of AMIs so alterations - // to the data don't affect the original - return append(AMIs{}, images.(AMIs)...), nil - } - amiFamily := GetAMIFamily(nodeClass.Spec.AMIFamily, &Options{}) - kubernetesVersion, err := p.versionProvider.Get(ctx) - if err != nil { - return nil, fmt.Errorf("getting kubernetes version %w", err) - } - defaultAMIs := amiFamily.DefaultAMIs(kubernetesVersion) - for _, ami := range defaultAMIs { - if id, err := p.resolveSSMParameter(ctx, ami.Query); err != nil { - log.FromContext(ctx).WithValues("query", ami.Query).Error(err, "failed discovering amis from ssm") - } else { - res = append(res, AMI{AmiID: id, Requirements: ami.Requirements}) - } - } - // Resolve Name and CreationDate information into the DefaultAMIs - if err = p.ec2api.DescribeImagesPagesWithContext(ctx, &ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{{Name: aws.String("image-id"), Values: aws.StringSlice(lo.Map(res, func(a AMI, _ int) string { return a.AmiID }))}}, - MaxResults: aws.Int64(500), - }, func(page *ec2.DescribeImagesOutput, _ bool) bool { - for i := range page.Images { - for j := range res { - if res[j].AmiID == aws.StringValue(page.Images[i].ImageId) { - res[j].Name = aws.StringValue(page.Images[i].Name) - res[j].CreationDate = aws.StringValue(page.Images[i].CreationDate) - } - } +func (p *DefaultProvider) DescribeImageQueries(ctx context.Context, nodeClass *v1.EC2NodeClass) ([]DescribeImageQuery, error) { + // Aliases are mutually exclusive, both on the term level and field level within a term. + // This is enforced by a CEL validation, we will treat this as an invariant. + if amiFamilyKey := nodeClass.AMIFamily(); amiFamilyKey != v1.AMIFamilyCustom { + amiVersion := nodeClass.AMIVersion() + amiFamily := GetAMIFamily(&amiFamilyKey, nil) + kubernetesVersion, err := p.versionProvider.Get(ctx) + if err != nil { + return nil, fmt.Errorf("getting kubernetes version, %w", err) } - return true - }); err != nil { - return nil, fmt.Errorf("describing images, %w", err) - } - p.cache.SetDefault(lo.FromPtr(nodeClass.Spec.AMIFamily), res) - return res, nil -} - -func (p *DefaultProvider) resolveSSMParameter(ctx context.Context, ssmQuery string) (string, error) { - output, err := p.ssm.GetParameterWithContext(ctx, &ssm.GetParameterInput{Name: aws.String(ssmQuery)}) - if err != nil { - return "", fmt.Errorf("getting ssm parameter %q, %w", ssmQuery, err) - } - ami := aws.StringValue(output.Parameter.Value) - return ami, nil -} - -func (p *DefaultProvider) getAMIs(ctx context.Context, terms []v1.AMISelectorTerm) (AMIs, error) { - filterAndOwnerSets := GetFilterAndOwnerSets(terms) - hash, err := hashstructure.Hash(filterAndOwnerSets, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) - if err != nil { - return nil, err - } - if images, ok := p.cache.Get(fmt.Sprintf("%d", hash)); ok { - // Ensure what's returned from this function is a deep-copy of AMIs so alterations - // to the data don't affect the original - return append(AMIs{}, images.(AMIs)...), nil - } - images := map[uint64]AMI{} - for _, filtersAndOwners := range filterAndOwnerSets { - if err = p.ec2api.DescribeImagesPagesWithContext(ctx, &ec2.DescribeImagesInput{ - // Don't include filters in the Describe Images call as EC2 API doesn't allow empty filters. - Filters: lo.Ternary(len(filtersAndOwners.Filters) > 0, filtersAndOwners.Filters, nil), - Owners: lo.Ternary(len(filtersAndOwners.Owners) > 0, aws.StringSlice(filtersAndOwners.Owners), nil), - MaxResults: aws.Int64(1000), - }, func(page *ec2.DescribeImagesOutput, _ bool) bool { - for i := range page.Images { - reqs := p.getRequirementsFromImage(page.Images[i]) - if !v1.WellKnownArchitectures.Has(reqs.Get(corev1.LabelArchStable).Any()) { - continue - } - reqsHash := lo.Must(hashstructure.Hash(reqs.NodeSelectorRequirements(), hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})) - // If the proposed image is newer, store it so that we can return it - if v, ok := images[reqsHash]; ok { - candidateCreationTime, _ := time.Parse(time.RFC3339, lo.FromPtr(page.Images[i].CreationDate)) - existingCreationTime, _ := time.Parse(time.RFC3339, v.CreationDate) - if existingCreationTime == candidateCreationTime && lo.FromPtr(page.Images[i].Name) < v.Name { - continue - } - if candidateCreationTime.Unix() < existingCreationTime.Unix() { - continue - } - } - 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, - } - } - return true - }); err != nil { - return nil, fmt.Errorf("describing images, %w", err) + query, err := amiFamily.DescribeImageQuery(ctx, p.ssmProvider, kubernetesVersion, amiVersion) + if err != nil { + return []DescribeImageQuery{}, err } + return []DescribeImageQuery{query}, nil } - p.cache.SetDefault(fmt.Sprintf("%d", hash), AMIs(lo.Values(images))) - return lo.Values(images), nil -} -type FiltersAndOwners struct { - Filters []*ec2.Filter - Owners []string -} - -func GetFilterAndOwnerSets(terms []v1.AMISelectorTerm) (res []FiltersAndOwners) { idFilter := &ec2.Filter{Name: aws.String("image-id")} - for _, term := range terms { + queries := []DescribeImageQuery{} + for _, term := range nodeClass.Spec.AMISelectorTerms { switch { case term.ID != "": idFilter.Values = append(idFilter.Values, aws.String(term.ID)) default: - elem := FiltersAndOwners{ + query := DescribeImageQuery{ Owners: lo.Ternary(term.Owner != "", []string{term.Owner}, []string{}), } if term.Name != "" { // Default owners to self,amazon to ensure Karpenter only discovers cross-account AMIs if the user specifically allows it. // Removing this default would cause Karpenter to discover publicly shared AMIs passing the name filter. - elem = FiltersAndOwners{ + query = DescribeImageQuery{ Owners: lo.Ternary(term.Owner != "", []string{term.Owner}, []string{"self", "amazon"}), } - elem.Filters = append(elem.Filters, &ec2.Filter{ + query.Filters = append(query.Filters, &ec2.Filter{ Name: aws.String("name"), Values: aws.StringSlice([]string{term.Name}), }) @@ -256,33 +123,90 @@ func GetFilterAndOwnerSets(terms []v1.AMISelectorTerm) (res []FiltersAndOwners) } for k, v := range term.Tags { if v == "*" { - elem.Filters = append(elem.Filters, &ec2.Filter{ + query.Filters = append(query.Filters, &ec2.Filter{ Name: aws.String("tag-key"), Values: []*string{aws.String(k)}, }) } else { - elem.Filters = append(elem.Filters, &ec2.Filter{ + query.Filters = append(query.Filters, &ec2.Filter{ Name: aws.String(fmt.Sprintf("tag:%s", k)), Values: []*string{aws.String(v)}, }) } } - res = append(res, elem) + queries = append(queries, query) } } if len(idFilter.Values) > 0 { - res = append(res, FiltersAndOwners{Filters: []*ec2.Filter{idFilter}}) + queries = append(queries, DescribeImageQuery{Filters: []*ec2.Filter{idFilter}}) } - return res + return queries, nil } -func (p *DefaultProvider) getRequirementsFromImage(ec2Image *ec2.Image) scheduling.Requirements { - requirements := scheduling.NewRequirements() - // Always add the architecture of an image as a requirement, irrespective of what's specified in EC2 tags. - architecture := *ec2Image.Architecture - if value, ok := v1.AWSToKubeArchitectures[architecture]; ok { - architecture = value +//nolint:gocyclo +func (p *DefaultProvider) amis(ctx context.Context, queries []DescribeImageQuery) (AMIs, error) { + hash, err := hashstructure.Hash(queries, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) + if err != nil { + return nil, err } - requirements.Add(scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, architecture)) - return requirements + if images, ok := p.cache.Get(fmt.Sprintf("%d", hash)); ok { + // Ensure what's returned from this function is a deep-copy of AMIs so alterations + // to the data don't affect the original + return append(AMIs{}, images.(AMIs)...), nil + } + images := map[uint64]AMI{} + for _, query := range queries { + if err = p.ec2api.DescribeImagesPagesWithContext(ctx, query.DescribeImagesInput(), func(page *ec2.DescribeImagesOutput, _ bool) bool { + for _, image := range page.Images { + arch, ok := v1.AWSToKubeArchitectures[lo.FromPtr(image.Architecture)] + if !ok { + continue + } + // Each image may have multiple associated sets of requirements. For example, an image may be compatible with Neuron instances + // and GPU instances. In that case, we'll have a set of requirements for each, and will create one "image" for each. + for _, reqs := range query.RequirementsForImageWithArchitecture(lo.FromPtr(image.ImageId), arch) { + // If we already have an image with the same set of requirements, but this image is newer, replace the previous image. + reqsHash := lo.Must(hashstructure.Hash(reqs.NodeSelectorRequirements(), hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})) + if v, ok := images[reqsHash]; ok { + candidateCreationTime, _ := time.Parse(time.RFC3339, lo.FromPtr(image.CreationDate)) + existingCreationTime, _ := time.Parse(time.RFC3339, v.CreationDate) + if existingCreationTime == candidateCreationTime && lo.FromPtr(image.Name) < v.Name { + continue + } + if candidateCreationTime.Unix() < existingCreationTime.Unix() { + continue + } + } + images[reqsHash] = AMI{ + Name: lo.FromPtr(image.Name), + AmiID: lo.FromPtr(image.ImageId), + CreationDate: lo.FromPtr(image.CreationDate), + Requirements: reqs, + } + } + } + return true + }); err != nil { + return nil, fmt.Errorf("describing images, %w", err) + } + } + p.cache.SetDefault(fmt.Sprintf("%d", hash), AMIs(lo.Values(images))) + return lo.Values(images), nil +} + +// MapToInstanceTypes returns a map of AMIIDs that are the most recent on creationDate to compatible instancetypes +func MapToInstanceTypes(instanceTypes []*cloudprovider.InstanceType, amis []v1.AMI) map[string][]*cloudprovider.InstanceType { + amiIDs := map[string][]*cloudprovider.InstanceType{} + for _, instanceType := range instanceTypes { + for _, ami := range amis { + if err := instanceType.Requirements.Compatible( + scheduling.NewNodeSelectorRequirements(ami.Requirements...), + scheduling.AllowUndefinedWellKnownLabels, + ); err == nil { + amiIDs[ami.ID] = append(amiIDs[ami.ID], instanceType) + break + } + } + } + return amiIDs } diff --git a/pkg/providers/amifamily/bottlerocket.go b/pkg/providers/amifamily/bottlerocket.go index 084004988697..703784275f30 100644 --- a/pkg/providers/amifamily/bottlerocket.go +++ b/pkg/providers/amifamily/bottlerocket.go @@ -15,19 +15,23 @@ limitations under the License. package amifamily import ( + "context" "fmt" + "strings" "github.com/samber/lo" - karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" + "sigs.k8s.io/controller-runtime/pkg/log" v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/scheduling" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" ) @@ -37,54 +41,43 @@ type Bottlerocket struct { *Options } -// DefaultAMIs returns the AMI name, and Requirements, with an SSM query -func (b Bottlerocket) DefaultAMIs(version string) []DefaultAMIOutput { - return []DefaultAMIOutput{ - { - Query: fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpDoesNotExist), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpDoesNotExist), - ), - }, - { - Query: fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s-nvidia/x86_64/latest/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpExists), - ), - }, - { - Query: fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s-nvidia/x86_64/latest/image_id", version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpExists), - ), - }, - { - Query: fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/%s/latest/image_id", version, karpv1.ArchitectureArm64), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureArm64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpDoesNotExist), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpDoesNotExist), - ), - }, - { - Query: fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s-nvidia/%s/latest/image_id", version, karpv1.ArchitectureArm64), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureArm64), - scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpExists), - ), - }, - { - Query: fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s-nvidia/%s/latest/image_id", version, karpv1.ArchitectureArm64), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureArm64), - scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpExists), - ), - }, +func (b Bottlerocket) DescribeImageQuery(ctx context.Context, ssmProvider ssm.Provider, k8sVersion string, amiVersion string) (DescribeImageQuery, error) { + imageIDs := make([]*string, 0, 5) + requirements := make(map[string][]scheduling.Requirements) + // Example Paths: + // - Latest EKS 1.30 amd64 Standard Image: /aws/service/bottlerocket/aws-k8s-1.30/x86_64/latest/image_id + // - Specific EKS 1.30 arm64 Nvidia Image: /aws/service/bottlerocket/aws-k8s-1.30-nvidia/arm64/1.10.0/image_id + for rootPath, variants := range map[string][]Variant{ + fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s", k8sVersion): []Variant{VariantStandard}, + fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s-nvidia", k8sVersion): []Variant{VariantNeuron, VariantNvidia}, + } { + results, err := ssmProvider.List(ctx, rootPath) + if err != nil { + log.FromContext(ctx).WithValues("path", rootPath, "family", "bottlerocket").Error(err, "discovering AMIs from ssm") + continue + } + for path, value := range results { + pathComponents := strings.Split(path, "/") + // Only select image_id paths which match the desired AMI version + // Note: The SSM path doesn't prefix the version with a v, but Bottlerocket's GitHub releases do. We'll support both. + if len(pathComponents) != 8 || pathComponents[7] != "image_id" || pathComponents[6] != strings.TrimPrefix(amiVersion, "v") { + continue + } + imageIDs = append(imageIDs, lo.ToPtr(value)) + requirements[value] = lo.Map(variants, func(v Variant, _ int) scheduling.Requirements { return v.Requirements() }) + } + } + // Failed to discover any AMIs, we should short circuit AMI discovery + if len(imageIDs) == 0 { + return DescribeImageQuery{}, fmt.Errorf(`failed to discover any AMIs for alias "bottlerocket@%s"`, amiVersion) } + return DescribeImageQuery{ + Filters: []*ec2.Filter{{ + Name: lo.ToPtr("image-id"), + Values: imageIDs, + }}, + KnownRequirements: requirements, + }, nil } // UserData returns the default userdata script for the AMI Family diff --git a/pkg/providers/amifamily/custom.go b/pkg/providers/amifamily/custom.go index 3b15d4060bca..6448cad83997 100644 --- a/pkg/providers/amifamily/custom.go +++ b/pkg/providers/amifamily/custom.go @@ -15,6 +15,8 @@ limitations under the License. package amifamily import ( + "context" + corev1 "k8s.io/api/core/v1" "sigs.k8s.io/karpenter/pkg/cloudprovider" @@ -22,6 +24,7 @@ import ( v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" ) type Custom struct { @@ -38,8 +41,8 @@ func (c Custom) UserData(_ *v1.KubeletConfiguration, _ []corev1.Taint, _ map[str } } -func (c Custom) DefaultAMIs(_ string) []DefaultAMIOutput { - return nil +func (c Custom) DescribeImageQuery(_ context.Context, _ ssm.Provider, _ string, _ string) (DescribeImageQuery, error) { + return DescribeImageQuery{}, nil } func (c Custom) DefaultBlockDeviceMappings() []*v1.BlockDeviceMapping { diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index 0c5838af5231..eb1e81690a49 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -15,6 +15,7 @@ limitations under the License. package amifamily import ( + "context" "fmt" "net" @@ -28,6 +29,7 @@ import ( v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" "github.com/aws/karpenter-provider-aws/pkg/utils" "sigs.k8s.io/karpenter/pkg/cloudprovider" @@ -77,7 +79,7 @@ type LaunchTemplate struct { // AMIFamily can be implemented to override the default logic for generating dynamic launch template parameters type AMIFamily interface { - DefaultAMIs(version string) []DefaultAMIOutput + DescribeImageQuery(ctx context.Context, ssmProvider ssm.Provider, k8sVersion string, amiVersion string) (DescribeImageQuery, error) UserData(kubeletConfig *v1.KubeletConfiguration, taints []corev1.Taint, labels map[string]string, caBundle *string, instanceTypes []*cloudprovider.InstanceType, customUserData *string, instanceStorePolicy *v1.InstanceStorePolicy) bootstrap.Bootstrapper DefaultBlockDeviceMappings() []*v1.BlockDeviceMapping DefaultMetadataOptions() *v1.MetadataOptions @@ -120,7 +122,7 @@ func NewResolver(amiProvider Provider) *Resolver { // Resolve generates launch templates using the static options and dynamically generates launch template parameters. // Multiple ResolvedTemplates are returned based on the instanceTypes passed in to support special AMIs for certain instance types like GPUs. func (r Resolver) Resolve(nodeClass *v1.EC2NodeClass, nodeClaim *karpv1.NodeClaim, instanceTypes []*cloudprovider.InstanceType, capacityType string, options *Options) ([]*LaunchTemplate, error) { - amiFamily := GetAMIFamily(nodeClass.Spec.AMIFamily, options) + amiFamily := GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), options) if len(nodeClass.Status.AMIs) == 0 { return nil, fmt.Errorf("no amis exist given constraints") } @@ -164,8 +166,6 @@ func GetAMIFamily(amiFamily *string, options *Options) AMIFamily { switch aws.StringValue(amiFamily) { case v1.AMIFamilyBottlerocket: return &Bottlerocket{Options: options} - case v1.AMIFamilyUbuntu: - return &Ubuntu{Options: options} case v1.AMIFamilyWindows2019: return &Windows{Options: options, Version: v1.Windows2019, Build: v1.Windows2019Build} case v1.AMIFamilyWindows2022: diff --git a/pkg/providers/amifamily/suite_test.go b/pkg/providers/amifamily/suite_test.go index 57e1f2f0c8dd..8e9110322e0d 100644 --- a/pkg/providers/amifamily/suite_test.go +++ b/pkg/providers/amifamily/suite_test.go @@ -134,7 +134,7 @@ var _ = Describe("AMIProvider", func() { nodeClass = test.EC2NodeClass() }) It("should succeed to resolve AMIs (AL2)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): amd64AMI, fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-gpu/recommended/image_id", version): amd64NvidiaAMI, @@ -145,7 +145,7 @@ var _ = Describe("AMIProvider", func() { Expect(amis).To(HaveLen(4)) }) It("should succeed to resolve AMIs (AL2023)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", version): amd64AMI, fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/arm64/standard/recommended/image_id", version): arm64AMI, @@ -155,7 +155,7 @@ var _ = Describe("AMIProvider", func() { Expect(amis).To(HaveLen(2)) }) It("should succeed to resolve AMIs (Bottlerocket)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): amd64AMI, fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s-nvidia/x86_64/latest/image_id", version): amd64NvidiaAMI, @@ -166,18 +166,8 @@ var _ = Describe("AMIProvider", func() { Expect(err).ToNot(HaveOccurred()) Expect(amis).To(HaveLen(6)) }) - It("should succeed to resolve AMIs (Ubuntu)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyUbuntu - awsEnv.SSMAPI.Parameters = map[string]string{ - fmt.Sprintf("/aws/service/canonical/ubuntu/eks/20.04/%s/stable/current/amd64/hvm/ebs-gp2/ami-id", version): amd64AMI, - fmt.Sprintf("/aws/service/canonical/ubuntu/eks/20.04/%s/stable/current/arm64/hvm/ebs-gp2/ami-id", version): arm64AMI, - } - amis, err := awsEnv.AMIProvider.List(ctx, nodeClass) - Expect(err).ToNot(HaveOccurred()) - Expect(amis).To(HaveLen(2)) - }) It("should succeed to resolve AMIs (Windows2019)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2019 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2019@latest"}} awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/ami-windows-latest/Windows_Server-2019-English-Core-EKS_Optimized-%s/image_id", version): amd64AMI, } @@ -186,7 +176,7 @@ var _ = Describe("AMIProvider", func() { Expect(amis).To(HaveLen(1)) }) It("should succeed to resolve AMIs (Windows2022)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2022 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}} awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/ami-windows-latest/Windows_Server-2022-English-Core-EKS_Optimized-%s/image_id", version): amd64AMI, } @@ -194,12 +184,6 @@ var _ = Describe("AMIProvider", func() { Expect(err).ToNot(HaveOccurred()) Expect(amis).To(HaveLen(1)) }) - It("should succeed to resolve AMIs (Custom)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom - amis, err := awsEnv.AMIProvider.List(ctx, nodeClass) - Expect(err).ToNot(HaveOccurred()) - Expect(amis).To(HaveLen(0)) - }) It("should not cause data races when calling Get() simultaneously", func() { nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ { @@ -245,7 +229,7 @@ var _ = Describe("AMIProvider", func() { }) Context("SSM Alias Missing", func() { It("should succeed to partially resolve AMIs if all SSM aliases don't exist (Al2)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} // No GPU AMI exists here awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): amd64AMI, @@ -257,7 +241,7 @@ var _ = Describe("AMIProvider", func() { Expect(amis).To(HaveLen(2)) }) It("should succeed to partially resolve AMIs if all SSM aliases don't exist (AL2023)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", version): amd64AMI, } @@ -266,7 +250,7 @@ var _ = Describe("AMIProvider", func() { Expect(amis).To(HaveLen(1)) }) It("should succeed to partially resolve AMIs if all SSM aliases don't exist (Bottlerocket)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} // No GPU AMI exists for AM64 here awsEnv.SSMAPI.Parameters = map[string]string{ fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): amd64AMI, @@ -278,17 +262,6 @@ var _ = Describe("AMIProvider", func() { Expect(err).ToNot(HaveOccurred()) Expect(amis).To(HaveLen(4)) }) - It("should succeed to partially resolve AMIs if all SSM aliases don't exist (Ubuntu)", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyUbuntu - // No AMD64 AMI exists here - awsEnv.SSMAPI.Parameters = map[string]string{ - fmt.Sprintf("/aws/service/canonical/ubuntu/eks/20.04/%s/stable/current/arm64/hvm/ebs-gp2/ami-id", version): arm64AMI, - } - // Only 1 of the requirements sets for the SSM aliases will resolve - amis, err := awsEnv.AMIProvider.List(ctx, nodeClass) - Expect(err).ToNot(HaveOccurred()) - Expect(amis).To(HaveLen(1)) - }) }) Context("AMI Tag Requirements", func() { var img *ec2.Image @@ -334,15 +307,17 @@ var _ = Describe("AMIProvider", func() { // When you tag public or shared resources, the tags you assign are available only to your AWS account; no other AWS account will have access to those tags // https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html#tag-restrictions It("should have empty owners and use tags when prefixes aren't set", func() { - amiSelectorTerms := []v1.AMISelectorTerm{ - { - Tags: map[string]string{ - "Name": "my-ami", - }, + queries, err := awsEnv.AMIProvider.DescribeImageQueries(ctx, &v1.EC2NodeClass{ + Spec: v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{{ + Tags: map[string]string{ + "Name": "my-ami", + }, + }}, }, - } - filterAndOwnersSets := amifamily.GetFilterAndOwnerSets(amiSelectorTerms) - ExpectConsistsOfFiltersAndOwners([]amifamily.FiltersAndOwners{ + }) + Expect(err).To(BeNil()) + ExpectConsistsOfAMIQueries([]amifamily.DescribeImageQuery{ { Filters: []*ec2.Filter{ { @@ -352,16 +327,18 @@ var _ = Describe("AMIProvider", func() { }, Owners: []string{}, }, - }, filterAndOwnersSets) + }, queries) }) It("should have default owners and use name when prefixed", func() { - amiSelectorTerms := []v1.AMISelectorTerm{ - { - Name: "my-ami", + queries, err := awsEnv.AMIProvider.DescribeImageQueries(ctx, &v1.EC2NodeClass{ + Spec: v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{{ + Name: "my-ami", + }}, }, - } - filterAndOwnersSets := amifamily.GetFilterAndOwnerSets(amiSelectorTerms) - ExpectConsistsOfFiltersAndOwners([]amifamily.FiltersAndOwners{ + }) + Expect(err).To(BeNil()) + ExpectConsistsOfAMIQueries([]amifamily.DescribeImageQuery{ { Filters: []*ec2.Filter{ { @@ -374,19 +351,23 @@ var _ = Describe("AMIProvider", func() { "self", }, }, - }, filterAndOwnersSets) + }, queries) }) It("should not set owners when legacy ids are passed", func() { - amiSelectorTerms := []v1.AMISelectorTerm{ - { - ID: "ami-abcd1234", - }, - { - ID: "ami-cafeaced", + queries, err := awsEnv.AMIProvider.DescribeImageQueries(ctx, &v1.EC2NodeClass{ + Spec: v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{ + { + ID: "ami-abcd1234", + }, + { + ID: "ami-cafeaced", + }, + }, }, - } - filterAndOwnersSets := amifamily.GetFilterAndOwnerSets(amiSelectorTerms) - ExpectConsistsOfFiltersAndOwners([]amifamily.FiltersAndOwners{ + }) + Expect(err).To(BeNil()) + ExpectConsistsOfAMIQueries([]amifamily.DescribeImageQuery{ { Filters: []*ec2.Filter{ { @@ -395,40 +376,48 @@ var _ = Describe("AMIProvider", func() { }, }, }, - }, filterAndOwnersSets) + }, queries) }) It("should allow only specifying owners", func() { - amiSelectorTerms := []v1.AMISelectorTerm{ - { - Owner: "abcdef", - }, - { - Owner: "123456789012", + queries, err := awsEnv.AMIProvider.DescribeImageQueries(ctx, &v1.EC2NodeClass{ + Spec: v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{ + { + Owner: "abcdef", + }, + { + Owner: "123456789012", + }, + }, }, - } - filterAndOwnersSets := amifamily.GetFilterAndOwnerSets(amiSelectorTerms) - ExpectConsistsOfFiltersAndOwners([]amifamily.FiltersAndOwners{ + }) + Expect(err).To(BeNil()) + ExpectConsistsOfAMIQueries([]amifamily.DescribeImageQuery{ { Owners: []string{"abcdef"}, }, { Owners: []string{"123456789012"}, }, - }, filterAndOwnersSets) + }, queries) }) It("should allow prefixed name and prefixed owners", func() { - amiSelectorTerms := []v1.AMISelectorTerm{ - { - Name: "my-name", - Owner: "0123456789", - }, - { - Name: "my-name", - Owner: "self", + queries, err := awsEnv.AMIProvider.DescribeImageQueries(ctx, &v1.EC2NodeClass{ + Spec: v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{ + { + Name: "my-name", + Owner: "0123456789", + }, + { + Name: "my-name", + Owner: "self", + }, + }, }, - } - filterAndOwnersSets := amifamily.GetFilterAndOwnerSets(amiSelectorTerms) - ExpectConsistsOfFiltersAndOwners([]amifamily.FiltersAndOwners{ + }) + Expect(err).To(BeNil()) + ExpectConsistsOfAMIQueries([]amifamily.DescribeImageQuery{ { Owners: []string{"0123456789"}, Filters: []*ec2.Filter{ @@ -447,7 +436,7 @@ var _ = Describe("AMIProvider", func() { }, }, }, - }, filterAndOwnersSets) + }, queries) }) It("should sort amis by creationDate", func() { amis := amifamily.AMIs{ @@ -567,11 +556,11 @@ var _ = Describe("AMIProvider", func() { }) }) -func ExpectConsistsOfFiltersAndOwners(expected, actual []amifamily.FiltersAndOwners) { +func ExpectConsistsOfAMIQueries(expected, actual []amifamily.DescribeImageQuery) { GinkgoHelper() Expect(actual).To(HaveLen(len(expected))) - for _, list := range [][]amifamily.FiltersAndOwners{expected, actual} { + for _, list := range [][]amifamily.DescribeImageQuery{expected, actual} { for _, elem := range list { for _, f := range elem.Filters { sort.Slice(f.Values, func(i, j int) bool { @@ -584,5 +573,5 @@ func ExpectConsistsOfFiltersAndOwners(expected, actual []amifamily.FiltersAndOwn }) } } - Expect(actual).To(ConsistOf(lo.Map(expected, func(f amifamily.FiltersAndOwners, _ int) interface{} { return f })...)) + Expect(actual).To(ConsistOf(lo.Map(expected, func(q amifamily.DescribeImageQuery, _ int) interface{} { return q })...)) } diff --git a/pkg/providers/amifamily/types.go b/pkg/providers/amifamily/types.go new file mode 100644 index 000000000000..ac37d20b06b0 --- /dev/null +++ b/pkg/providers/amifamily/types.go @@ -0,0 +1,119 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package amifamily + +import ( + "fmt" + "sort" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/samber/lo" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/karpenter/pkg/scheduling" + + v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" +) + +const ( + // AMIVersionLatest is the version used in EKS aliases to represent the latest version. This maps to different + // values in the SSM path, depending on the AMI type (e.g. "recommended" for AL2/AL2023)). + AMIVersionLatest = "latest" +) + +type AMI struct { + Name string + AmiID string + CreationDate string + Requirements scheduling.Requirements +} + +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() { + sort.Slice(a, func(i, j int) bool { + itime, _ := time.Parse(time.RFC3339, a[i].CreationDate) + jtime, _ := time.Parse(time.RFC3339, a[j].CreationDate) + if itime.Unix() != jtime.Unix() { + return itime.Unix() > jtime.Unix() + } + return a[i].AmiID < a[j].AmiID + }) +} + +type Variant string + +var ( + VariantStandard Variant = "standard" + VariantNvidia Variant = "nvidia" + VariantNeuron Variant = "neuron" +) + +func NewVariant(v string) (Variant, error) { + var wellKnownVariants = sets.New(VariantStandard, VariantNvidia, VariantNeuron) + variant := Variant(v) + if !wellKnownVariants.Has(variant) { + return variant, fmt.Errorf("%q is not a well-known variant", variant) + } + return variant, nil +} + +func (v Variant) Requirements() scheduling.Requirements { + switch v { + case VariantStandard: + return scheduling.NewRequirements( + scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpDoesNotExist), + scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpDoesNotExist), + ) + case VariantNvidia: + return scheduling.NewRequirements(scheduling.NewRequirement(v1.LabelInstanceAcceleratorCount, corev1.NodeSelectorOpExists)) + case VariantNeuron: + return scheduling.NewRequirements(scheduling.NewRequirement(v1.LabelInstanceGPUCount, corev1.NodeSelectorOpExists)) + } + return nil +} + +type DescribeImageQuery struct { + Filters []*ec2.Filter + Owners []string + // KnownRequirements is a map from image IDs to a set of known requirements. + // When discovering image IDs via SSM we know additional requirements which aren't surfaced by ec2:DescribeImage (e.g. GPU / Neuron compatibility) + // Sometimes, an image may have multiple sets of known requirements. For example, the AL2 GPU AMI is compatible with both Neuron and Nvidia GPU + // instances, which means we need a set of requirements for either instance type. + KnownRequirements map[string][]scheduling.Requirements +} + +func (q DescribeImageQuery) DescribeImagesInput() *ec2.DescribeImagesInput { + return &ec2.DescribeImagesInput{ + // Don't include filters in the Describe Images call as EC2 API doesn't allow empty filters. + Filters: lo.Ternary(len(q.Filters) > 0, q.Filters, nil), + Owners: lo.Ternary(len(q.Owners) > 0, lo.ToSlicePtr(q.Owners), nil), + MaxResults: aws.Int64(1000), + } +} + +func (q DescribeImageQuery) RequirementsForImageWithArchitecture(image string, arch string) []scheduling.Requirements { + if knownRequirements, ok := q.KnownRequirements[image]; ok { + return lo.Map(knownRequirements, func(r scheduling.Requirements, _ int) scheduling.Requirements { + r.Add(scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, arch)) + return r + }) + } + return []scheduling.Requirements{scheduling.NewRequirements(scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, arch))} +} diff --git a/pkg/providers/amifamily/windows.go b/pkg/providers/amifamily/windows.go index e92c801bcf0b..3324e6d1c540 100644 --- a/pkg/providers/amifamily/windows.go +++ b/pkg/providers/amifamily/windows.go @@ -15,9 +15,11 @@ limitations under the License. package amifamily import ( + "context" "fmt" + "regexp" + "strings" - karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" "sigs.k8s.io/karpenter/pkg/scheduling" v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" @@ -26,8 +28,10 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" corev1 "k8s.io/api/core/v1" @@ -37,21 +41,51 @@ import ( type Windows struct { DefaultFamily *Options + // Version is the major version of Windows Server (2019 or 2022). + // Only the core version of each version is supported by Karpenter, so this field only indicates the year. Version string - Build string + // Build is a specific build code associated with the Version + Build string } -func (w Windows) DefaultAMIs(version string) []DefaultAMIOutput { - return []DefaultAMIOutput{ - { - Query: fmt.Sprintf("/aws/service/ami-windows-latest/Windows_Server-%s-English-%s-EKS_Optimized-%s/image_id", w.Version, v1.WindowsCore, version), - Requirements: scheduling.NewRequirements( - scheduling.NewRequirement(corev1.LabelArchStable, corev1.NodeSelectorOpIn, karpv1.ArchitectureAmd64), - scheduling.NewRequirement(corev1.LabelOSStable, corev1.NodeSelectorOpIn, string(corev1.Windows)), - scheduling.NewRequirement(corev1.LabelWindowsBuild, corev1.NodeSelectorOpIn, w.Build), - ), - }, +func (w Windows) DescribeImageQuery(ctx context.Context, ssmProvider ssm.Provider, k8sVersion string, amiVersion string) (DescribeImageQuery, error) { + requirements := make(map[string][]scheduling.Requirements) + imageIDs := make([]*string, 0, 5) + // SSM aliases are only maintained for the latest Windows AMI releases + if amiVersion != AMIVersionLatest { + return DescribeImageQuery{}, fmt.Errorf(`discovering AMIs for alias "windows%s@%s", %q is not a supported version`, w.Version, amiVersion, amiVersion) + } + // Example Path: /aws/service/ami-windows-latest/Windows_Server-2022-English-Core-EKS_Optimized-1.30/image_id + results, err := ssmProvider.List(ctx, "/aws/service/ami-windows-latest") + if err != nil { + return DescribeImageQuery{}, fmt.Errorf("discovering AMIs from ssm") + } + for path, value := range results { + pathComponents := strings.Split(path, "/") + if len(pathComponents) != 6 || pathComponents[5] != "image_id" { + continue + } + matches := regexp.MustCompile(`^Windows_Server-(\d+)-English-Core-EKS_Optimized-(\d\.\d+)$`).FindStringSubmatch(pathComponents[4]) + if len(matches) != 3 || matches[1] != w.Version || matches[2] != k8sVersion { + continue + } + imageIDs = append(imageIDs, lo.ToPtr(value)) + requirements[value] = []scheduling.Requirements{scheduling.NewRequirements( + scheduling.NewRequirement(corev1.LabelOSStable, corev1.NodeSelectorOpIn, string(corev1.Windows)), + scheduling.NewRequirement(corev1.LabelWindowsBuild, corev1.NodeSelectorOpIn, w.Build), + )} + } + // Failed to discover any AMIs, we should short circuit AMI discovery + if len(imageIDs) == 0 { + return DescribeImageQuery{}, fmt.Errorf(`failed to discover any AMIs for alias "windows%s@%s"`, w.Version, amiVersion) } + return DescribeImageQuery{ + Filters: []*ec2.Filter{&ec2.Filter{ + Name: lo.ToPtr("image-id"), + Values: imageIDs, + }}, + KnownRequirements: requirements, + }, nil } // UserData returns the default userdata script for the AMI Family diff --git a/pkg/providers/instancetype/instancetype.go b/pkg/providers/instancetype/instancetype.go index f34e2c25c039..e95c01bb16bd 100644 --- a/pkg/providers/instancetype/instancetype.go +++ b/pkg/providers/instancetype/instancetype.go @@ -131,8 +131,8 @@ func (p *DefaultProvider) List(ctx context.Context, kc *v1.KubeletConfiguration, subnetZonesHash, kcHash, blockDeviceMappingsHash, - aws.StringValue((*string)(nodeClass.Spec.InstanceStorePolicy)), - aws.StringValue(nodeClass.Spec.AMIFamily), + lo.FromPtr((*string)(nodeClass.Spec.InstanceStorePolicy)), + nodeClass.AMIFamily(), ) if item, ok := p.instanceTypesCache.Get(key); ok { // Ensure what's returned from this function is a shallow-copy of the slice (not a deep-copy of the data itself) @@ -151,7 +151,7 @@ func (p *DefaultProvider) List(ctx context.Context, kc *v1.KubeletConfiguration, if p.cm.HasChanged("zones", allZones) { log.FromContext(ctx).WithValues("zones", allZones.UnsortedList()).V(1).Info("discovered zones") } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) result := lo.Map(p.instanceTypesInfo, func(i *ec2.InstanceTypeInfo, _ int) *cloudprovider.InstanceType { instanceTypeVCPU.With(prometheus.Labels{ instanceTypeLabel: *i.InstanceType, diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index 569647e2b99a..9ff96447c008 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -168,7 +168,9 @@ var _ = Describe("InstanceTypeProvider", func() { }) windowsNodeClass = test.EC2NodeClass(v1.EC2NodeClass{ Spec: v1.EC2NodeClassSpec{ - AMIFamily: &v1.AMIFamilyWindows2022, + AMISelectorTerms: []v1.AMISelectorTerm{{ + Alias: "windows2022@latest", + }}, }, Status: v1.EC2NodeClassStatus{ InstanceProfile: "test-profile", @@ -618,7 +620,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(supportsPodENI()).To(Equal(true)) }) It("should launch vpc.amazonaws.com/PrivateIPv4Address on a compatible instance type", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2022 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: corev1.ResourceRequirements{ @@ -679,7 +681,7 @@ var _ = Describe("InstanceTypeProvider", func() { Values: []string{"test"}, }, }) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2022 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: corev1.ResourceRequirements{ @@ -910,7 +912,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(err).To(BeNil()) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -933,7 +935,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(err).To(BeNil()) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(windowsNodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(windowsNodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1052,7 +1054,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) Context("System Reserved Resources", func() { It("should use defaults when no kubelet is specified", func() { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} it := instancetype.NewInstanceType(ctx, info, @@ -1080,7 +1082,7 @@ var _ = Describe("InstanceTypeProvider", func() { string(corev1.ResourceEphemeralStorage): "10Gi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1102,8 +1104,8 @@ var _ = Describe("InstanceTypeProvider", func() { }) Context("Kube Reserved Resources", func() { It("should use defaults when no kubelet is specified", func() { + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1135,7 +1137,7 @@ var _ = Describe("InstanceTypeProvider", func() { string(corev1.ResourceEphemeralStorage): "2Gi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1174,7 +1176,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "500Mi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1203,7 +1205,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "10%", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1232,7 +1234,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "100%", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1261,7 +1263,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "50Mi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1292,7 +1294,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "500Mi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1324,7 +1326,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "10%", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1353,7 +1355,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "100%", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1371,7 +1373,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(it.Overhead.EvictionThreshold.Memory().String()).To(Equal("0")) }) It("should ignore eviction threshold when using Bottlerocket AMI", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{ SystemReserved: map[string]string{ string(corev1.ResourceMemory): "20Gi", @@ -1386,7 +1388,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "10Gi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1405,8 +1407,8 @@ var _ = Describe("InstanceTypeProvider", func() { }) }) It("should take the default eviction threshold when none is specified", func() { + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1440,7 +1442,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "1Gi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1472,7 +1474,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "5%", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1504,7 +1506,7 @@ var _ = Describe("InstanceTypeProvider", func() { instancetype.MemoryAvailable: "1Gi", }, } - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1528,7 +1530,7 @@ var _ = Describe("InstanceTypeProvider", func() { nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} for _, info := range instanceInfo.InstanceTypes { if *info.InstanceType == "t3.large" { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1546,7 +1548,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(it.Capacity.Pods().Value()).To(BeNumerically("==", 35)) } if *info.InstanceType == "m6idn.32xlarge" { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1572,7 +1574,7 @@ var _ = Describe("InstanceTypeProvider", func() { MaxPods: lo.ToPtr(int32(10)), } for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1597,7 +1599,7 @@ var _ = Describe("InstanceTypeProvider", func() { MaxPods: lo.ToPtr(int32(10)), } for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1626,7 +1628,7 @@ var _ = Describe("InstanceTypeProvider", func() { return *info.InstanceType == "t3.large" }) Expect(ok).To(Equal(true)) - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} it := instancetype.NewInstanceType(ctx, t3Large, @@ -1661,7 +1663,7 @@ var _ = Describe("InstanceTypeProvider", func() { return *info.InstanceType == "t3.large" }) Expect(ok).To(Equal(true)) - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} it := instancetype.NewInstanceType(ctx, t3Large, @@ -1693,7 +1695,7 @@ var _ = Describe("InstanceTypeProvider", func() { PodsPerCore: lo.ToPtr(int32(1)), } for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1719,7 +1721,7 @@ var _ = Describe("InstanceTypeProvider", func() { MaxPods: lo.ToPtr(int32(20)), } for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1740,12 +1742,12 @@ var _ = Describe("InstanceTypeProvider", func() { It("should ignore pods-per-core when using Bottlerocket AMI", func() { instanceInfo, err := awsEnv.EC2API.DescribeInstanceTypesWithContext(ctx, &ec2.DescribeInstanceTypesInput{}) Expect(err).To(BeNil()) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{ PodsPerCore: lo.ToPtr(int32(1)), } for _, info := range instanceInfo.InstanceTypes { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1772,7 +1774,7 @@ var _ = Describe("InstanceTypeProvider", func() { } for _, info := range instanceInfo.InstanceTypes { if *info.InstanceType == "t3.large" { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -1790,7 +1792,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(it.Capacity.Pods().Value()).To(BeNumerically("==", 35)) } if *info.InstanceType == "m6idn.32xlarge" { - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, fake.DefaultRegion, @@ -2185,7 +2187,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) Context("Ephemeral Storage", func() { BeforeEach(func() { - nodeClass.Spec.AMIFamily = aws.String(v1.AMIFamilyAL2) + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.BlockDeviceMappings = []*v1.BlockDeviceMapping{ { DeviceName: aws.String("/dev/xvda"), @@ -2196,7 +2198,6 @@ var _ = Describe("InstanceTypeProvider", func() { } }) It("should default to EBS defaults when volumeSize is not defined in blockDeviceMappings for custom AMIs", func() { - nodeClass.Spec.AMIFamily = aws.String(v1.AMIFamilyCustom) nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ { Tags: map[string]string{ @@ -2230,7 +2231,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) }) It("should default to EBS defaults when volumeSize is not defined in blockDeviceMappings for AL2023 Root volume", func() { - nodeClass.Spec.AMIFamily = aws.String(v1.AMIFamilyAL2023) + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} awsEnv.LaunchTemplateProvider.CABundle = lo.ToPtr("Y2EtYnVuZGxlCg==") awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(lo.ToPtr("10.100.0.0/16")) ExpectApplied(ctx, env.Client, nodePool, nodeClass) @@ -2246,7 +2247,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) }) It("should default to EBS defaults when volumeSize is not defined in blockDeviceMappings for Bottlerocket Root volume", func() { - nodeClass.Spec.AMIFamily = aws.String(v1.AMIFamilyBottlerocket) + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.BlockDeviceMappings[0].DeviceName = aws.String("/dev/xvdb") ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() @@ -2261,21 +2262,6 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(*ltInput.LaunchTemplateData.BlockDeviceMappings[0].Ebs.SnapshotId).To(Equal("snap-xxxxxxxx")) }) }) - It("should default to EBS defaults when volumeSize is not defined in blockDeviceMappings for Ubuntu Root volume", func() { - nodeClass.Spec.AMIFamily = aws.String(v1.AMIFamilyUbuntu) - nodeClass.Spec.BlockDeviceMappings[0].DeviceName = aws.String("/dev/sda1") - ExpectApplied(ctx, env.Client, nodePool, nodeClass) - pod := coretest.UnschedulablePod() - ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) - node := ExpectScheduled(ctx, env.Client, pod) - Expect(*node.Status.Capacity.StorageEphemeral()).To(Equal(resource.MustParse("20Gi"))) - Expect(awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Len()).To(BeNumerically(">=", 1)) - awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.ForEach(func(ltInput *ec2.CreateLaunchTemplateInput) { - Expect(ltInput.LaunchTemplateData.BlockDeviceMappings).To(HaveLen(1)) - Expect(*ltInput.LaunchTemplateData.BlockDeviceMappings[0].DeviceName).To(Equal("/dev/sda1")) - Expect(*ltInput.LaunchTemplateData.BlockDeviceMappings[0].Ebs.SnapshotId).To(Equal("snap-xxxxxxxx")) - }) - }) }) Context("Metadata Options", func() { It("should default metadata options on generated launch template", func() { @@ -2369,7 +2355,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("changes to nodeclass fields should result in a different set of instances types", func() { // We should expect these nodeclass fields to change the result of the instance type // nodeClass.instanceStorePolicy - // nodeClass.amiFamily + // nodeClass.amiSelectorTerms (alias) // nodeClass.blockDeviceMapping.rootVolume // nodeClass.blockDeviceMapping.volumeSize // nodeClass.blockDeviceMapping.deviceName @@ -2383,7 +2369,13 @@ var _ = Describe("InstanceTypeProvider", func() { nodeClassChanges := []*v1.EC2NodeClass{ {}, // Testing the base case black EC2NodeClass {Spec: v1.EC2NodeClassSpec{InstanceStorePolicy: lo.ToPtr(v1.InstanceStorePolicyRAID0)}}, - {Spec: v1.EC2NodeClassSpec{AMIFamily: &v1.AMIFamilyUbuntu}}, + { + Spec: v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{{ + Alias: "bottlerocket@latest", + }}, + }, + }, { Spec: v1.EC2NodeClassSpec{BlockDeviceMappings: []*v1.BlockDeviceMapping{ { diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go index cf724f665b69..32b23ea80e2e 100644 --- a/pkg/providers/launchtemplate/suite_test.go +++ b/pkg/providers/launchtemplate/suite_test.go @@ -594,7 +594,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) Context("Block Device Mappings", func() { It("should default AL2 block device mappings", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) @@ -608,7 +608,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should default AL2023 block device mappings", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} awsEnv.LaunchTemplateProvider.CABundle = lo.ToPtr("Y2EtYnVuZGxlCg==") awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(lo.ToPtr("10.100.0.0/16")) ExpectApplied(ctx, env.Client, nodePool, nodeClass) @@ -624,7 +624,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should use custom block device mapping", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.BlockDeviceMappings = []*v1.BlockDeviceMapping{ { DeviceName: aws.String("/dev/xvda"), @@ -674,7 +674,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should round up for custom block device mappings when specified in gigabytes", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.BlockDeviceMappings = []*v1.BlockDeviceMapping{ { DeviceName: aws.String("/dev/xvda"), @@ -711,7 +711,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should default bottlerocket second volume with root volume size", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) @@ -730,7 +730,6 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should not default block device mappings for custom AMIFamilies", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() @@ -742,7 +741,6 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should use custom block device mapping for custom AMIFamilies", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} nodeClass.Spec.BlockDeviceMappings = []*v1.BlockDeviceMapping{ { @@ -909,7 +907,6 @@ var _ = Describe("LaunchTemplate Provider", func() { ExpectScheduled(ctx, env.Client, pod) }) It("should pack pods using blockdevicemappings for Custom AMIFamily", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} nodeClass.Spec.BlockDeviceMappings = []*v1.BlockDeviceMapping{ { @@ -939,7 +936,6 @@ var _ = Describe("LaunchTemplate Provider", func() { ExpectScheduled(ctx, env.Client, pod) }) It("should pack pods using the configured root volume in blockdevicemappings", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} nodeClass.Spec.BlockDeviceMappings = []*v1.BlockDeviceMapping{ { @@ -1008,9 +1004,9 @@ var _ = Describe("LaunchTemplate Provider", func() { VMMemoryOverheadPercent: lo.ToPtr[float64](0), })) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, "", @@ -1062,9 +1058,9 @@ var _ = Describe("LaunchTemplate Provider", func() { VMMemoryOverheadPercent: lo.ToPtr[float64](0), })) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{} - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, "", @@ -1088,9 +1084,9 @@ var _ = Describe("LaunchTemplate Provider", func() { VMMemoryOverheadPercent: lo.ToPtr[float64](0), })) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{MaxPods: lo.ToPtr[int32](110)} - amiFamily := amifamily.GetAMIFamily(nodeClass.Spec.AMIFamily, &amifamily.Options{}) + amiFamily := amifamily.GetAMIFamily(lo.ToPtr(nodeClass.AMIFamily()), &amifamily.Options{}) it := instancetype.NewInstanceType(ctx, info, "", @@ -1360,7 +1356,7 @@ var _ = Describe("LaunchTemplate Provider", func() { ExpectLaunchTemplatesCreatedWithUserDataNotContaining(corev1.LabelNamespaceNodeRestriction) }) It("should specify --local-disks raid0 when instance-store policy is set on AL2", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.InstanceStorePolicy = lo.ToPtr(v1.InstanceStorePolicyRAID0) ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() @@ -1370,7 +1366,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) Context("Bottlerocket", func() { BeforeEach(func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{MaxPods: lo.ToPtr[int32](110)} }) It("should merge in custom user data", func() { @@ -1644,7 +1640,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) Context("AL2023", func() { BeforeEach(func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} // base64 encoded version of "ca-bundle" to ensure the nodeadm bootstrap provider can decode successfully awsEnv.LaunchTemplateProvider.CABundle = lo.ToPtr("Y2EtYnVuZGxlCg==") @@ -1866,7 +1862,6 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should copy over userData untouched when AMIFamily is Custom", func() { nodeClass.Spec.UserData = aws.String("special user data") nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Status.AMIs = []v1.AMI{ { ID: "ami-123", @@ -2074,7 +2069,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Context("Windows Custom UserData", func() { BeforeEach(func() { nodePool.Spec.Template.Spec.Requirements = []karpv1.NodeSelectorRequirementWithMinValues{{NodeSelectorRequirement: corev1.NodeSelectorRequirement{Key: corev1.LabelOSStable, Operator: corev1.NodeSelectorOpIn, Values: []string{string(corev1.Windows)}}}} - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2022 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}} nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{MaxPods: lo.ToPtr[int32](110)} }) It("should merge and bootstrap with custom user data", func() { @@ -2114,7 +2109,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) Context("Detailed Monitoring", func() { It("should default detailed monitoring to off", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) @@ -2125,7 +2120,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should pass detailed monitoring setting to the launch template at creation", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.DetailedMonitoring = aws.Bool(true) ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() diff --git a/pkg/providers/securitygroup/suite_test.go b/pkg/providers/securitygroup/suite_test.go index 7adcbc95cb8e..6a2e18fe8111 100644 --- a/pkg/providers/securitygroup/suite_test.go +++ b/pkg/providers/securitygroup/suite_test.go @@ -70,7 +70,9 @@ var _ = BeforeEach(func() { ctx = options.ToContext(ctx, test.Options()) nodeClass = test.EC2NodeClass(v1.EC2NodeClass{ Spec: v1.EC2NodeClassSpec{ - AMIFamily: aws.String(v1.AMIFamilyAL2), + AMISelectorTerms: []v1.AMISelectorTerm{{ + Alias: "al2@latest", + }}, SubnetSelectorTerms: []v1.SubnetSelectorTerm{ { Tags: map[string]string{ diff --git a/pkg/providers/ssm/provider.go b/pkg/providers/ssm/provider.go new file mode 100644 index 000000000000..216615573713 --- /dev/null +++ b/pkg/providers/ssm/provider.go @@ -0,0 +1,70 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ssm + +import ( + "context" + "fmt" + "sync" + + "github.com/aws/aws-sdk-go/service/ssm" + "github.com/aws/aws-sdk-go/service/ssm/ssmiface" + "github.com/patrickmn/go-cache" + "github.com/samber/lo" +) + +type Provider interface { + List(context.Context, string) (map[string]string, error) +} + +type DefaultProvider struct { + sync.Mutex + cache *cache.Cache + ssmapi ssmiface.SSMAPI +} + +func NewDefaultProvider(ssmapi ssmiface.SSMAPI, cache *cache.Cache) *DefaultProvider { + return &DefaultProvider{ + ssmapi: ssmapi, + cache: cache, + } +} + +// List calls GetParametersByPath recursively with the provided input path. +// The result is a map of paths to values for those paths. +func (p *DefaultProvider) List(ctx context.Context, path string) (map[string]string, error) { + p.Lock() + defer p.Unlock() + if paths, ok := p.cache.Get(path); ok { + return paths.(map[string]string), nil + } + values := map[string]string{} + if err := p.ssmapi.GetParametersByPathPagesWithContext(ctx, &ssm.GetParametersByPathInput{ + Recursive: lo.ToPtr(true), + Path: &path, + }, func(out *ssm.GetParametersByPathOutput, _ bool) bool { + for _, parameter := range out.Parameters { + if parameter.Name == nil || parameter.Value == nil { + continue + } + values[*parameter.Name] = *parameter.Value + } + return true + }); err != nil { + return nil, fmt.Errorf("getting ssm parameters for path %q, %w", path, err) + } + p.cache.SetDefault(path, values) + return values, nil +} diff --git a/pkg/providers/subnet/suite_test.go b/pkg/providers/subnet/suite_test.go index 068fb93f4676..f6dedfa1d2e7 100644 --- a/pkg/providers/subnet/suite_test.go +++ b/pkg/providers/subnet/suite_test.go @@ -22,7 +22,6 @@ import ( "sigs.k8s.io/karpenter/pkg/test/v1alpha1" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/samber/lo" @@ -70,7 +69,9 @@ var _ = BeforeEach(func() { ctx = options.ToContext(ctx, test.Options()) nodeClass = test.EC2NodeClass(v1.EC2NodeClass{ Spec: v1.EC2NodeClassSpec{ - AMIFamily: aws.String(v1.AMIFamilyAL2), + AMISelectorTerms: []v1.AMISelectorTerm{{ + Alias: "al2@latest", + }}, SubnetSelectorTerms: []v1.SubnetSelectorTerm{ { Tags: map[string]string{ diff --git a/pkg/providers/version/version.go b/pkg/providers/version/version.go index 4050c797350f..4aa8a01f4abe 100644 --- a/pkg/providers/version/version.go +++ b/pkg/providers/version/version.go @@ -17,9 +17,11 @@ package version import ( "context" "fmt" + "strconv" "strings" "github.com/patrickmn/go-cache" + "github.com/samber/lo" "k8s.io/apimachinery/pkg/util/version" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/log" @@ -75,6 +77,19 @@ func (p *DefaultProvider) Get(ctx context.Context) (string, error) { return version, nil } +// SupportedK8sVersions returns a slice of version strings in format "major.minor" for all versions of k8s supported by +// this version of Karpenter. +// Note: Assumes k8s only has a single major version (1.x) +func SupportedK8sVersions() []string { + minMinor := lo.Must(strconv.Atoi(strings.Split(MinK8sVersion, ".")[1])) + maxMinor := lo.Must(strconv.Atoi(strings.Split(MaxK8sVersion, ".")[1])) + versions := make([]string, 0, maxMinor-minMinor+1) + for i := minMinor; i <= maxMinor; i++ { + versions = append(versions, fmt.Sprintf("1.%d", i)) + } + return versions +} + func validateK8sVersion(v string) error { k8sVersion := version.MustParseGeneric(v) diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 015d5ae9a5eb..ac4c357bfee1 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -34,6 +34,7 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" "github.com/aws/karpenter-provider-aws/pkg/providers/pricing" "github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup" + ssmp "github.com/aws/karpenter-provider-aws/pkg/providers/ssm" "github.com/aws/karpenter-provider-aws/pkg/providers/subnet" "github.com/aws/karpenter-provider-aws/pkg/providers/version" @@ -66,6 +67,7 @@ type Environment struct { AssociatePublicIPAddressCache *cache.Cache SecurityGroupCache *cache.Cache InstanceProfileCache *cache.Cache + SSMCache *cache.Cache // Providers InstanceTypesProvider *instancetype.DefaultProvider @@ -98,6 +100,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment associatePublicIPAddressCache := cache.New(awscache.AssociatePublicIPAddressTTL, awscache.DefaultCleanupInterval) securityGroupCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) instanceProfileCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) + ssmCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) fakePricingAPI := &fake.PricingAPI{} // Providers @@ -106,7 +109,8 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment securityGroupProvider := securitygroup.NewDefaultProvider(ec2api, securityGroupCache) versionProvider := version.NewDefaultProvider(env.KubernetesInterface, kubernetesVersionCache) instanceProfileProvider := instanceprofile.NewDefaultProvider(fake.DefaultRegion, iamapi, instanceProfileCache) - amiProvider := amifamily.NewDefaultProvider(versionProvider, ssmapi, ec2api, ec2Cache) + ssmProvider := ssmp.NewDefaultProvider(ssmapi, ssmCache) + amiProvider := amifamily.NewDefaultProvider(versionProvider, ssmProvider, ec2api, ec2Cache) amiResolver := amifamily.NewResolver(amiProvider) instanceTypesProvider := instancetype.NewDefaultProvider(fake.DefaultRegion, instanceTypeCache, ec2api, subnetProvider, unavailableOfferingsCache, pricingProvider) launchTemplateProvider := @@ -149,6 +153,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment SecurityGroupCache: securityGroupCache, InstanceProfileCache: instanceProfileCache, UnavailableOfferingsCache: unavailableOfferingsCache, + SSMCache: ssmCache, InstanceTypesProvider: instanceTypesProvider, InstanceProvider: instanceProvider, @@ -181,7 +186,7 @@ func (env *Environment) Reset() { env.AvailableIPAdressCache.Flush() env.SecurityGroupCache.Flush() env.InstanceProfileCache.Flush() - + env.SSMCache.Flush() mfs, err := crmetrics.Registry.Gather() if err != nil { for _, mf := range mfs { diff --git a/pkg/test/nodeclass.go b/pkg/test/nodeclass.go index 6b8858832a29..ecba27c5c8c9 100644 --- a/pkg/test/nodeclass.go +++ b/pkg/test/nodeclass.go @@ -38,8 +38,8 @@ func EC2NodeClass(overrides ...v1.EC2NodeClass) *v1.EC2NodeClass { panic(fmt.Sprintf("Failed to merge settings: %s", err)) } } - if options.Spec.AMIFamily == nil { - options.Spec.AMIFamily = &v1.AMIFamilyAL2 + if len(options.Spec.AMISelectorTerms) == 0 { + options.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} options.Status.AMIs = []v1.AMI{ { ID: "ami-test1", diff --git a/test/pkg/environment/aws/environment.go b/test/pkg/environment/aws/environment.go index 64d307bfb58e..8df6765a9915 100644 --- a/test/pkg/environment/aws/environment.go +++ b/test/pkg/environment/aws/environment.go @@ -143,7 +143,7 @@ func GetTimeStreamAPI(session *session.Session) timestreamwriteiface.TimestreamW func (env *Environment) DefaultEC2NodeClass() *v1.EC2NodeClass { nodeClass := test.EC2NodeClass() - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} nodeClass.Spec.Tags = map[string]string{ "testing/cluster": env.ClusterName, } diff --git a/test/suites/ami/suite_test.go b/test/suites/ami/suite_test.go index 85e2519819e5..ca8b119e0f6a 100644 --- a/test/suites/ami/suite_test.go +++ b/test/suites/ami/suite_test.go @@ -71,8 +71,12 @@ var _ = AfterEach(func() { env.AfterEach() }) var _ = Describe("AMI", func() { var customAMI string + var customUserData *string BeforeEach(func() { customAMI = env.GetAMIBySSMPath(fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", env.K8sVersion())) + rawContent, err := os.ReadFile("testdata/al2023_userdata_input.yaml") + Expect(err).ToNot(HaveOccurred()) + customUserData = lo.ToPtr(fmt.Sprintf(string(rawContent), env.ClusterName, env.ClusterEndpoint, env.ExpectCABundle())) }) It("should use the AMI defined by the AMI Selector Terms", func() { @@ -82,6 +86,7 @@ var _ = Describe("AMI", func() { ID: customAMI, }, } + nodeClass.Spec.UserData = customUserData env.ExpectCreated(pod, nodeClass, nodePool) env.EventuallyExpectHealthy(pod) env.ExpectCreatedNodeCount("==", 1) @@ -99,6 +104,7 @@ var _ = Describe("AMI", func() { ID: oldCustomAMI, }, } + nodeClass.Spec.UserData = customUserData pod := coretest.Pod() env.ExpectCreated(pod, nodeClass, nodePool) @@ -119,6 +125,7 @@ var _ = Describe("AMI", func() { Owner: "fakeOwnerValue", }, } + nodeClass.Spec.UserData = customUserData pod := coretest.Pod() env.ExpectCreated(pod, nodeClass, nodePool) @@ -137,6 +144,7 @@ var _ = Describe("AMI", func() { Name: *output.Images[0].Name, }, } + nodeClass.Spec.UserData = customUserData pod := coretest.Pod() env.ExpectCreated(pod, nodeClass, nodePool) @@ -151,6 +159,7 @@ var _ = Describe("AMI", func() { ID: customAMI, }, } + nodeClass.Spec.UserData = customUserData pod := coretest.Pod() env.ExpectCreated(pod, nodeClass, nodePool) @@ -163,49 +172,28 @@ var _ = Describe("AMI", func() { Context("AMIFamily", func() { It("should provision a node using the AL2 family", func() { pod := coretest.Pod() - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} env.ExpectCreated(nodeClass, nodePool, pod) env.EventuallyExpectHealthy(pod) env.ExpectCreatedNodeCount("==", 1) }) It("should provision a node using the AL2023 family", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}} pod := coretest.Pod() env.ExpectCreated(nodeClass, nodePool, pod) env.EventuallyExpectHealthy(pod) env.ExpectCreatedNodeCount("==", 1) }) It("should provision a node using the Bottlerocket family", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket - pod := coretest.Pod() - env.ExpectCreated(nodeClass, nodePool, pod) - env.EventuallyExpectHealthy(pod) - env.ExpectCreatedNodeCount("==", 1) - }) - It("should provision a node using the Ubuntu family", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyUbuntu - // TODO (jmdeal@): remove once 22.04 AMIs are supported - if env.K8sMinorVersion() >= 29 { - nodeClass.Spec.AMISelectorTerms = lo.Map([]string{ - "/aws/service/canonical/ubuntu/eks/20.04/1.28/stable/current/amd64/hvm/ebs-gp2/ami-id", - "/aws/service/canonical/ubuntu/eks/20.04/1.28/stable/current/arm64/hvm/ebs-gp2/ami-id", - }, func(ssmPath string, _ int) v1.AMISelectorTerm { - return v1.AMISelectorTerm{ID: env.GetAMIBySSMPath(ssmPath)} - }) - } + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} pod := coretest.Pod() env.ExpectCreated(nodeClass, nodePool, pod) env.EventuallyExpectHealthy(pod) env.ExpectCreatedNodeCount("==", 1) }) It("should support Custom AMIFamily with AMI Selectors", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom al2023AMI := env.GetAMIBySSMPath(fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", env.K8sVersion())) - nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: al2023AMI, - }, - } + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: al2023AMI}} rawContent, err := os.ReadFile("testdata/al2023_userdata_input.yaml") Expect(err).ToNot(HaveOccurred()) nodeClass.Spec.UserData = lo.ToPtr(fmt.Sprintf(string(rawContent), env.ClusterName, @@ -257,7 +245,7 @@ var _ = Describe("AMI", func() { It("should merge UserData contents for AL2 AMIFamily", func() { content, err := os.ReadFile("testdata/al2_userdata_input.sh") Expect(err).ToNot(HaveOccurred()) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.UserData = awssdk.String(string(content)) nodePool.Spec.Template.Spec.Taints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}} nodePool.Spec.Template.Spec.StartupTaints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}} @@ -278,7 +266,7 @@ var _ = Describe("AMI", func() { It("should merge non-MIME UserData contents for AL2 AMIFamily", func() { content, err := os.ReadFile("testdata/al2_no_mime_userdata_input.sh") Expect(err).ToNot(HaveOccurred()) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} nodeClass.Spec.UserData = awssdk.String(string(content)) nodePool.Spec.Template.Spec.Taints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}} nodePool.Spec.Template.Spec.StartupTaints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}} @@ -299,7 +287,7 @@ var _ = Describe("AMI", func() { It("should merge UserData contents for Bottlerocket AMIFamily", func() { content, err := os.ReadFile("testdata/br_userdata_input.sh") Expect(err).ToNot(HaveOccurred()) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} nodeClass.Spec.UserData = awssdk.String(string(content)) nodePool.Spec.Template.Spec.Taints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}} nodePool.Spec.Template.Spec.StartupTaints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}} @@ -328,7 +316,7 @@ var _ = Describe("AMI", func() { content, err := os.ReadFile("testdata/windows_userdata_input.ps1") Expect(err).ToNot(HaveOccurred()) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2022 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}} nodeClass.Spec.UserData = awssdk.String(string(content)) nodePool.Spec.Template.Spec.Taints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}} nodePool.Spec.Template.Spec.StartupTaints = []corev1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}} diff --git a/test/suites/drift/suite_test.go b/test/suites/drift/suite_test.go index 5590fd20b064..7d2d2f989c0f 100644 --- a/test/suites/drift/suite_test.go +++ b/test/suites/drift/suite_test.go @@ -16,11 +16,17 @@ package drift_test import ( "fmt" + "os" "sort" "testing" "time" + awssdk "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/eks" "github.com/awslabs/operatorpkg/object" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "github.com/samber/lo" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -31,21 +37,14 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/karpenter/pkg/utils/resources" - - awssdk "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" - "github.com/aws/aws-sdk-go/service/eks" karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1" coretest "sigs.k8s.io/karpenter/pkg/test" + "sigs.k8s.io/karpenter/pkg/utils/resources" v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" "github.com/aws/karpenter-provider-aws/pkg/test" "github.com/aws/karpenter-provider-aws/test/pkg/environment/aws" "github.com/aws/karpenter-provider-aws/test/pkg/environment/common" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" ) var env *aws.Environment @@ -76,6 +75,7 @@ var _ = Describe("Drift", func() { var dep *appsv1.Deployment var selector labels.Selector var numPods int + var customUserData *string BeforeEach(func() { amdAMI = env.GetAMIBySSMPath(fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", env.K8sVersion())) numPods = 1 @@ -95,6 +95,10 @@ var _ = Describe("Drift", func() { }, }) selector = labels.SelectorFromSet(dep.Spec.Selector.MatchLabels) + + rawContent, err := os.ReadFile("testdata/al2023_userdata_input.yaml") + Expect(err).ToNot(HaveOccurred()) + customUserData = lo.ToPtr(fmt.Sprintf(string(rawContent), env.ClusterName, env.ClusterEndpoint, env.ExpectCABundle())) }) Context("Budgets", func() { It("should respect budgets for empty drift", func() { @@ -377,8 +381,8 @@ var _ = Describe("Drift", func() { "/aws/service/eks/optimized-ami/1.23/amazon-linux-2023/x86_64/standard/amazon-eks-node-al2023-x86_64-standard-1.23-v20240307/image_id", fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", env.K8sVersionWithOffset(1)), )) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: oldCustomAMI}} + nodeClass.Spec.UserData = customUserData env.ExpectCreated(dep, nodeClass, nodePool) pod := env.EventuallyExpectHealthyPodCount(selector, numPods)[0] @@ -398,8 +402,8 @@ var _ = Describe("Drift", func() { }) It("should return drifted if the AMI no longer matches the existing NodeClaims instance type", func() { armAMI := env.GetAMIBySSMPath(fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/arm64/standard/recommended/image_id", env.K8sVersion())) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: armAMI}} + nodeClass.Spec.UserData = customUserData env.ExpectCreated(dep, nodeClass, nodePool) pod := env.EventuallyExpectHealthyPodCount(selector, numPods)[0] @@ -425,8 +429,8 @@ var _ = Describe("Drift", func() { "/aws/service/eks/optimized-ami/1.23/amazon-linux-2023/x86_64/standard/amazon-eks-node-al2023-x86_64-standard-1.23-v20240307/image_id", fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2023/x86_64/standard/recommended/image_id", env.K8sVersionWithOffset(1)), )) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2023 nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: oldCustomAMI}} + nodeClass.Spec.UserData = customUserData env.ExpectCreated(dep, nodeClass, nodePool) env.EventuallyExpectHealthyPodCount(selector, numPods) @@ -654,7 +658,9 @@ var _ = Describe("Drift", func() { }, }}}), Entry("DetailedMonitoring", v1.EC2NodeClassSpec{DetailedMonitoring: awssdk.Bool(true)}), - Entry("AMIFamily", v1.EC2NodeClassSpec{AMIFamily: awssdk.String(v1.AMIFamilyBottlerocket)}), + Entry("AMIFamily", v1.EC2NodeClassSpec{ + AMISelectorTerms: []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}}, + }), Entry("KubeletConfiguration", v1.EC2NodeClassSpec{ Kubelet: &v1.KubeletConfiguration{ EvictionSoft: map[string]string{"memory.available": "5%"}, diff --git a/test/suites/drift/testdata/al2023_userdata_input.yaml b/test/suites/drift/testdata/al2023_userdata_input.yaml new file mode 100644 index 000000000000..e400f3259490 --- /dev/null +++ b/test/suites/drift/testdata/al2023_userdata_input.yaml @@ -0,0 +1,15 @@ +apiVersion: node.eks.aws/v1alpha1 +kind: NodeConfig +spec: + cluster: + name: %s + apiServerEndpoint: %s + certificateAuthority: %s + cidr: 10.100.0.0/16 + kubelet: + config: + clusterDNS: + - 10.0.100.10 + flags: + - --node-labels='testing/cluster=unspecified' + - --register-with-taints='karpenter.sh/unregistered:NoExecute' diff --git a/test/suites/integration/ec2nodeclass_kubelet_test.go b/test/suites/integration/ec2nodeclass_kubelet_test.go index a34724abf4cd..0462d9f4b0fd 100644 --- a/test/suites/integration/ec2nodeclass_kubelet_test.go +++ b/test/suites/integration/ec2nodeclass_kubelet_test.go @@ -74,8 +74,38 @@ var _ = Describe("EC2nodeClass Kubelet Configuration", func() { Requirements: []karpv1beta1.NodeSelectorRequirementWithMinValues{ { NodeSelectorRequirement: corev1.NodeSelectorRequirement{ - Key: karpv1beta1.CapacityTypeLabelKey, - Operator: corev1.NodeSelectorOpExists, + Key: corev1.LabelOSStable, + Operator: corev1.NodeSelectorOpIn, + Values: []string{string(corev1.Linux)}, + }, + }, + { + NodeSelectorRequirement: corev1.NodeSelectorRequirement{ + Key: karpv1.CapacityTypeLabelKey, + Operator: corev1.NodeSelectorOpIn, + Values: []string{karpv1.CapacityTypeOnDemand}, + }, + }, + { + NodeSelectorRequirement: corev1.NodeSelectorRequirement{ + Key: v1.LabelInstanceCategory, + Operator: corev1.NodeSelectorOpIn, + Values: []string{"c", "m", "r"}, + }, + }, + { + NodeSelectorRequirement: corev1.NodeSelectorRequirement{ + Key: v1.LabelInstanceGeneration, + Operator: corev1.NodeSelectorOpGt, + Values: []string{"2"}, + }, + }, + // Filter out a1 instance types, which are incompatible with AL2023 AMIs + { + NodeSelectorRequirement: corev1.NodeSelectorRequirement{ + Key: v1.LabelInstanceFamily, + Operator: corev1.NodeSelectorOpNotIn, + Values: []string{"a1"}, }, }, }, diff --git a/test/suites/integration/extended_resources_test.go b/test/suites/integration/extended_resources_test.go index fcc36a081a79..070f8cca07b3 100644 --- a/test/suites/integration/extended_resources_test.go +++ b/test/suites/integration/extended_resources_test.go @@ -45,7 +45,7 @@ var _ = Describe("Extended Resources", func() { It("should provision nodes for a deployment that requests nvidia.com/gpu", func() { ExpectNvidiaDevicePluginCreated() // TODO: jmdeal@ remove AL2 pin once AL2023 accelerated AMIs are available - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} numPods := 1 dep := test.Deployment(test.DeploymentOptions{ Replicas: int32(numPods), @@ -77,7 +77,7 @@ var _ = Describe("Extended Resources", func() { }) It("should provision nodes for a deployment that requests nvidia.com/gpu (Bottlerocket)", func() { // For Bottlerocket, we are testing that resources are initialized without needing a device plugin - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} numPods := 1 dep := test.Deployment(test.DeploymentOptions{ Replicas: int32(numPods), @@ -144,12 +144,7 @@ var _ = Describe("Extended Resources", func() { // We use a Custom AMI so that we can reboot after we start the kubelet service rawContent, err := os.ReadFile("testdata/amd_driver_input.sh") Expect(err).ToNot(HaveOccurred()) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom - nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: customAMI, - }, - } + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: customAMI}} nodeClass.Spec.UserData = lo.ToPtr(fmt.Sprintf(string(rawContent), env.ClusterName, env.ClusterEndpoint, env.ExpectCABundle(), nodePool.Name)) @@ -228,7 +223,7 @@ var _ = Describe("Extended Resources", func() { // Only select private subnets since instances with multiple network instances at launch won't get a public IP. nodeClass.Spec.SubnetSelectorTerms[0].Tags["Name"] = "*Private*" // TODO: jmdeal@ remove AL2 pin once AL2023 accelerated AMIs are available - nodeClass.Spec.AMIFamily = &v1.AMIFamilyAL2 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}} numPods := 1 dep := test.Deployment(test.DeploymentOptions{ diff --git a/test/suites/integration/kubelet_config_test.go b/test/suites/integration/kubelet_config_test.go index 142c59d55d87..b7f06f3175c8 100644 --- a/test/suites/integration/kubelet_config_test.go +++ b/test/suites/integration/kubelet_config_test.go @@ -84,17 +84,9 @@ var _ = Describe("KubeletConfiguration Overrides", func() { } }) DescribeTable("Linux AMIFamilies", - func(amiFamily *string) { - nodeClass.Spec.AMIFamily = amiFamily + func(term v1.AMISelectorTerm) { + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{term} // TODO (jmdeal@): remove once 22.04 AMIs are supported - if *amiFamily == v1.AMIFamilyUbuntu && env.K8sMinorVersion() >= 29 { - nodeClass.Spec.AMISelectorTerms = lo.Map([]string{ - "/aws/service/canonical/ubuntu/eks/20.04/1.28/stable/current/amd64/hvm/ebs-gp2/ami-id", - "/aws/service/canonical/ubuntu/eks/20.04/1.28/stable/current/arm64/hvm/ebs-gp2/ami-id", - }, func(ssmPath string, _ int) v1.AMISelectorTerm { - return v1.AMISelectorTerm{ID: env.GetAMIBySSMPath(ssmPath)} - }) - } pod := test.Pod(test.PodOptions{ NodeSelector: map[string]string{ corev1.LabelOSStable: string(corev1.Linux), @@ -105,19 +97,18 @@ var _ = Describe("KubeletConfiguration Overrides", func() { env.EventuallyExpectHealthy(pod) env.ExpectCreatedNodeCount("==", 1) }, - Entry("when the AMIFamily is AL2", &v1.AMIFamilyAL2), - Entry("when the AMIFamily is AL2023", &v1.AMIFamilyAL2023), - Entry("when the AMIFamily is Ubuntu", &v1.AMIFamilyUbuntu), - Entry("when the AMIFamily is Bottlerocket", &v1.AMIFamilyBottlerocket), + Entry("when the AMIFamily is AL2", v1.AMISelectorTerm{Alias: "al2@latest"}), + Entry("when the AMIFamily is AL2023", v1.AMISelectorTerm{Alias: "al2023@latest"}), + Entry("when the AMIFamily is Bottlerocket", v1.AMISelectorTerm{Alias: "bottlerocket@latest"}), ) DescribeTable("Windows AMIFamilies", - func(amiFamily *string) { + func(term v1.AMISelectorTerm) { env.ExpectWindowsIPAMEnabled() DeferCleanup(func() { env.ExpectWindowsIPAMDisabled() }) - nodeClass.Spec.AMIFamily = amiFamily + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{term} // Need to enable nodepool-level OS-scoping for now since DS evaluation is done off of the nodepool // requirements, not off of the instance type options so scheduling can fail if nodepool aren't // properly scoped @@ -146,8 +137,8 @@ var _ = Describe("KubeletConfiguration Overrides", func() { // If the instance type is not supported by the controller resource `vpc.amazonaws.com/PrivateIPv4Address` will not register. // Issue: https://github.com/aws/karpenter-provider-aws/issues/4472 // See: https://github.com/aws/amazon-vpc-resource-controller-k8s/blob/master/pkg/aws/vpc/limits.go - Entry("when the AMIFamily is Windows2019", &v1.AMIFamilyWindows2019), - Entry("when the AMIFamily is Windows2022", &v1.AMIFamilyWindows2022), + Entry("when the AMIFamily is Windows2019", v1.AMISelectorTerm{Alias: "windows2019@latest"}), + Entry("when the AMIFamily is Windows2022", v1.AMISelectorTerm{Alias: "windows2022@latest"}), ) }) It("should schedule pods onto separate nodes when maxPods is set", func() { @@ -221,7 +212,7 @@ var _ = Describe("KubeletConfiguration Overrides", func() { env.EventuallyExpectUniqueNodeNames(selector, 2) }) It("should ignore podsPerCore value when Bottlerocket is used", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} // All pods should schedule to a single node since we are ignoring podsPerCore value // This would normally schedule to 3 nodes if not using Bottlerocket test.ReplaceRequirements(nodePool, diff --git a/test/suites/integration/validation_test.go b/test/suites/integration/validation_test.go index d5c5d498cf74..0c0671dbe856 100644 --- a/test/suites/integration/validation_test.go +++ b/test/suites/integration/validation_test.go @@ -157,8 +157,8 @@ var _ = Describe("Validation", func() { }) }) Context("EC2NodeClass", func() { - It("should error when amiSelectorTerms are not defined for amiFamily Custom", func() { - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom + It("should error when amiSelectorTerms are not defined", func() { + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{} Expect(env.Client.Create(env.Context, nodeClass)).ToNot(Succeed()) }) It("should fail for poorly formatted AMI ids", func() { diff --git a/test/suites/nodeclaim/nodeclaim_test.go b/test/suites/nodeclaim/nodeclaim_test.go index 4fb499886025..81738fa335bc 100644 --- a/test/suites/nodeclaim/nodeclaim_test.go +++ b/test/suites/nodeclaim/nodeclaim_test.go @@ -244,7 +244,6 @@ var _ = Describe("StandaloneNodeClaim", func() { Expect(err).ToNot(HaveOccurred()) // Create userData that adds custom labels through the --node-labels - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: customAMI}} nodeClass.Spec.UserData = lo.ToPtr(fmt.Sprintf(string(rawContent), env.ClusterName, env.ClusterEndpoint, env.ExpectCABundle())) @@ -296,7 +295,6 @@ var _ = Describe("StandaloneNodeClaim", func() { Expect(err).ToNot(HaveOccurred()) // Create userData that adds custom labels through the --node-labels - nodeClass.Spec.AMIFamily = &v1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{ID: customAMI}} // Giving bad clusterName and clusterEndpoint to the userData diff --git a/test/suites/nodeclaim/testdata/al2023_userdata_input.yaml b/test/suites/nodeclaim/testdata/al2023_userdata_input.yaml index b0ce7a5e8496..e400f3259490 100644 --- a/test/suites/nodeclaim/testdata/al2023_userdata_input.yaml +++ b/test/suites/nodeclaim/testdata/al2023_userdata_input.yaml @@ -11,4 +11,5 @@ spec: clusterDNS: - 10.0.100.10 flags: - - --node-labels="testing/cluster=unspecified" \ No newline at end of file + - --node-labels='testing/cluster=unspecified' + - --register-with-taints='karpenter.sh/unregistered:NoExecute' diff --git a/test/suites/scale/deprovisioning_test.go b/test/suites/scale/deprovisioning_test.go index 33c711d2dccc..cac0d34e064c 100644 --- a/test/suites/scale/deprovisioning_test.go +++ b/test/suites/scale/deprovisioning_test.go @@ -256,7 +256,7 @@ var _ = Describe("Deprovisioning", Label(debug.NoWatch), Label(debug.NoEvents), nodePoolMap[expirationValue].Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(time.Duration(0)) nodePoolMap[expirationValue].Spec.Limits = disableProvisioningLimits // Update the drift NodeClass to start drift on Nodes assigned to this NodeClass - driftNodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + driftNodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} // Create test assertions to ensure during the multiple deprovisioner scale-downs type testAssertions struct { @@ -678,7 +678,7 @@ var _ = Describe("Deprovisioning", Label(debug.NoWatch), Label(debug.NoEvents), env.MeasureDeprovisioningDurationFor(func() { By("kicking off deprovisioning drift by changing the nodeClass AMIFamily") - nodeClass.Spec.AMIFamily = &v1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "bottlerocket@latest"}} env.ExpectCreatedOrUpdated(nodeClass) env.EventuallyExpectDeletedNodeCount("==", expectedNodeCount) diff --git a/test/suites/scheduling/suite_test.go b/test/suites/scheduling/suite_test.go index 3801f6113768..23248c43d174 100644 --- a/test/suites/scheduling/suite_test.go +++ b/test/suites/scheduling/suite_test.go @@ -302,7 +302,7 @@ var _ = Describe("Scheduling", Ordered, ContinueOnFailure, func() { NodeRequirements: requirements, Image: environmentaws.WindowsDefaultImage, }}) - nodeClass.Spec.AMIFamily = &v1.AMIFamilyWindows2022 + nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}} test.ReplaceRequirements(nodePool, karpv1.NodeSelectorRequirementWithMinValues{ NodeSelectorRequirement: corev1.NodeSelectorRequirement{ diff --git a/website/content/en/docs/getting-started/getting-started-with-karpenter/cloudformation.yaml b/website/content/en/docs/getting-started/getting-started-with-karpenter/cloudformation.yaml index b20462aa1d6d..a9760c9451d6 100644 --- a/website/content/en/docs/getting-started/getting-started-with-karpenter/cloudformation.yaml +++ b/website/content/en/docs/getting-started/getting-started-with-karpenter/cloudformation.yaml @@ -352,4 +352,4 @@ Resources: - EC2 Instance State-change Notification Targets: - Id: KarpenterInterruptionQueueTarget - Arn: !GetAtt KarpenterInterruptionQueue.Arn \ No newline at end of file + Arn: !GetAtt KarpenterInterruptionQueue.Arn diff --git a/website/content/en/preview/getting-started/getting-started-with-karpenter/cloudformation.yaml b/website/content/en/preview/getting-started/getting-started-with-karpenter/cloudformation.yaml index b20462aa1d6d..dde2e9999849 100644 --- a/website/content/en/preview/getting-started/getting-started-with-karpenter/cloudformation.yaml +++ b/website/content/en/preview/getting-started/getting-started-with-karpenter/cloudformation.yaml @@ -181,7 +181,7 @@ Resources: "Sid": "AllowSSMReadActions", "Effect": "Allow", "Resource": "arn:${AWS::Partition}:ssm:${AWS::Region}::parameter/aws/service/*", - "Action": "ssm:GetParameter" + "Action": "ssm:GetParametersByPath" }, { "Sid": "AllowPricingReadActions", @@ -352,4 +352,4 @@ Resources: - EC2 Instance State-change Notification Targets: - Id: KarpenterInterruptionQueueTarget - Arn: !GetAtt KarpenterInterruptionQueue.Arn \ No newline at end of file + Arn: !GetAtt KarpenterInterruptionQueue.Arn diff --git a/website/content/en/preview/reference/cloudformation.md b/website/content/en/preview/reference/cloudformation.md index 99a084661338..c9153487df46 100644 --- a/website/content/en/preview/reference/cloudformation.md +++ b/website/content/en/preview/reference/cloudformation.md @@ -310,7 +310,7 @@ This allows the Karpenter controller to do any of those read-only actions across #### AllowSSMReadActions -The AllowSSMReadActions Sid allows the Karpenter controller to read SSM parameters (`ssm:GetParameter`) from the current region for SSM parameters generated by ASW services. +The AllowSSMReadActions Sid allows the Karpenter controller to list SSM parameters (`ssm:GetParametersByPath`) from the current region for SSM parameters generated by ASW services. **NOTE**: If potentially sensitive information is stored in SSM parameters, you could consider restricting access to these messages further. ```json @@ -318,7 +318,7 @@ The AllowSSMReadActions Sid allows the Karpenter controller to read SSM paramete "Sid": "AllowSSMReadActions", "Effect": "Allow", "Resource": "arn:${AWS::Partition}:ssm:${AWS::Region}::parameter/aws/service/*", - "Action": "ssm:GetParameter" + "Action": "ssm:GetParametersByPath" } ```