-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DynamoDB V2019.11.21 #12342
DynamoDB V2019.11.21 #12342
Conversation
Have you tried importing an existing 2019 replicated table? Can you drop a sample bit of TF code as an example of the configuration? |
If the table is replicated using version 2017, I'm not sure you can use the API to convert the table to a 2019 version. Let me check more on this but I think you may have to do it in the AWS console. Is this what you mean?
Configuration for what I've implemented so far ( resource "aws_dynamodb_table" "test" {
name = "MyTestTable"
hash_key = "TestTableHashKey"
billing_mode = "PAY_PER_REQUEST"
stream_enabled = true
stream_view_type = "NEW_AND_OLD_IMAGES"
attribute {
name = "TestTableHashKey"
type = "S"
}
replica {
region_name = "us-west-1"
}
replica {
region_name = "au-southeast-2"
}
} |
No, I was more thinking about a table I had in 2017, that I moved to 2019 via the console, and now want to import that change in to my existing statefiles. Currently I have:
I'll need to remove the table definition in region 2 and global table definition, in favor of an update table definition in region 1 Now that I'm thinking about it, I may not have to import it at all....since the end state will already match the described state. I'll give it a test and see what happens. |
I'm running into issues being able to test additional attributes for Pay-Per-Request is "easy" in that I can set the When adding the app auto-scaling targets and policies I still get a validation error from AWS, What are the typical strategies for applying changes that take 24 hours to apply? Would it be reasonable to accept the PR as-is with only adding / deleting regions and not configuring read capacity overrides? |
Typically things that I expect to take a long time to run, like a table convert to PPR, I will do in the console, or via the CLI, so that TF doesn't have to sit and wait for it. |
After converting my tables to PROVISIONED, and back to PPR, my plan's are clean....not sure I can explain that. On one, I did have to do an attribute reorder on a GSI, but that was it. I've only been operating on my existing tables, I can do a couple table create/update/deletes to ensure those run smoothly. |
This is what I expected and makes me think updating |
Any update on what's needed for merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @cmonty 👋 Thank you for submitting this -- overall its off to a good start code-wise. Please see the below for some initial feedback and let us know if you have any questions or do not have time to implement the items.
Some other things to consider:
- Documentation updates will need to be made within
website/docs/r/dynamodb_table.html.markdown
to add the newreplica
configuration block and its nested argument. The documentation for the configuration block should link to the AWS documentation for V2 global tables and explicitly note that it is for version 2019.11.21 while version 2017.11.29 must be managed via theaws_dynamodb_global_table
resource. We should also consider adding something like the replica test configuration as an example to the documentation page. - Documentation updates to
website/docs/r/dynamodb_global_table.html.markdown
to denote that it is only for version 2017.11.29 global tables, linking to their AWS documentation - Potentially refactoring the suggested logic within the loops of
updateDynamoDbReplica
into thedeleteDynamoDbReplicas
/createDynamoDbReplicas
, then just calling those two functions in theupdateDynamoDbReplica
instead of duplicating the logic both places.
Thanks again.
Not yet, but I should be doing a bit more testing today. I don't think we can not apply the change assuming that people will do via CLI or console....the state should match the code, unless there is a specific |
Any chance we can get the suggested changes merged in so we can do some additional testing? |
Co-Authored-By: Brian Flad <bflad417@gmail.com>
Thanks for you feedback! I've incorporated the suggestions and will work on the remaining updates to documentation and refactoring. Sorry for the delay! |
I've just accepted the suggestions. If travis fails I can fix it up tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi again @cmonty 👋 Thanks so much for those updates. Since this was pretty close (e.g. just a lingering CI failure and a few other small things), we went ahead and took care of this. Appreciate all your hard work here!
Output from acceptance testing:
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (588.92s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (7.18s)
--- PASS: TestAccAWSDynamoDbTable_basic (30.14s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_PayPerRequestToProvisioned (72.32s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_ProvisionedToPayPerRequest (882.42s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_PayPerRequestToProvisioned (49.30s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_ProvisionedToPayPerRequest (1025.15s)
--- PASS: TestAccAWSDynamoDbTable_disappears (17.38s)
--- PASS: TestAccAWSDynamoDbTable_disappears_PayPerRequestWithGSI (69.76s)
--- PASS: TestAccAWSDynamoDbTable_enablePitr (95.50s)
--- PASS: TestAccAWSDynamoDbTable_encryption (169.91s)
--- PASS: TestAccAWSDynamoDbTable_extended (275.87s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (66.23s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (282.44s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (656.30s)
--- PASS: TestAccAWSDynamoDbTable_Replica (538.95s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (45.82s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecificationValidation (5.76s)
--- PASS: TestAccAWSDynamoDbTable_tags (40.50s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Disabled (41.32s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Enabled (28.51s)
--- PASS: TestAccDataSourceAwsDynamoDbTable_basic (44.33s)
@bflad thanks for your help and merging! |
This has been released in version 2.58.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
Trying to create a global dynamodb table, with GSI, provisioned billing mode and using autoscaling policy |
I'm having some troubles with Global tables V2 and multiple replicas. |
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! |
Attempts to add support for DynamoDB Global Table V2019.11.21.
The PR introduces a new resourceaws_dynamodb_table_2019
as initially discussed in #11096. After implementing though, it may be better to foldreplica
intoaws_dynamodb_table
as you can addreplica
to existing tables (just not existing tables already replicated with V2017).I've decided to change the initial PR to add
replica
toaws_dynamodb_table
. I think there are benefits if you have an existing DynamoDB table without any GlobalTables, being able to addreplica
to the existing resource and having it add the new version of GlobalTables. Otherwise, you'd need to recreate the resource which would require a new table name or deleting an existing one.Community Note
Closes #11096
Release note for CHANGELOG:
Output from acceptance testing: