Add instance group option to use LaunchTemplate

[hakman]

As agreed during Office Hours, launch templates should move from experimental to an InstanceGroup option.

At the moment, the main reason to switch to launch templates would be to get tags for root volumes and other resources. Versions should also be explored, instead of creating a launch template on each update.

This should be merged before #8693 to see how those changes affect the launch templates.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hakman
To complete the pull request process, please assign chrislovecnm
You can assign the PR to them by writing /assign @chrislovecnm in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[hakman]

/cc @rifelpet @KashifSaadat @granular-ryanbonham

[rifelpet]

This ASG resource doesnt have the launch_template property that associates it with the aws_launch_template defined below.

https://www.terraform.io/docs/providers/aws/r/autoscaling_group.html#with-latest-version-of-launch-template

I know your other PR mentions that LT werent working at all, but it seems it only fixes CF, not TF. It may be out of scope for this PR but we should definitely fix that because it seems that launch templates wont work with terraform at all currently.

Side note: i'm hoping that #8648 will prevent this from happening again in the future, since I believe this would be caught during a terraform validate or terraform plan

[hakman]

Yes, that was the main problem with TF. The other PR should fix it with this:

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

Lines 736 to 740 in 1294d97

	} else if e.LaunchTemplate != nil {
	tf.LaunchTemplate = &terraformAutoscalingLaunchTemplateSpecification{
	LaunchTemplateID: e.LaunchTemplate.TerraformLink(),
	Version: e.LaunchTemplate.VersionLink(),
	}

[johngmyers]

I don't remember that agreement; that comment must have passed under my radar.

What is the value in keeping support for launch configurations long-term? Wouldn't a better progression of the feature be to change the default of the feature flag and then later remove support for launch configurations?

With the launch templates code needing so many fixes this round, perhaps taking them out from under the feature flag should be deferred to kops 1.19.

[hakman]

@johngmyers this was the agreement at the Jan 31 meeting. I asked if the flag can be removed. The answer was that yes, because people are already using it in mixed instances. But, to be safe, first change it to instancegroup option, disabled by default.

There is no value in keeping support for launch configuration long term. We cannot remove them until we are sure that the upgrade for existing clusters is without issues.

Kops feature flags are hard to use and test. This is why it's best to move it to option for now and once we are comfortable, we can set it as default.

[rifelpet]

One other benefit of using an API field rather than feature flag is it allows the cluster administrator to rollout the LC -> LT migration on a per-IG basis rather than all InstanceGroups at once. The downside is that the field's relevance is relatively short. At some point we'd want to have new clusters default to using LaunchTemplates and eventually consider having existing clusters migrated by default as well.

If we're looking for precedence for making this an API field, I think a comparable field would be the etcd Provider, it was primarily relevant during the etcd2/etcd3 migration but long term I don't see us needing to support the standalone value.

[johngmyers]

The disadvantage to an API field is that it takes an extremely long time to remove.

Is there a particular reason an admin would want to rollout the migration on a per-IG basis? This doesn't seem like it should be as impactful as changing the etcd provider.

This isn't a strong objection; it just seems a bit overcomplicated and overly user visible.

[hakman]

/assign @justinsb

[hakman]

@justinsb Can you please check that this is as we agreed during office hours?

[hakman]

Closing in favour of #8758