fix: Allow instance Name tag to be overwritten

[aidan-mundy]

PR o'clock

Description

Allow instance Name tag to be overwritten by the user-defined node-group variable additional_tags. Without this PR, it is impossible to define custom names for instances created as part of a managed node group while using a generated launch template. It was possible to affect the name of the instance by setting a different name for the node-group, but there does not appear to be a technical reason for this to be mandated.

Checklist

• README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

[daroga0002]

@aidan-mundy very smart solution 🥇

@antonbabenko 👍

[antonbabenko]

@bryantbiggs @barryib This is going to be a breaking change and users expect this feature to be in a major release, right?

I will mark it with the breaking change label and maybe group it with some other change.

@barryib What do you normally do with this kind of change?

[daroga0002]

Why this will be breaking change?

I dont think it will change anything as node_groups_defaults by default is empty

terraform-aws-eks/variables.tf

Lines 322 to 326 in 3b12298

	variable "node_groups_defaults" {
	description = "Map of values to be applied to all node groups. See `node_groups` module's documentation for more details"
	type = any
	default = {}
	}

var.tags is also empty:

https://github.com/terraform-aws-modules/terraform-aws-eks/blob/3b1229837ad61d6e3420196fe3870cc406940cc8/variables.tf#L95-L99

In worst case change with will be done is overwrite Name tag which doesnt have a bigger difference from EKS point of view (overwrite only if somoebody specified into node_groups_defaults map Name tag.

To summarise I think this can be safely merged

[antonbabenko]

@daroga0002 I think you are right. This change is definitely a fix rather than a breaking change. Too late for me & too much EKS for today :)

PS: I will merge it tomorrow morning if other guys don't have anything against it.

[goetzc]

Is this (v17.18.0) only for launch_template? Instances generated by node_groups still have no Name tag.

[daroga0002]

this is for node groups which are creating AMI launch template when you have create_launch_template=true

[aidan-mundy]

@goetzc As far as I know, an ASG a node group cannot propagate tags to instances without a launch template. Create a launch template and use it for your node group (or use create_launch_template=true) in order to support tags (including the "Name" parameter, which is just a resource tag that is treated specially in the Console UI)

[lisfo4ka]

Folks, it's a nice idea, but what about worker groups? Should we provide the same option to define custom names to instances for worker groups as well?

There is quite a different approach, but it's hardcoded to the exact pattern without the option to redefine.

@aidan-mundy @daroga0002 @antonbabenko

[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.