Change instances in target pool to be a set

google/resource_compute_target_pool.go

@@ -60,29 +60,18 @@ func resourceComputeTargetPool() *schema.Resource {
 			},
 
 			"instances": {
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
 				Computed: true,
 				ForceNew: false,
-				Elem:     &schema.Schema{Type: schema.TypeString},
-				DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
-					// instances are stored in state as "zone/name"
-					oldParts := strings.Split(old, "/")
-
-					// instances can also be specified in the config as a URL
-					if parts := instancesSelfLinkPattern.FindStringSubmatch(new); len(oldParts) == 2 && len(parts) == 4 {
-						// parts[0] = full match
-						// parts[1] = project
-						// parts[2] = zone
-						// parts[3] = instance name
-
-						oZone, oName := oldParts[0], oldParts[1]
-						nZone, nName := parts[2], parts[3]
-						if oZone == nZone && oName == nName {
-							return true
-						}
-					}
-					return false
+				Elem: &schema.Schema{
+					Type: schema.TypeString,
+					StateFunc: func(v interface{}) string {
+						return canonicalizeInstanceRef(v.(string))
+					},
+				},
+				Set: func(v interface{}) int {
+					return schema.HashString(canonicalizeInstanceRef(v.(string)))
 				},
 			},
 
@@ -115,6 +104,22 @@ func resourceComputeTargetPool() *schema.Resource {
 	}
 }
 
