fix: Add instance tag specification to launch template

[js-timbirkett]

PR o'clock

Description

Tags are being applied after instances are launched whils in pending state.

Adding a tag_specification for instance resources allows ensures that
instances are tagged during launch.

Fixes: #753

Checklist

• Change added to CHANGELOG.md. All changes must be added and breaking changes and highlighted
• CI tests are passing
• README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

[max-rocket-internet]

TBH I don't really understand the changes in this PR as I don't have experience here.

@barryib do you know how tag_specifications works and what "Tags are being applied after instances are launched" does?

[barryib]

@max-rocket-internet I don't use it either, but the use case sounds legit to me. Maybe @dpiddockcmp ?

From what I understood in https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html, it's add tags to EC2 instances during their creation.

@Art3mK does this resolve your issue #753 ?

[Art3mK]

yeah, I think so! Thanks for checking this!

Use case is that with spot instances ASG will add tags to instances after those are created

The exception is when you use a launch configuration to launch Spot Instances. For this scenario, your Auto Scaling group adds tags while the instances are in the Pending lifecycle state

Problem with this is that if you have policy, which denies you to launch instance without proper tagging - there is no way currently with this module to do that. And error message from API is very cryptic. See issue. That took me some time to solve that :)

This could be solved with adding that tag_specification for instances also in launch template (there is already one for volumes).

I'm not sure though how these tags for instances defined in launch template will work with tags defined on ASG itself. Would be there any conflicts/overwrites if both defines the same tag?

[max-rocket-internet]

I'm not sure though how these tags for instances defined in launch template will work with tags defined on ASG itself. Would be there any conflicts/overwrites if both defines the same tag?

True. I'm not sure. As there would be a lot of overlap. @js-timbirkett do you know? Are they just merged?

[barryib]

I'm not sure though how these tags for instances defined in launch template will work with tags defined on ASG itself. Would be there any conflicts/overwrites if both defines the same tag?

True. I'm not sure. As there would be a lot of overlap. @js-timbirkett do you know? Are they just merged?

@js-timbirkett can you confirm please and remove your changes from changelog. It's now auto generated during the release process.

[barryib]

@js-timbirkett are you still working on this ? There are some questions to answer before we get this PR merged.

[barryib]

Can you please your changes in changelog ? It's now generated from git commits during release.

[Reuuke]

I'm not sure though how these tags for instances defined in launch template will work with tags defined on ASG itself. Would be there any conflicts/overwrites if both defines the same tag?

True. I'm not sure. As there would be a lot of overlap. @js-timbirkett do you know? Are they just merged?

@max-rocket-internet Yep. They just merged. And tags from ASG have a higher priority then tags from LT if they match on tag key (I tested today)

[barryib]

Great closing this in favor of #822

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.