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

provider/google: Add node_pool field in resource_container_cluster #13402

Merged
merged 1 commit into from
Apr 12, 2017

Conversation

danawillow
Copy link
Contributor

As @paddyforan and I discussed offline, even though nodepools could be their own resource, it makes more sense to keep track of them inside the cluster resource. Each nodepool belongs to exactly one cluster, some of the update methods on nodepools are actually part of the clusters API, and the creation of a cluster without a specified nodepool will create a default one that we might want terraform to be able to manage.

For this PR, I've just included the minimum functionality. In follow-ups I'll add the ability to add and remove nodepools from a cluster and add more fields to the node_pool schema.

Copy link
Contributor

@paddycarver paddycarver left a 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. 👍 Merge at will, thanks!

@danawillow danawillow merged commit 11a20dd into hashicorp:master Apr 12, 2017
@danawillow danawillow deleted the gke-nodepool-in-cluster branch April 12, 2017 19:58
@imkira
Copy link

imkira commented Apr 21, 2017

@danawillow It would be nice to have the ability to configure as one would do with the default pool otherwise you get something like:

googleapi: Error 400: It's invalid to specify both cluster.node_config and a node pool. Please only provide a node pool.., badRequest

Maybe adding config (like node_config under google_container_cluster resource) to the node_pool object would be nice:

https://godoc.org/google.golang.org/api/container/v1#NodePool

@danawillow
Copy link
Contributor Author

@imkira Yup, that's exactly the plan!

@ghost
Copy link

ghost commented Apr 13, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
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.

4 participants