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

Add feature to concurrently update/delete AWS API Gateway method settings #13497

Conversation

saurabh-hirani
Copy link
Contributor

This PR fixes the error encountered while updating/deleting aws_api_gateway_method_settings resources concurrently. Right now we get the following error if parallelism is > 1 and we try to update/delete > 1 method_settings

Error: Updating API Gateway Stage failed: ConflictException: Unable to complete operation due to concurrent modification. Please try again later.

This issue was highlighted in a comment - #483 (comment) - which suggested using daisy chaining to resolve it. But when using for_each we cannot use daisy chaining as per this discussion - https://discuss.hashicorp.com/t/is-there-a-way-to-daisy-chain-depends-on-for-resources-created-using-count-or-for-each/8467

There is no other workaround except for using parallelism value 1 which slows down the entire deployment as API Gateway is just one part of many resources created/updated.

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

Relates #483

Release note for CHANGELOG:

NONE

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAPIGatewayRestApi'

=== RUN   TestAccAWSAPIGatewayRestApi_basic
=== PAUSE TestAccAWSAPIGatewayRestApi_basic
=== RUN   TestAccAWSAPIGatewayRestApi_tags
=== PAUSE TestAccAWSAPIGatewayRestApi_tags
=== RUN   TestAccAWSAPIGatewayRestApi_disappears
=== PAUSE TestAccAWSAPIGatewayRestApi_disappears
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration_VPCEndpoint
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration_VPCEndpoint
=== RUN   TestAccAWSAPIGatewayRestApi_api_key_source
=== PAUSE TestAccAWSAPIGatewayRestApi_api_key_source
=== RUN   TestAccAWSAPIGatewayRestApi_policy
=== PAUSE TestAccAWSAPIGatewayRestApi_policy
=== RUN   TestAccAWSAPIGatewayRestApi_openapi
=== PAUSE TestAccAWSAPIGatewayRestApi_openapi
=== CONT  TestAccAWSAPIGatewayRestApi_basic
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration_VPCEndpoint
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== CONT  TestAccAWSAPIGatewayRestApi_openapi
=== CONT  TestAccAWSAPIGatewayRestApi_disappears
=== CONT  TestAccAWSAPIGatewayRestApi_api_key_source
=== CONT  TestAccAWSAPIGatewayRestApi_policy
=== CONT  TestAccAWSAPIGatewayRestApi_tags
--- PASS: TestAccAWSAPIGatewayRestApi_disappears (58.28s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private (78.31s)
--- PASS: TestAccAWSAPIGatewayRestApi_policy (114.82s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration (134.65s)
--- PASS: TestAccAWSAPIGatewayRestApi_api_key_source (158.21s)
--- PASS: TestAccAWSAPIGatewayRestApi_basic (195.04s)

@saurabh-hirani saurabh-hirani requested a review from a team May 26, 2020 14:26
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/apigateway Issues and PRs that pertain to the apigateway service. needs-triage Waiting for first response or review from a maintainer. labels May 26, 2020
@saurabh-hirani
Copy link
Contributor Author

Can someone please review and provide feedback? Doing parallelism=1 workaround leads to unwanted delay in infra deployments.

@bflad bflad self-assigned this Jan 13, 2021
@bflad bflad added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 13, 2021
@bflad
Copy link
Contributor

bflad commented Jan 13, 2021

Hi folks 👋 After creating some additional testing for this resource that included concurrent aws_api_gateway_methods_settings resources, I was able to reproduce what appears to be the problem here. To speed things up in the future for the maintainers and get bug fixes in, it is helpful to ensure there is an open GitHub issue, per Terraform resource, that describes the problem including a reproducing configuration and typically debug logging.

In this case, we'll be taking a slightly different approach instead of a mutex where we introduce the retry logic directly into the AWS Go SDK service client since it will yield the simplest fix. Thank you @saurabh-hirani for contributing this and the service client implementation will be refactored on top of this commit along with the covering testing. 👍

@bflad bflad added this to the v3.24.0 milestone Jan 13, 2021
@bflad bflad merged commit 26f4b5f into hashicorp:master Jan 13, 2021
bflad added a commit that referenced this pull request Jan 13, 2021
@saurabh-hirani
Copy link
Contributor Author

Thanks @bflad 👍

@ghost
Copy link

ghost commented Jan 15, 2021

This has been released in version 3.24.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 Feb 12, 2021

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 Feb 12, 2021
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/apigateway Issues and PRs that pertain to the apigateway service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants