-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add support for node pool autoscaling #157
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.
Minor comment about OOB changes to whether autoscaling is enabled or not.
@@ -129,10 +180,70 @@ func resourceContainerNodePoolRead(d *schema.ResourceData, meta interface{}) err | |||
|
|||
d.Set("name", nodePool.Name) | |||
d.Set("initial_node_count", nodePool.InitialNodeCount) | |||
if nodePool.Autoscaling != nil && nodePool.Autoscaling.Enabled { |
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.
How do we detect if autoscaling has been turned on out of band and needs to be switched back on?
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.
Ah, that's a good point.
I went ahead and checked what the cloud console does- when you turn autoscaling off and then back on, you lose the min/max node count you had originally set. Based on that, I removed the enabled attribute so we can easily detect out-of-band changes (this way we don't need to differentiate between set to false and not set at all, although in retrospect maybe a DiffSuppressFn
on the autoscaling
attribute would work? not sure). Let me know what you think.
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.
LGTM, and tests pass.
* add node pool autoscaling * docs for node pool autoscaling * remove enabled attribute * remove enabled from docs
* add node pool autoscaling * docs for node pool autoscaling * remove enabled attribute * remove enabled from docs
* add node pool autoscaling * docs for node pool autoscaling * remove enabled attribute * remove enabled from docs
Add to state before we send the create call.
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! |
Fixes #44.
This is essentially the same code as hashicorp/terraform#12779, except I added the
enabled
attribute and support for update.