Add remove_default_node_pool

[mkubaczyk]

In the #15 issue there was a request for remove_default_node_pool. I'm interested in introducing this to the module. There's no point in keeping the default node pool the google_container_cluster forces us to have on the beginning. Usually, the one wants to manage the pools explicitly, and so do I, hence this PR.

Adding the remove_default_node_pool was not enough to make it work perfectly. A change in the manner of providing the google_container_cluster with the network and subnetwork parameters had to be changed as well. Without it, terraform produced permadiff with every plan and apply. That's the issue with google provider, more on that is described in this google provider issue.

The approach I propose is a workaround which won't cause any backward incompatibilities. The issue I faced might be the problem described from the #15.

If anyone is having the better solution, feel free to propose it!

[morgante]

Thanks for looking into this. I believe there are some complexities with the 0-node default pool we currently create for clusters with node_pools configured - could you take a look at that?

The other thing to test/confirm is compatibility with Shared VPC clusters.

[morgante]

Isn't that equivalent to the self_link?

The issue with formatting it like this is if the cluster is being launched on a shared VPC, in which case the network is in a different project.

[mkubaczyk]

self_link adds https://www.googleapis.com/compute/v1 before the projects/[..], hence the permadiff since terraform sees the current path as the one starting from projects/[..] and tries to update it.

I'll take a look at the shared VPC case and will come with a solution/an idea hopefully. :-)

[mkubaczyk]

@morgante I reused the 'data.google_compute_network` to get the project_id, which takes the shared VPC into account right now. Proposed in 8f96807

[morgante]

One alternative idea: what if we used the self_link but used replace to strip the URL prefix? https://www.terraform.io/docs/configuration/interpolation.html#replace-string-search-replace-

[mkubaczyk]

Sounds good.
As simple as network = "${replace(data.google_compute_network.gke_network.self_link, "https://www.googleapis.com/compute/v1/", "")}" ?

[mkubaczyk]

Added in e997e66. :-)

[morgante]

Yes I think that should work.

…

On Fri, Jan 4, 2019 at 10:18 Mateusz Kubaczyk ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In cluster_zonal.tf <#55 (comment)> : > @@ -26,8 +26,8 @@ resource "google_container_cluster" "zonal_primary" { zone = "${var.zones[0]}" additional_zones = ["${slice(var.zones,1,length(var.zones))}"] - network = "${data.google_compute_network.gke_network.self_link}" - subnetwork = "${data.google_compute_subnetwork.gke_subnetwork.self_link}" + network = "projects/${var.project_id}/global/networks/${data.google_compute_network.gke_network.name}" Sounds good. As simple as network = "${replace(data.google_compute_network.gke_network.self_link, " https://www.googleapis.com/compute/v1/", "")}" ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjWmftRAA4mwVbh92E__hOg1gpzVR8vks5u_3CsgaJpZM4Zo1Fv> .

[morgante]

LGTM! Thanks for the contribution.