From 6da595b86e9fcfe43e9f0527275977b2cacc3136 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Tue, 26 Feb 2019 14:26:41 -0800 Subject: [PATCH 1/3] fix remove_default_node_pool import, add docs --- .../resource_container_cluster.go.erb | 80 +++++++++++++------ .../resource_container_cluster_test.go.erb | 7 +- .../docs/r/container_cluster.html.markdown | 5 ++ 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/third_party/terraform/resources/resource_container_cluster.go.erb b/third_party/terraform/resources/resource_container_cluster.go.erb index d8a33473e55e..ab9e5898dd7c 100644 --- a/third_party/terraform/resources/resource_container_cluster.go.erb +++ b/third_party/terraform/resources/resource_container_cluster.go.erb @@ -671,11 +671,6 @@ func resourceContainerCluster() *schema.Resource { } } -func cidrOrSizeDiffSuppress(k, old, new string, d *schema.ResourceData) bool { - // If the user specified a size and the API returned a full cidr block, suppress. - return strings.HasPrefix(new, "/") && strings.HasSuffix(old, new) -} - func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) @@ -1497,11 +1492,15 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er name := fmt.Sprintf("%s/nodePools/%s", containerClusterFullName(project, location, clusterName), "default-pool") op, err := config.clientContainerBeta.Projects.Locations.Clusters.NodePools.Delete(name).Do() if err != nil { - return errwrap.Wrapf("Error deleting default node pool: {{err}}", err) - } - err = containerOperationWait(config, op, project, location, "removing default node pool", timeoutInMinutes) - if err != nil { - return errwrap.Wrapf("Error deleting default node pool: {{err}}", err) + if !isGoogleApiErrorWithCode(err, 404) { + return errwrap.Wrapf("Error deleting default node pool: {{err}}", err) + } + log.Printf("[WARN] Container cluster %q default node pool already removed, no change", d.Id()) + } else { + err = containerOperationWait(config, op, project, location, "removing default node pool", timeoutInMinutes) + if err != nil { + return errwrap.Wrapf("Error deleting default node pool: {{err}}", err) + } } } @@ -2108,29 +2107,55 @@ func flattenPodSecurityPolicyConfig(c *containerBeta.PodSecurityPolicyConfig) [] <% end -%> func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - parts := strings.Split(d.Id(), "/") + config := meta.(*Config) + parts := strings.Split(d.Id(), "/") + var project, location, clusterName string switch len(parts) { case 2: - if loc := parts[0]; isZone(loc) { - d.Set("zone", loc) - } else { - d.Set("region", loc) - } - d.Set("name", parts[1]) + location = parts[0] + clusterName = parts[1] case 3: - d.Set("project", parts[0]) - if loc := parts[1]; isZone(loc) { - d.Set("zone", loc) - } else { - d.Set("region", loc) - } - d.Set("name", parts[2]) + project = parts[0] + location = parts[1] + clusterName = parts[2] default: return nil, fmt.Errorf("Invalid container cluster specifier. Expecting {zone}/{name} or {project}/{zone}/{name}") } - d.SetId(parts[len(parts)-1]) + if len(project) > 0 { + d.Set("project", project) + } else { + var err error + project, err = getProject(d, config) + if err != nil { + return nil, err + } + } + + if isZone(location) { + d.Set("zone", location) + } else { + d.Set("region", location) + } + + d.Set("name", clusterName) + d.SetId(clusterName) + + // Try to determine remove_default_node_pool from presence of default + // node pool; if still present and user has it set to true, the pool + // will get removed on next apply. + nodePool := fmt.Sprintf("%s/nodePools/%s", containerClusterFullName(project, location, clusterName), "default-pool") + _, err := config.clientContainerBeta.Projects.Locations.Clusters.NodePools.Get(nodePool).Do() + if err != nil && isGoogleApiErrorWithCode(err, 404) { + d.Set("remove_default_node_pool", true) + } else { + d.Set("remove_default_node_pool", false) + if err != nil { + log.Printf("[WARN] Unable to import value for remove_default_node_pool, got error while trying to get default node pool: %s", err) + } + } + return []*schema.ResourceData{d}, nil } @@ -2160,6 +2185,11 @@ func extractNodePoolInformationFromCluster(d *schema.ResourceData, config *Confi }, nil } +func cidrOrSizeDiffSuppress(k, old, new string, d *schema.ResourceData) bool { + // If the user specified a size and the API returned a full cidr block, suppress. + return strings.HasPrefix(new, "/") && strings.HasSuffix(old, new) +} + // We want to suppress diffs for empty or default client certificate configs, i.e: // [{ "issue_client_certificate": true}] --> [] // [] -> [{ "issue_client_certificate": true}] diff --git a/third_party/terraform/tests/resource_container_cluster_test.go.erb b/third_party/terraform/tests/resource_container_cluster_test.go.erb index 1f234d6601d7..65bed67f8635 100644 --- a/third_party/terraform/tests/resource_container_cluster_test.go.erb +++ b/third_party/terraform/tests/resource_container_cluster_test.go.erb @@ -6,7 +6,7 @@ import ( "bytes" "fmt" "testing" - + "regexp" "strconv" "github.com/hashicorp/terraform/helper/acctest" @@ -276,7 +276,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) { ImportStateIdPrefix: "us-central1-a/", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"remove_default_node_pool"}, }, { Config: testAccContainerCluster_removeNetworkPolicy(clusterName), @@ -290,7 +289,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) { ImportStateIdPrefix: "us-central1-a/", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"remove_default_node_pool"}, }, { Config: testAccContainerCluster_withNetworkPolicyDisabled(clusterName), @@ -304,7 +302,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) { ImportStateIdPrefix: "us-central1-a/", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"remove_default_node_pool"}, }, { Config: testAccContainerCluster_withNetworkPolicyConfigDisabled(clusterName), @@ -318,7 +315,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) { ImportStateIdPrefix: "us-central1-a/", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"remove_default_node_pool"}, }, { Config: testAccContainerCluster_withNetworkPolicyConfigDisabled(clusterName), @@ -1214,7 +1210,6 @@ func TestAccContainerCluster_withDefaultNodePoolRemoved(t *testing.T) { ImportStateIdPrefix: "us-central1-a/", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"remove_default_node_pool"}, }, }, }) diff --git a/third_party/terraform/website/docs/r/container_cluster.html.markdown b/third_party/terraform/website/docs/r/container_cluster.html.markdown index d1c380d08e93..e38fc9df3ed6 100644 --- a/third_party/terraform/website/docs/r/container_cluster.html.markdown +++ b/third_party/terraform/website/docs/r/container_cluster.html.markdown @@ -577,3 +577,8 @@ $ terraform import google_container_cluster.mycluster my-gcp-project/us-east1-a/ $ terraform import google_container_cluster.mycluster us-east1-a/my-cluster ``` + +~> **Note:** This resource has several convenience fields that are state-only or used on creation and are not persisted by the API. If you have these fields set in config and import a cluster, you may see diffs that may or may not require actual operations against the resource. Example behavior: + +- `min_master_version` will not be set on import and will show a no-op diff if set in config. +- `remove_default_node_pool`: If the default node pool exists at import, this value will be set to false in state (or true if non-existant). If set to true in config but the node pool exists, a follow-up diff/apply will delete the default node pool. From d1334c4b2dbde41841a8b2a19bcfa5343e535ada Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Thu, 28 Feb 2019 10:16:47 -0800 Subject: [PATCH 2/3] use Riley's good words Co-Authored-By: emilymye --- .../terraform/website/docs/r/container_cluster.html.markdown | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/third_party/terraform/website/docs/r/container_cluster.html.markdown b/third_party/terraform/website/docs/r/container_cluster.html.markdown index e38fc9df3ed6..29f45579f1de 100644 --- a/third_party/terraform/website/docs/r/container_cluster.html.markdown +++ b/third_party/terraform/website/docs/r/container_cluster.html.markdown @@ -578,7 +578,9 @@ $ terraform import google_container_cluster.mycluster my-gcp-project/us-east1-a/ $ terraform import google_container_cluster.mycluster us-east1-a/my-cluster ``` -~> **Note:** This resource has several convenience fields that are state-only or used on creation and are not persisted by the API. If you have these fields set in config and import a cluster, you may see diffs that may or may not require actual operations against the resource. Example behavior: +~> **Note:** This resource has several fields that control Terraform-specific behavior and aren't present in the API. If they are set in config and you import a cluster, Terraform may need to perform an update immediately after import. Some of these updates are no-ops, and some may modify your cluster. + +For example: - `min_master_version` will not be set on import and will show a no-op diff if set in config. - `remove_default_node_pool`: If the default node pool exists at import, this value will be set to false in state (or true if non-existant). If set to true in config but the node pool exists, a follow-up diff/apply will delete the default node pool. From 31543d4b31ac68d0961a436e9a6eb3cad5e951fb Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Thu, 28 Feb 2019 21:12:49 +0000 Subject: [PATCH 3/3] Update tracked submodules -> HEAD on Thu Feb 28 21:12:49 UTC 2019 Tracked submodules are build/terraform-beta build/terraform build/ansible build/inspec. --- build/terraform | 2 +- build/terraform-beta | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/terraform b/build/terraform index 1496810ca60b..77c086de1c53 160000 --- a/build/terraform +++ b/build/terraform @@ -1 +1 @@ -Subproject commit 1496810ca60b208016da0f3087e699048430b4a0 +Subproject commit 77c086de1c533e1ad4ea23a153d4266775c9ab2d diff --git a/build/terraform-beta b/build/terraform-beta index e578cd0d6109..d9c8f2bb9e5b 160000 --- a/build/terraform-beta +++ b/build/terraform-beta @@ -1 +1 @@ -Subproject commit e578cd0d61097c714bbbf69cb3fcbd633905a997 +Subproject commit d9c8f2bb9e5b04c839e9077f836e46f7cdfc4e86