feat: Allow override of timeouts in node_groups #1552
Conversation
modules/node_groups/node_groups.tf
Outdated
| @@ -69,6 +69,12 @@ resource "aws_eks_node_group" "workers" { | |||
| } | |||
| } | |||
|
|
|||
| timeouts { | |||
| create = lookup(each.value["timeouts"], "create", "60m") | |||
There was a problem hiding this comment.
for the fallback on all of these, can we use null so that we use whats provided by the upstream provider (looks like it defaults to an hour https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eks_node_group#timeouts )
| create = lookup(each.value["timeouts"], "create", "60m") | |
| create = lookup(each.value["timeouts"], "create", null) | |
| update = lookup(each.value["timeouts"], "update", null) | |
| delete = lookup(each.value["timeouts"], "delete", null) |
There was a problem hiding this comment.
I'll try that. When I was doing my testing it didn't actually fallback and created empty values that broke the plan. But I will try again.
|
actually, looks like theres a bit more - I think what is captured in this PR is what we are looking for #1209 if you update with those changes we can get this merged |
|
I'll try again and get this updated after some testing on my end |
|
@bryantbiggs OK. all checked in and ready |
antonbabenko
changed the title
|
Thanks @RobertKozak ! v17.9.0 has been just released. |
| @@ -96,6 +96,7 @@ locals { | |||
| additional_ebs_volumes = [] # A list of additional volumes to be attached to the instances on this Auto Scaling group. Each volume should be an object with the following: block_device_name (required), volume_size, volume_type, iops, throughput, encrypted, kms_key_id (only on launch-template), delete_on_termination. Optional values are grabbed from root volume or from defaults | |||
| additional_instance_store_volumes = [] # A list of additional instance store (local disk) volumes to be attached to the instances on this Auto Scaling group. Each volume should be an object with the following: block_device_name (required), virtual_name. | |||
| warm_pool = null # If this block is configured, add a Warm Pool to the specified Auto Scaling group. | |||
| timeouts = {} # A map of timeouts for create/update/delete operations | |||
There was a problem hiding this comment.
should this have been:
timeouts = var.timeouts
The timeouts variable in the variable.tf file does not seem to actually be used during my testing
|
This merge was basically reverted by #1583 |
I still see this https://github.com/RobertKozak/terraform-aws-eks/blob/d46c49562771f247bf012031337968aef616332b/modules/node_groups/locals.tf#L27 so this feature is still available |
|
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
Description
This allows timeouts to be configured for aws_eks_node_group using the standard timeout block.
Updating node groups with large nodes often takes more than the default 1 hour and terraform can time out.
This pull request handles this by allowing you to add a timeout block to either the node_groups_defaults or to individual node_groups.
This PR also respects the timeouts described here
NOTE: You can set either / or create, update, delete and you will get defaults for the ones not supplied
There are a couple other PRs for this same issue #1173 but one PR #1209 is very old with no changes since Jan 30 and the other PR #1467 is just too simplistic for a long term solution.
Linked PRs and Issues
Checklist