Skip to content

Commit

Permalink
Add schema update support to spanner db 2082 (#3947) (#7279)
Browse files Browse the repository at this point in the history 
* eoncders and customdiff added for spanner DB ddl update

* config update test case added

* customdiff modified to handle out-of-index issue

* new lines added

* indent fixed

* indent fixed for tests

* test added for ddl update condition

* mock added Terraformresourcediff, unit tests added

* test fixed

* more unit tests added

* tests fixed

* PR comments implemented

* unit tests converted to table driven tests

* ImportStateVerifyIgnore flag added to tests

* syntax corrected in test

Signed-off-by: Modular Magician <[email protected]>
  • Loading branch information
@modular-magician
modular-magician authored Sep 16, 2020
1 parent 6d0c641 commit ca6062f
Show file tree
Hide file tree
Showing 6 changed files with 272 additions and 19 deletions.
3 changes: 3 additions & 0 deletions .changelog/3947.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
spanner: Added schema update/update ddl support for `google_spanner_database`
```
124 changes: 123 additions & 1 deletion google/resource_spanner_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,49 @@ import (
"fmt"
"log"
"reflect"
"strings"
"time"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

// customizeDiff func for additional checks on google_spanner_database properties:
func resourceSpannerDBDdlCustomDiffFunc(diff TerraformResourceDiff) error {
old, new := diff.GetChange("ddl")
oldDdls := old.([]interface{})
newDdls := new.([]interface{})
var err error

if len(newDdls) < len(oldDdls) {
err = diff.ForceNew("ddl")
if err != nil {
return fmt.Errorf("ForceNew failed for ddl, old - %v and new - %v", oldDdls, newDdls)
}
return nil
}

for i := range oldDdls {
if newDdls[i].(string) != oldDdls[i].(string) {
err = diff.ForceNew("ddl")
if err != nil {
return fmt.Errorf("ForceNew failed for ddl, old - %v and new - %v", oldDdls, newDdls)
}
return nil
}
}
return nil
}

func resourceSpannerDBDdlCustomDiff(diff *schema.ResourceDiff, meta interface{}) error {
// separate func to allow unit testing
return resourceSpannerDBDdlCustomDiffFunc(diff)
}

func resourceSpannerDatabase() *schema.Resource {
return &schema.Resource{
Create: resourceSpannerDatabaseCreate,
Read: resourceSpannerDatabaseRead,
Update: resourceSpannerDatabaseUpdate,
Delete: resourceSpannerDatabaseDelete,

Importer: &schema.ResourceImporter{
Expand All @@ -35,9 +69,12 @@ func resourceSpannerDatabase() *schema.Resource {

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(4 * time.Minute),
Update: schema.DefaultTimeout(4 * time.Minute),
Delete: schema.DefaultTimeout(4 * time.Minute),
},

CustomizeDiff: resourceSpannerDBDdlCustomDiff,

Schema: map[string]*schema.Schema{
"instance": {
Type: schema.TypeString,
Expand All @@ -57,7 +94,6 @@ the instance is created. Values are of the form [a-z][-a-z0-9]*[a-z0-9].`,
"ddl": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Description: `An optional list of DDL statements to run inside the newly created
database. Statements can create tables, indexes, etc. These statements
execute atomically with the creation of the database: if there is an
Expand Down Expand Up @@ -231,6 +267,66 @@ func resourceSpannerDatabaseRead(d *schema.ResourceData, meta interface{}) error
return nil
}

func resourceSpannerDatabaseUpdate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

billingProject := ""

project, err := getProject(d, config)
if err != nil {
return err
}
billingProject = project

d.Partial(true)

if d.HasChange("ddl") {
obj := make(map[string]interface{})

extraStatementsProp, err := expandSpannerDatabaseDdl(d.Get("ddl"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("ddl"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, extraStatementsProp)) {
obj["extraStatements"] = extraStatementsProp
}

obj, err = resourceSpannerDatabaseUpdateEncoder(d, meta, obj)
if err != nil {
return err
}

url, err := replaceVars(d, config, "{{SpannerBasePath}}projects/{{project}}/instances/{{instance}}/databases/{{name}}/ddl")
if err != nil {
return err
}

// err == nil indicates that the billing_project value was found
if bp, err := getBillingProject(d, config); err == nil {
billingProject = bp
}

res, err := sendRequestWithTimeout(config, "PATCH", billingProject, url, obj, d.Timeout(schema.TimeoutUpdate))
if err != nil {
return fmt.Errorf("Error updating Database %q: %s", d.Id(), err)
} else {
log.Printf("[DEBUG] Finished updating Database %q: %#v", d.Id(), res)
}

err = spannerOperationWaitTime(
config, res, project, "Updating Database",
d.Timeout(schema.TimeoutUpdate))
if err != nil {
return err
}

d.SetPartial("ddl")
}

d.Partial(false)

return resourceSpannerDatabaseRead(d, meta)
}

func resourceSpannerDatabaseDelete(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

Expand Down Expand Up @@ -260,6 +356,14 @@ func resourceSpannerDatabaseDelete(d *schema.ResourceData, meta interface{}) err
return handleNotFoundError(err, d, "Database")
}

err = spannerOperationWaitTime(
config, res, project, "Deleting Database",
d.Timeout(schema.TimeoutDelete))

if err != nil {
return err
}

log.Printf("[DEBUG] Finished deleting Database %q: %#v", d.Id(), res)
return nil
}
Expand Down Expand Up @@ -326,6 +430,24 @@ func resourceSpannerDatabaseEncoder(d *schema.ResourceData, meta interface{}, ob
return obj, nil
}

func resourceSpannerDatabaseUpdateEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) {
old, new := d.GetChange("ddl")
oldDdls := old.([]interface{})
newDdls := new.([]interface{})
updateDdls := []string{}

//Only new ddl statments to be add to update call
for i := len(oldDdls); i < len(newDdls); i++ {
updateDdls = append(updateDdls, newDdls[i].(string))
}

obj["statements"] = strings.Join(updateDdls, ",")
delete(obj, "name")
delete(obj, "instance")
delete(obj, "extraStatements")
return obj, nil
}

func resourceSpannerDatabaseDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) {
config := meta.(*Config)
d.SetId(res["name"].(string))
Expand Down
150 changes: 135 additions & 15 deletions google/resource_spanner_database_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,44 @@ func TestAccSpannerDatabase_basic(t *testing.T) {
},
{
// Test import with default Terraform ID
ResourceName: "google_spanner_database.basic",
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_spanner_database.basic",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"ddl"},
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
Config: testAccSpannerDatabase_basicUpdate(instanceName, databaseName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"),
),
},
{
// Test import with default Terraform ID
ResourceName: "google_spanner_database.basic",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"ddl"},
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"ddl"},
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("%s/%s", instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"ddl"},
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("%s/%s", instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"ddl"},
},
},
})
Expand All @@ -66,6 +83,31 @@ resource "google_spanner_instance" "basic" {
resource "google_spanner_database" "basic" {
instance = google_spanner_instance.basic.name
name = "%s"
ddl = [
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
]
}
`, instanceName, instanceName, databaseName)
}

