Skip to content

Commit

Permalink
provider/aws: Retry IAM Role deletion on DeleteConflict (#14707)
Browse files Browse the repository at this point in the history 
* provider/aws: Retry IAM Role deletion on DeleteConflict

* provider/aws: Add 'IAM' to relevant test names
  • Loading branch information
@radeksimko @stack72
radeksimko authored and stack72 committed May 21, 2017
1 parent 752c38b commit 4a671fc
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 25 deletions.
2 changes: 1 addition & 1 deletion builtin/providers/aws/import_aws_iam_role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestAccAWSIAMRole_importBasic(t *testing.T) {
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRoleConfig(rName),
Config: testAccAWSIAMRoleConfig(rName),
},

{
Expand Down
17 changes: 13 additions & 4 deletions builtin/providers/aws/resource_aws_iam_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,17 @@ func resourceAwsIamRoleDelete(d *schema.ResourceData, meta interface{}) error {
RoleName: aws.String(d.Id()),
}

if _, err := iamconn.DeleteRole(request); err != nil {
return fmt.Errorf("Error deleting IAM Role %s: %s", d.Id(), err)
}
return nil
// IAM is eventually consistent and deletion of attached policies may take time
return resource.Retry(30*time.Second, func() *resource.RetryError {
_, err := iamconn.DeleteRole(request)
if err != nil {
awsErr, ok := err.(awserr.Error)
if ok && awsErr.Code() == "DeleteConflict" {
return resource.RetryableError(err)
}

return resource.NonRetryableError(fmt.Errorf("Error deleting IAM Role %s: %s", d.Id(), err))
}
return nil
})
}
40 changes: 20 additions & 20 deletions builtin/providers/aws/resource_aws_iam_role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/hashicorp/terraform/terraform"
)

func TestAccAWSRole_basic(t *testing.T) {
func TestAccAWSIAMRole_basic(t *testing.T) {
var conf iam.GetRoleOutput
rName := acctest.RandString(10)

Expand All @@ -25,7 +25,7 @@ func TestAccAWSRole_basic(t *testing.T) {
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRoleConfig(rName),
Config: testAccAWSIAMRoleConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role", &conf),
resource.TestCheckResourceAttr("aws_iam_role.role", "path", "/"),
Expand All @@ -36,7 +36,7 @@ func TestAccAWSRole_basic(t *testing.T) {
})
}

func TestAccAWSRole_basicWithDescription(t *testing.T) {
func TestAccAWSIAMRole_basicWithDescription(t *testing.T) {
var conf iam.GetRoleOutput
rName := acctest.RandString(10)

Expand All @@ -46,23 +46,23 @@ func TestAccAWSRole_basicWithDescription(t *testing.T) {
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRoleConfigWithDescription(rName),
Config: testAccAWSIAMRoleConfigWithDescription(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role", &conf),
resource.TestCheckResourceAttr("aws_iam_role.role", "path", "/"),
resource.TestCheckResourceAttr("aws_iam_role.role", "description", "This 1s a D3scr!pti0n with weird content: &@90ë“‘{«¡Çø}"),
),
},
{
Config: testAccAWSRoleConfigWithUpdatedDescription(rName),
Config: testAccAWSIAMRoleConfigWithUpdatedDescription(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role", &conf),
resource.TestCheckResourceAttr("aws_iam_role.role", "path", "/"),
resource.TestCheckResourceAttr("aws_iam_role.role", "description", "This 1s an Upd@ted D3scr!pti0n with weird content: &90ë“‘{«¡Çø}"),
),
},
{
Config: testAccAWSRoleConfig(rName),
Config: testAccAWSIAMRoleConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role", &conf),
resource.TestCheckResourceAttrSet("aws_iam_role.role", "create_date"),
Expand All @@ -73,7 +73,7 @@ func TestAccAWSRole_basicWithDescription(t *testing.T) {
})
}

func TestAccAWSRole_namePrefix(t *testing.T) {
func TestAccAWSIAMRole_namePrefix(t *testing.T) {
var conf iam.GetRoleOutput
rName := acctest.RandString(10)

Expand All @@ -85,7 +85,7 @@ func TestAccAWSRole_namePrefix(t *testing.T) {
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRolePrefixNameConfig(rName),
Config: testAccAWSIAMRolePrefixNameConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role", &conf),
testAccCheckAWSRoleGeneratedNamePrefix(
Expand All @@ -96,7 +96,7 @@ func TestAccAWSRole_namePrefix(t *testing.T) {
})
}

func TestAccAWSRole_testNameChange(t *testing.T) {
func TestAccAWSIAMRole_testNameChange(t *testing.T) {
var conf iam.GetRoleOutput
rName := acctest.RandString(10)

Expand All @@ -106,14 +106,14 @@ func TestAccAWSRole_testNameChange(t *testing.T) {
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRolePre(rName),
Config: testAccAWSIAMRolePre(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role_update_test", &conf),
),
},

{
Config: testAccAWSRolePost(rName),
Config: testAccAWSIAMRolePost(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSRoleExists("aws_iam_role.role_update_test", &conf),
),
Expand All @@ -122,7 +122,7 @@ func TestAccAWSRole_testNameChange(t *testing.T) {
})
}

func TestAccAWSRole_badJSON(t *testing.T) {
func TestAccAWSIAMRole_badJSON(t *testing.T) {
rName := acctest.RandString(10)

resource.Test(t, resource.TestCase{
Expand All @@ -131,7 +131,7 @@ func TestAccAWSRole_badJSON(t *testing.T) {
CheckDestroy: testAccCheckAWSRoleDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSRoleConfig_badJson(rName),
Config: testAccAWSIAMRoleConfig_badJson(rName),
ExpectError: regexp.MustCompile(`.*contains an invalid JSON:.*`),
},
},
Expand Down Expand Up @@ -210,7 +210,7 @@ func testAccCheckAWSRoleGeneratedNamePrefix(resource, prefix string) resource.Te
}
}

func testAccAWSRoleConfig(rName string) string {
func testAccAWSIAMRoleConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role" {
name = "test-role-%s"
Expand All @@ -220,7 +220,7 @@ resource "aws_iam_role" "role" {
`, rName)
}

func testAccAWSRoleConfigWithDescription(rName string) string {
func testAccAWSIAMRoleConfigWithDescription(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role" {
name = "test-role-%s"
Expand All @@ -231,7 +231,7 @@ resource "aws_iam_role" "role" {
`, rName)
}

func testAccAWSRoleConfigWithUpdatedDescription(rName string) string {
func testAccAWSIAMRoleConfigWithUpdatedDescription(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role" {
name = "test-role-%s"
Expand All @@ -242,7 +242,7 @@ resource "aws_iam_role" "role" {
`, rName)
}

func testAccAWSRolePrefixNameConfig(rName string) string {
func testAccAWSIAMRolePrefixNameConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role" {
name_prefix = "test-role-%s"
Expand All @@ -252,7 +252,7 @@ resource "aws_iam_role" "role" {
`, rName)
}

func testAccAWSRolePre(rName string) string {
func testAccAWSIAMRolePre(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role_update_test" {
name = "tf_old_name_%s"
Expand Down Expand Up @@ -302,7 +302,7 @@ resource "aws_iam_instance_profile" "role_update_test" {
`, rName, rName, rName)
}

func testAccAWSRolePost(rName string) string {
func testAccAWSIAMRolePost(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "role_update_test" {
name = "tf_new_name_%s"
Expand Down Expand Up @@ -352,7 +352,7 @@ resource "aws_iam_instance_profile" "role_update_test" {
`, rName, rName, rName)
}

func testAccAWSRoleConfig_badJson(rName string) string {
func testAccAWSIAMRoleConfig_badJson(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "my_instance_role" {
name = "test-role-%s"
Expand Down

0 comments on commit 4a671fc

Please sign in to comment.