-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
initial_node_count and node_count both set when node pool autoscaling is disabled #311
Comments
Actually, even for a node pool with autoscaling it seems to be set to |
That PR is not yet merged because it is incorrect, so I wouldn't rely on it for documentation. As such, the best reference is still the actual code: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/cluster.tf#L140 From this, you can see that if autoscaling is disabled we will set both Could you share the error message you see and your config? |
@morgante I believe it's this error I am seeing I will re-run my script to verify |
Looking at the code, this line https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/cluster.tf#L146 will set |
Yep. just tried re-running and got this
Looking at the plan I can see both being set
For my node pool
If I uncomment |
Yup, looks like a bug! Specifically, we need to update this line to not set initial_node_count if autoscaling is false. |
…m-google-modules#311 Dont set initial_node_count when autoscaling is disabled on node pool. Use new node pool var when setting desired size of pool - matches provider var
…m-google-modules#311 Dont set initial_node_count when autoscaling is disabled on node pool. Use new node pool var when setting desired size of pool - matches provider var
Hi
I've noticed that if
autoscaling=false
is set on a node pool, theninitial_node_count
will be set to1
(and not0
as documented in #297) and so isnode_count
, which causes an error since both can't be set. What is the intended behavior here?Thanks
The text was updated successfully, but these errors were encountered: