Skip to content

Commit

Permalink
builtin: Refactor resource.Retry to clarify return
Browse files Browse the repository at this point in the history
Change the `RetryFunc` from a plain `error` return type to a
specialized `RetryError` which must decide whether it is
retryable or not.

Add `RetryableError` / `NonRetryableError` factory functions that
callers are meant to use to build up these errors.

This makes it eminently clear whether or not a given error is
retryable from inside the client code.

Goal here is to _not_ change any behavior, simply reflect the
existing behavior with the new, clearer, API.
  • Loading branch information
phinze committed Mar 9, 2016
1 parent de65694 commit c52aee7
Show file tree
Hide file tree
Showing 51 changed files with 317 additions and 292 deletions.
4 changes: 2 additions & 2 deletions builtin/providers/aws/resource_aws_api_gateway_api_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func resourceAwsApiGatewayApiKeyDelete(d *schema.ResourceData, meta interface{})
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway API Key: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteApiKey(&apigateway.DeleteApiKeyInput{
ApiKey: aws.String(d.Id()),
})
Expand All @@ -161,6 +161,6 @@ func resourceAwsApiGatewayApiKeyDelete(d *schema.ResourceData, meta interface{})
return nil
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
6 changes: 3 additions & 3 deletions builtin/providers/aws/resource_aws_api_gateway_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func resourceAwsApiGatewayDeploymentDelete(d *schema.ResourceData, meta interfac
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Deployment: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
log.Printf("[DEBUG] schema is %#v", d)
if _, err := conn.DeleteStage(&apigateway.DeleteStageInput{
StageName: aws.String(d.Get("stage_name").(string)),
Expand All @@ -161,9 +161,9 @@ func resourceAwsApiGatewayDeploymentDelete(d *schema.ResourceData, meta interfac
}

if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
6 changes: 3 additions & 3 deletions builtin/providers/aws/resource_aws_api_gateway_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func resourceAwsApiGatewayIntegrationDelete(d *schema.ResourceData, meta interfa
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Integration: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteIntegration(&apigateway.DeleteIntegrationInput{
HttpMethod: aws.String(d.Get("http_method").(string)),
ResourceId: aws.String(d.Get("resource_id").(string)),
Expand All @@ -177,9 +177,9 @@ func resourceAwsApiGatewayIntegrationDelete(d *schema.ResourceData, meta interfa
}

if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func resourceAwsApiGatewayIntegrationResponseDelete(d *schema.ResourceData, meta
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Integration Response: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteIntegrationResponse(&apigateway.DeleteIntegrationResponseInput{
HttpMethod: aws.String(d.Get("http_method").(string)),
ResourceId: aws.String(d.Get("resource_id").(string)),
Expand All @@ -133,9 +133,9 @@ func resourceAwsApiGatewayIntegrationResponseDelete(d *schema.ResourceData, meta
}

if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
6 changes: 3 additions & 3 deletions builtin/providers/aws/resource_aws_api_gateway_method.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func resourceAwsApiGatewayMethodDelete(d *schema.ResourceData, meta interface{})
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Method: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteMethod(&apigateway.DeleteMethodInput{
HttpMethod: aws.String(d.Get("http_method").(string)),
ResourceId: aws.String(d.Get("resource_id").(string)),
Expand All @@ -170,9 +170,9 @@ func resourceAwsApiGatewayMethodDelete(d *schema.ResourceData, meta interface{})
}

if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func resourceAwsApiGatewayMethodResponseDelete(d *schema.ResourceData, meta inte
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Method Response: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteMethodResponse(&apigateway.DeleteMethodResponseInput{
HttpMethod: aws.String(d.Get("http_method").(string)),
ResourceId: aws.String(d.Get("resource_id").(string)),
Expand All @@ -153,9 +153,9 @@ func resourceAwsApiGatewayMethodResponseDelete(d *schema.ResourceData, meta inte
}

if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
6 changes: 3 additions & 3 deletions builtin/providers/aws/resource_aws_api_gateway_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func resourceAwsApiGatewayModelDelete(d *schema.ResourceData, meta interface{})
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Model: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
log.Printf("[DEBUG] schema is %#v", d)
_, err := conn.DeleteModel(&apigateway.DeleteModelInput{
ModelName: aws.String(d.Get("name").(string)),
Expand All @@ -160,9 +160,9 @@ func resourceAwsApiGatewayModelDelete(d *schema.ResourceData, meta interface{})
}

if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
4 changes: 2 additions & 2 deletions builtin/providers/aws/resource_aws_api_gateway_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func resourceAwsApiGatewayResourceDelete(d *schema.ResourceData, meta interface{
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway Resource: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
log.Printf("[DEBUG] schema is %#v", d)
_, err := conn.DeleteResource(&apigateway.DeleteResourceInput{
ResourceId: aws.String(d.Id()),
Expand All @@ -143,6 +143,6 @@ func resourceAwsApiGatewayResourceDelete(d *schema.ResourceData, meta interface{
return nil
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
4 changes: 2 additions & 2 deletions builtin/providers/aws/resource_aws_api_gateway_rest_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func resourceAwsApiGatewayRestApiDelete(d *schema.ResourceData, meta interface{}
conn := meta.(*AWSClient).apigateway
log.Printf("[DEBUG] Deleting API Gateway: %s", d.Id())

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteRestApi(&apigateway.DeleteRestApiInput{
RestApiId: aws.String(d.Id()),
})
Expand All @@ -156,6 +156,6 @@ func resourceAwsApiGatewayRestApiDelete(d *schema.ResourceData, meta interface{}
return nil
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
})
}
18 changes: 10 additions & 8 deletions builtin/providers/aws/resource_aws_autoscaling_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{})
// We retry the delete operation to handle InUse/InProgress errors coming
// from scaling operations. We should be able to sneak in a delete in between
// scaling operations within 5m.
err = resource.Retry(5*time.Minute, func() error {
err = resource.Retry(5*time.Minute, func() *resource.RetryError {
if _, err := conn.DeleteAutoScalingGroup(&deleteopts); err != nil {
if awserr, ok := err.(awserr.Error); ok {
switch awserr.Code() {
Expand All @@ -459,11 +459,11 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{})
return nil
case "ResourceInUse", "ScalingActivityInProgress":
// These are retryable
return awserr
return resource.RetryableError(awserr)
}
}
// Didn't recognize the error, so shouldn't retry.
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}
// Successful delete
return nil
Expand All @@ -472,9 +472,10 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{})
return err
}

return resource.Retry(5*time.Minute, func() error {
return resource.Retry(5*time.Minute, func() *resource.RetryError {
if g, _ = getAwsAutoscalingGroup(d.Id(), conn); g != nil {
return fmt.Errorf("Auto Scaling Group still exists")
return resource.RetryableError(
fmt.Errorf("Auto Scaling Group still exists"))
}
return nil
})
Expand Down Expand Up @@ -531,10 +532,10 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{})

// Next, wait for the autoscale group to drain
log.Printf("[DEBUG] Waiting for group to have zero instances")
return resource.Retry(10*time.Minute, func() error {
return resource.Retry(10*time.Minute, func() *resource.RetryError {
g, err := getAwsAutoscalingGroup(d.Id(), conn)
if err != nil {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}
if g == nil {
log.Printf("[INFO] Autoscaling Group %q not found", d.Id())
Expand All @@ -546,7 +547,8 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{})
return nil
}

return fmt.Errorf("group still has %d instances", len(g.Instances))
return resource.RetryableError(
fmt.Errorf("group still has %d instances", len(g.Instances)))
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ func waitForASGCapacity(

log.Printf("[DEBUG] Waiting on %s for capacity...", d.Id())

return resource.Retry(wait, func() error {
return resource.Retry(wait, func() *resource.RetryError {
g, err := getAwsAutoscalingGroup(d.Id(), meta.(*AWSClient).autoscalingconn)
if err != nil {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}
if g == nil {
log.Printf("[INFO] Autoscaling Group %q not found", d.Id())
Expand All @@ -44,7 +44,7 @@ func waitForASGCapacity(
}
lbis, err := getLBInstanceStates(g, meta)
if err != nil {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

haveASG := 0
Expand Down Expand Up @@ -86,7 +86,8 @@ func waitForASGCapacity(
return nil
}

return fmt.Errorf("%q: Waiting up to %s: %s", d.Id(), wait, reason)
return resource.RetryableError(
fmt.Errorf("%q: Waiting up to %s: %s", d.Id(), wait, reason))
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,16 @@ func resourceAwsAutoscalingLifecycleHookPut(d *schema.ResourceData, meta interfa
params := getAwsAutoscalingPutLifecycleHookInput(d)

log.Printf("[DEBUG] AutoScaling PutLifecyleHook: %s", params)
err := resource.Retry(5*time.Minute, func() error {
err := resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.PutLifecycleHook(&params)

if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
if strings.Contains(awsErr.Message(), "Unable to publish test message to notification target") {
return fmt.Errorf("[DEBUG] Retrying AWS AutoScaling Lifecycle Hook: %s", params)
return resource.RetryableError(fmt.Errorf("[DEBUG] Retrying AWS AutoScaling Lifecycle Hook: %s", params))
}
}
return resource.RetryError{Err: fmt.Errorf("Error putting lifecycle hook: %s", err)}
return resource.NonRetryableError(fmt.Errorf("Error putting lifecycle hook: %s", err))
}
return nil
})
Expand Down
16 changes: 6 additions & 10 deletions builtin/providers/aws/resource_aws_cloudwatch_event_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,18 @@ func resourceAwsCloudWatchEventRuleCreate(d *schema.ResourceData, meta interface

// IAM Roles take some time to propagate
var out *events.PutRuleOutput
err := resource.Retry(30*time.Second, func() error {
err := resource.Retry(30*time.Second, func() *resource.RetryError {
var err error
out, err = conn.PutRule(input)
pattern := regexp.MustCompile("cannot be assumed by principal '[a-z]+\\.amazonaws\\.com'\\.$")
if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "ValidationException" && pattern.MatchString(awsErr.Message()) {
log.Printf("[DEBUG] Retrying creation of CloudWatch Event Rule %q", *input.Name)
return err
return resource.RetryableError(err)
}
}
return resource.RetryError{
Err: err,
}
return resource.NonRetryableError(err)
}
return nil
})
Expand Down Expand Up @@ -157,20 +155,18 @@ func resourceAwsCloudWatchEventRuleUpdate(d *schema.ResourceData, meta interface

// IAM Roles take some time to propagate
var out *events.PutRuleOutput
err := resource.Retry(30*time.Second, func() error {
err := resource.Retry(30*time.Second, func() *resource.RetryError {
var err error
out, err = conn.PutRule(input)
pattern := regexp.MustCompile("cannot be assumed by principal '[a-z]+\\.amazonaws\\.com'\\.$")
if err != nil {
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "ValidationException" && pattern.MatchString(awsErr.Message()) {
log.Printf("[DEBUG] Retrying update of CloudWatch Event Rule %q", *input.Name)
return err
return resource.RetryableError(err)
}
}
return resource.RetryError{
Err: err,
}
return resource.NonRetryableError(err)
}
return nil
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,20 @@ func resourceAwsCodeDeployDeploymentGroupCreate(d *schema.ResourceData, meta int
// Retry to handle IAM role eventual consistency.
var resp *codedeploy.CreateDeploymentGroupOutput
var err error
err = resource.Retry(2*time.Minute, func() error {
err = resource.Retry(2*time.Minute, func() *resource.RetryError {
resp, err = conn.CreateDeploymentGroup(&input)
if err != nil {
codedeployErr, ok := err.(awserr.Error)
if !ok {
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}
if codedeployErr.Code() == "InvalidRoleException" {
log.Printf("[DEBUG] Trying to create deployment group again: %q",
codedeployErr.Message())
return err
return resource.RetryableError(err)
}

return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}
return nil
})
Expand Down
8 changes: 4 additions & 4 deletions builtin/providers/aws/resource_aws_dynamodb_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,25 +676,25 @@ func resourceAwsDynamoDbTableDelete(d *schema.ResourceData, meta interface{}) er
TableName: aws.String(d.Id()),
}

err = resource.Retry(10*time.Minute, func() error {
err = resource.Retry(10*time.Minute, func() *resource.RetryError {
t, err := dynamodbconn.DescribeTable(params)
if err != nil {
if awserr, ok := err.(awserr.Error); ok && awserr.Code() == "ResourceNotFoundException" {
return nil
}
// Didn't recognize the error, so shouldn't retry.
return resource.RetryError{Err: err}
return resource.NonRetryableError(err)
}

if t != nil {
if t.Table.TableStatus != nil && strings.ToLower(*t.Table.TableStatus) == "deleting" {
log.Printf("[DEBUG] AWS Dynamo DB table (%s) is still deleting", d.Id())
return fmt.Errorf("still deleting")
return resource.RetryableError(fmt.Errorf("still deleting"))
}
}

// we should be not found or deleting, so error here
return resource.RetryError{Err: fmt.Errorf("[ERR] Error deleting Dynamo DB table, unexpected state: %s", t)}
return resource.NonRetryableError(err)
})

// check error from retry
Expand Down
Loading

0 comments on commit c52aee7

Please sign in to comment.