From 12757bee7b5ef8d9a84a0235f473162cbe3e0064 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Mon, 19 Aug 2019 22:35:47 +0000 Subject: [PATCH] Throw an error on a duplicate copy, expand website Signed-off-by: Modular Magician --- google/resource_bigtable_instance.go | 117 ++--------------- google/resource_bigtable_instance_iam_test.go | 29 ++--- google/resource_bigtable_instance_test.go | 121 ++---------------- 3 files changed, 33 insertions(+), 234 deletions(-) diff --git a/google/resource_bigtable_instance.go b/google/resource_bigtable_instance.go index ead906450e3..ed1435b54d5 100644 --- a/google/resource_bigtable_instance.go +++ b/google/resource_bigtable_instance.go @@ -5,7 +5,6 @@ import ( "fmt" "log" - "github.com/hashicorp/terraform/helper/customdiff" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" @@ -16,14 +15,8 @@ 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, @@ -32,9 +25,10 @@ func resourceBigtableInstance() *schema.Resource { }, "cluster": { - Type: schema.TypeList, - Optional: true, - Computed: true, + Type: schema.TypeSet, + Required: true, + ForceNew: true, + MaxItems: 4, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "cluster_id": { @@ -50,6 +44,7 @@ func resourceBigtableInstance() *schema.Resource { "num_nodes": { Type: schema.TypeInt, Optional: true, + ForceNew: true, ValidateFunc: validation.IntAtLeast(3), }, "storage_type": { @@ -62,6 +57,7 @@ func resourceBigtableInstance() *schema.Resource { }, }, }, + "display_name": { Type: schema.TypeString, Optional: true, @@ -141,7 +137,7 @@ func resourceBigtableInstanceCreate(d *schema.ResourceData, meta interface{}) er conf.InstanceType = bigtable.PRODUCTION } - conf.Clusters = expandBigtableClusters(d.Get("cluster").([]interface{}), conf.InstanceID) + conf.Clusters = expandBigtableClusters(d.Get("cluster").(*schema.Set).List(), conf.InstanceID) c, err := config.bigtableClientFactory.NewInstanceAdminClient(project) if err != nil { @@ -185,7 +181,7 @@ func resourceBigtableInstanceRead(d *schema.ResourceData, meta interface{}) erro d.Set("project", project) - clusters := d.Get("cluster").([]interface{}) + clusters := d.Get("cluster").(*schema.Set).List() clusterState := []map[string]interface{}{} for _, cl := range clusters { cluster := cl.(map[string]interface{}) @@ -211,47 +207,6 @@ 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() @@ -310,59 +265,3 @@ 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/resource_bigtable_instance_iam_test.go b/google/resource_bigtable_instance_iam_test.go index a9b80fad0ea..a67b6f0eaa0 100644 --- a/google/resource_bigtable_instance_iam_test.go +++ b/google/resource_bigtable_instance_iam_test.go @@ -12,7 +12,6 @@ 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" @@ -25,7 +24,7 @@ func TestAccBigtableInstanceIamBinding(t *testing.T) { Steps: []resource.TestStep{ { // Test IAM Binding creation - Config: testAccBigtableInstanceIamBinding_basic(instance, cluster, account, role), + Config: testAccBigtableInstanceIamBinding_basic(instance, account, role), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_bigtable_instance_iam_binding.binding", "role", role), @@ -39,7 +38,7 @@ func TestAccBigtableInstanceIamBinding(t *testing.T) { }, { // Test IAM Binding update - Config: testAccBigtableInstanceIamBinding_update(instance, cluster, account, role), + Config: testAccBigtableInstanceIamBinding_update(instance, account, role), }, { ResourceName: "google_bigtable_instance_iam_binding.binding", @@ -55,7 +54,6 @@ 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" @@ -71,7 +69,7 @@ func TestAccBigtableInstanceIamMember(t *testing.T) { Steps: []resource.TestStep{ { // Test IAM Binding creation - Config: testAccBigtableInstanceIamMember(instance, cluster, account, role), + Config: testAccBigtableInstanceIamMember(instance, account, role), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_bigtable_instance_iam_member.member", "role", role), @@ -93,7 +91,6 @@ 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" @@ -106,7 +103,7 @@ func TestAccBigtableInstanceIamPolicy(t *testing.T) { Steps: []resource.TestStep{ { // Test IAM Binding creation - Config: testAccBigtableInstanceIamPolicy(instance, cluster, account, role), + Config: testAccBigtableInstanceIamPolicy(instance, account, role), }, { ResourceName: "google_bigtable_instance_iam_policy.policy", @@ -118,7 +115,7 @@ func TestAccBigtableInstanceIamPolicy(t *testing.T) { }) } -func testAccBigtableInstanceIamBinding_basic(instance, cluster, account, role string) string { +func testAccBigtableInstanceIamBinding_basic(instance, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account1" { @@ -138,10 +135,10 @@ resource "google_bigtable_instance_iam_binding" "binding" { "serviceAccount:${google_service_account.test-account1.email}", ] } -`, instance, cluster, account, account, role) +`, instance, acctest.RandString(10), account, account, role) } -func testAccBigtableInstanceIamBinding_update(instance, cluster, account, role string) string { +func testAccBigtableInstanceIamBinding_update(instance, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account1" { account_id = "%s-1" @@ -161,10 +158,10 @@ resource "google_bigtable_instance_iam_binding" "binding" { "serviceAccount:${google_service_account.test-account2.email}", ] } -`, instance, cluster, account, account, role) +`, instance, acctest.RandString(10), account, account, role) } -func testAccBigtableInstanceIamMember(instance, cluster, account, role string) string { +func testAccBigtableInstanceIamMember(instance, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account" { account_id = "%s" @@ -176,10 +173,10 @@ resource "google_bigtable_instance_iam_member" "member" { role = "%s" member = "serviceAccount:${google_service_account.test-account.email}" } -`, instance, cluster, account, role) +`, instance, acctest.RandString(10), account, role) } -func testAccBigtableInstanceIamPolicy(instance, cluster, account, role string) string { +func testAccBigtableInstanceIamPolicy(instance, account, role string) string { return fmt.Sprintf(testBigtableInstanceIam+` resource "google_service_account" "test-account" { account_id = "%s" @@ -197,7 +194,7 @@ resource "google_bigtable_instance_iam_policy" "policy" { instance = "${google_bigtable_instance.instance.name}" policy_data = "${data.google_iam_policy.policy.policy_data}" } -`, instance, cluster, account, role) +`, instance, acctest.RandString(10), account, role) } // Smallest instance possible for testing @@ -207,7 +204,7 @@ resource "google_bigtable_instance" "instance" { instance_type = "DEVELOPMENT" cluster { - cluster_id = "%s" + cluster_id = "c-%s" zone = "us-central1-b" storage_type = "HDD" } diff --git a/google/resource_bigtable_instance_test.go b/google/resource_bigtable_instance_test.go index 16426f7f6c7..d3bb9d69cc4 100644 --- a/google/resource_bigtable_instance_test.go +++ b/google/resource_bigtable_instance_test.go @@ -21,22 +21,18 @@ 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", 3), + "google_bigtable_instance.instance"), ), }, { Config: testAccBigtableInstance(instanceName, 4), Check: resource.ComposeTestCheckFunc( testAccBigtableInstanceExists( - "google_bigtable_instance.instance", 4), + "google_bigtable_instance.instance"), ), }, }, @@ -58,17 +54,10 @@ 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, 3), - Check: resource.ComposeTestCheckFunc( - testAccBigtableInstanceExists( - "google_bigtable_instance.instance", 3), - ), - }, - { - Config: testAccBigtableInstance_cluster_reordered(instanceName, 5), + Config: testAccBigtableInstance_cluster(instanceName), Check: resource.ComposeTestCheckFunc( testAccBigtableInstanceExists( - "google_bigtable_instance.instance", 5), + "google_bigtable_instance.instance"), ), }, }, @@ -85,19 +74,11 @@ 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", 0), + "google_bigtable_instance.instance"), ), }, }, @@ -128,7 +109,7 @@ func testAccCheckBigtableInstanceDestroy(s *terraform.State) error { return nil } -func testAccBigtableInstanceExists(n string, numNodes int) resource.TestCheckFunc { +func testAccBigtableInstanceExists(n string) resource.TestCheckFunc { var ctx = context.Background() return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -152,21 +133,6 @@ func testAccBigtableInstanceExists(n string, numNodes int) resource.TestCheckFun 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 } } @@ -185,44 +151,36 @@ resource "google_bigtable_instance" "instance" { `, instanceName, instanceName, numNodes) } -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 { +func testAccBigtableInstance_cluster(instanceName string) string { return fmt.Sprintf(` resource "google_bigtable_instance" "instance" { name = "%s" cluster { cluster_id = "%s-a" zone = "us-central1-a" - num_nodes = %d + num_nodes = 3 storage_type = "HDD" } cluster { cluster_id = "%s-b" zone = "us-central1-b" - num_nodes = %d + num_nodes = 3 storage_type = "HDD" } cluster { cluster_id = "%s-c" zone = "us-central1-c" - num_nodes = %d + num_nodes = 3 storage_type = "HDD" } cluster { cluster_id = "%s-d" zone = "us-central1-f" - num_nodes = %d + num_nodes = 3 storage_type = "HDD" } } -`, instanceName, instanceName, numNodes, instanceName, numNodes, instanceName, numNodes, instanceName, numNodes) +`, instanceName, instanceName, instanceName, instanceName, instanceName) } func testAccBigtableInstance_clusterMax(instanceName string) string { @@ -263,38 +221,6 @@ 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" { @@ -307,26 +233,3 @@ 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) -}