add `auto_provisioning_defaults` to `google_container_cluster.cluster_autoscaling` #2747

megan07 · 2019-11-25T20:03:14Z

Fixes hashicorp/terraform-provider-google#4061

Release Note Template for Downstream PRs (will be copied)

`container`: added `auto_provisioning_defaults` to `google_container_cluster.cluster_autoscaling`

modular-magician · 2019-11-25T20:12:38Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, a702925.

Pull request statuses

No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google-beta#1434
depends: GoogleCloudPlatform/terraform-google-conversion#268
depends: hashicorp/terraform-provider-google#4991

third_party/terraform/resources/resource_container_cluster.go.erb

rileykarson · 2019-11-26T17:25:08Z

third_party/terraform/resources/resource_container_cluster.go.erb

+
+	defaultScopes := []string{
+		"https://www.googleapis.com/auth/logging.write",
+		"https://www.googleapis.com/auth/monitoring",


Was it your experience that monitoring was a default sometimes, and other times it was monitoring.write? Did the default change based on logging_service and monitoring_service values?

Good catch, yes, it did change. When monitoring_service was none the default was monitoring.write and when it is not none the default is monitoring.

Talked in Slack- let's only suppress monitoring.write. While monitoring and logging.write are conditionally enabled based on GKE Stackdriver Monitoring being enabled, I think it's a valid behaviour for Terraform to show a diff in that case- the user's config isn't the real state.

third_party/terraform/resources/resource_container_cluster.go.erb

rileykarson · 2019-11-26T17:29:31Z

third_party/terraform/resources/resource_container_cluster.go.erb

@@ -2840,6 +2879,35 @@ func cidrOrSizeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
 	return strings.HasPrefix(new, "/") && strings.HasSuffix(old, new)
 }

+// We want to suppress diffs for the default auto provisioning defaults


"Defaults" is somewhat overloaded, since https://cloud.google.com/kubernetes-engine/docs/how-to/node-auto-provisioning#identity actually refers to a different set of scopes as defaults. Can we refer to this as suppressing scopes that are automatically added by GKE?

Additionally, I wonder if we can apply the same DSF to node_config fields because I believe they have the same behaviour.

third_party/terraform/resources/resource_container_cluster.go.erb

modular-magician · 2019-11-26T19:00:45Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 938bb29.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

third_party/terraform/resources/resource_container_cluster.go.erb

rileykarson · 2019-11-26T20:04:27Z

third_party/terraform/resources/resource_container_cluster.go.erb

@@ -2840,6 +2879,34 @@ func cidrOrSizeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
 	return strings.HasPrefix(new, "/") && strings.HasSuffix(old, new)
 }

+// We want to suppress diffs for the scopes automatically added by GKE


Suggested change

// We want to suppress diffs for the scopes automatically added by GKE

// Suppress unremovable default scope values from GCP.

// If the default service account would not otherwise have it, the `monitoring.write` scope

// is added to a GKE cluster's scopes regardless of what the user provided.

// monitoring.write is inherited from monitoring (rw) and cloud-platform, so it won't always

// be present.

// Enabling Stackdriver features through logging_service and monitoring_service may enable

// monitoring or logging.write. We've chosen not to suppress in those cases because they're

// removable by disabling those features.

rileykarson · 2019-11-26T20:06:32Z

third_party/terraform/tests/resource_container_cluster_test.go.erb

@@ -2642,6 +2644,12 @@ resource "google_container_cluster" "with_autoprovisioning" {
      resource_type = "memory"
      maximum       = 2048
    }
+    auto_provisioning_defaults {


What happens if I include cluster_autoscaling without auto_provisioning_defaults or auto_provisioning_defaults without oauth_scopes? Will I get a diff?

Also, can you include an _updated config including monitoring.write and check that there are no changes in plan? (I think that's one of the options we have for a test step)

I am adding O+C to auto_provisioning_defaults which should get rid of the diff if auto_provisioning_defaults is not included in a cluster_autoscaling block.

rileykarson · 2019-11-26T20:07:23Z

third_party/terraform/website/docs/r/container_cluster.html.markdown

@@ -372,6 +375,14 @@ for a list of types.

 * `maximum` - (Optional) Maximum amount of the resource in the cluster.

+The `auto_provisioning_defaults` block supports:
+
+* `oauth_scopes` - (Optional) Scopes that are used by NAP when creating node pools.


Can you note that monitoring.write will always be enabled regardless of user input, and add an infobox ( with ->) to say that GKE may enable monitoring or logging.write based on logging_service and monitoring_service values?

rileykarson

LGTM. Thanks for thoroughly working through the idiosyncrasies here!

modular-magician · 2019-11-26T23:05:16Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 23a93c5.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

modular-magician · 2019-11-26T23:06:21Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 6552bdb.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

modular-magician · 2019-11-26T23:07:44Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 6552bdb.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

modular-magician · 2019-11-26T23:09:31Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 14ce2c3.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

… o+c

Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.

googlebot added the cla: yes label Nov 25, 2019

megan07 requested a review from rileykarson November 25, 2019 20:05

modular-magician added downstream-generated terraform-google-conversion terraform-provider-google-beta terraform-provider-google labels Nov 25, 2019

rileykarson reviewed Nov 25, 2019

View reviewed changes

third_party/terraform/resources/resource_container_cluster.go.erb Outdated Show resolved Hide resolved

rileykarson reviewed Nov 26, 2019

View reviewed changes

megan07 commented Nov 26, 2019

View reviewed changes

third_party/terraform/resources/resource_container_cluster.go.erb Show resolved Hide resolved

megan07 requested a review from rileykarson November 26, 2019 19:04

rileykarson reviewed Nov 26, 2019

View reviewed changes

rileykarson approved these changes Nov 26, 2019

View reviewed changes

modular-magician added the automerged label Nov 26, 2019

megan07 and others added 7 commits November 26, 2019 23:30

add support for auto_provisioning_defaults on cluster_autoscaling block

1948e8c

add support for auto_provisioning_defaults on cluster_autoscaling block

7190dd0

updates from review comments

a1e991a

updated tests and website docs. made auto_provisioning_defaults block…

1af8846

… o+c

fix new line

78a7c3e

remove new line

e1765ef

Update tracked submodules -> HEAD on Tue Nov 26 23:30:40 UTC 2019

96ee7fb

Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.

modular-magician force-pushed the megan_autoprovisiondefaults branch from 14ce2c3 to 96ee7fb Compare November 26, 2019 23:30

modular-magician merged commit 020073b into GoogleCloudPlatform:master Nov 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `auto_provisioning_defaults` to `google_container_cluster.cluster_autoscaling` #2747

add `auto_provisioning_defaults` to `google_container_cluster.cluster_autoscaling` #2747

megan07 commented Nov 25, 2019

modular-magician commented Nov 25, 2019

rileykarson Nov 26, 2019

megan07 Nov 26, 2019

rileykarson Nov 26, 2019

rileykarson Nov 26, 2019

modular-magician commented Nov 26, 2019

rileykarson Nov 26, 2019

rileykarson Nov 26, 2019

rileykarson Nov 26, 2019

megan07 Nov 26, 2019

rileykarson Nov 26, 2019

rileykarson left a comment

modular-magician commented Nov 26, 2019

modular-magician commented Nov 26, 2019

modular-magician commented Nov 26, 2019

modular-magician commented Nov 26, 2019

-// We want to suppress diffs for the scopes automatically added by GKE
+// Suppress unremovable default scope values from GCP.
+// If the default service account would not otherwise have it, the `monitoring.write` scope
+// is added to a GKE cluster's scopes regardless of what the user provided.
+// monitoring.write is inherited from monitoring (rw) and cloud-platform, so it won't always
+// be present.
+// Enabling Stackdriver features through logging_service and monitoring_service may enable
+// monitoring or logging.write. We've chosen not to suppress in those cases because they're
+// removable by disabling those features.

add auto_provisioning_defaults to google_container_cluster.cluster_autoscaling #2747

add auto_provisioning_defaults to google_container_cluster.cluster_autoscaling #2747

Conversation

megan07 commented Nov 25, 2019

modular-magician commented Nov 25, 2019

Pull request statuses

New Pull Requests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modular-magician commented Nov 26, 2019

Pull request statuses

New Pull Requests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rileykarson left a comment

Choose a reason for hiding this comment

modular-magician commented Nov 26, 2019

Pull request statuses

New Pull Requests

modular-magician commented Nov 26, 2019

Pull request statuses

New Pull Requests

modular-magician commented Nov 26, 2019

Pull request statuses

New Pull Requests

modular-magician commented Nov 26, 2019

Pull request statuses

New Pull Requests

add `auto_provisioning_defaults` to `google_container_cluster.cluster_autoscaling` #2747

add `auto_provisioning_defaults` to `google_container_cluster.cluster_autoscaling` #2747