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.

Sign up for GitHub

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

Jump to bottom

Retry LB listener methods after timeout #8630

Merged
merged 3 commits into from
May 15, 2019
Merged

Retry LB listener methods after timeout #8630

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7bd175
Retry modifying LB listener after timeout
ryndaniels May 14, 2019
a6f58f9
Add final retries to the other methods' timeouts as well
ryndaniels May 14, 2019
efa1e28
Use direct assignment for the retry methods
ryndaniels May 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions aws/resource_aws_lb_listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,14 @@ func resourceAwsLbListenerCreate(d *schema.ResourceData, meta interface{}) error
return nil
})

if isResourceTimeoutError(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When the error is a resource.TimeoutError and we hit this condition, its important that we reset the error object so the later error checking logic is not triggered. Otherwise as written this will work like the following:

  1. resource.Retry sets top level err to resource.TimeoutError
  2. isResourceTimeoutError(err) conditional runs CreateListener again (great!)
  3. If CreateListener returns an error, it returns the error
  4. If CreateListener doesn't return an error, the isResourceTimeoutError() conditional exits, but the below error check (if err != nil) will still have the original resource.TimeoutError error, so it will still return an error

The trick here is using a direct = assignment in Go instead of :=

This allows us to simplify this logic like the following:

	// ... new logic here ...
	if isResourceTimeoutError(err) {
		_, err = elbconn.CreateListener(params)
	}

	// ... existing error handling below here ...
	if err != nil {
		return fmt.Errorf("Error creating LB Listener: %s", err)
	}

	// ... existing happy path ...

You'll now get the outcome you're expecting both ways when resource.Retry has a timeout error:

  1. resource.Retry sets top level err to resource.TimeoutError
  2. isResourceTimeoutError(err) conditional runs CreateListener again (great!)
  3. If CreateListener returns an error, it resets the top level error object and the below error check (if err != nil) will return that error
  4. If CreateListener doesn't return an error, the top level error object is reset to nil and the below error check (if err != nil) will not trigger so the resource will continue on its merry way on the happy path

Hope this helps!

_, err := elbconn.CreateListener(params)

if err != nil {
return fmt.Errorf("Error creating LB Listener: %s", err)
}
}

if err != nil {
return fmt.Errorf("Error creating LB Listener: %s", err)
}
Expand Down Expand Up @@ -526,6 +534,14 @@ func resourceAwsLbListenerRead(d *schema.ResourceData, meta interface{}) error {
return nil
})

if isResourceTimeoutError(err) {
_, err := elbconn.DescribeListeners(request)

if err != nil {
return fmt.Errorf("Error retrieving Listener: %s", err)
}
}

if isAWSErr(err, elbv2.ErrCodeListenerNotFoundException, "") {
log.Printf("[WARN] ELBv2 Listener (%s) not found - removing from state", d.Id())
d.SetId("")
Expand Down Expand Up @@ -799,6 +815,15 @@ func resourceAwsLbListenerUpdate(d *schema.ResourceData, meta interface{}) error
}
return nil
})

if isResourceTimeoutError(err) {
_, err := elbconn.ModifyListener(params)

if err != nil {
return fmt.Errorf("Error modifying LB Listener: %s", err)
}
}

if err != nil {
return fmt.Errorf("Error modifying LB Listener: %s", err)
}
Expand Down