-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add Tags to ASG when using node groups #1558
Comments
you have map terraform-aws-eks/modules/node_groups/node_groups.tf Lines 79 to 83 in 979d62d
|
But those tags are going to the node group not to the autoscaling group related to the node group |
autoscaling group for node groups is done thru EKS api, so you cannot influence it. If you want have own autoscaling group then follow worker nodes approach with custom AMI |
I like the nodegroups managed by aws.
I can do a proposed PR but only if the maintainers agree to the idea |
this approach is not supported by AWS, so probably we shouldn't implement it (even if possible). EKS node group has own limitations which people must accept choosing it, if they want to hack or use unsupported tricks then I think they make it on own responsibility. |
@marianobilli you can do this with a new resource that will be released this week, but as @daroga0002 stated, we won't be adding that here hashicorp/terraform-provider-aws#20009 |
@bryantbiggs the resource you shared is exactly the same I proposed, which I am proposing to add in the terraform-aws-eks module for easier use. |
man you need to relax a bit, if you are in krakow I'll buy you a beer. |
I will admit, I just don't see the benefit of adding it here; adding it externally seems to be a very good approach. The module is already overly complex and we are trying to be thoughtful in what gets added because currently its quite involved in managing it, and tags are an area that create a lot of noise because you can have 100 different users of the module and 199 different ways they want to manage tags |
@bryantbiggs I can appreciate being defensive on adding new features;
I think the argument to add it isn't quite that complex. I see it more as "do you want your node group tagged by the module that created it?" bringing 199 (at least!) usecases to one "yes or no" question ;), such that 'yes' is passing a map through to provider resource ( |
I guess lets see what a PR looks like then |
@marianobilli Let me know if you're still keen to open a PR or want me to run with it. |
|
for some reference |
@wgj Are you working on this? This is how I implemented it on our custom eks module (extending this module):
If you are going to do this on the |
Hello there, I've implemented the tagging of ASGs in managed node groups and I though someone else might find it useful. This is how I did it: Logic in modules/node_groups/locals.tf:
resource call in modules/node_groups/main.tf:
After that, you just need to declare a group to be passed to the node_groups variable like this:
I have the change ready in a fork I created in case you want to have a look (here). Regards. |
Related to my previous message, I've created a PR https://github.com/terraform-aws-modules/terraform-aws-eks/pull/1705/files |
@gabops thanks a lot, it was very helpful! Based on your PR I've added that tags outside of EKS module and it works! locals {
eks_asg_tags = [
{
key = "Create_Auto_Alarms"
value = "true"
propagate_at_launch = true
},
{
key = "AutoAlarm-AWS/EC2-CPUCreditBalance-LessThanThreshold-5m-Average"
value = "30"
propagate_at_launch = true
}
]
eks_asg_tag_list = flatten([
for name, info in module.my_eks_cluster.node_groups : [
[
for tag in local.eks_asg_tags : {
group_name = name
key = tag.key
propagate_at_launch = try(tag.propagate_at_launch, false)
value = tag.value
}
],
[
{
group_name = name
key = "Name"
propagate_at_launch = true
value = "eks-${name}"
}
]
]
])
}
resource "aws_autoscaling_group_tag" "eks" {
for_each = { for map in local.eks_asg_tag_list : "${map.group_name}%${map.key}" => map }
autoscaling_group_name = module.my_eks_cluster.node_groups[split("%", each.key)[0]].resources[0].autoscaling_groups[0].name
tag {
key = each.value.key
value = each.value.value
propagate_at_launch = each.value.propagate_at_launch
}
} |
Hi all, I've updated my PR with some fixes for a few issues I've found when using the module with the changes I've added. I also tested it deeper. I don't have any hopes on having the PR approved and merged given the complaints about the complexity of this module (which I agree with). I suspect the way to go if you want to have this functionality is to add the changes I made to a personal fork of yours or use an approach similar to the great solution proposed by @rkul. Regards |
@marianobilli @bryantbiggs I have used the "external" approach, using the autoscaling_group_tag resource to tag my EKS node_group's ASG with propagate_at_launch set to true. I have confirmed via aws cli that this ASG does indeed have the desired Tags with Key/Value/PropagateAtLaunch, however the EC2 instance launched does NOT have these Tags set. This is all after a complete recreation of my EKS cluster (I'm using the terraform eks module to do this). Can you think of anything that I might have missed that could be preventing the ASG tags from propagating to the EC2 instance launched in the managed node group? Below are the commands I used after creating the cluster to verify my EKS's MNG/ASG/EC2 tags:
|
@marianobilli @bryantbiggs @gabops @rkul I have a theory why my implementation of @rkul 's external solution is not resulting in the first launched ec2 node having the right tags. I believe what is happening is that because the asg tag is being applied outside the eks module, it is happening AFTER the launching of the first ec2 node in the asg. Because ASG tags are only propagated to ec2 instances upon launch, and any changes to asg tags will NOT be applied to already-launched instances, my first-launched ec2 doesn't exhibit the desired tags. Presumably, if I could find a way to force the ASG to launch another ec2 to validate this, but I do not know how to do this at the moment. EDIT: I have proven my theory correct (I think). I have gone into the AWS console and manually increased the "min size" of my node group, and witnessed the second ec2 instance created has the correct tags (propagated from ASG tags) while the original ec2 does NOT. Furthermore, this begs the question, how does this workaround, using the aws_autoscaling_group_tag resource, external-to-the-module solution work for anyone? Or was this 1st instance issue just an oversight? I mention all this because it seems to be a solid case for why this feature should be implemented in the module |
The recommendation for AWS EKS managed node groups is to create a custom launch template. When opting in to the custom launch template route, users are able to specify tags that will be propagated to instances launched. |
Exactly, this PR is not to tag ec2s you can do that from the LT. this is to tag the ASG alone which is useful for the autoscaler or company tagging requirements |
Thanks for that, @bryantbiggs, @jaimehrubiks. If I'm specifying a launch_template (with specified tags) with a managed node group, don't I automatically get new ec2 instance with the prescribed tags, even without the ASG having the tags? |
Exactly, if you put them in the template (either a manually created launch_template, or with the option to create it for you and just specifying tags in this module), yes, you will see them in the ec2 and not in the asg |
This issue has been resolved in version 18.0.0 🎉 |
Is this issue the reason default_tags from the aws provider are not applied to the EC2 instances created when using the resource "aws_eks_node_group"? |
I'm afraid this issue still exists in version ~> 18.0.0. But we can still use the workaround suggested in #1558 (comment) with slight modification, by getting a generated ASG name via |
@tmokmss I am also not seeing tags propagated to the ASG(s). The tags are visible on the Launch Template, and instances, but not the ASG. @antonbabenko Do you know if there is somewhere that shows a working example of this feature? |
Yes, see the |
@bryantbiggs I took a look at the mentioned example. I saw the following comment:
Based on this I left the Here is my simple TF:
Version details for testing:
Unfortunately with the above, the tag I have specified is not propogated to the ASG:
Am I missing anything here? |
Hi @mckennajones, you should be able to add tags to ASGs by the code below: resource "aws_autoscaling_group_tag" "this" {
autoscaling_group_name = module.eks.eks_managed_node_groups["default"].node_group_resources[0].autoscaling_groups[0].name
tag {
key = "k8s.io/cluster-autoscaler/node-template/label/foo"
value = "bar"
propagate_at_launch = false
}
} And I guess it might help if we add this snippet to the examples... |
ya, unfortunately this one is not solved. I tried a bunch of different ways but it doesn't look like we can put the Note: this solution doesn't work resource "aws_autoscaling_group_tag" "this" {
# Build map of maps to iterate over = `for_each` won't take a list of maps here
for_each = { for tag in flatten([
for asg in flatten([
# Unpack autoscaling group name from EKS node group
for resources in try(aws_eks_node_group.this[0].resources, {}) : resources.autoscaling_groups
]) : [
# Map each tag in `var.tags` to each autoscaling group in EKS node group => returns list of maps
for k, v in merge(var.tags, var.autoscaling_group_tags) : { asg = asg.name, key = k, val = v }
]
]) : "${tag.asg}-${tag.key}" => { asg = tag.asg, key = tag.key, val = tag.val } if var.create }
autoscaling_group_name = each.value.asg
tag {
key = each.value.key
value = each.value.val
propagate_at_launch = true
}
depends_on = [
aws_eks_node_group.this
]
} |
@tmokmss thanks for the suggestion. I had to slightly modify the
Difference is referencing @bryantbiggs or @antonbabenko should this issue be re-opened? |
@mckennajones thanks, I fixed my comment. |
We could but like I said above, there doesn't appear to be any path to a resolution within this module |
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. |
Is your request related to a new offering from AWS?
No
Is your request related to a problem? Please describe.
When using nodegroups it is necessary to be able to add tags to the autoscaling groups as described in AWS EKS Cluster Autoscaler Doc
in my case I need to add tags related to taints
Key: k8s.io/cluster-autoscaler/node-template/taint/$TAINT_KEY Value: NoSchedule
Describe the solution you'd like.
Include in the node group module, the positiblity to add
asg_tags
per each nodegroup and in the module use the terraform resourceaws_autoscaling_group_tag
Describe alternatives you've considered.
Manually mantain the ASG tags. t(-_-t)
The text was updated successfully, but these errors were encountered: