allow updating additional_zones, turn it into a set (#152)

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
+}

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))

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)
 }

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 <N> 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
+}

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)
+	}
+}

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" {