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

provider/aws: Support eventually consistent aws_security_group_rule #6325

Merged

Conversation

jrnt30
Copy link
Contributor

@jrnt30 jrnt30 commented Apr 25, 2016

Found that there are several instances when AWS API is busy that the SG describe will not return the appropriate set of permissions. We have observed this taking over 2 minutes in the past.

  • Refactor the IP Permission rule matching logic to be reused
  • Create now retries for up to 5 minutes to get a matching rule set appropriately
  • Minor, but added the SG ID into the error message to make it easier to track down conflicting SG rules

NOTE: There are a few things that I could use another set of eyes on or clarification around:

  • The TestAccAWSSecurityGroupRule_Egress test fails for me, but it also fails on the master branch without any of my modifications as well.
  • The initial implementation that reused the resourceAwsSecurityGroupRuleRead to lookup the item after creation does quite a bit there. I was unclear why the state file was mapped to a ipPermission with the expandIPPerm, an explicit match was found and then those same values were reset on the state file with the values from the EC2 ip permission. If this is significant I could not understand why

Fixes #6256

- Preferring slower but consistent runs when AWS API calls do not properly return the SG Rule in the list of ingress/egress rules.
- Testing has shown that several times that we had to exceed 20 attempts
before the SG was actually returned
- Adjusting to use resource.Retry
- Extract lookup method for matching ipPermissions set
@catsby
Copy link
Contributor

catsby commented May 3, 2016

Checked this out locally, looks good! All the acceptance tests passed for me, so we'll pull this in. Nice work!

@catsby catsby merged commit 7f738be into hashicorp:master May 3, 2016
@jrnt30
Copy link
Contributor Author

jrnt30 commented May 3, 2016

Great, thanks!

@cmlad
Copy link

cmlad commented May 3, 2016

Great!

bigkraig pushed a commit to ticketmaster/terraform that referenced this pull request May 5, 2016
…ashicorp#6325)

* TF-6256 - SG Rule Retry

- Preferring slower but consistent runs when AWS API calls do not properly return the SG Rule in the list of ingress/egress rules.
- Testing has shown that several times that we had to exceed 20 attempts
before the SG was actually returned

* TF-6256 - Refactor of rule lookup

- Adjusting to use resource.Retry
- Extract lookup method for matching ipPermissions set
bigkraig pushed a commit to ticketmaster/terraform that referenced this pull request May 5, 2016
…ashicorp#6325)

* TF-6256 - SG Rule Retry

- Preferring slower but consistent runs when AWS API calls do not properly return the SG Rule in the list of ingress/egress rules.
- Testing has shown that several times that we had to exceed 20 attempts
before the SG was actually returned

* TF-6256 - Refactor of rule lookup

- Adjusting to use resource.Retry
- Extract lookup method for matching ipPermissions set
xsellier pushed a commit to xsellier/terraform that referenced this pull request May 17, 2016
…ashicorp#6325)

* TF-6256 - SG Rule Retry

- Preferring slower but consistent runs when AWS API calls do not properly return the SG Rule in the list of ingress/egress rules.
- Testing has shown that several times that we had to exceed 20 attempts
before the SG was actually returned

* TF-6256 - Refactor of rule lookup

- Adjusting to use resource.Retry
- Extract lookup method for matching ipPermissions set
cristicalin pushed a commit to cristicalin/terraform that referenced this pull request May 24, 2016
…ashicorp#6325)

* TF-6256 - SG Rule Retry

- Preferring slower but consistent runs when AWS API calls do not properly return the SG Rule in the list of ingress/egress rules.
- Testing has shown that several times that we had to exceed 20 attempts
before the SG was actually returned

* TF-6256 - Refactor of rule lookup

- Adjusting to use resource.Retry
- Extract lookup method for matching ipPermissions set
@ghost
Copy link

ghost commented Apr 26, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to find matching ingress Security Group Rule
4 participants