From ae5ea9fd0fbc2f689384a635d114050d61a54d3d Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Wed, 5 Jul 2017 16:00:49 -0700 Subject: [PATCH] allow updating additional_zones, turn it into a set (#152) --- google/provider.go | 11 +++ google/resource_compute_target_pool.go | 11 --- google/resource_container_cluster.go | 76 ++++++++++++++----- google/resource_container_cluster_migrate.go | 70 +++++++++++++++++ ...resource_container_cluster_migrate_test.go | 75 ++++++++++++++++++ google/resource_container_cluster_test.go | 75 ++++++++++++++++-- 6 files changed, 283 insertions(+), 35 deletions(-) create mode 100644 google/resource_container_cluster_migrate.go create mode 100644 google/resource_container_cluster_migrate_test.go diff --git a/google/provider.go b/google/provider.go index 6c08fd11..94e76d08 100644 --- a/google/provider.go +++ b/google/provider.go @@ -286,3 +286,14 @@ func linkDiffSuppress(k, old, new string, d *schema.ResourceData) bool { } return false } + +func convertStringArr(ifaceArr []interface{}) []string { + var arr []string + for _, v := range ifaceArr { + if v == nil { + continue + } + arr = append(arr, v.(string)) + } + return arr +} diff --git a/google/resource_compute_target_pool.go b/google/resource_compute_target_pool.go index 8f3b2219..68264d97 100644 --- a/google/resource_compute_target_pool.go +++ b/google/resource_compute_target_pool.go @@ -88,17 +88,6 @@ func resourceComputeTargetPool() *schema.Resource { } } -func convertStringArr(ifaceArr []interface{}) []string { - var arr []string - for _, v := range ifaceArr { - if v == nil { - continue - } - arr = append(arr, v.(string)) - } - return arr -} - // Healthchecks need to exist before being referred to from the target pool. func convertHealthChecks(config *Config, project string, names []string) ([]string, error) { urls := make([]string, len(names)) diff --git a/google/resource_container_cluster.go b/google/resource_container_cluster.go index fb8da86d..b3096da4 100644 --- a/google/resource_container_cluster.go +++ b/google/resource_container_cluster.go @@ -29,6 +29,9 @@ func resourceContainerCluster() *schema.Resource { Delete: schema.DefaultTimeout(10 * time.Minute), }, + SchemaVersion: 1, + MigrateState: resourceContainerClusterMigrateState, + Schema: map[string]*schema.Schema{ "master_auth": { Type: schema.TypeList, @@ -106,10 +109,9 @@ func resourceContainerCluster() *schema.Resource { }, "additional_zones": { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, Computed: true, - ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, }, @@ -386,7 +388,7 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er } if v, ok := d.GetOk("additional_zones"); ok { - locationsList := v.([]interface{}) + locationsList := v.(*schema.Set).List() locations := []string{} for _, v := range locationsList { location := v.(string) @@ -634,29 +636,65 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er zoneName := d.Get("zone").(string) clusterName := d.Get("name").(string) - desiredNodeVersion := d.Get("node_version").(string) timeoutInMinutes := int(d.Timeout(schema.TimeoutUpdate).Minutes()) - req := &container.UpdateClusterRequest{ - Update: &container.ClusterUpdate{ - DesiredNodeVersion: desiredNodeVersion, - }, - } - op, err := config.clientContainer.Projects.Zones.Clusters.Update( - project, zoneName, clusterName, req).Do() - if err != nil { - return err + d.Partial(true) + + if d.HasChange("node_version") { + desiredNodeVersion := d.Get("node_version").(string) + + req := &container.UpdateClusterRequest{ + Update: &container.ClusterUpdate{ + DesiredNodeVersion: desiredNodeVersion, + }, + } + op, err := config.clientContainer.Projects.Zones.Clusters.Update( + project, zoneName, clusterName, req).Do() + if err != nil { + return err + } + + // Wait until it's updated + waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE cluster version", timeoutInMinutes, 2) + if waitErr != nil { + return waitErr + } + + log.Printf("[INFO] GKE cluster %s has been updated to %s", d.Id(), + desiredNodeVersion) + + d.SetPartial("node_version") } - // Wait until it's updated + if d.HasChange("additional_zones") { + azSet := d.Get("additional_zones").(*schema.Set) + if azSet.Contains(zoneName) { + return fmt.Errorf("additional_zones should not contain the original 'zone'.") + } + azs := convertStringArr(azSet.List()) + locations := append(azs, zoneName) + req := &container.UpdateClusterRequest{ + Update: &container.ClusterUpdate{ + DesiredLocations: locations, + }, + } + op, err := config.clientContainer.Projects.Zones.Clusters.Update( + project, zoneName, clusterName, req).Do() + if err != nil { + return err + } - waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE cluster", timeoutInMinutes, 2) - if waitErr != nil { - return waitErr + // Wait until it's updated + waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE cluster locations", timeoutInMinutes, 2) + if waitErr != nil { + return waitErr + } + + log.Printf("[INFO] GKE cluster %s locations have been updated to %v", d.Id(), + locations) } - log.Printf("[INFO] GKE cluster %s has been updated to %s", d.Id(), - desiredNodeVersion) + d.Partial(false) return resourceContainerClusterRead(d, meta) } diff --git a/google/resource_container_cluster_migrate.go b/google/resource_container_cluster_migrate.go new file mode 100644 index 00000000..720936b2 --- /dev/null +++ b/google/resource_container_cluster_migrate.go @@ -0,0 +1,70 @@ +package google + +import ( + "fmt" + "log" + "strconv" + "strings" + + "github.com/hashicorp/terraform/helper/schema" + "github.com/hashicorp/terraform/terraform" +) + +func resourceContainerClusterMigrateState( + v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + if is.Empty() { + log.Println("[DEBUG] Empty InstanceState; nothing to migrate.") + return is, nil + } + + switch v { + case 0: + log.Println("[INFO] Found Container Cluster State v0; migrating to v1") + return migrateClusterStateV0toV1(is) + default: + return is, fmt.Errorf("Unexpected schema version: %d", v) + } +} + +func migrateClusterStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { + log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) + + newZones := []string{} + + for k, v := range is.Attributes { + if !strings.HasPrefix(k, "additional_zones.") { + continue + } + + if k == "additional_zones.#" { + continue + } + + // Key is now of the form additional_zones.%d + kParts := strings.Split(k, ".") + + // Sanity check: two parts should be there and should be a number + badFormat := false + if len(kParts) != 2 { + badFormat = true + } else if _, err := strconv.Atoi(kParts[1]); err != nil { + badFormat = true + } + + if badFormat { + return is, fmt.Errorf("migration error: found additional_zones key in unexpected format: %s", k) + } + + newZones = append(newZones, v) + delete(is.Attributes, k) + } + + for _, v := range newZones { + hash := schema.HashString(v) + newKey := fmt.Sprintf("additional_zones.%d", hash) + is.Attributes[newKey] = v + } + + log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) + return is, nil +} diff --git a/google/resource_container_cluster_migrate_test.go b/google/resource_container_cluster_migrate_test.go new file mode 100644 index 00000000..84fa535b --- /dev/null +++ b/google/resource_container_cluster_migrate_test.go @@ -0,0 +1,75 @@ +package google + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestContainerClusterMigrateState(t *testing.T) { + cases := map[string]struct { + StateVersion int + Attributes map[string]string + Expected map[string]string + Meta interface{} + }{ + "change additional_zones from list to set": { + StateVersion: 0, + Attributes: map[string]string{ + "additional_zones.#": "2", + "additional_zones.0": "us-central1-c", + "additional_zones.1": "us-central1-b", + }, + Expected: map[string]string{ + "additional_zones.#": "2", + "additional_zones.90274510": "us-central1-c", + "additional_zones.1919306328": "us-central1-b", + }, + Meta: &Config{}, + }, + } + + for tn, tc := range cases { + is := &terraform.InstanceState{ + ID: "i-abc123", + Attributes: tc.Attributes, + } + is, err := resourceContainerClusterMigrateState( + tc.StateVersion, is, tc.Meta) + + if err != nil { + t.Fatalf("bad: %s, err: %#v", tn, err) + } + + for k, v := range tc.Expected { + if is.Attributes[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + tn, k, v, k, is.Attributes[k], is.Attributes) + } + } + } +} + +func TestContainerClusterMigrateState_empty(t *testing.T) { + var is *terraform.InstanceState + var meta *Config + + // should handle nil + is, err := resourceContainerClusterMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } + if is != nil { + t.Fatalf("expected nil instancestate, got: %#v", is) + } + + // should handle non-nil but empty + is = &terraform.InstanceState{} + is, err = resourceContainerClusterMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } +} diff --git a/google/resource_container_cluster_test.go b/google/resource_container_cluster_test.go index ff9db81b..fd0b00cc 100644 --- a/google/resource_container_cluster_test.go +++ b/google/resource_container_cluster_test.go @@ -2,6 +2,9 @@ package google import ( "fmt" + "reflect" + "sort" + "strings" "testing" "strconv" @@ -63,13 +66,22 @@ func TestAccContainerCluster_withMasterAuth(t *testing.T) { } func TestAccContainerCluster_withAdditionalZones(t *testing.T) { + clusterName := fmt.Sprintf("cluster-test-%s", acctest.RandString(10)) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckContainerClusterDestroy, Steps: []resource.TestStep{ { - Config: testAccContainerCluster_withAdditionalZones, + Config: testAccContainerCluster_withAdditionalZones(clusterName), + Check: resource.ComposeTestCheckFunc( + testAccCheckContainerCluster( + "google_container_cluster.with_additional_zones"), + ), + }, + { + Config: testAccContainerCluster_updateAdditionalZones(clusterName), Check: resource.ComposeTestCheckFunc( testAccCheckContainerCluster( "google_container_cluster.with_additional_zones"), @@ -236,6 +248,10 @@ func testAccCheckContainerClusterDestroy(s *terraform.State) error { return nil } +var setFields map[string]struct{} = map[string]struct{}{ + "additional_zones": struct{}{}, +} + func testAccCheckContainerCluster(n string) resource.TestCheckFunc { return func(s *terraform.State) error { attributes, err := getResourceAttributes(n, s) @@ -354,6 +370,9 @@ func getResourceAttributes(n string, s *terraform.State) (map[string]string, err func checkMatch(attributes map[string]string, attr string, gcp interface{}) string { if gcpList, ok := gcp.([]string); ok { + if _, ok := setFields[attr]; ok { + return checkSetMatch(attributes, attr, gcpList) + } return checkListMatch(attributes, attr, gcpList) } if gcpMap, ok := gcp.(map[string]string); ok { @@ -366,6 +385,30 @@ func checkMatch(attributes map[string]string, attr string, gcp interface{}) stri return "" } +func checkSetMatch(attributes map[string]string, attr string, gcpList []string) string { + num, err := strconv.Atoi(attributes[attr+".#"]) + if err != nil { + return fmt.Sprintf("Error in number conversion for attribute %s: %s", attr, err) + } + if num != len(gcpList) { + return fmt.Sprintf("Cluster has mismatched %s size.\nTF Size: %d\nGCP Size: %d", attr, num, len(gcpList)) + } + + // We don't know the exact keys of the elements, so go through the whole list looking for matching ones + tfAttr := []string{} + for k, v := range attributes { + if strings.HasPrefix(k, attr) && !strings.HasSuffix(k, "#") { + tfAttr = append(tfAttr, v) + } + } + sort.Strings(tfAttr) + sort.Strings(gcpList) + if reflect.DeepEqual(tfAttr, gcpList) { + return "" + } + return matchError(attr, tfAttr, gcpList) +} + func checkListMatch(attributes map[string]string, attr string, gcpList []string) string { num, err := strconv.Atoi(attributes[attr+".#"]) if err != nil { @@ -402,7 +445,7 @@ func checkMapMatch(attributes map[string]string, attr string, gcpMap map[string] return "" } -func matchError(attr, tf string, gcp interface{}) string { +func matchError(attr, tf interface{}, gcp interface{}) string { return fmt.Sprintf("Cluster has mismatched %s.\nTF State: %+v\nGCP State: %+v", attr, tf, gcp) } @@ -438,9 +481,10 @@ resource "google_container_cluster" "with_master_auth" { } }`, acctest.RandString(10)) -var testAccContainerCluster_withAdditionalZones = fmt.Sprintf(` +func testAccContainerCluster_withAdditionalZones(clusterName string) string { + return fmt.Sprintf(` resource "google_container_cluster" "with_additional_zones" { - name = "cluster-test-%s" + name = "%s" zone = "us-central1-a" initial_node_count = 1 @@ -453,7 +497,28 @@ resource "google_container_cluster" "with_additional_zones" { username = "mr.yoda" password = "adoy.rm" } -}`, acctest.RandString(10)) +}`, clusterName) +} + +func testAccContainerCluster_updateAdditionalZones(clusterName string) string { + return fmt.Sprintf(` +resource "google_container_cluster" "with_additional_zones" { + name = "%s" + zone = "us-central1-a" + initial_node_count = 1 + + additional_zones = [ + "us-central1-f", + "us-central1-b", + "us-central1-c", + ] + + master_auth { + username = "mr.yoda" + password = "adoy.rm" + } +}`, clusterName) +} var testAccContainerCluster_withVersion = fmt.Sprintf(` data "google_container_engine_versions" "central1a" {