Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow to choose launch template version for Managed Node Groups when create_launch_template is set to true #1419

Merged

Conversation

ArchiFleKs
Copy link
Contributor

@ArchiFleKs ArchiFleKs commented Jun 1, 2021

PR o'clock

Description

By default choose latest LT version. This trigger a rollout of all the instances. I noticed this because of the description change here

This PR allow to override launch template version to postpone instances rollout if needed

Checklist

@barryib
Copy link
Member

barryib commented Jun 1, 2021

😢

In docs https://docs.aws.amazon.com/eks/latest/userguide/launch-templates.html it says:

When you update your node group to a different version of your launch template, all of the nodes in the group are recycled to match the new configuration of the specified launch template version. Existing node groups that do not use a custom launch template can't be updated directly. Rather, you must create a new node group with a custom launch template to do so.

@barryib barryib changed the title fix: allow to choose LT version with create_launch_template feat: Allow to choose launch template version with create_launch_template Jun 1, 2021
@ArchiFleKs
Copy link
Contributor Author

😢

In docs https://docs.aws.amazon.com/eks/latest/userguide/launch-templates.html it says:

When you update your node group to a different version of your launch template, all of the nodes in the group are recycled to match the new configuration of the specified launch template version. Existing node groups that do not use a custom launch template can't be updated directly. Rather, you must create a new node group with a custom launch template to do so.

@barryib Do you see any issue with that ?

For me when not using Create/Custom LT the only thing that is going to trigger an update if changing the label maybe and the version right ?

@@ -48,7 +48,7 @@ resource "aws_eks_node_group" "workers" {
dynamic "launch_template" {
for_each = each.value["launch_template_id"] == null && each.value["create_launch_template"] ? [{
id = aws_launch_template.workers[each.key].id
version = aws_launch_template.workers[each.key].latest_version
version = each.value["launch_template_version"] != null ? each.value["launch_template_version"] : aws_launch_template.workers[each.key].latest_version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just wondering how user will get the launch_template_version if it's created directly by the module ? They have to go through the aws cli or aws console ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think they can see it in the TF or in cli/console. Most of the time if think you want to upgrade (that's one of the perk of MNG vs Self managed without instance refresh) but it might be useful when you have large pool and do not want to trigger an upgrade yet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that launch_template_version = "$Latest" by default https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/local.tf#L104. In that case, should we need this ternary evaluation ?

Does version = each.value["launch_template_version"] enough ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should be, fixed

Copy link
Contributor

@jaimehrubiks jaimehrubiks Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barryib
Copy link
Member

barryib commented Jun 1, 2021

@barryib Do you see any issue with that ?

I was just saying that it is intended behavior ^^.

For me when not using Create/Custom LT the only thing that is going to trigger an update if changing the label maybe and the version right ?

I think you're right. I don't use MNG at all (for now).

@barryib barryib changed the title feat: Allow to choose launch template version with create_launch_template feat: Allow to choose launch template version for Managed Node Groups when create_launch_template is set to true Jun 1, 2021
@barryib barryib merged commit 4c1f272 into terraform-aws-modules:master Jun 3, 2021
@barryib
Copy link
Member

barryib commented Jun 3, 2021

Thanks @ArchiFleKs for your contribution.

@barryib
Copy link
Member

barryib commented Jun 9, 2021

Just released it in v17.1.0

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants