From 8583cb32a3dfd9a42825cc798e3306d5fdf152ec Mon Sep 17 00:00:00 2001 From: jacob Date: Mon, 2 Aug 2021 19:02:11 -0700 Subject: [PATCH 1/4] Initial pass at hard-coding $Default LT version --- pkg/cloudprovider/aws/constraints.go | 30 ++++++++++------------------ pkg/cloudprovider/aws/suite_test.go | 24 +++++----------------- 2 files changed, 15 insertions(+), 39 deletions(-) diff --git a/pkg/cloudprovider/aws/constraints.go b/pkg/cloudprovider/aws/constraints.go index 706239907861..e02a005d6067 100644 --- a/pkg/cloudprovider/aws/constraints.go +++ b/pkg/cloudprovider/aws/constraints.go @@ -32,18 +32,16 @@ const ( ) var ( - AWSLabelPrefix = "node.k8s.aws/" - CapacityTypeLabel = AWSLabelPrefix + "capacity-type" - LaunchTemplateIdLabel = AWSLabelPrefix + "launch-template-id" - LaunchTemplateVersionLabel = AWSLabelPrefix + "launch-template-version" - SubnetNameLabel = AWSLabelPrefix + "subnet-name" - SubnetTagKeyLabel = AWSLabelPrefix + "subnet-tag-key" - SecurityGroupNameLabel = AWSLabelPrefix + "security-group-name" - SecurityGroupTagKeyLabel = AWSLabelPrefix + "security-group-tag-key" - AllowedLabels = []string{ + AWSLabelPrefix = "node.k8s.aws/" + CapacityTypeLabel = AWSLabelPrefix + "capacity-type" + LaunchTemplateIdLabel = AWSLabelPrefix + "launch-template-id" + SubnetNameLabel = AWSLabelPrefix + "subnet-name" + SubnetTagKeyLabel = AWSLabelPrefix + "subnet-tag-key" + SecurityGroupNameLabel = AWSLabelPrefix + "security-group-name" + SecurityGroupTagKeyLabel = AWSLabelPrefix + "security-group-tag-key" + AllowedLabels = []string{ CapacityTypeLabel, LaunchTemplateIdLabel, - LaunchTemplateVersionLabel, SubnetNameLabel, SubnetTagKeyLabel, SecurityGroupNameLabel, @@ -79,13 +77,9 @@ func (c *Constraints) GetLaunchTemplate() *LaunchTemplate { if !ok { return nil } - version, ok := c.Labels[LaunchTemplateVersionLabel] - if !ok { - version = DefaultLaunchTemplateVersion - } return &LaunchTemplate{ Id: id, - Version: version, + Version: DefaultLaunchTemplateVersion, } } @@ -152,11 +146,7 @@ func (c *Constraints) validateCapacityType(ctx context.Context) (errs *apis.Fiel } func (c *Constraints) validateLaunchTemplate(ctx context.Context) (errs *apis.FieldError) { - if _, versionExists := c.Labels[LaunchTemplateVersionLabel]; versionExists { - if _, bothExist := c.Labels[LaunchTemplateIdLabel]; !bothExist { - return errs.Also(apis.ErrMissingField(fmt.Sprintf("spec.labels[%s]", LaunchTemplateIdLabel))) - } - } + // nothing to validate at the moment return errs } diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index 669ed0901952..8644c15df761 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -277,7 +277,6 @@ var _ = Describe("Allocation", func() { // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) Expect(node.Labels).ToNot(HaveKey(LaunchTemplateIdLabel)) - Expect(node.Labels).ToNot(HaveKey(LaunchTemplateVersionLabel)) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) @@ -288,21 +287,18 @@ var _ = Describe("Allocation", func() { It("should default to a provisioner's launch template id and version", func() { // Setup provisioner.Spec.Labels = map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), - LaunchTemplateVersionLabel: randomdata.SillyName(), + LaunchTemplateIdLabel: randomdata.SillyName(), } ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, test.PendingPod()) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, provisioner.Spec.Labels[LaunchTemplateIdLabel])) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateVersionLabel, provisioner.Spec.Labels[LaunchTemplateVersionLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification Expect(*launchTemplate.LaunchTemplateId).To(Equal(provisioner.Spec.Labels[LaunchTemplateIdLabel])) - Expect(*launchTemplate.Version).To(Equal(provisioner.Spec.Labels[LaunchTemplateVersionLabel])) }) It("should default to a provisioner's launch template and the default launch template version", func() { @@ -313,7 +309,6 @@ var _ = Describe("Allocation", func() { // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, provisioner.Spec.Labels[LaunchTemplateIdLabel])) - Expect(node.Labels).ToNot(HaveKey(LaunchTemplateVersionLabel)) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) @@ -324,26 +319,22 @@ var _ = Describe("Allocation", func() { It("should allow a pod to override the launch template id and version", func() { // Setup provisioner.Spec.Labels = map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), - LaunchTemplateVersionLabel: randomdata.SillyName(), + LaunchTemplateIdLabel: randomdata.SillyName(), } ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, test.PendingPod(test.PodOptions{NodeSelector: map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), - LaunchTemplateVersionLabel: randomdata.SillyName(), + LaunchTemplateIdLabel: randomdata.SillyName(), }}), ) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateVersionLabel, pods[0].Spec.NodeSelector[LaunchTemplateVersionLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification Expect(*launchTemplate.LaunchTemplateId).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) - Expect(*launchTemplate.Version).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateVersionLabel])) }) It("should allow a pod to override the launch template id and use the default launch template version", func() { // Setup @@ -355,7 +346,6 @@ var _ = Describe("Allocation", func() { // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) - Expect(node.Labels).ToNot(HaveKey(LaunchTemplateVersionLabel)) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) @@ -366,8 +356,7 @@ var _ = Describe("Allocation", func() { It("should allow a pod to override the launch template id and use the provisioner's launch template version", func() { // Setup provisioner.Spec.Labels = map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), - LaunchTemplateVersionLabel: randomdata.SillyName(), + LaunchTemplateIdLabel: randomdata.SillyName(), } ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, @@ -376,13 +365,11 @@ var _ = Describe("Allocation", func() { // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateVersionLabel, provisioner.Spec.Labels[LaunchTemplateVersionLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification Expect(*launchTemplate.LaunchTemplateId).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) - Expect(*launchTemplate.Version).To(Equal(provisioner.Spec.Labels[LaunchTemplateVersionLabel])) }) }) Context("Subnets", func() { @@ -594,8 +581,7 @@ var _ = Describe("Allocation", func() { }) It("should support launch templates", func() { provisioner.Spec.Labels = map[string]string{ - "node.k8s.aws/launch-template-version": randomdata.SillyName(), - "node.k8s.aws/launch-template-id": "23", + "node.k8s.aws/launch-template-id": "23", } Expect(provisioner.Validate(ctx)).To(Succeed()) }) From c3ff233849bab3ce52f728e4c5431626709c1c32 Mon Sep 17 00:00:00 2001 From: jacob Date: Tue, 3 Aug 2021 21:54:09 -0700 Subject: [PATCH 2/4] checkpoint: unit tests pass after id-> name change --- designs/aws-launch-templates-options.md | 33 +++++++++++-------- pkg/cloudprovider/aws/constraints.go | 10 +++--- pkg/cloudprovider/aws/fake/ec2api.go | 7 ++-- pkg/cloudprovider/aws/instance.go | 4 +-- pkg/cloudprovider/aws/launchtemplate.go | 2 +- pkg/cloudprovider/aws/suite_test.go | 44 ++++++++++++------------- 6 files changed, 52 insertions(+), 48 deletions(-) diff --git a/designs/aws-launch-templates-options.md b/designs/aws-launch-templates-options.md index 2bbc25c039c8..fd04586a3a82 100644 --- a/designs/aws-launch-templates-options.md +++ b/designs/aws-launch-templates-options.md @@ -32,7 +32,7 @@ spec: # behavior. Additional labels may be supported by your cloudprovider. labels: # These are AWS-specific - kubernetes.amazonaws.com/launchTemplateId: + kubernetes.amazonaws.com/launchTemplateName: kubernetes.amazonaws.com/launchTemplateVersion: kubernetes.amazonaws.com/capacityType: ``` @@ -123,10 +123,17 @@ Furthermore, in keeping with Kubernetes we should use `node.k8s.aws/` as the prefix for any node labels we choose to use. -Lastly, in keeping with another Kubernetes convention (such as +In keeping with another Kubernetes convention (such as `kubernetes.io/service-name`), we should use `lower-case-with-hypens`, not `camelCase` for words following the `/`. +Lastly, using launch template names, rather than ids, will be more +familiar to Kubernetes users and should cut down on manual work and +errors. Since the EC2 APIs that use launch templates can take either +names or ids, it seems like names are an more useful choice. In the +future we can consider adding id support as well, if there is demand +for it. + ## Architecture One partial solution to the architecture issue brought up above would @@ -146,9 +153,9 @@ spec: labels: # applied only to arm64 arm64: - node.k8s.aws/launch-template-id: id-of-arm64-lt + node.k8s.aws/launch-template-name: name-of-arm64-lt x86_64: - node.k8s.aws/launch-template-id: id-of-x86_64-lt + node.k8s.aws/launch-template-name: name-of-x86_64-lt # applied everywhere other-label: other-value ``` @@ -164,7 +171,7 @@ spec: other-label: other-value # applied only to arm64 arm64-labels: - node.k8s.aws/launch-template-id: id-of-arm64-based-lt + node.k8s.aws/launch-template-name: name-of-arm64-based-lt # applied only to x86_64 x86_64-labels: ``` @@ -177,9 +184,9 @@ apiVersion: provisioning.karpenter.sh/v1alpha2 kind: Provisioner spec: labels: - node.k8s.aws/launch-template-id/arm64: id-of-arm64-lt + node.k8s.aws/launch-template-name/arm64: name-of-arm64-lt # or? - node.k8s.aws/arm64/launch-template-id: id-of-arm64-lt + node.k8s.aws/arm64/launch-template-name: name-of-arm64-lt ``` This might be very non-standard however and defy expectations. Also, @@ -208,12 +215,12 @@ apiVersion: provisioning.karpenter.sh/v1alpha2 kind: Provisioner spec: labels: - node.k8s.aws/launch-template-id: id-of-lt-with-x86_64-based-ami + node.k8s.aws/launch-template-name: name-of-lt-with-x86_64-based-ami ``` Then it could ignore that pending pod since there is no way it could possibly work. Another possiblity is that it could ignore the launch -template `node.k8s.aws/launch-template-id` for that pod and revert to +template `node.k8s.aws/launch-template-name` for that pod and revert to the default dynamically-generated ([Bottlerocket](https://aws.amazon.com/bottlerocket/)) launch template that would work for that architecture. @@ -224,11 +231,11 @@ that would work for that architecture. For now the recommendation is to support the following in provisioner `spec.labels`: -- `node.k8s.aws/launch-template-id`: (optional) id of launch template +- `node.k8s.aws/launch-template-name`: (optional) id of launch template and cannot be specified if `architecture` - `node.k8s.aws/launch-template-version`: version number or `$LATEST` (optional, default `$LATEST`) (cannot be specified unless - `node.k8s.aws/launch-template-id` is present + `node.k8s.aws/launch-template-name` is present - `node.k8s.aws/capacity-type`: listed here for completeness ### The Fine Print @@ -254,7 +261,7 @@ metadata: name: podA spec: nodeSelector: - node.k8s.aws/launch-template-id=x + node.k8s.aws/launch-template-name=x --- apiVersion: v1 kind: Pod @@ -268,7 +275,7 @@ two instances would launch. #### Multiple Provisioners -While specifying a `launch-template-id` on a provisioner will limit +While specifying a `launch-template-name` on a provisioner will limit that provisioner to a single default launch template (pod specs can still override by specifying a launch template), it seems like the complexity of the other solutions that might allow multiple launch diff --git a/pkg/cloudprovider/aws/constraints.go b/pkg/cloudprovider/aws/constraints.go index e02a005d6067..e73d2d2bb8f3 100644 --- a/pkg/cloudprovider/aws/constraints.go +++ b/pkg/cloudprovider/aws/constraints.go @@ -34,14 +34,14 @@ const ( var ( AWSLabelPrefix = "node.k8s.aws/" CapacityTypeLabel = AWSLabelPrefix + "capacity-type" - LaunchTemplateIdLabel = AWSLabelPrefix + "launch-template-id" + LaunchTemplateNameLabel = AWSLabelPrefix + "launch-template-name" SubnetNameLabel = AWSLabelPrefix + "subnet-name" SubnetTagKeyLabel = AWSLabelPrefix + "subnet-tag-key" SecurityGroupNameLabel = AWSLabelPrefix + "security-group-name" SecurityGroupTagKeyLabel = AWSLabelPrefix + "security-group-tag-key" AllowedLabels = []string{ CapacityTypeLabel, - LaunchTemplateIdLabel, + LaunchTemplateNameLabel, SubnetNameLabel, SubnetTagKeyLabel, SecurityGroupNameLabel, @@ -68,17 +68,17 @@ func (c *Constraints) GetCapacityType() string { } type LaunchTemplate struct { - Id string + Name string Version string } func (c *Constraints) GetLaunchTemplate() *LaunchTemplate { - id, ok := c.Labels[LaunchTemplateIdLabel] + name, ok := c.Labels[LaunchTemplateNameLabel] if !ok { return nil } return &LaunchTemplate{ - Id: id, + Name: name, Version: DefaultLaunchTemplateVersion, } } diff --git a/pkg/cloudprovider/aws/fake/ec2api.go b/pkg/cloudprovider/aws/fake/ec2api.go index 353fd600dcce..3e72491a16d9 100644 --- a/pkg/cloudprovider/aws/fake/ec2api.go +++ b/pkg/cloudprovider/aws/fake/ec2api.go @@ -62,9 +62,8 @@ func (e *EC2API) Reset() { func (e *EC2API) CreateFleetWithContext(ctx context.Context, input *ec2.CreateFleetInput, options ...request.Option) (*ec2.CreateFleetOutput, error) { e.CalledWithCreateFleetInput.Add(input) - if input.LaunchTemplateConfigs[0].LaunchTemplateSpecification.LaunchTemplateId == nil && - input.LaunchTemplateConfigs[0].LaunchTemplateSpecification.LaunchTemplateName == nil { - return nil, fmt.Errorf("missing launch template id or name") + if input.LaunchTemplateConfigs[0].LaunchTemplateSpecification.LaunchTemplateName == nil { + return nil, fmt.Errorf("missing launch template name") } instance := &ec2.Instance{ InstanceId: aws.String(randomdata.SillyName()), @@ -78,7 +77,7 @@ func (e *EC2API) CreateFleetWithContext(ctx context.Context, input *ec2.CreateFl func (e *EC2API) CreateLaunchTemplateWithContext(ctx context.Context, input *ec2.CreateLaunchTemplateInput, options ...request.Option) (*ec2.CreateLaunchTemplateOutput, error) { e.CalledWithCreateLaunchTemplateInput.Add(input) - launchTemplate := &ec2.LaunchTemplate{LaunchTemplateName: input.LaunchTemplateName, LaunchTemplateId: aws.String("test-launch-template-id")} + launchTemplate := &ec2.LaunchTemplate{LaunchTemplateName: input.LaunchTemplateName} e.LaunchTemplates.Store(input.LaunchTemplateName, launchTemplate) return &ec2.CreateLaunchTemplateOutput{LaunchTemplate: launchTemplate}, nil } diff --git a/pkg/cloudprovider/aws/instance.go b/pkg/cloudprovider/aws/instance.go index a8a36f419695..b89aa2c9e4b7 100644 --- a/pkg/cloudprovider/aws/instance.go +++ b/pkg/cloudprovider/aws/instance.go @@ -147,8 +147,8 @@ func (p *InstanceProvider) launchInstance(ctx context.Context, }, LaunchTemplateConfigs: []*ec2.FleetLaunchTemplateConfigRequest{{ LaunchTemplateSpecification: &ec2.FleetLaunchTemplateSpecificationRequest{ - LaunchTemplateId: aws.String(launchTemplate.Id), - Version: aws.String(launchTemplate.Version), + LaunchTemplateName: aws.String(launchTemplate.Name), + Version: aws.String(launchTemplate.Version), }, Overrides: overrides, }}, diff --git a/pkg/cloudprovider/aws/launchtemplate.go b/pkg/cloudprovider/aws/launchtemplate.go index ed8c9ff784f3..fc25f5cc9f0f 100644 --- a/pkg/cloudprovider/aws/launchtemplate.go +++ b/pkg/cloudprovider/aws/launchtemplate.go @@ -114,7 +114,7 @@ func (p *LaunchTemplateProvider) Get(ctx context.Context, provisioner *v1alpha3. return nil, err } return &LaunchTemplate{ - Id: aws.StringValue(launchTemplate.LaunchTemplateId), + Name: aws.StringValue(launchTemplate.LaunchTemplateName), Version: fmt.Sprint(DefaultLaunchTemplateVersion), }, nil } diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index 8644c15df761..4dd4117a536c 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -276,100 +276,98 @@ var _ = Describe("Allocation", func() { pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, test.PendingPod()) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) - Expect(node.Labels).ToNot(HaveKey(LaunchTemplateIdLabel)) + Expect(node.Labels).ToNot(HaveKey(LaunchTemplateNameLabel)) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification - Expect(*launchTemplate.LaunchTemplateId).To(Equal("test-launch-template-id")) Expect(*launchTemplate.Version).To(Equal(DefaultLaunchTemplateVersion)) }) It("should default to a provisioner's launch template id and version", func() { // Setup provisioner.Spec.Labels = map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), + LaunchTemplateNameLabel: randomdata.SillyName(), } ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, test.PendingPod()) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, provisioner.Spec.Labels[LaunchTemplateIdLabel])) + Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateNameLabel, provisioner.Spec.Labels[LaunchTemplateNameLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification - Expect(*launchTemplate.LaunchTemplateId).To(Equal(provisioner.Spec.Labels[LaunchTemplateIdLabel])) - + Expect(*launchTemplate.LaunchTemplateName).To(Equal(provisioner.Spec.Labels[LaunchTemplateNameLabel])) }) It("should default to a provisioner's launch template and the default launch template version", func() { // Setup - provisioner.Spec.Labels = map[string]string{LaunchTemplateIdLabel: randomdata.SillyName()} + provisioner.Spec.Labels = map[string]string{LaunchTemplateNameLabel: randomdata.SillyName()} ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, test.PendingPod()) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, provisioner.Spec.Labels[LaunchTemplateIdLabel])) + Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateNameLabel, provisioner.Spec.Labels[LaunchTemplateNameLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification - Expect(*launchTemplate.LaunchTemplateId).To(Equal(provisioner.Spec.Labels[LaunchTemplateIdLabel])) + Expect(*launchTemplate.LaunchTemplateName).To(Equal(provisioner.Spec.Labels[LaunchTemplateNameLabel])) Expect(*launchTemplate.Version).To(Equal(DefaultLaunchTemplateVersion)) }) It("should allow a pod to override the launch template id and version", func() { // Setup provisioner.Spec.Labels = map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), + LaunchTemplateNameLabel: randomdata.SillyName(), } ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, test.PendingPod(test.PodOptions{NodeSelector: map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), + LaunchTemplateNameLabel: randomdata.SillyName(), }}), ) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) + Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateNameLabel, pods[0].Spec.NodeSelector[LaunchTemplateNameLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification - Expect(*launchTemplate.LaunchTemplateId).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) + Expect(*launchTemplate.LaunchTemplateName).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateNameLabel])) }) It("should allow a pod to override the launch template id and use the default launch template version", func() { // Setup - provisioner.Spec.Labels = map[string]string{LaunchTemplateIdLabel: randomdata.SillyName()} + provisioner.Spec.Labels = map[string]string{LaunchTemplateNameLabel: randomdata.SillyName()} ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, - test.PendingPod(test.PodOptions{NodeSelector: map[string]string{LaunchTemplateIdLabel: randomdata.SillyName()}}), + test.PendingPod(test.PodOptions{NodeSelector: map[string]string{LaunchTemplateNameLabel: randomdata.SillyName()}}), ) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) + Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateNameLabel, pods[0].Spec.NodeSelector[LaunchTemplateNameLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification - Expect(*launchTemplate.LaunchTemplateId).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) + Expect(*launchTemplate.LaunchTemplateName).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateNameLabel])) Expect(*launchTemplate.Version).To(Equal(DefaultLaunchTemplateVersion)) }) It("should allow a pod to override the launch template id and use the provisioner's launch template version", func() { // Setup provisioner.Spec.Labels = map[string]string{ - LaunchTemplateIdLabel: randomdata.SillyName(), + LaunchTemplateNameLabel: randomdata.SillyName(), } ExpectCreated(env.Client, provisioner) pods := ExpectProvisioningSucceeded(ctx, env.Client, controller, provisioner, - test.PendingPod(test.PodOptions{NodeSelector: map[string]string{LaunchTemplateIdLabel: randomdata.SillyName()}}), + test.PendingPod(test.PodOptions{NodeSelector: map[string]string{LaunchTemplateNameLabel: randomdata.SillyName()}}), ) // Assertions node := ExpectNodeExists(env.Client, pods[0].Spec.NodeName) - Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateIdLabel, pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) + Expect(node.Labels).To(HaveKeyWithValue(LaunchTemplateNameLabel, pods[0].Spec.NodeSelector[LaunchTemplateNameLabel])) Expect(fakeEC2API.CalledWithCreateFleetInput.Cardinality()).To(Equal(1)) input := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput) Expect(input.LaunchTemplateConfigs).To(HaveLen(1)) launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification - Expect(*launchTemplate.LaunchTemplateId).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateIdLabel])) + Expect(*launchTemplate.LaunchTemplateName).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateNameLabel])) }) }) Context("Subnets", func() { @@ -581,12 +579,12 @@ var _ = Describe("Allocation", func() { }) It("should support launch templates", func() { provisioner.Spec.Labels = map[string]string{ - "node.k8s.aws/launch-template-id": "23", + "node.k8s.aws/launch-template-name": "23", } Expect(provisioner.Validate(ctx)).To(Succeed()) }) It("should allow launch template id to be specified alone", func() { - provisioner.Spec.Labels = map[string]string{"node.k8s.aws/launch-template-id": "23"} + provisioner.Spec.Labels = map[string]string{"node.k8s.aws/launch-template-name": "23"} Expect(provisioner.Validate(ctx)).To(Succeed()) }) It("should fail if only launch template version label present", func() { From cf2aaa1b535e3857f86a6d224335cb9ad0fb4691 Mon Sep 17 00:00:00 2001 From: jacob Date: Tue, 3 Aug 2021 21:58:44 -0700 Subject: [PATCH 3/4] Further test clean up --- pkg/cloudprovider/aws/suite_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index 4dd4117a536c..a198d5bce762 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -579,18 +579,14 @@ var _ = Describe("Allocation", func() { }) It("should support launch templates", func() { provisioner.Spec.Labels = map[string]string{ - "node.k8s.aws/launch-template-name": "23", + "node.k8s.aws/launch-template-name": randomdata.SillyName(), } Expect(provisioner.Validate(ctx)).To(Succeed()) }) It("should allow launch template id to be specified alone", func() { - provisioner.Spec.Labels = map[string]string{"node.k8s.aws/launch-template-name": "23"} + provisioner.Spec.Labels = map[string]string{"node.k8s.aws/launch-template-name": randomdata.SillyName()} Expect(provisioner.Validate(ctx)).To(Succeed()) }) - It("should fail if only launch template version label present", func() { - provisioner.Spec.Labels = map[string]string{"node.k8s.aws/launch-template-version": randomdata.SillyName()} - Expect(provisioner.Validate(ctx)).ToNot(Succeed()) - }) It("should support on demand capacity type", func() { provisioner.Spec.Labels = map[string]string{"node.k8s.aws/capacity-type": CapacityTypeOnDemand} Expect(provisioner.Validate(ctx)).To(Succeed()) From 903c177407677e18de622399146effafba6f8fa8 Mon Sep 17 00:00:00 2001 From: jacob Date: Tue, 3 Aug 2021 22:02:41 -0700 Subject: [PATCH 4/4] fix overzealously-edited doc change --- designs/aws-launch-templates-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/aws-launch-templates-options.md b/designs/aws-launch-templates-options.md index fd04586a3a82..5e2409d55e94 100644 --- a/designs/aws-launch-templates-options.md +++ b/designs/aws-launch-templates-options.md @@ -32,7 +32,7 @@ spec: # behavior. Additional labels may be supported by your cloudprovider. labels: # These are AWS-specific - kubernetes.amazonaws.com/launchTemplateName: + kubernetes.amazonaws.com/launchTemplateId: kubernetes.amazonaws.com/launchTemplateVersion: kubernetes.amazonaws.com/capacityType: ```