-
-
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
feat: Supporting update_config in EKS managed node groups #1560
feat: Supporting update_config in EKS managed node groups #1560
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but minor things are left.
examples/managed_node_groups/main.tf
Outdated
@@ -95,6 +95,8 @@ module "eks" { | |||
effect = "NO_SCHEDULE" | |||
} | |||
] | |||
max_unavailable_type = "percent" | |||
max_unavailable = 50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to run pre-commit run -a
to fix formatting and documentation.
…aws-modules#1554) Setting default as max_unavailable=1, same as default for the tf aws_eks_node_group resource. Using a 2 level map in order to be able to set node_group_defaults and be able to overwrite with node_group specific config
…aws-modules#1554) Add documentation for update_config fields in node_groups module README.md
@antonbabenko regarding the conflict due to the version change by pre-commit you will be resolving them right? |
Yes, I will fix that |
Thanks, @marianobilli for the PR. I have simplified code a bit and verified it locally. v17.10.0 has been just released. |
Thank you @antonbabenko ! I was not sure about using lookup() vs try(), and also i didnt know i could put two tries inside the same try() function. So I learnt something new. Thanks again! |
Since we agreed to increase the minimum supported version of Terraform to 0.13 across all |
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. |
PR o'clock
Fixes #1554
Description
Adding functionality proposed in Supporting update_config in EKS managed node groups
Checklist