func testAccSpannerDatabase_basicUpdate(instanceName, databaseName string) string {
return fmt.Sprintf(`
resource "google_spanner_instance" "basic" {
name = "%s"
config = "regional-us-central1"
display_name = "display-%s"
num_nodes = 1
}
resource "google_spanner_database" "basic" {
instance = google_spanner_instance.basic.name
name = "%s"
ddl = [
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
"CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)",
]
}
`, instanceName, instanceName, databaseName)
}
Expand All @@ -81,3 +123,81 @@ func TestDatabaseNameForApi(t *testing.T) {
expected := "projects/project123/instances/instance456/databases/db789"
expectEquals(t, expected, actual)
}

// Unit Tests for ForceNew when the change in ddl
func TestSpannerDatabase_resourceSpannerDBDdlCustomDiffFuncForceNew(t *testing.T) {
t.Parallel()

cases := map[string]struct {
before interface{}
after interface{}
forcenew bool
}{
"remove_old_statements": {
before: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"},
after: []interface{}{
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)"},
forcenew: true,
},
"append_new_statements": {
before: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"},
after: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
},
forcenew: false,
},
"no_change": {
before: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"},
after: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"},
forcenew: false,
},
"order_of_statments_change": {
before: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
"CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)",
},
after: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
},
forcenew: true,
},
"missing_an_old_statement": {
before: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
"CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)",
},
after: []interface{}{
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)",
},
forcenew: true,
},
}

for tn, tc := range cases {
d := &ResourceDiffMock{
Before: map[string]interface{}{
"ddl": tc.before,
},
After: map[string]interface{}{
"ddl": tc.after,
},
}
err := resourceSpannerDBDdlCustomDiffFunc(d)
if err != nil {
t.Errorf("failed, expected no error but received - %s for the condition %s", err, tn)
}
if d.IsForceNew != tc.forcenew {
t.Errorf("ForceNew not setup correctly for the condition-'%s', expected:%v;actual:%v", tn, tc.forcenew, d.IsForceNew)
}
}
}
12 changes: 9 additions & 3 deletions google/test_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ func (d *ResourceDataMock) Id() string {
}

type ResourceDiffMock struct {
Before map[string]interface{}
After map[string]interface{}
Cleared map[string]struct{}
Before map[string]interface{}
After map[string]interface{}
Cleared map[string]struct{}
IsForceNew bool
}

func (d *ResourceDiffMock) GetChange(key string) (interface{}, interface{}) {
Expand All @@ -83,6 +84,11 @@ func (d *ResourceDiffMock) Clear(key string) error {
return nil
}

func (d *ResourceDiffMock) ForceNew(key string) error {
d.IsForceNew = true
return nil
}

func checkDataSourceStateMatchesResourceState(dataSourceName, resourceName string) func(*terraform.State) error {
return checkDataSourceStateMatchesResourceStateWithIgnores(dataSourceName, resourceName, map[string]struct{}{})
}
Expand Down
1 change: 1 addition & 0 deletions google/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type TerraformResourceDiff interface {
GetChange(string) (interface{}, interface{})
Get(string) interface{}
Clear(string) error
ForceNew(string) error
}

// getRegionFromZone returns the region from a zone for Google cloud.
Expand Down
1 change: 1 addition & 0 deletions website/docs/r/spanner_database.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ This resource provides the following
[Timeouts](/docs/configuration/resources.html#timeouts) configuration options:

- `create` - Default is 4 minutes.
- `update` - Default is 4 minutes.
- `delete` - Default is 4 minutes.

## Import
Expand Down

0 comments on commit ca6062f

Please sign in to comment.