provider/google: turn compute_instance_group.instances into a set (#12790)

builtin/providers/google/resource_compute_instance_group.go

@@ -18,6 +18,8 @@ func resourceComputeInstanceGroup() *schema.Resource {
 		Update: resourceComputeInstanceGroupUpdate,
 		Delete: resourceComputeInstanceGroupDelete,
 
+		SchemaVersion: 1,
+
 		Schema: map[string]*schema.Schema{
 			"name": &schema.Schema{
 				Type:     schema.TypeString,
@@ -38,9 +40,10 @@ func resourceComputeInstanceGroup() *schema.Resource {
 			},
 
 			"instances": &schema.Schema{
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
 				Elem:     &schema.Schema{Type: schema.TypeString},
+				Set:      schema.HashString,
 			},
 
 			"named_port": &schema.Schema{
@@ -142,7 +145,7 @@ func resourceComputeInstanceGroupCreate(d *schema.ResourceData, meta interface{}
 	}
 
 	if v, ok := d.GetOk("instances"); ok {
-		instanceUrls := convertStringArr(v.([]interface{}))
+		instanceUrls := convertStringArr(v.(*schema.Set).List())
 		if !validInstanceURLs(instanceUrls) {
 			return fmt.Errorf("Error invalid instance URLs: %v", instanceUrls)
 		}
@@ -239,8 +242,8 @@ func resourceComputeInstanceGroupUpdate(d *schema.ResourceData, meta interface{}
 		// to-do check for no instances
 		from_, to_ := d.GetChange("instances")
 
-		from := convertStringArr(from_.([]interface{}))
-		to := convertStringArr(to_.([]interface{}))
+		from := convertStringArr(from_.(*schema.Set).List())
+		to := convertStringArr(to_.(*schema.Set).List())
 
 		if !validInstanceURLs(from) {
 			return fmt.Errorf("Error invalid instance URLs: %v", from)

builtin/providers/google/resource_compute_instance_group_migrate.go

@@ -0,0 +1,74 @@
+package google
+
+import (
+	"fmt"
+	"log"
+	"strconv"
+	"strings"
+
+	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func resourceComputeInstanceGroupMigrateState(
+	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 Compute Instance Group State v0; migrating to v1")
+		is, err := migrateInstanceGroupStateV0toV1(is)
+		if err != nil {
+			return is, err
+		}
+		return is, nil
+	default:
+		return is, fmt.Errorf("Unexpected schema version: %d", v)
+	}
+}
+
+func migrateInstanceGroupStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
+	log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes)
+
+	newInstances := []string{}
+
+	for k, v := range is.Attributes {
+		if !strings.HasPrefix(k, "instances.") {
+			continue
+		}
+
+		if k == "instances.#" {
+			continue
+		}
+
+		// Key is now of the form instances.%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 instances key in unexpected format: %s", k)
+		}
+
+		newInstances = append(newInstances, v)
+		delete(is.Attributes, k)
+	}
+
+	for _, v := range newInstances {
+		hash := schema.HashString(v)
+		newKey := fmt.Sprintf("instances.%d", hash)
+		is.Attributes[newKey] = v
+	}
+
+	log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes)
+	return is, nil
+}

builtin/providers/google/resource_compute_instance_group_migrate_test.go

@@ -0,0 +1,75 @@
+package google
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func TestComputeInstanceGroupMigrateState(t *testing.T) {
+	cases := map[string]struct {
+		StateVersion int
+		Attributes   map[string]string
+		Expected     map[string]string
+		Meta         interface{}
+	}{
+		"change instances from list to set": {
+			StateVersion: 0,
+			Attributes: map[string]string{
+				"instances.#": "1",
+				"instances.0": "https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instancegroup-test-1",
+				"instances.1": "https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instancegroup-test-0",
+			},
+			Expected: map[string]string{
+				"instances.#":          "1",
+				"instances.764135222":  "https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instancegroup-test-1",
+				"instances.1519187872": "https://www.googleapis.com/compute/v1/projects/project_name/zones/zone_name/instances/instancegroup-test-0",
+			},
+			Meta: &Config{},
+		},
+	}
+
+	for tn, tc := range cases {
+		is := &terraform.InstanceState{
+			ID:         "i-abc123",
+			Attributes: tc.Attributes,
+		}
+		is, err := resourceComputeInstanceGroupMigrateState(
+			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 TestComputeInstanceGroupMigrateState_empty(t *testing.T) {
+	var is *terraform.InstanceState
+	var meta *Config
+
+	// should handle nil
+	is, err := resourceComputeInstanceGroupMigrateState(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 = resourceComputeInstanceGroupMigrateState(0, is, meta)
+
+	if err != nil {
+		t.Fatalf("err: %#v", err)
+	}
+}

builtin/providers/google/resource_compute_instance_group_test.go

@@ -70,6 +70,26 @@ func TestAccComputeInstanceGroup_update(t *testing.T) {
 	})
 }
 
+func TestAccComputeInstanceGroup_outOfOrderInstances(t *testing.T) {
+	var instanceGroup compute.InstanceGroup
+	var instanceName = fmt.Sprintf("instancegroup-test-%s", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccComputeInstanceGroup_destroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeInstanceGroup_outOfOrderInstances(instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccComputeInstanceGroup_exists(
+						"google_compute_instance_group.group", &instanceGroup),
+				),
+			},
+		},
+	})
+}
+
 func testAccComputeInstanceGroup_destroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 
@@ -297,3 +317,51 @@ func testAccComputeInstanceGroup_update2(instance string) string {
 		}
 	}`, instance, instance)
 }
+
+func testAccComputeInstanceGroup_outOfOrderInstances(instance string) string {
+	return fmt.Sprintf(`
+	resource "google_compute_instance" "ig_instance" {
+		name = "%s-1"
+		machine_type = "n1-standard-1"
+		can_ip_forward = false
+		zone = "us-central1-c"
+
+		disk {
+			image = "debian-8-jessie-v20160803"
+		}
+
+		network_interface {
+			network = "default"
+		}
+	}
+
+	resource "google_compute_instance" "ig_instance_2" {
+		name = "%s-2"
+		machine_type = "n1-standard-1"
+		can_ip_forward = false
+		zone = "us-central1-c"
+
+		disk {
+			image = "debian-8-jessie-v20160803"
+		}
+
+		network_interface {
+			network = "default"
+		}
+	}
+
+	resource "google_compute_instance_group" "group" {
+		description = "Terraform test instance group"
+		name = "%s"
+		zone = "us-central1-c"
+		instances = [ "${google_compute_instance.ig_instance_2.self_link}", "${google_compute_instance.ig_instance.self_link}" ]
+		named_port {
+			name = "http"
+			port = "8080"
+		}
+		named_port {
+			name = "https"
+			port = "8443"
+		}
+	}`, instance, instance, instance)
+}