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

Re-enable ec2_vpc_vpn_facts test #58893

Merged
merged 1 commit into from
Jul 18, 2019
Merged

Conversation

jillr
Copy link
Contributor

@jillr jillr commented Jul 10, 2019

Unable to reproduce failure locally, let's see if shippable still fails

SUMMARY

Test re-enabling ec2_vpc_vpn_facts test.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_vpc_vpn_facts integration test

ADDITIONAL INFORMATION

Unable to reproduce timeout failures locally in us-east-1 or us-west-2 though nothing has changed in the test or the module. ¯_(ツ)_/¯

Unable to reproduce failure locally, let's see if shippable still fails
@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Jul 10, 2019
@jillr
Copy link
Contributor Author

jillr commented Jul 16, 2019

@s-hertel @mattclay I've been unable to get these tests to produce any failure locally or in shippable. What do you think about re-enabling them?

@mattclay
Copy link
Member

@jillr Go ahead and enable them. If they start failing again we can disable and investigate further.

@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Jul 18, 2019
@jillr jillr changed the title [WIP] Re-enable ec2_vpc_vpn_facts test Re-enable ec2_vpc_vpn_facts test Jul 18, 2019
@jillr jillr merged commit 1c1da3c into ansible:devel Jul 18, 2019
@jillr
Copy link
Contributor Author

jillr commented Jul 18, 2019

Fixes: #53185

@s-hertel
Copy link
Contributor

@jillr 👍

Actually closes: #49751. I think the errors in #53185 are a subset of the ones for #49751, but the ec2_vpc_vgw tests themselves are still disabled. If you'd like to re-enable those too, feel free. If you add Fixes #... in your description or commit msg it will close the issue.

Just some background on unstable tests since these are tricky and sporadic and painstaking to actually solve - As you noted, the module code and tests can be unchanged and suddenly work great (or conversely, fail all over the place). In most cases the reason the tests are unstable is because many AWS services (like EC2, IAM, and S3) use an eventual consistency model. Because of this the failures can be pretty unpredictable (I'd loop tests to run 50 times and do that several times when trying to fix these bugs to address all the different possible places for failure). AWS seems to occasionally make changes to the model which have caused many widespread failures in tests that have never had a problem before. ec2_vpc_vgw was one of these - it has been disabled a couple times now through no fault of Ansible. Even if the tests don't fail when you run them x times, adding AWSRetry and waiters between API calls made by the module are good preventative measures and make the module more resilient against AWS making changes to their distributed system that supports the API.

@jillr
Copy link
Contributor Author

jillr commented Jul 19, 2019

@s-hertel ok cool, that's super useful info - thanks!

@ansible ansible locked and limited conversation to collaborators Aug 15, 2019
@jillr jillr deleted the reenable_ec2_vpc_vpn_facts branch December 4, 2019 22:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. support:community This issue/PR relates to code supported by the Ansible community. WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants