-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
r/aws_route_table_association: Wait for 40 NotFoundChecks #21062
r/aws_route_table_association: Wait for 40 NotFoundChecks #21062
Conversation
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.
Welcome @ryancragun 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
Signed-off-by: Ryan Cragun <me@ryan.ec>
Signed-off-by: Ryan Cragun <me@ryan.ec>
3df3439
to
ea669b3
Compare
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.
LGTM 🚀.
Commercial
% make testacc TESTARGS='-run=TestAccAWSRouteTableAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRouteTableAssociation_ -timeout 180m
=== RUN TestAccAWSRouteTableAssociation_Subnet_basic
=== PAUSE TestAccAWSRouteTableAssociation_Subnet_basic
=== RUN TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable
=== PAUSE TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable
=== RUN TestAccAWSRouteTableAssociation_Gateway_basic
=== PAUSE TestAccAWSRouteTableAssociation_Gateway_basic
=== RUN TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable
=== PAUSE TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable
=== RUN TestAccAWSRouteTableAssociation_disappears
=== PAUSE TestAccAWSRouteTableAssociation_disappears
=== CONT TestAccAWSRouteTableAssociation_Subnet_basic
=== CONT TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable
=== CONT TestAccAWSRouteTableAssociation_Gateway_basic
=== CONT TestAccAWSRouteTableAssociation_disappears
=== CONT TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable
--- PASS: TestAccAWSRouteTableAssociation_disappears (46.16s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_basic (48.98s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable (71.13s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_basic (80.13s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable (103.93s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 108.328s
GovCloud
% make testacc TESTARGS='-run=TestAccAWSRouteTableAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRouteTableAssociation_ -timeout 180m
=== RUN TestAccAWSRouteTableAssociation_Subnet_basic
=== PAUSE TestAccAWSRouteTableAssociation_Subnet_basic
=== RUN TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable
=== PAUSE TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable
=== RUN TestAccAWSRouteTableAssociation_Gateway_basic
=== PAUSE TestAccAWSRouteTableAssociation_Gateway_basic
=== RUN TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable
=== PAUSE TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable
=== RUN TestAccAWSRouteTableAssociation_disappears
=== PAUSE TestAccAWSRouteTableAssociation_disappears
=== CONT TestAccAWSRouteTableAssociation_Subnet_basic
=== CONT TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable
=== CONT TestAccAWSRouteTableAssociation_disappears
=== CONT TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable
=== CONT TestAccAWSRouteTableAssociation_Gateway_basic
--- PASS: TestAccAWSRouteTableAssociation_disappears (47.91s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_basic (51.82s)
--- PASS: TestAccAWSRouteTableAssociation_Subnet_ChangeRouteTable (73.77s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_basic (84.15s)
--- PASS: TestAccAWSRouteTableAssociation_Gateway_ChangeRouteTable (106.92s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 112.590s
@ryancragun Thanks for the contribution 🎉 👏. |
This functionality has been released in v3.61.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. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
We have many automated tests that create route table associations and have witnessed extreme variance in the duration when waiting for the
aws_route_table_association
resource to be found after it is initially created. Most of the time, theaws_route_table_association
resource finishes in about 1 second, while in others it'll retry until we hit the default 20MaxNotFound
checks in theStateChangeConf
waiter, which causes the resource to always fail around the 2.5 minute mark when this condition is exhibited. This change increases the maxNotFoundChecks
for theRouteTableAssociationCreated
StateChangeConf
waiter to 40, which gets us closer to the max default timeout of 5 minutes. After having built my own copy of the provider plugin with this change we no longer see this issue.Before:
After:
I didn't include a modified acceptance test because the issue itself is quite sporadic and we cannot control the conditions of the AWS Ec2 API to sufficiently test the change. Looking further into the waiter and finder themselves, it would appear that testing at that level would require building an
ec2.EC2
connection mock, which in turn means that we'd really only be testing theStateChangeConf
implementation, which has nothing to do with our changes.Community Note
Output from acceptance testing:
Note that ACCTEST_PARALLELISM is at 2 because of the maximum allowed internet gateways in my ec2 account. With the default of 20 we'd run into that limit.