Skip to content
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

node_count is required #3485

Closed
wants to merge 1 commit into from
Closed

node_count is required #3485

wants to merge 1 commit into from

Conversation

minac
Copy link
Contributor

@minac minac commented Apr 24, 2019

Without node_count, autoscaling will never kick in because e.g. 0 is not between 2 and 5.

Without node_count, autoscaling will never kick in because e.g. 0 is not between 2 and 5.
@danawillow
Copy link
Contributor

Hi @minac, I'm not convinced this is the correct approach. This variable essentially says "node count should be X, hard stop". If autoscaling is also defined, then it won't work: any time terraform is re-run, it'll try to set the node count back to whatever was defined in the configuration. Instead, I'd recommend using initial_node_count to create the node pool with the specified number of nodes, while not enforcing that it remains the same size it was when it was originally created.

Because either initial_node_count or node_count can be specified, I don't feel comfortable changing the docs to say either are required. However, I'm happy to accept a PR that clarifies when to use one vs the other, especially in relation to autoscaling.

@minac
Copy link
Contributor Author

minac commented May 21, 2019

Hi @danawillow! Using initial_node_count seems like a better alternative. I just wish it was required to avoid the issue described above. If one does not use the field and sets the min of an autoscaling group to 2 and max to 4, the cluster will never create correctly, because it will never reach the minimum.
This was my experience. As you say this may not be a documentation issue, but a module issue, as an autoscaling group should guarantee that the number of nodes is between a min and a max.

@ghost ghost removed the waiting-response label May 21, 2019
@danawillow
Copy link
Contributor

Neither field should be required though, because they both have their use cases. initial_node_count works better when you want to use autoscaling, and node_count works better when you don't. We also used to check that node pools had a count > 0, but people wanted the ability to scale their cluster down (#752).

I think the correct approach is not to say that the field is required, but instead to add a note to the autoscaling docs to say that it only works if the initial node count starts in that range.

@minac
Copy link
Contributor Author

minac commented May 21, 2019

Sounds good

@ghost ghost removed the waiting-response label May 21, 2019
@rileykarson
Copy link
Collaborator

rileykarson commented Jun 26, 2019

Based on https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/FAQ.md#my-cluster-is-below-minimum--above-maximum-number-of-nodes-but-ca-did-not-fix-that-why, I don't think that your initial_node_count needs to be within autoscaler ranges, so I'm going to close this out. If I'm wrong, please correct me though!

@ghost
Copy link

ghost commented Jul 27, 2019

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants