New Resource: aws_dynamodb_global_table #2517

bflad · 2017-12-02T21:39:33Z

Sorry for the additional changes inside aws_dynamodb_table, however I was receiving these errors and decided to clean up the deletion logic a little.

# Consistently after first implementing global table deletion

        * aws_dynamodb_table.us-east-1 (destroy): 1 error(s) occurred:

        * aws_dynamodb_table.us-east-1: ResourceInUseException: Attempt to change a resource which is still in use: Table is being deleted: tf-acc-test-k9l0s
            status code: 400, request id: E45O0ASHK3HNUP8KMU11L75D3FVV4KQNSO5AEMVJF66Q9ASUAAJG

# Consistently after fixing the above

        * aws_dynamodb_table.us-east-1 (destroy): 1 error(s) occurred:

        * aws_dynamodb_table.us-east-1: Error waiting for Dynamo DB Table update: ResourceNotFoundException: Requested resource not found: Table: tf-acc-test-khj7g not found
            status code: 400, request id: RRD7G8M109TJFBH3F8L6JFMHKRVV4KQNSO5AEMVJF66Q9ASUAAJG

Passes testing for me:

make testacc TEST=./aws TESTARGS='-run=TestAccAwsDynamoDbGlobalTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsDynamoDbGlobalTable -timeout 120m
=== RUN   TestAccAwsDynamoDbGlobalTable_basic
--- PASS: TestAccAwsDynamoDbGlobalTable_basic (92.97s)
=== RUN   TestAccAwsDynamoDbGlobalTable_import
--- PASS: TestAccAwsDynamoDbGlobalTable_import (55.70s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	148.715s

make testacc TEST=./aws TESTARGS='-run=TestAccAWSDynamoDbTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSDynamoDbTable -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_importBasic
--- PASS: TestAccAWSDynamoDbTable_importBasic (71.61s)
=== RUN   TestAccAWSDynamoDbTable_importTags
--- PASS: TestAccAWSDynamoDbTable_importTags (76.10s)
=== RUN   TestAccAWSDynamoDbTable_basic
--- PASS: TestAccAWSDynamoDbTable_basic (287.11s)
=== RUN   TestAccAWSDynamoDbTable_streamSpecification
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (72.92s)
=== RUN   TestAccAWSDynamoDbTable_tags
--- PASS: TestAccAWSDynamoDbTable_tags (69.01s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdate
--- PASS: TestAccAWSDynamoDbTable_gsiUpdate (182.58s)
=== RUN   TestAccAWSDynamoDbTable_ttl
--- PASS: TestAccAWSDynamoDbTable_ttl (91.00s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	850.376s

Ninir

Hi @bflad

Thanks a lot for the awesome & clean work, really appreciated! :)

Just left a few comments to address, mostly nitpicks & comments to better understand :)

$ make testacc TEST=./aws TESTARGS='-run=TestAccAwsDynamoDbGlobalTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsDynamoDbGlobalTable -timeout 120m
=== RUN   TestAccAwsDynamoDbGlobalTable_basic
--- PASS: TestAccAwsDynamoDbGlobalTable_basic (144.88s)
=== RUN   TestAccAwsDynamoDbGlobalTable_import
--- PASS: TestAccAwsDynamoDbGlobalTable_import (51.27s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	196.195s

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSDynamoDbTable'      
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSDynamoDbTable -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_importBasic
--- PASS: TestAccAWSDynamoDbTable_importBasic (105.07s)
=== RUN   TestAccAWSDynamoDbTable_importTags
--- PASS: TestAccAWSDynamoDbTable_importTags (62.06s)
=== RUN   TestAccAWSDynamoDbTable_basic
--- PASS: TestAccAWSDynamoDbTable_basic (346.33s)
=== RUN   TestAccAWSDynamoDbTable_streamSpecification
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (55.17s)
=== RUN   TestAccAWSDynamoDbTable_tags
--- PASS: TestAccAWSDynamoDbTable_tags (55.60s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdate
--- PASS: TestAccAWSDynamoDbTable_gsiUpdate (134.70s)
=== RUN   TestAccAWSDynamoDbTable_ttl
--- PASS: TestAccAWSDynamoDbTable_ttl (65.53s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	824.510s

Ninir · 2017-12-07T21:28:04Z

aws/resource_aws_dynamodb_global_table.go

+		Pending: []string{
+			dynamodb.GlobalTableStatusCreating,
+			dynamodb.GlobalTableStatusDeleting,
+			dynamodb.GlobalTableStatusUpdating,


👍 for constants usage

Ninir · 2017-12-07T21:34:41Z

aws/resource_aws_dynamodb_global_table.go

+						},
+					},
+				},
+				Set: resourceAwsDynamoDbGlobalTableReplicaHash,


The logic managing hashes in the background already takes care of managing hashes so this shouldn't be needed

I was prematurely optimizing here for if/when Amazon adds other attributes for replicas since region_name is what will always trigger the create/remove replica calls. I'll remove the currently extraneous configuration.

Actually, I think this is required. 😄 We cannot use schema.HashString here since this is a map element, not a set of just string elements.

panic: interface conversion: interface {} is map[string]interface {}, not string goroutine 693 [running]: github.com/terraform-providers/terraform-provider-aws/vendor/github.com/hashicorp/terraform/helper/schema.HashString(0x31eaac0, 0xc420576210, 0xc4205b4ef8)

Ninir · 2017-12-07T21:39:29Z

website/docs/r/dynamodb_global_table.html.markdown

+}
+
+resource "aws_dynamodb_global_table" "myTable" {
+	depends_on = ["aws_dynamodb_table.us-east-1", "aws_dynamodb_table.us-west-2"]


Can you fix indentations?

Ninir · 2017-12-08T12:41:05Z

aws/resource_aws_dynamodb_global_table.go

+	if err != nil {
+		return err
+	}
+	if globalTableDescription == nil {


We should log log.Printf("[WARN] DynamoDB Global Table %q not found, removing from state", dashboardName)

I will add this, thanks!

Ninir · 2017-12-08T12:42:07Z

aws/resource_aws_dynamodb_global_table.go

+	stateConf := &resource.StateChangeConf{
+		Pending: []string{
+			dynamodb.GlobalTableStatusCreating,
+			dynamodb.GlobalTableStatusDeleting,


Do we need to handle this status when creating a table (and the updating one also)?

I added this as edge-case protection if we miss ACTIVE between check intervals for an immediate update or something like deletion happens outside Terraform during the creation.

When StateChangeConf sees an unexpected state, it will complain with an error that seems less friendly than our refresh function's error: Error on retrieving DynamoDB Global Table when waiting or hitting a timeout.

Ninir · 2017-12-08T12:42:18Z

aws/resource_aws_dynamodb_global_table.go

+		stateConf := &resource.StateChangeConf{
+			Pending: []string{
+				dynamodb.GlobalTableStatusCreating,
+				dynamodb.GlobalTableStatusDeleting,


Do we need to handle this status or the one above when updating?

Same logic as mentioned before, I added this purely for friendlier edge-case messaging. Maybe we could drop these, but leaving them could still handle the extremely weird case someone manages to delete and recreate a global table while updating (e.g. tainting one of two overlapping resources then Terraform running both in parallel).

Ninir · 2017-12-08T13:19:01Z

aws/resource_aws_dynamodb_global_table.go

+		return nil
+	}
+
+	return flattenAwsDynamoDbGlobalTable(d, globalTableDescription)


Is there a reason to have this as a separated function? I think it would be more consistent with other resources to keep setting attributes in the read function. What do you think?

This is so we can reuse this code support a aws_dynamodb_global_table data source in the future, if we would like.

Ninir · 2017-12-08T13:19:58Z

aws/resource_aws_dynamodb_global_table_test.go

+%s
+
+resource "aws_dynamodb_global_table" "test" {
+	depends_on = ["aws_dynamodb_table.us-east-1"]


Can you fix indentations?

Sorry about that! VS Code thinks I want to continue with tabs to match the rest of the file instead of noticing its inside backticks 🙁

Ninir · 2017-12-08T13:20:39Z

aws/resource_aws_dynamodb_table.go

+	stateConf := &resource.StateChangeConf{
+		Pending: []string{
+			dynamodb.TableStatusActive,
+			dynamodb.TableStatusCreating,


Could this be a pending state when destroying?

Same logic about edge-cases as mentioned before. Let me know if you think I'm overthinking this.

Ninir · 2017-12-08T13:23:33Z

aws/resource_aws_dynamodb_global_table.go

+			"arn": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},


Do you think it is worth having ReplicationGroup as an attribute also?

I felt the extra attribute nesting to strictly follow the API would've made the resource needlessly complex here and opted for the Set of replica. If you feel like it should be nested under a replication_group, please let me know and I'll add that in.

* Fix using schema.TimeoutCreate on create * WARN messaging on read when not found and removing from state * Fix testing indentations

bflad · 2017-12-09T05:12:16Z

PR is updated with:

Fix using schema.TimeoutCreate on create
WARN messaging on read when not found and removing from state
Fix testing indentations

Let me know how you would like to handle the Set hashing/StateChangeConf given my feedback. Thanks!

make testacc TEST=./aws TESTARGS='-run=TestAccAwsDynamoDbGlobalTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsDynamoDbGlobalTable -timeout 120m
=== RUN   TestAccAwsDynamoDbGlobalTable_basic
--- PASS: TestAccAwsDynamoDbGlobalTable_basic (76.49s)
=== RUN   TestAccAwsDynamoDbGlobalTable_import
--- PASS: TestAccAwsDynamoDbGlobalTable_import (60.13s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	136.652s

CumpsD · 2017-12-25T20:44:45Z

Thanks @bflad for your work on this! Looking forward to the evolution of this PR into a release 👍

unixengineer · 2018-01-10T00:55:55Z

Would it be possible to know when this PR would be into the release

grayaii · 2018-01-22T14:34:05Z

I just wanted to say that I compiled the for darwin and linux and I have already started using this for our environments. No issues at all so far! The documentation is spot on. Thank you for doing the work to make this a reality!

catsby · 2018-01-24T22:44:25Z

@bflad / @Ninir is this ready for another review?

bflad · 2018-01-25T01:21:38Z

@catsby yes please!

…ant usage

… set

bflad · 2018-01-25T15:56:03Z

@Ninir per your request I have removed the custom Set hashing 👍

make testacc TEST=./aws TESTARGS='-run=TestAccAwsDynamoDbGlobalTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsDynamoDbGlobalTable -timeout 120m
=== RUN   TestAccAwsDynamoDbGlobalTable_basic
--- PASS: TestAccAwsDynamoDbGlobalTable_basic (77.41s)
=== RUN   TestAccAwsDynamoDbGlobalTable_import
--- PASS: TestAccAwsDynamoDbGlobalTable_import (54.38s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	131.844s

Ninir

Hey @bflad

Looks very good to me, thanks! 🤖 🚀

$ make testacc TEST=./aws TESTARGS='-run=\(TestAccAwsDynamoDbGlobalTable\|TestAccAWSDynamoDbTable\)'            
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=\(TestAccAwsDynamoDbGlobalTable\|TestAccAWSDynamoDbTable\) -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_importBasic
--- PASS: TestAccAWSDynamoDbTable_importBasic (73.49s)
=== RUN   TestAccAWSDynamoDbTable_importTags
--- PASS: TestAccAWSDynamoDbTable_importTags (91.57s)
=== RUN   TestAccAwsDynamoDbGlobalTable_basic
--- PASS: TestAccAwsDynamoDbGlobalTable_basic (128.93s)
=== RUN   TestAccAwsDynamoDbGlobalTable_import
--- PASS: TestAccAwsDynamoDbGlobalTable_import (87.83s)
=== RUN   TestAccAWSDynamoDbTable_basic
--- PASS: TestAccAWSDynamoDbTable_basic (265.44s)
=== RUN   TestAccAWSDynamoDbTable_streamSpecification
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (75.28s)
=== RUN   TestAccAWSDynamoDbTable_tags
--- PASS: TestAccAWSDynamoDbTable_tags (104.93s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdate
--- PASS: TestAccAWSDynamoDbTable_gsiUpdate (184.11s)
=== RUN   TestAccAWSDynamoDbTable_ttl
--- PASS: TestAccAWSDynamoDbTable_ttl (89.09s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1100.729s

bflad · 2018-01-25T17:35:13Z

Hi, everyone! This has landed in master and will ship with the v1.8.0 release of the provider, expected tomorrow. Happy Terraform'ing! 🎉

eric-young-work · 2018-01-25T23:40:40Z

Thanks, this is great! Sorry if newbish question: will corresponding documentation get released at the same time as the feature release?

grayaii · 2018-01-26T00:59:17Z

@eric-young-work Yes, the PR contains the documentation. I used it myself to create my global tables and it works like a charm. If you click the "Files changed" at the link, you'll see the documentation changes.

eric-young-work · 2018-01-26T21:01:25Z

Great, thanks very much!

bflad · 2018-01-29T19:51:17Z

This has been released in terraform-provider-aws version 1.8.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-08T17:30:45Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

New Resouce: aws_dynamodb_global_table

d94d1d2

This was referenced Dec 2, 2017

Add DynamoDB Global Table Support #2482

Closed

Support for DynamoDB Global Tables #2506

Closed

Ninir added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 4, 2017

radeksimko added new-resource Introduces a new resource. and removed enhancement Requests to existing resources that expand the functionality or scope. labels Dec 6, 2017

Ninir suggested changes Dec 8, 2017

View reviewed changes

Ninir added the waiting-response Maintainers are waiting on response from community or contributor. label Dec 8, 2017

r/aws_dynamodb_global_table: hashicorp#2517 review

90d65cc

* Fix using schema.TimeoutCreate on create * WARN messaging on read when not found and removing from state * Fix testing indentations

radeksimko removed the waiting-response Maintainers are waiting on response from community or contributor. label Dec 11, 2017

radeksimko requested a review from Ninir January 8, 2018 12:52

bflad changed the title ~~New Resouce: aws_dynamodb_global_table~~ New Resource: aws_dynamodb_global_table Jan 11, 2018

bflad added the service/dynamodb Issues and PRs that pertain to the dynamodb service. label Jan 11, 2018

bflad requested a review from radeksimko January 19, 2018 19:23

bflad added 2 commits January 24, 2018 21:18

resource/aws_dynamodb_global_table: Final indentation fixes and const…

4294c64

…ant usage

resource/aws_dynamodb_global_table: Remove hash function for replicas…

b734ff0

… set

Ninir approved these changes Jan 25, 2018

View reviewed changes

Ninir merged commit 61aa6fe into hashicorp:master Jan 25, 2018

Ninir added a commit that referenced this pull request Jan 25, 2018

Update CHANGELOG for #2517

c205f07

bflad added this to the v1.8.0 milestone Jan 25, 2018

bflad deleted the aws_dynamodb_global_table branch January 25, 2018 17:32

bflad mentioned this pull request Feb 13, 2018

resource/aws_dynamodb_table: Retry on deletion ResourceInUseException and refactor #3355

Merged

ghost locked and limited conversation to collaborators Apr 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_dynamodb_global_table #2517

New Resource: aws_dynamodb_global_table #2517

bflad commented Dec 2, 2017

Ninir left a comment

Ninir Dec 7, 2017

Ninir Dec 7, 2017

bflad Dec 9, 2017

bflad Dec 9, 2017

Ninir Dec 7, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

Ninir Dec 8, 2017

bflad Dec 9, 2017

bflad commented Dec 9, 2017

CumpsD commented Dec 25, 2017

unixengineer commented Jan 10, 2018

grayaii commented Jan 22, 2018

catsby commented Jan 24, 2018

bflad commented Jan 25, 2018

bflad commented Jan 25, 2018

Ninir left a comment

bflad commented Jan 25, 2018

eric-young-work commented Jan 25, 2018

grayaii commented Jan 26, 2018

eric-young-work commented Jan 26, 2018

bflad commented Jan 29, 2018

ghost commented Apr 8, 2020

New Resource: aws_dynamodb_global_table #2517

New Resource: aws_dynamodb_global_table #2517

Conversation

bflad commented Dec 2, 2017

Ninir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad commented Dec 9, 2017

CumpsD commented Dec 25, 2017

unixengineer commented Jan 10, 2018

grayaii commented Jan 22, 2018

catsby commented Jan 24, 2018

bflad commented Jan 25, 2018

bflad commented Jan 25, 2018

Ninir left a comment

Choose a reason for hiding this comment

bflad commented Jan 25, 2018

eric-young-work commented Jan 25, 2018

grayaii commented Jan 26, 2018

eric-young-work commented Jan 26, 2018

bflad commented Jan 29, 2018

ghost commented Apr 8, 2020