Skip to content
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

Replace DyanmoDB table when local secondary index changes (#12334) #12335

Conversation

tjoris
Copy link
Contributor

@tjoris tjoris commented Mar 10, 2020

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #12334

Release note for CHANGELOG:

Changing the name, range_key, projection_type or non_key_attributes of a local_secondary_index of an aws_dynamodb_table forces a replace

Output from acceptance testing:

Without fix

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSDynamoDbTable_lsiUpdate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDynamoDbTable_lsiUpdate -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_lsiUpdate
=== PAUSE TestAccAWSDynamoDbTable_lsiUpdate
=== CONT  TestAccAWSDynamoDbTable_lsiUpdate
    TestAccAWSDynamoDbTable_lsiUpdate: testing.go:654: Step 2 error: After applying this step and refreshing, the plan was not empty:

        DIFF:

        UPDATE: aws_dynamodb_table.test
          arn:                                          "arn:aws:dynamodb:us-west-2:067346078311:table/TerraformTestTable--7952334412318672221" => "arn:aws:dynamodb:us-west-2:067346078311:table/TerraformTestTable--7952334412318672221"
          attribute.#:                                  "3" => "3"
          attribute.0.name:                             "staticHashKey" => "staticHashKey"
          attribute.0.type:                             "S" => "S"
          attribute.1.name:                             "staticLSIRangeKey" => "staticLSIRangeKey"
          attribute.1.type:                             "S" => "S"
          attribute.2.name:                             "staticRangeKey" => "staticRangeKey"
          attribute.2.type:                             "S" => "S"
          billing_mode:                                 "PROVISIONED" => "PROVISIONED"
          global_secondary_index.#:                     "0" => "0"
          hash_key:                                     "staticHashKey" => "staticHashKey"
          id:                                           "TerraformTestTable--7952334412318672221" => "TerraformTestTable--7952334412318672221"
          local_secondary_index.#:                      "1" => "1"
          local_secondary_index.0.name:                 "lsi-original" => "lsi-changed"
          local_secondary_index.0.non_key_attributes.#: "0" => "0"
          local_secondary_index.0.projection_type:      "KEYS_ONLY" => "KEYS_ONLY"
          local_secondary_index.0.range_key:            "staticLSIRangeKey" => "staticLSIRangeKey"
          name:                                         "TerraformTestTable--7952334412318672221" => "TerraformTestTable--7952334412318672221"
          point_in_time_recovery.#:                     "1" => "1"
          point_in_time_recovery.0.enabled:             "false" => "false"
          range_key:                                    "staticRangeKey" => "staticRangeKey"
          read_capacity:                                "10" => "10"
          server_side_encryption.#:                     "0" => "0"
          stream_arn:                                   "" => ""
          stream_enabled:                               "false" => "false"
          stream_label:                                 "" => ""
          stream_view_type:                             "" => ""
          ttl.#:                                        "1" => "1"
          ttl.0.attribute_name:                         "" => ""
          ttl.0.enabled:                                "false" => "false"
          write_capacity:                               "10" => "10"



        STATE:

        aws_dynamodb_table.test:
          ID = TerraformTestTable--7952334412318672221
          provider = provider.aws
          arn = arn:aws:dynamodb:us-west-2:067346078311:table/TerraformTestTable--7952334412318672221
          attribute.# = 3
          attribute.0.name = staticHashKey
          attribute.0.type = S
          attribute.1.name = staticLSIRangeKey
          attribute.1.type = S
          attribute.2.name = staticRangeKey
          attribute.2.type = S
          billing_mode = PROVISIONED
          hash_key = staticHashKey
          local_secondary_index.# = 1
          local_secondary_index.0.name = lsi-original
          local_secondary_index.0.projection_type = KEYS_ONLY
          local_secondary_index.0.range_key = staticLSIRangeKey
          name = TerraformTestTable--7952334412318672221
          point_in_time_recovery.# = 1
          point_in_time_recovery.0.enabled = false
          range_key = staticRangeKey
          read_capacity = 10
          stream_arn =
          stream_enabled = false
          stream_label =
          stream_view_type =
          ttl.# = 1
          ttl.0.attribute_name =
          ttl.0.enabled = false
          write_capacity = 10
--- FAIL: TestAccAWSDynamoDbTable_lsiUpdate (79.60s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       79.823s
FAIL
GNUmakefile:26: recipe for target 'testacc' failed
make: *** [testacc] Error 1

With fix

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSDynamoDbTable_lsiUpdate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDynamoDbTable_lsiUpdate -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_lsiUpdate
=== PAUSE TestAccAWSDynamoDbTable_lsiUpdate
=== CONT  TestAccAWSDynamoDbTable_lsiUpdate
--- PASS: TestAccAWSDynamoDbTable_lsiUpdate (87.18s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       87.512s

@tjoris tjoris requested a review from a team March 10, 2020 16:16
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/dynamodb Issues and PRs that pertain to the dynamodb service. labels Mar 10, 2020
@tjoris tjoris force-pushed the replace-dynamo-table-for-lsi-change branch from a5d4e76 to 419f09c Compare March 11, 2020 14:21
@ghost ghost added size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed size/XS Managed by automation to categorize the size of a PR. labels Mar 11, 2020
@tjoris tjoris force-pushed the replace-dynamo-table-for-lsi-change branch from 419f09c to e3d8a7c Compare March 11, 2020 14:34
@tjoris tjoris changed the title [WIP] Replace DyanmoDB table when local secondary index changes (#12334) Replace DyanmoDB table when local secondary index changes (#12334) Mar 12, 2020
@tjoris
Copy link
Contributor Author

tjoris commented Mar 12, 2020

What can I do to help progress this Pull Request?

@tjoris tjoris force-pushed the replace-dynamo-table-for-lsi-change branch from e3d8a7c to 863e24d Compare March 12, 2020 12:32
@bflad bflad added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 18, 2020
@anGie44 anGie44 self-assigned this Sep 18, 2020
Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks @tjoris for this fix! 🚀

Output of acceptance tests:

--- PASS: TestDiffDynamoDbGSI (0.01s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecificationValidation (17.56s)
--- PASS: TestAccAWSDynamoDbTable_disappears (48.00s)
--- PASS: TestAccAWSDynamoDbTableItem_withMultipleItems (67.15s)
--- PASS: TestAccAWSDynamoDbTableItem_basic (69.77s)
--- PASS: TestAccAWSDynamoDbTableItem_rangeKey (70.26s)
--- PASS: TestAccAWSDynamoDbTable_basic (72.30s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (31.12s)
--- PASS: TestAccAWSDynamoDbTable_tags (104.07s)
--- PASS: TestAccAWSDynamoDbTableItem_updateWithRangeKey (117.57s)
--- PASS: TestAccAWSDynamoDbTableItem_update (118.49s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Enabled (77.46s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes_emptyPlan (114.12s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_PayPerRequestToProvisioned (133.48s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (140.54s)
--- PASS: TestAccAWSDynamoDbTable_enablePitr (142.17s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (146.29s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_PayPerRequestToProvisioned (145.92s)
--- PASS: TestAccAWSDynamoDbTable_disappears_PayPerRequestWithGSI (151.94s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Disabled (90.00s)
--- PASS: TestAccAWSDynamoDbTable_lsiUpdate (93.40s)
--- PASS: TestAccAWSDynamoDbTable_encryption (148.55s)
--- PASS: TestAccAWSDynamoDbTable_extended (329.25s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (330.08s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (660.61s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (593.16s)
--- PASS: TestAccAWSDynamoDbTable_Replica_Single (594.71s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_ProvisionedToPayPerRequest (734.38s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_ProvisionedToPayPerRequest (869.16s)
--- PASS: TestAccAWSDynamoDbTable_Replica_Multiple (1583.47s)

@anGie44 anGie44 added this to the v3.8.0 milestone Sep 18, 2020
@anGie44 anGie44 merged commit 7670a78 into hashicorp:master Sep 18, 2020
anGie44 added a commit that referenced this pull request Sep 18, 2020
@ghost
Copy link

ghost commented Sep 24, 2020

This has been released in version 3.8.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!

@ghost
Copy link

ghost commented Oct 19, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/dynamodb Issues and PRs that pertain to the dynamodb service. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DynamoDB table not replaced when changing name or range_key of local_secondary_index
3 participants