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

Retries and retry removal for opsworks resources #9818

Merged
merged 1 commit into from
Aug 20, 2019
Merged

Retries and retry removal for opsworks resources #9818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 5 additions & 34 deletions aws/resource_aws_opsworks_application.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ import (
"fmt"
"log"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/opsworks"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
)
Expand Down Expand Up @@ -312,24 +310,9 @@ func resourceAwsOpsworksApplicationCreate(d *schema.ResourceData, meta interface
Attributes: resourceAwsOpsworksApplicationAttributes(d),
}

var resp *opsworks.CreateAppOutput
err = resource.Retry(2*time.Minute, func() *resource.RetryError {
var cerr error
resp, cerr = client.CreateApp(req)
if cerr != nil {
log.Printf("[INFO] client error")
if opserr, ok := cerr.(awserr.Error); ok {
// XXX: handle errors
log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
return resource.RetryableError(cerr)
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears this resource.RetryableError() usage may have been legitimate because unlike the UpdateApp usage below, its retrying on AWS Go SDK errors. That said, it is probably okay to remove this and see if anything comes up where we discover the appropriate errors (if any) to actually retry on. The acceptance testing is not finding it currently.

}
return resource.NonRetryableError(cerr)
}
return nil
})

resp, err := client.CreateApp(req)
if err != nil {
return err
return fmt.Errorf("Error creating OpsWorks application: %s", err)
}

appID := *resp.AppId
Expand Down Expand Up @@ -362,23 +345,11 @@ func resourceAwsOpsworksApplicationUpdate(d *schema.ResourceData, meta interface

log.Printf("[DEBUG] Updating OpsWorks layer: %s", d.Id())

err = resource.Retry(2*time.Minute, func() *resource.RetryError {
_, cerr := client.UpdateApp(req)
if cerr != nil {
log.Printf("[INFO] client error")
if opserr, ok := cerr.(awserr.Error); ok {
// XXX: handle errors
log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
return resource.NonRetryableError(cerr)
}
return resource.RetryableError(cerr)
}
return nil
})

_, err = client.UpdateApp(req)
if err != nil {
return err
return fmt.Errorf("Error updating OpsWorks app: %s", err)
}

return resourceAwsOpsworksApplicationRead(d, meta)
}

Expand Down
65 changes: 13 additions & 52 deletions aws/resource_aws_opsworks_rds_db_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@ package aws
import (
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/opsworks"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

Expand Down Expand Up @@ -68,23 +65,10 @@ func resourceAwsOpsworksRdsDbInstanceUpdate(d *schema.ResourceData, meta interfa
if requestUpdate {
log.Printf("[DEBUG] Opsworks RDS DB Instance Modification request: %s", req)

err := resource.Retry(2*time.Minute, func() *resource.RetryError {
var cerr error
_, cerr = client.UpdateRdsDbInstance(req)
if cerr != nil {
log.Printf("[INFO] client error")
if opserr, ok := cerr.(awserr.Error); ok {
log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
}
return resource.NonRetryableError(cerr)
}
return nil
})

_, err := client.UpdateRdsDbInstance(req)
if err != nil {
return err
return fmt.Errorf("Error updating Opsworks RDS DB instance: %s", err)
}

}

d.Partial(false)
Expand All @@ -101,27 +85,17 @@ func resourceAwsOpsworksRdsDbInstanceDeregister(d *schema.ResourceData, meta int

log.Printf("[DEBUG] Unregistering rds db instance '%s' from stack: %s", d.Get("rds_db_instance_arn"), d.Get("stack_id"))

err := resource.Retry(2*time.Minute, func() *resource.RetryError {
var cerr error
_, cerr = client.DeregisterRdsDbInstance(req)
if cerr != nil {
log.Printf("[INFO] client error")
if opserr, ok := cerr.(awserr.Error); ok {
if opserr.Code() == "ResourceNotFoundException" {
log.Printf("[INFO] The db instance could not be found. Remove it from state.")
d.SetId("")

return nil
}
log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
}
return resource.NonRetryableError(cerr)
_, err := client.DeregisterRdsDbInstance(req)
if err != nil {
if isAWSErr(err, "ResourceNotFoundException", "") {
log.Printf("[INFO] The db instance could not be found. Remove it from state.")
d.SetId("")
return nil
}
return fmt.Errorf("Error deregistering Opsworks RDS DB instance: %s", err)
}

return nil
})

return err
return nil
}

func resourceAwsOpsworksRdsDbInstanceRead(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -171,22 +145,9 @@ func resourceAwsOpsworksRdsDbInstanceRegister(d *schema.ResourceData, meta inter
DbPassword: aws.String(d.Get("db_password").(string)),
}

err := resource.Retry(2*time.Minute, func() *resource.RetryError {
var cerr error
_, cerr = client.RegisterRdsDbInstance(req)
if cerr != nil {
log.Printf("[INFO] client error")
if opserr, ok := cerr.(awserr.Error); ok {
log.Printf("[ERROR] OpsWorks error: %s message: %s", opserr.Code(), opserr.Message())
}
return resource.NonRetryableError(cerr)
}

return nil
})

_, err := client.RegisterRdsDbInstance(req)
if err != nil {
return err
return fmt.Errorf("Error registering Opsworks RDS DB instance: %s", err)
}

return resourceAwsOpsworksRdsDbInstanceRead(d, meta)
Expand Down
43 changes: 20 additions & 23 deletions aws/resource_aws_opsworks_stack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package aws
import (
"fmt"
"log"
"strings"
"time"

"github.com/hashicorp/terraform/helper/resource"
Expand Down Expand Up @@ -436,33 +435,31 @@ func resourceAwsOpsworksStackCreate(d *schema.ResourceData, meta interface{}) er

var resp *opsworks.CreateStackOutput
err = resource.Retry(20*time.Minute, func() *resource.RetryError {
var cerr error
resp, cerr = client.CreateStack(req)
if cerr != nil {
if opserr, ok := cerr.(awserr.Error); ok {
// If Terraform is also managing the service IAM role,
// it may have just been created and not yet be
// propagated.
// AWS doesn't provide a machine-readable code for this
// specific error, so we're forced to do fragile message
// matching.
// The full error we're looking for looks something like
// the following:
// Service Role Arn: [...] is not yet propagated, please try again in a couple of minutes
propErr := "not yet propagated"
trustErr := "not the necessary trust relationship"
validateErr := "validate IAM role permission"
if opserr.Code() == "ValidationException" && (strings.Contains(opserr.Message(), trustErr) || strings.Contains(opserr.Message(), propErr) || strings.Contains(opserr.Message(), validateErr)) {
log.Printf("[INFO] Waiting for service IAM role to propagate")
return resource.RetryableError(cerr)
}
resp, err = client.CreateStack(req)
if err != nil {
// If Terraform is also managing the service IAM role, it may have just been created and not yet be
// propagated. AWS doesn't provide a machine-readable code for this specific error, so we're forced
// to do fragile message matching.
// The full error we're looking for looks something like the following:
// Service Role Arn: [...] is not yet propagated, please try again in a couple of minutes
propErr := "not yet propagated"
trustErr := "not the necessary trust relationship"
validateErr := "validate IAM role permission"

if isAWSErr(err, "ValidationException", propErr) || isAWSErr(err, "ValidationException", trustErr) || isAWSErr(err, "ValidationException", validateErr) {
log.Printf("[INFO] Waiting for service IAM role to propagate")
return resource.RetryableError(err)
}
return resource.NonRetryableError(cerr)

return resource.NonRetryableError(err)
}
return nil
})
if isResourceTimeoutError(err) {
resp, err = client.CreateStack(req)
}
if err != nil {
return err
return fmt.Errorf("Error creating Opsworks stack: %s", err)
}

stackId := *resp.StackId
Expand Down
10 changes: 5 additions & 5 deletions aws/resource_aws_opsworks_stack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/aws/aws-sdk-go/service/opsworks"
)

func TestAccAWSOpsworksStackImportBasic(t *testing.T) {
func TestAccAWSOpsworksStack_ImportBasic(t *testing.T) {
name := acctest.RandString(10)

resourceName := "aws_opsworks_stack.tf-acc"
Expand All @@ -40,7 +40,7 @@ func TestAccAWSOpsworksStackImportBasic(t *testing.T) {
//// Tests for the No-VPC case
///////////////////////////////

func TestAccAWSOpsworksStackNoVpc(t *testing.T) {
func TestAccAWSOpsworksStack_NoVpc(t *testing.T) {
stackName := fmt.Sprintf("tf-opsworks-acc-%d", acctest.RandInt())
var opsstack opsworks.Stack
resource.ParallelTest(t, resource.TestCase{
Expand All @@ -63,7 +63,7 @@ func TestAccAWSOpsworksStackNoVpc(t *testing.T) {
})
}

func TestAccAWSOpsworksStackNoVpcChangeServiceRoleForceNew(t *testing.T) {
func TestAccAWSOpsworksStack_NoVpcChangeServiceRoleForceNew(t *testing.T) {
stackName := fmt.Sprintf("tf-opsworks-acc-%d", acctest.RandInt())
var before, after opsworks.Stack
resource.ParallelTest(t, resource.TestCase{
Expand All @@ -90,7 +90,7 @@ func TestAccAWSOpsworksStackNoVpcChangeServiceRoleForceNew(t *testing.T) {
})
}

func TestAccAWSOpsworksStackVpc(t *testing.T) {
func TestAccAWSOpsworksStack_Vpc(t *testing.T) {
stackName := fmt.Sprintf("tf-opsworks-acc-%d", acctest.RandInt())
var opsstack opsworks.Stack
resource.ParallelTest(t, resource.TestCase{
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestAccAWSOpsworksStackVpc(t *testing.T) {
})
}

func TestAccAWSOpsworksStackNoVpcCreateTags(t *testing.T) {
func TestAccAWSOpsworksStack_NoVpcCreateTags(t *testing.T) {
stackName := fmt.Sprintf("tf-opsworks-acc-%d", acctest.RandInt())
var opsstack opsworks.Stack
resource.ParallelTest(t, resource.TestCase{
Expand Down