Use LaunchTemplate versions instead of timestamped LaunchTemplates #10151

hakman · 2020-11-02T08:32:39Z

Historically, kOps used LaunchConfigurations for configuring AutoScalingGroups with the IG name as prefix and creation timestamp as suffix (nodes-a.k8s.example.com-20200806091534).

Newer versions of kOps added the possibility of using LaunchTemplates for configuring AutoScalingGroups, as some features are not available for when using LaunchConfigurations. Even though LaunchTemplates allow versioning, kOps uses the same naming convention as for LaunchConfigurations, but having to deal with LaunchTemplateVersions also and generating many requests to the AWS API, as seen in #9806.

kOps 1.19 introduces LaunchTemplates as the default AutoScalingGroups configuration method and switching to versions should simplify things:

the LaunchTemplate would have a fixed name identical to the IG name
ASGs will use the $Latest LaunchTemplate version
all versions will be kept (for now) as there AWS limit is 10.000 versions for each LaunchTemplate
all previous LaunchTemplates with timestamp prefixed names will be deleted, reducing the clutter in the LaunchTemplate list

CloudFormation output will not be affected by these changes.

Terraform output will change to using name instead of name_prefix (another leftover from the LaunchConfigurations days).

k8s-ci-robot · 2020-11-02T08:32:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [hakman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

hakman · 2020-11-02T13:36:50Z

/hold for feedback

hakman · 2020-11-03T14:41:06Z

/test pull-kops-bazel-test

hakman · 2020-11-03T16:29:40Z

/retest

pkg/resources/aws/aws.go

upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go

johngmyers · 2020-11-06T06:48:01Z

upup/pkg/fi/cloudup/awstasks/launchtemplate.go

 	for _, configuration := range configurations {
-		removals = append(removals, &deleteLaunchTemplate{lc: configuration})
+		if strings.HasPrefix(aws.StringValue(configuration.LaunchTemplateName), prefix) {


Why is this necessary? Wouldn't all the old launch templates be deleted as owned by the cluster but unreferenced by an ASG?

There can be N previous LTs for each ASG. already unreferenced that should be deleted.
Maybe there's some mechanism that I'm missing.

Looks like I misunderstood the Find / FindDeletions mechanisms. I thought it would look for resources tagged as owned by the cluster and delete them if they're not expected.

johngmyers · 2020-11-06T06:53:50Z

upup/pkg/fi/cloudup/awstasks/launchtemplate_target_api.go

+		}
+		e.ID = output.LaunchTemplate.LaunchTemplateId
+	} else {
+		// TODO: Update the LaunchTemplate tags


I think implementing this might block merging.

Hmm, already implemented it in 852c0e2 and seems to work without issues. Nothing actually reads these tags after creation, they are just used for deleting the LT when needed.

upup/pkg/fi/cloudup/awsup/aws_cloud.go

hakman · 2020-11-08T04:05:35Z

/hold cancel

johngmyers · 2020-11-11T06:42:15Z

/lgtm

hakman · 2020-11-11T06:51:37Z

Thanks @johngmyers :)

…-upstream-release-1.19 Automated cherry pick of #10151: Use LT versions instead of timestamped LTs

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 2, 2020

k8s-ci-robot added the area/provider/aws Issues or PRs related to aws provider label Nov 2, 2020

k8s-ci-robot requested review from johngmyers and KashifSaadat November 2, 2020 08:32

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 2, 2020

hakman force-pushed the launch-template-versions branch 2 times, most recently from bbbc5ba to bf79b86 Compare November 2, 2020 11:11

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 2, 2020

hakman force-pushed the launch-template-versions branch from bf79b86 to 4328a00 Compare November 2, 2020 13:47

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2020

hakman force-pushed the launch-template-versions branch from 4328a00 to dcc7444 Compare November 3, 2020 14:38

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 3, 2020

hakman force-pushed the launch-template-versions branch from dcc7444 to 992b16d Compare November 4, 2020 10:05

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2020

hakman force-pushed the launch-template-versions branch from 992b16d to 852c0e2 Compare November 5, 2020 18:11

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2020

johngmyers requested changes Nov 6, 2020

View reviewed changes

hakman force-pushed the launch-template-versions branch from b1537a5 to 705fd35 Compare November 7, 2020 08:20

hakman requested a review from johngmyers November 7, 2020 08:22

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2020

hakman mentioned this pull request Nov 8, 2020

Fix disabling spot instances when using launch templates #10198

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2020

Ciprian Hacman added 7 commits November 9, 2020 21:40

Use LaunchTemplate versions instead of timestamped LaunchTemplates

565adce

Use LaunchTemplate name instead of name_prefix for Terraform

0c3e378

Update LaunchTemplate tags on changes

057ab28

Use LaunchTemplate id instead of name for rolling updates

7b04b97

Use LaunchTemplate id instead of name for ASGs

ed37c34

Use pagination when listing LaunchTemplates

fdf9768

Address review comments

1d6a51a

hakman force-pushed the launch-template-versions branch from 705fd35 to 1d6a51a Compare November 9, 2020 19:42

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2020

kubernetes deleted a comment from k8s-ci-robot Nov 10, 2020

k8s-ci-robot assigned johngmyers Nov 11, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 11, 2020

k8s-ci-robot merged commit 9b3f13d into kubernetes:master Nov 11, 2020

k8s-ci-robot added this to the v1.20 milestone Nov 11, 2020

hakman deleted the launch-template-versions branch November 11, 2020 07:24

hakman mentioned this pull request Nov 12, 2020

Automated cherry pick of #10151: Use LT versions instead of timestamped LTs #10226

Merged

k8s-ci-robot added a commit that referenced this pull request Nov 16, 2020

Merge pull request #10226 from hakman/automated-cherry-pick-of-#10151…

6cf691b

…-upstream-release-1.19 Automated cherry pick of #10151: Use LT versions instead of timestamped LTs

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use LaunchTemplate versions instead of timestamped LaunchTemplates #10151

Use LaunchTemplate versions instead of timestamped LaunchTemplates #10151

hakman commented Nov 2, 2020 •

edited

Loading

k8s-ci-robot commented Nov 2, 2020

hakman commented Nov 2, 2020

hakman commented Nov 3, 2020

hakman commented Nov 3, 2020

johngmyers Nov 6, 2020

hakman Nov 6, 2020

johngmyers Nov 11, 2020

johngmyers Nov 6, 2020

hakman Nov 6, 2020

hakman commented Nov 8, 2020

johngmyers commented Nov 11, 2020

hakman commented Nov 11, 2020

Use LaunchTemplate versions instead of timestamped LaunchTemplates #10151

Use LaunchTemplate versions instead of timestamped LaunchTemplates #10151

Conversation

hakman commented Nov 2, 2020 • edited Loading

k8s-ci-robot commented Nov 2, 2020

hakman commented Nov 2, 2020

hakman commented Nov 3, 2020

hakman commented Nov 3, 2020

johngmyers Nov 6, 2020

Choose a reason for hiding this comment

hakman Nov 6, 2020

Choose a reason for hiding this comment

johngmyers Nov 11, 2020

Choose a reason for hiding this comment

johngmyers Nov 6, 2020

Choose a reason for hiding this comment

hakman Nov 6, 2020

Choose a reason for hiding this comment

hakman commented Nov 8, 2020

johngmyers commented Nov 11, 2020

hakman commented Nov 11, 2020

hakman commented Nov 2, 2020 •

edited

Loading