Skip to content

Commit

Permalink
add ability to update Bigtable instance cluster node count (#4026)
Browse files Browse the repository at this point in the history 
* add ability to update Bigtable instance cluster node count

* cleanup diff

* test is passing for reordering clusters

* refactor and address code review comments

* add check for MaxItems:4 for cluster attribute

* fixup diff after rebasing master

* add additional tests and logic for validating DEVELOPMENT instances per review comments
- add ForceNew back to display_name attribute

* remove explicit check for DEVELOPMENT instance in update method
  • Loading branch information
@mackenziestarr @rileykarson
mackenziestarr authored and rileykarson committed Aug 19, 2019
1 parent f12d6db commit b753d89
Show file tree
Hide file tree
Showing 3 changed files with 234 additions and 33 deletions.
117 changes: 109 additions & 8 deletions google/resource_bigtable_instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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,
Expand All @@ -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": {
Expand All @@ -44,7 +50,6 @@ func resourceBigtableInstance() *schema.Resource {
"num_nodes": {
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
ValidateFunc: validation.IntAtLeast(3),
},
"storage_type": {
Expand All @@ -57,7 +62,6 @@ func resourceBigtableInstance() *schema.Resource {
},
},
},

"display_name": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{})
Expand All @@ -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()
Expand Down Expand Up @@ -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
}
29 changes: 16 additions & 13 deletions google/resource_bigtable_instance_iam_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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),
Expand All @@ -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",
Expand All @@ -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"

Expand All @@ -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),
Expand All @@ -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"

Expand All @@ -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",
Expand All @@ -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" {
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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"
}
Expand Down
Loading

0 comments on commit b753d89

Please sign in to comment.