From 82608ca54ba7bd698d030bfcb22540186ebb356c Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Wed, 22 Mar 2017 17:47:41 -0700 Subject: [PATCH] provider/google: turn compute_instance_group.instances into a set (#12790) --- .../google/resource_compute_instance_group.go | 11 ++- ...resource_compute_instance_group_migrate.go | 74 ++++++++++++++++++ ...rce_compute_instance_group_migrate_test.go | 75 +++++++++++++++++++ .../resource_compute_instance_group_test.go | 68 +++++++++++++++++ 4 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 builtin/providers/google/resource_compute_instance_group_migrate.go create mode 100644 builtin/providers/google/resource_compute_instance_group_migrate_test.go diff --git a/builtin/providers/google/resource_compute_instance_group.go b/builtin/providers/google/resource_compute_instance_group.go index a6ece3a41..1f2b93e06 100644 --- a/builtin/providers/google/resource_compute_instance_group.go +++ b/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) diff --git a/builtin/providers/google/resource_compute_instance_group_migrate.go b/builtin/providers/google/resource_compute_instance_group_migrate.go new file mode 100644 index 000000000..1db04c22a --- /dev/null +++ b/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 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 +} diff --git a/builtin/providers/google/resource_compute_instance_group_migrate_test.go b/builtin/providers/google/resource_compute_instance_group_migrate_test.go new file mode 100644 index 000000000..88057d99e --- /dev/null +++ b/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) + } +} diff --git a/builtin/providers/google/resource_compute_instance_group_test.go b/builtin/providers/google/resource_compute_instance_group_test.go index 4435454c1..2dfe63d34 100644 --- a/builtin/providers/google/resource_compute_instance_group_test.go +++ b/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) +}