search

terraform-aws-modules / terraform-aws-eks

Terraform module to create Amazon Elastic Kubernetes (EKS) resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/eks/aws
Apache License 2.0
4.44k stars 4.06k forks source link

Changing instance_types in MNGs causes destroy/recreate #1415

Closed cabrinha closed 3 years ago

cabrinha commented 3 years ago

Description

Seems like if I deploy a MNG with a list of instance types, then remove or add an instance type, I get the error:

ResourceInUseException: NodeGroup already exists with name <MNG name> and cluster name <cluster name>

Versions

Reproduction

Steps to reproduce the behavior: Create a MNG with instance_types of any value.

Edit the instance_types list by adding or removing any instance types.

Code Snippet to Reproduce

Expected behavior

I expect the launch template to be updated and don't expect the MNG to need to be recreated entirely.

Actual behavior

ResourceInUseException says the MNG already exists with the same name.

Additional context

I wonder if create_before_destroy should be implemented here if the MNGs really do need to be recreated due to instance types changing.

barryib commented 3 years ago

Is this still happen in v17.x.x ? We dropped random pets. Can you please try to upgrade your module ?

Please see https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/docs/upgrades.md and changelog for more info.

cabrinha commented 3 years ago

Is this still happen in v17.x.x ? We dropped random pets. Can you please try to upgrade your module ?

Please see https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/docs/upgrades.md and changelog for more info.

I missed the (by removing the name argument) part. Seems to be working well now, I'll do a little more testing and then close the issue.

cabrinha commented 3 years ago

Working well now! Closing.

jaimehrubiks commented 3 years ago

@cabrinha @barryib Is it even possible to change the instance_types of managed node groups in this module? My terraform will destroy/recreate the whole node group when attempt so.

Thing is, I don't even see that option in the AWS console. When I click on modify Node Group, I can only change number of instances, labels, taints and tags. I also see that the instance type is specified only at the node group level (which is also reflected in the default instance list of the underlying autoscaling group), but not on the launch template itself.

According to https://github.com/aws/containers-roadmap/issues/746 it is not possible.

But also, it says that this is solved by the introduction of launch templates: https://github.com/aws/containers-roadmap/issues/585#issuecomment-675093297

So this means that it order to benefit from this feature we should be passing the instance type to the launch template.

This is related to https://github.com/terraform-aws-modules/terraform-aws-eks/issues/1386

It turns out that this module is specifying the instance_type on the node group creation API call, and leaving it empty on the launch template (It is clear that you cannot do it in both places). I would expect that if the "create_launch_template" variable is set to true (or a new variable not to cause breaking changes), the instance_type should go there, in the template, so we can benefit from rolling updates.

Edit: Didn't know that the variable "set_instance_types_on_lt" existed. So it is fine, we have both options :) I guess that for regular instances it is better to set it to true and use LT for the instance so you benefit from rolling updates. For spot I think it is better false because you can have more instance types in the node groups than you can in the LT

github-actions[bot] commented 1 year ago

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.