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

aws_lb_listener: wait for listener creation #5167

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

mildred
Copy link
Contributor

@mildred mildred commented Jul 12, 2018

AWS resources can take some time to appear in listings. When we created
a listener before, it could happen that the creation succeeded but the
listing of the resource right after creation would return a resource not
found.

This can me normal on AWS where changes can take some time to propagate.
The correct behaviour in this case is to retry reading the resource
until we find it (because we know that it has been created
successfully).

We don't change the behaviour on resource reads that are not following a
creation where a resource not found is still going to remove the
resource from the tfstate.

This should fix #2456

AWS resources can take some time to appear in listings. When we created
a listener before, it could happen that the creation succeeded but the
listing of the resource right after creation would return a resource not
found.

This can me normal on AWS where changes can take some time to propagate.
The correct behaviour in this case is to retry reading the resource
until we find it (because we know that it has been created
successfully).

We don't change the behaviour on resource reads that are not following a
creation where a resource not found is still going to remove the
resource from the tfstate.

This should fix hashicorp#2456
@ghost ghost added the size/S Managed by automation to categorize the size of a PR. label Jul 12, 2018
@bflad bflad added the service/elbv2 Issues and PRs that pertain to the elbv2 service. label Jul 12, 2018
ListenerArns: []*string{aws.String(d.Id())},
}

err := resource.Retry(d.Timeout(schema.TimeoutRead), func() *resource.RetryError {
Copy link
Contributor

Choose a reason for hiding this comment

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

When dealing with AWS service eventual consistency, we prefer to hardcode 1 minute timeouts. Generally its not desired to allow that timeout to be user configurable as its a knob that is unlikely to handle the situation properly by waiting longer.

@@ -25,6 +25,10 @@ func resourceAwsLbListener() *schema.Resource {
State: schema.ImportStatePassthrough,
},

Timeouts: &schema.ResourceTimeout{
Copy link
Contributor

Choose a reason for hiding this comment

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

If we do decide to add user configurable timeouts for the resource (I would recommend against in this situation), it should be documented in the resource documentation under a ## Timeouts header.

@bflad bflad added bug Addresses a defect in current functionality. waiting-response Maintainers are waiting on response from community or contributor. labels Jul 12, 2018
@runtheops
Copy link

@mildred , many thanks for the fix, this particular bug is super nasty in my scenario, since I have tons of listeners and rules created and destroyed. Looking forward to get this merged

@ngortheone
Copy link

+1 for getting this merged. I experience this on a daily basis

bflad added a commit that referenced this pull request Oct 9, 2018
@bflad bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Oct 9, 2018
@bflad bflad added this to the v1.40.0 milestone Oct 9, 2018
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

PR feedback addressed in 1085a58 🚀

--- PASS: TestAccAWSLBListenerRule_multipleConditionThrowsError (1.79s)
--- PASS: TestAccAWSLBListener_basic (187.43s)
--- PASS: TestAccAWSLBListenerRule_fixedResponse (189.04s)
--- PASS: TestAccAWSLBListenerRule_redirect (197.55s)
--- PASS: TestAccAWSLBListener_cognito (201.62s)
--- PASS: TestAccAWSLBListener_oidc (207.87s)
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (208.18s)
--- PASS: TestAccAWSLBListenerRule_oidc (209.59s)
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (209.97s)
--- PASS: TestAccAWSLBListener_fixedResponse (210.71s)
--- PASS: TestAccAWSLBListener_https (211.47s)
--- PASS: TestAccAWSLBListener_redirect (220.32s)
--- PASS: TestAccAWSLBListenerRule_basic (225.98s)
--- PASS: TestAccAWSLBListenerRule_cognito (229.10s)
--- PASS: TestAccAWSLBListenerRule_priority (288.32s)

@bflad bflad merged commit dfc900a into hashicorp:master Oct 9, 2018
bflad added a commit that referenced this pull request Oct 9, 2018
@bflad
Copy link
Contributor

bflad commented Oct 10, 2018

This has been released in version 1.40.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link

ghost commented Apr 2, 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 and limited conversation to collaborators Apr 2, 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/elbv2 Issues and PRs that pertain to the elbv2 service. size/S Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS ALB http/https listener creation/destruction unstable and caused errors for dependencies
4 participants