diff --git a/google/resource_compute_target_pool.go b/google/resource_compute_target_pool.go index 33ef305c..859aba85 100644 --- a/google/resource_compute_target_pool.go +++ b/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 / + 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( diff --git a/google/resource_compute_target_pool_test.go b/google/resource_compute_target_pool_test.go index 793f3171..b9c29e86 100644 --- a/google/resource_compute_target_pool_test.go +++ b/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) +}