+func canonicalizeInstanceRef(instanceRef string) string {
+	// instances can also be specified in the config as a URL or <zone>/<project>
+	parts := instancesSelfLinkPattern.FindStringSubmatch(instanceRef)
+	// parts[0] = full match
+	// parts[1] = project
+	// parts[2] = zone
+	// parts[3] = instance name
+
+	if len(parts) < 4 {
+		return instanceRef
+	}
+
+	return fmt.Sprintf("%s/%s", parts[2], parts[3])
+	// return fmt.Sprintf("%s/%s/%s", parts[1], parts[2], parts[3])
+}
+
 // Healthchecks need to exist before being referred to from the target pool.
 func convertHealthChecks(healthChecks []interface{}, d *schema.ResourceData, config *Config) ([]string, error) {
 	if healthChecks == nil || len(healthChecks) == 0 {
@@ -131,9 +136,10 @@ func convertHealthChecks(healthChecks []interface{}, d *schema.ResourceData, con
 
 // Instances do not need to exist yet, so we simply generate URLs.
 // Instances can be full URLS or zone/name
-func convertInstancesToUrls(config *Config, project string, names []string) ([]string, error) {
-	urls := make([]string, len(names))
-	for i, name := range names {
+func convertInstancesToUrls(project string, names *schema.Set) ([]string, error) {
+	urls := make([]string, len(names.List()))
+	for i, nameI := range names.List() {
+		name := nameI.(string)
 		if strings.HasPrefix(name, "https://www.googleapis.com/compute/v1/") {
 			urls[i] = name
 		} else {
@@ -168,8 +174,7 @@ func resourceComputeTargetPoolCreate(d *schema.ResourceData, meta interface{}) e
 		return err
 	}
 
-	instanceUrls, err := convertInstancesToUrls(
-		config, project, convertStringArr(d.Get("instances").([]interface{})))
+	instanceUrls, err := convertInstancesToUrls(project, d.Get("instances").(*schema.Set))
 	if err != nil {
 		return err
 	}
@@ -301,23 +306,23 @@ func resourceComputeTargetPoolUpdate(d *schema.ResourceData, meta interface{}) e
 
 	if d.HasChange("instances") {
 
-		from_, to_ := d.GetChange("instances")
-		from := convertStringArr(from_.([]interface{}))
-		to := convertStringArr(to_.([]interface{}))
-		fromUrls, err := convertInstancesToUrls(config, project, from)
+		old_, new_ := d.GetChange("instances")
+		old := old_.(*schema.Set)
+		new := new_.(*schema.Set)
+
+		addUrls, err := convertInstancesToUrls(project, new.Difference(old))
 		if err != nil {
 			return err
 		}
-		toUrls, err := convertInstancesToUrls(config, project, to)
+		removeUrls, err := convertInstancesToUrls(project, old.Difference(new))
 		if err != nil {
 			return err
 		}
-		add, remove := calcAddRemove(fromUrls, toUrls)
 
 		addReq := &compute.TargetPoolsAddInstanceRequest{
-			Instances: make([]*compute.InstanceReference, len(add)),
+			Instances: make([]*compute.InstanceReference, len(addUrls)),
 		}
-		for i, v := range add {
+		for i, v := range addUrls {
 			addReq.Instances[i] = &compute.InstanceReference{Instance: v}
 		}
 		op, err := config.clientCompute.TargetPools.AddInstance(
@@ -331,9 +336,9 @@ func resourceComputeTargetPoolUpdate(d *schema.ResourceData, meta interface{}) e
 			return err
 		}
 		removeReq := &compute.TargetPoolsRemoveInstanceRequest{
-			Instances: make([]*compute.InstanceReference, len(remove)),
+			Instances: make([]*compute.InstanceReference, len(removeUrls)),
 		}
-		for i, v := range remove {
+		for i, v := range removeUrls {
 			removeReq.Instances[i] = &compute.InstanceReference{Instance: v}
 		}
 		op, err = config.clientCompute.TargetPools.RemoveInstance(

google/resource_compute_target_pool_test.go

@@ -37,6 +37,49 @@ func TestAccComputeTargetPool_basic(t *testing.T) {
 	})
 }
 
+func TestAccComputeTargetPool_update(t *testing.T) {
+	t.Parallel()
+
+	tpname := fmt.Sprintf("tptest-%s", acctest.RandString(10))
+	name1 := fmt.Sprintf("tptest-%s", acctest.RandString(10))
+	name2 := fmt.Sprintf("tptest-%s", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeTargetPoolDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				// Create target pool with no instances attached
+				Config: testAccComputeTargetPool_update(tpname, "", name1, name2),
+			},
+			resource.TestStep{
+				ResourceName:      "google_compute_target_pool.foo",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			resource.TestStep{
+				// Add the two instances to the pool
+				Config: testAccComputeTargetPool_update(tpname,
+					`"${google_compute_instance.foo.self_link}", "${google_compute_instance.bar.self_link}"`,
+					name1, name2),
+			},
+			resource.TestStep{
+				ResourceName:      "google_compute_target_pool.foo",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			resource.TestStep{
+				// Reversing the order of instances or changing import format shouldn't matter
+				Config: testAccComputeTargetPool_update(tpname,
+					fmt.Sprintf(`"${google_compute_instance.bar.self_link}", "us-central1-a/%s"`, name1),
+					name1, name2),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 func testAccCheckComputeTargetPoolDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 
@@ -149,3 +192,45 @@ resource "google_compute_target_pool" "bar" {
 	]
 }`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10), acctest.RandString(10))
 }
+
+func testAccComputeTargetPool_update(tpname, instances, name1, name2 string) string {
+	return fmt.Sprintf(`
+resource "google_compute_target_pool" "foo" {
+	description = "Resource created for Terraform acceptance testing"
+	name = "tpool-test-%s"
+	instances = [%s]
+}
+
+resource "google_compute_instance" "foo" {
+	name         = "%s"
+	machine_type = "n1-standard-1"
+	zone         = "us-central1-a"
+
+  boot_disk {
+    initialize_params {
+      image = "debian-cloud/debian-9"
+    }
+  }
+
+	network_interface {
+		network = "default"
+	}
+}
+
+resource "google_compute_instance" "bar" {
+	name         = "%s"
+	machine_type = "n1-standard-1"
+	zone         = "us-central1-a"
+
+  boot_disk {
+    initialize_params {
+      image = "debian-cloud/debian-9"
+    }
+  }
+
+	network_interface {
+		network = "default"
+	}
+}
+`, tpname, instances, name1, name2)
+}