From 732d717e6b311a690ffd99425f27f5b431f23dc4 Mon Sep 17 00:00:00 2001 From: The Magician Date: Mon, 19 Aug 2019 15:27:24 -0700 Subject: [PATCH] add ability to update Bigtable instance cluster node count (#1067) Signed-off-by: Modular Magician --- google-beta/resource_bigtable_instance.go | 117 +++++++++++++++-- .../resource_bigtable_instance_iam_test.go | 29 +++-- .../resource_bigtable_instance_test.go | 121 ++++++++++++++++-- 3 files changed, 234 insertions(+), 33 deletions(-) diff --git a/google-beta/resource_bigtable_instance.go b/google-beta/resource_bigtable_instance.go index ed1435b54d5..ead906450e3 100644 --- a/google-beta/resource_bigtable_instance.go +++ b/google-beta/resource_bigtable_instance.go @@ -5,6 +5,7 @@ import ( "fmt" "log" + "github.com/hashicorp/terraform/helper/customdiff" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" @@ -15,8 +16,14 @@ func resourceBigtableInstance() *schema.Resource { return &schema.Resource{ Create: resourceBigtableInstanceCreate, Read: resourceBigtableInstanceRead, + Update: resourceBigtableInstanceUpdate, Delete: resourceBigtableInstanceDestroy, + CustomizeDiff: customdiff.All( + resourceBigtableInstanceValidateDevelopment, + resourceBigtableInstanceClusterReorderTypeList, + ), + Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -25,10 +32,9 @@ func resourceBigtableInstance() *schema.Resource { }, "cluster": { - Type: schema.TypeSet, - Required: true, - ForceNew: true, - MaxItems: 4, + Type: schema.TypeList, + Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "cluster_id": { @@ -44,7 +50,6 @@ func resourceBigtableInstance() *schema.Resource { "num_nodes": { Type: schema.TypeInt, Optional: true, - ForceNew: true, ValidateFunc: validation.IntAtLeast(3), }, "storage_type": { @@ -57,7 +62,6 @@ func resourceBigtableInstance() *schema.Resource { }, }, }, - "display_name": { Type: schema.TypeString, Optional: true, @@ -137,7 +141,7 @@ func resourceBigtableInstanceCreate(d *schema.ResourceData, meta interface{}) er conf.InstanceType = bigtable.PRODUCTION } - conf.Clusters = expandBigtableClusters(d.Get("cluster").(*schema.Set).List(), conf.InstanceID) + conf.Clusters = expandBigtableClusters(d.Get("cluster").([]interface{}), conf.InstanceID) c, err := config.bigtableClientFactory.NewInstanceAdminClient(project) if err != nil { @@ -181,7 +185,7 @@ func resourceBigtableInstanceRead(d *schema.ResourceData, meta interface{}) erro d.Set("project", project) - clusters := d.Get("cluster").(*schema.Set).List() + clusters := d.Get("cluster").([]interface{}) clusterState := []map[string]interface{}{} for _, cl := range clusters { cluster := cl.(map[string]interface{}) @@ -207,6 +211,47 @@ func resourceBigtableInstanceRead(d *schema.ResourceData, meta interface{}) erro return nil } +func resourceBigtableInstanceUpdate(d *schema.ResourceData, meta interface{}) error { + config := meta.(*Config) + ctx := context.Background() + + project, err := getProject(d, config) + if err != nil { + return err + } + + c, err := config.bigtableClientFactory.NewInstanceAdminClient(project) + if err != nil { + return fmt.Errorf("Error starting instance admin client. %s", err) + } + defer c.Close() + + clusters, err := c.Clusters(ctx, d.Get("name").(string)) + if err != nil { + return fmt.Errorf("Error retrieving clusters for instance %s", err.Error()) + } + + clusterMap := make(map[string]*bigtable.ClusterInfo, len(clusters)) + for _, cluster := range clusters { + clusterMap[cluster.Name] = cluster + } + + for _, cluster := range d.Get("cluster").([]interface{}) { + config := cluster.(map[string]interface{}) + cluster_id := config["cluster_id"].(string) + if cluster, ok := clusterMap[cluster_id]; ok { + if cluster.ServeNodes != config["num_nodes"].(int) { + err = c.UpdateCluster(ctx, d.Get("name").(string), cluster.Name, int32(config["num_nodes"].(int))) + if err != nil { + return fmt.Errorf("Error updating cluster %s for instance %s", cluster.Name, d.Get("name").(string)) + } + } + } + } + + return resourceBigtableInstanceRead(d, meta) +} + func resourceBigtableInstanceDestroy(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) ctx := context.Background() @@ -265,3 +310,59 @@ func expandBigtableClusters(clusters []interface{}, instanceID string) []bigtabl } return results } + +func resourceBigtableInstanceValidateDevelopment(diff *schema.ResourceDiff, meta interface{}) error { + if diff.Get("instance_type").(string) != "DEVELOPMENT" { + return nil + } + if diff.Get("cluster.#").(int) != 1 { + return fmt.Errorf("config is invalid: instance with instance_type=\"DEVELOPMENT\" should have exactly one \"cluster\" block") + } + if diff.Get("cluster.0.num_nodes").(int) != 0 { + return fmt.Errorf("config is invalid: num_nodes cannot be set for instance_type=\"DEVELOPMENT\"") + } + return nil +} + +func resourceBigtableInstanceClusterReorderTypeList(diff *schema.ResourceDiff, meta interface{}) error { + old_count, new_count := diff.GetChange("cluster.#") + + // simulate Required:true, MinItems:1, MaxItems:4 for "cluster" + if new_count.(int) < 1 { + return fmt.Errorf("config is invalid: Too few cluster blocks: Should have at least 1 \"cluster\" block") + } + if new_count.(int) > 4 { + return fmt.Errorf("config is invalid: Too many cluster blocks: No more than 4 \"cluster\" blocks are allowed") + } + + if old_count.(int) != new_count.(int) { + return nil + } + + var old_ids []string + clusters := make(map[string]interface{}, new_count.(int)) + + for i := 0; i < new_count.(int); i++ { + old_id, new_id := diff.GetChange(fmt.Sprintf("cluster.%d.cluster_id", i)) + if old_id != nil && old_id != "" { + old_ids = append(old_ids, old_id.(string)) + } + _, c := diff.GetChange(fmt.Sprintf("cluster.%d", i)) + clusters[new_id.(string)] = c + } + + // reorder clusters according to the old cluster order + var old_cluster_order []interface{} + for _, id := range old_ids { + if c, ok := clusters[id]; ok { + old_cluster_order = append(old_cluster_order, c) + } + } + + err := diff.SetNew("cluster", old_cluster_order) + if err != nil { + return fmt.Errorf("Error setting cluster diff: %s", err) + } + + return nil +} diff --git a/google-beta/resource_bigtable_instance_iam_test.go b/google-beta/resource_bigtable_instance_iam_test.go index a67b6f0eaa0..a9b80fad0ea 100644 --- a/google-beta/resource_bigtable_instance_iam_test.go +++ b/google-beta/resource_bigtable_instance_iam_test.go @@ -12,6 +12,7 @@ func TestAccBigtableInstanceIamBinding(t *testing.T) { t.Parallel() instance := "tf-bigtable-iam-" + acctest.RandString(10) + cluster := "c-" + acctest.RandString(10) account := "tf-bigtable-iam-" + acctest.RandString(10) role := "roles/bigtable.user" @@ -24,7 +25,7 @@ func TestAccBigtableInstanceIamBinding(t *testing.T) { Steps: []resource.TestStep{ { // Test IAM Binding creation - Config: testAccBigtableInstanceIamBinding_basic(instance, account, role), + Config: testAccBigtableInstanceIamBinding_basic(instance, cluster, account, role), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_bigtable_instance_iam_binding.binding", "role", role), @@ -38,7 +39,7 @@ func TestAccBigtableInstanceIamBinding(t *testing.T) { }, { // Test IAM Binding update - Config: testAccBigtableInstanceIamBinding_update(instance, account, role), + Config: testAccBigtableInstanceIamBinding_update(instance, cluster, account, role), }, { ResourceName: "google_bigtable_instance_iam_binding.binding", @@ -54,6 +55,7 @@ func TestAccBigtableInstanceIamMember(t *testing.T) { t.Parallel() instance := "tf-bigtable-iam-" + acctest.RandString(10) + cluster := "c-" + acctest.RandString(10) account := "tf-bigtable-iam-" + acctest.RandString(10) role := "roles/bigtable.user" @@ -69,7 +71,7 @@ func TestAccBigtableInstanceIamMember(t *testing.T) { Steps: []resource.TestStep{ { // Test IAM Binding creation - Config: testAccBigtableInstanceIamMember(instance, account, role), + Config: testAccBigtableInstanceIamMember(instance, cluster, account, role), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_bigtable_instance_iam_member.member", "role", role), @@ -91,6 +93,7 @@ func TestAccBigtableInstanceIamPolicy(t *testing.T) { t.Parallel() instance := "tf-bigtable-iam-" + acctest.RandString(10) + cluster := "c-" + acctest.RandString(10) account := "tf-bigtable-iam-" + acctest.RandString(10) role := "roles/bigtable.user" @@ -103,7 +106,7 @@ func TestAccBigtableInstanceIamPolicy(t *testing.T) { Steps: []resource.TestStep{ { // Test IAM Binding creation - Config: testAccBigtableInstanceIamPolicy(instance, account, role), + Config: testAccBigtableInstanceIamPolicy(instance, cluster, account, role), }, { ResourceName: "google_bigtable_instance_iam_policy.policy", @@ -115,7 +118,7 @@ func TestAccBigtableInstanceIamPolicy(t *testing.T) { }) } -func testAccBigtableInstanceIamBinding_basic(instance, account, role string) string { +func testAccBigtableInstanceIamBinding_basic(instance, cluster, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account1" { @@ -135,10 +138,10 @@ resource "google_bigtable_instance_iam_binding" "binding" { "serviceAccount:${google_service_account.test-account1.email}", ] } -`, instance, acctest.RandString(10), account, account, role) +`, instance, cluster, account, account, role) } -func testAccBigtableInstanceIamBinding_update(instance, account, role string) string { +func testAccBigtableInstanceIamBinding_update(instance, cluster, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account1" { account_id = "%s-1" @@ -158,10 +161,10 @@ resource "google_bigtable_instance_iam_binding" "binding" { "serviceAccount:${google_service_account.test-account2.email}", ] } -`, instance, acctest.RandString(10), account, account, role) +`, instance, cluster, account, account, role) } -func testAccBigtableInstanceIamMember(instance, account, role string) string { +func testAccBigtableInstanceIamMember(instance, cluster, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account" { account_id = "%s" @@ -173,10 +176,10 @@ resource "google_bigtable_instance_iam_member" "member" { role = "%s" member = "serviceAccount:${google_service_account.test-account.email}" } -`, instance, acctest.RandString(10), account, role) +`, instance, cluster, account, role) } -func testAccBigtableInstanceIamPolicy(instance, account, role string) string { +func testAccBigtableInstanceIamPolicy(instance, cluster, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account" { account_id = "%s" @@ -194,7 +197,7 @@ resource "google_bigtable_instance_iam_policy" "policy" { instance = "${google_bigtable_instance.instance.name}" policy_data = "${data.google_iam_policy.policy.policy_data}" } -`, instance, acctest.RandString(10), account, role) +`, instance, cluster, account, role) } // Smallest instance possible for testing @@ -204,7 +207,7 @@ resource "google_bigtable_instance" "instance" { instance_type = "DEVELOPMENT" cluster { - cluster_id = "c-%s" + cluster_id = "%s" zone = "us-central1-b" storage_type = "HDD" } diff --git a/google-beta/resource_bigtable_instance_test.go b/google-beta/resource_bigtable_instance_test.go index d3bb9d69cc4..16426f7f6c7 100644 --- a/google-beta/resource_bigtable_instance_test.go +++ b/google-beta/resource_bigtable_instance_test.go @@ -21,18 +21,22 @@ func TestAccBigtableInstance_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckBigtableInstanceDestroy, Steps: []resource.TestStep{ + { + Config: testAccBigtableInstance_invalid(instanceName), + ExpectError: regexp.MustCompile("config is invalid: Too few cluster blocks: Should have at least 1 \"cluster\" block"), + }, { Config: testAccBigtableInstance(instanceName, 3), Check: resource.ComposeTestCheckFunc( testAccBigtableInstanceExists( - "google_bigtable_instance.instance"), + "google_bigtable_instance.instance", 3), ), }, { Config: testAccBigtableInstance(instanceName, 4), Check: resource.ComposeTestCheckFunc( testAccBigtableInstanceExists( - "google_bigtable_instance.instance"), + "google_bigtable_instance.instance", 4), ), }, }, @@ -54,10 +58,17 @@ func TestAccBigtableInstance_cluster(t *testing.T) { ExpectError: regexp.MustCompile("config is invalid: Too many cluster blocks: No more than 4 \"cluster\" blocks are allowed"), }, { - Config: testAccBigtableInstance_cluster(instanceName), + Config: testAccBigtableInstance_cluster(instanceName, 3), + Check: resource.ComposeTestCheckFunc( + testAccBigtableInstanceExists( + "google_bigtable_instance.instance", 3), + ), + }, + { + Config: testAccBigtableInstance_cluster_reordered(instanceName, 5), Check: resource.ComposeTestCheckFunc( testAccBigtableInstanceExists( - "google_bigtable_instance.instance"), + "google_bigtable_instance.instance", 5), ), }, }, @@ -74,11 +85,19 @@ func TestAccBigtableInstance_development(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckBigtableInstanceDestroy, Steps: []resource.TestStep{ + { + Config: testAccBigtableInstance_development_invalid_no_cluster(instanceName), + ExpectError: regexp.MustCompile("config is invalid: instance with instance_type=\"DEVELOPMENT\" should have exactly one \"cluster\" block"), + }, + { + Config: testAccBigtableInstance_development_invalid_num_nodes(instanceName), + ExpectError: regexp.MustCompile("config is invalid: num_nodes cannot be set for instance_type=\"DEVELOPMENT\""), + }, { Config: testAccBigtableInstance_development(instanceName), Check: resource.ComposeTestCheckFunc( testAccBigtableInstanceExists( - "google_bigtable_instance.instance"), + "google_bigtable_instance.instance", 0), ), }, }, @@ -109,7 +128,7 @@ func testAccCheckBigtableInstanceDestroy(s *terraform.State) error { return nil } -func testAccBigtableInstanceExists(n string) resource.TestCheckFunc { +func testAccBigtableInstanceExists(n string, numNodes int) resource.TestCheckFunc { var ctx = context.Background() return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -133,6 +152,21 @@ func testAccBigtableInstanceExists(n string) resource.TestCheckFunc { return fmt.Errorf("Error retrieving instance %s.", rs.Primary.Attributes["name"]) } + clusters, err := c.Clusters(ctx, rs.Primary.Attributes["name"]) + if err != nil { + return fmt.Errorf("Error retrieving cluster list for instance %s.", rs.Primary.Attributes["name"]) + } + + for _, c := range clusters { + if c.ServeNodes != numNodes { + return fmt.Errorf("Expected cluster %s to have %d nodes but got %d nodes for instance %s.", + c.Name, + numNodes, + c.ServeNodes, + rs.Primary.Attributes["name"]) + } + } + return nil } } @@ -151,36 +185,44 @@ resource "google_bigtable_instance" "instance" { `, instanceName, instanceName, numNodes) } -func testAccBigtableInstance_cluster(instanceName string) string { +func testAccBigtableInstance_invalid(instanceName string) string { + return fmt.Sprintf(` +resource "google_bigtable_instance" "instance" { + name = "%s" +} +`, instanceName) +} + +func testAccBigtableInstance_cluster(instanceName string, numNodes int) string { return fmt.Sprintf(` resource "google_bigtable_instance" "instance" { name = "%s" cluster { cluster_id = "%s-a" zone = "us-central1-a" - num_nodes = 3 + num_nodes = %d storage_type = "HDD" } cluster { cluster_id = "%s-b" zone = "us-central1-b" - num_nodes = 3 + num_nodes = %d storage_type = "HDD" } cluster { cluster_id = "%s-c" zone = "us-central1-c" - num_nodes = 3 + num_nodes = %d storage_type = "HDD" } cluster { cluster_id = "%s-d" zone = "us-central1-f" - num_nodes = 3 + num_nodes = %d storage_type = "HDD" } } -`, instanceName, instanceName, instanceName, instanceName, instanceName) +`, instanceName, instanceName, numNodes, instanceName, numNodes, instanceName, numNodes, instanceName, numNodes) } func testAccBigtableInstance_clusterMax(instanceName string) string { @@ -221,6 +263,38 @@ resource "google_bigtable_instance" "instance" { `, instanceName, instanceName, instanceName, instanceName, instanceName, instanceName) } +func testAccBigtableInstance_cluster_reordered(instanceName string, numNodes int) string { + return fmt.Sprintf(` +resource "google_bigtable_instance" "instance" { + name = "%s" + cluster { + cluster_id = "%s-c" + zone = "us-central1-c" + num_nodes = %d + storage_type = "HDD" + } + cluster { + cluster_id = "%s-d" + zone = "us-central1-f" + num_nodes = %d + storage_type = "HDD" + } + cluster { + cluster_id = "%s-a" + zone = "us-central1-a" + num_nodes = %d + storage_type = "HDD" + } + cluster { + cluster_id = "%s-b" + zone = "us-central1-b" + num_nodes = %d + storage_type = "HDD" + } +} +`, instanceName, instanceName, numNodes, instanceName, numNodes, instanceName, numNodes, instanceName, numNodes) +} + func testAccBigtableInstance_development(instanceName string) string { return fmt.Sprintf(` resource "google_bigtable_instance" "instance" { @@ -233,3 +307,26 @@ resource "google_bigtable_instance" "instance" { } `, instanceName, instanceName) } + +func testAccBigtableInstance_development_invalid_num_nodes(instanceName string) string { + return fmt.Sprintf(` +resource "google_bigtable_instance" "instance" { + name = "%s" + cluster { + cluster_id = "%s" + zone = "us-central1-b" + num_nodes = 3 + } + instance_type = "DEVELOPMENT" +} +`, instanceName, instanceName) +} + +func testAccBigtableInstance_development_invalid_no_cluster(instanceName string) string { + return fmt.Sprintf(` +resource "google_bigtable_instance" "instance" { + name = "%s" + instance_type = "DEVELOPMENT" +} +`, instanceName) +}