Change google_container_node_pool ID format to zone/cluster/name to remove artificial restriction on node pool name across clusters (#304)

google/resource_container_node_pool.go

@@ -18,6 +18,9 @@ func resourceContainerNodePool() *schema.Resource {
 		Delete: resourceContainerNodePoolDelete,
 		Exists: resourceContainerNodePoolExists,
 
+		SchemaVersion: 1,
+		MigrateState:  resourceContainerNodePoolMigrateState,
+
 		Importer: &schema.ResourceImporter{
 			State: resourceContainerNodePoolStateImporter,
 		},
@@ -217,7 +220,7 @@ func resourceContainerNodePoolCreate(d *schema.ResourceData, meta interface{}) e
 
 	log.Printf("[INFO] GKE NodePool %s has been created", name)
 
-	d.SetId(name)
+	d.SetId(fmt.Sprintf("%s/%s/%s", zone, cluster, name))
 
 	return resourceContainerNodePoolRead(d, meta)
 }

google/resource_container_node_pool_migrate.go

@@ -0,0 +1,32 @@
+package google
+
+import (
+	"fmt"
+	"github.com/hashicorp/terraform/terraform"
+	"log"
+)
+
+func resourceContainerNodePoolMigrateState(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 Node Pool State v0; migrating to v1")
+		return migrateNodePoolStateV0toV1(is)
+	default:
+		return is, fmt.Errorf("Unexpected schema version: %d", v)
+	}
+}
+
+func migrateNodePoolStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
+	log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes)
+	log.Printf("[DEBUG] ID before migration: %s", is.ID)
+
+	is.ID = fmt.Sprintf("%s/%s/%s", is.Attributes["zone"], is.Attributes["cluster"], is.Attributes["name"])
+
+	log.Printf("[DEBUG] ID after migration: %s", is.ID)
+	return is, nil
+}

google/resource_container_node_pool_migrate_test.go

@@ -0,0 +1,65 @@
+package google
+
+import (
+	"github.com/hashicorp/terraform/terraform"
+	"testing"
+)
+
+func TestContainerNodePoolMigrateState(t *testing.T) {
+	cases := map[string]struct {
+		StateVersion int
+		Attributes   map[string]string
+		ExpectedId   string
+		Meta         interface{}
+	}{
+		"update id from name to zone/cluster/name": {
+			StateVersion: 0,
+			Attributes: map[string]string{
+				"name":    "node-pool-1",
+				"zone":    "us-central1-c",
+				"cluster": "cluster-1",
+			},
+			ExpectedId: "us-central1-c/cluster-1/node-pool-1",
+		},
+	}
+
+	for tn, tc := range cases {
+		is := &terraform.InstanceState{
+			ID:         tc.Attributes["name"],
+			Attributes: tc.Attributes,
+		}
+
+		is, err := resourceContainerNodePoolMigrateState(tc.StateVersion, is, tc.Meta)
+
+		if err != nil {
+			t.Fatalf("bad: %s, err: %#v", tn, err)
+		}
+
+		if is.ID != tc.ExpectedId {
+			t.Fatalf("Id should be set to `%s` but is `%s`", tc.ExpectedId, is.ID)
+		}
+	}
+}
+
+func TestContainerNodePoolMigrateState_empty(t *testing.T) {
+	var is *terraform.InstanceState
+	var meta *Config
+
+	// should handle nil
+	is, err := resourceContainerNodePoolMigrateState(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 = resourceContainerNodePoolMigrateState(0, is, meta)
+
+	if err != nil {
+		t.Fatalf("err: %#v", err)
+	}
+}