chrisghill commented 1 year ago

Description

The desired_size argument in the scaling_config block of the aws_eks_node_group resource is currently a required argument and I'd like to make it optional. I'm assuming the reason it is currently required is the aws-sdk-go states that "When creating a node group, you must specify all or none of the properties.". However, on updates you can specify "any or none".

This field being required creates problems when there is a cluster-autoscaler installed since the field may change outside of terraform. The official docs even suggest that users add the desired_size field to an ignore_changes block to prevent a cluster-autoscaler from resetting the value on applies. This creates problems when users try to change the min_size or max_size arguments since min_size can't be greater than desired_size and max_size can't be less than desired_size. Take for example, this config:

resource "aws_eks_node_group" "example" {
  # ... other configurations ...

  scaling_config {
    min_size = 1
    desired_size = 1
    max_size = 3
  }

  lifecycle {
    ignore_changes = [scaling_config[0].desired_size]
  }
}

If I later tried to update the scaling config to:

  scaling_config {
    min_size = 2
    desired_size = 2
    max_size = 3
  }

This would cause an API error since the desired_size field has changes ignored and thus will still be set at 1 while the min_size increased to 2. This behavior is reported many times in the official EKS module (see references).

I'm proposing that we make the desired_size field optional. The API only requires the field on creation (not updates), and we can just default the desired_size to min_size during creation if its omitted (and make this behavior clear in the documentation). This would allow users to simply omit the field if an autoscaler is installed.

Happy to implement the fix if approved.

Affected Resource(s) and/or Data Source(s)

aws_eks_node_group

Potential Terraform Configuration

resource "aws_eks_node_group" "example" {
  # ... other configurations ...

  scaling_config {
    min_size = 1
    max_size = 3
  }
}

References

Would you like to implement a fix?

Yes

github-actions[bot] commented 1 year ago

Community Note

Voting for Prioritization

Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
Please see our prioritization guide for information on how we prioritize.
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

If you are interested in working on this issue, please leave a comment.
If this would be your first contribution, please review the contribution guide.

hongbo-miao commented 2 weeks ago

Thanks for opening this ticket!

I tried to find a solution before this implemented, and always ended to this ticket. Just posted a workaround solution before this is ready to help people save some time. 😃

Step 1

Step 2

Step 3

Once you increased "Desired size" and "Minimum size" here, then you can update your Terraform code min_size and desired_size to match this latest size. And it should show "No changes. Your infrastructure matches the configuration."

bryantbiggs commented 2 weeks ago

@hongbo-miao https://github.com/bryantbiggs/eks-desired-size-hack

hashicorp / terraform-provider-aws