Merge pull request #9812 from terraform-providers/rfd-retry-security

Security group retries
hashicorp · Aug 21, 2019 · 1f4d45a · 1f4d45a
2 parents f90dfa8 + bd25ebd
commit 1f4d45a
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 23 deletions.
diff --git a/aws/resource_aws_security_group.go b/aws/resource_aws_security_group.go
@@ -458,31 +458,33 @@ func resourceAwsSecurityGroupDelete(d *schema.ResourceData, meta interface{}) er
 			return err
 		}
 	}
-
-	return resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
-		_, err := conn.DeleteSecurityGroup(&ec2.DeleteSecurityGroupInput{
-			GroupId: aws.String(d.Id()),
-		})
+	input := &ec2.DeleteSecurityGroupInput{
+		GroupId: aws.String(d.Id()),
+	}
+	err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
+		_, err := conn.DeleteSecurityGroup(input)
 		if err != nil {
-			ec2err, ok := err.(awserr.Error)
-			if !ok {
-				return resource.RetryableError(err)
-			}
-
-			switch ec2err.Code() {
-			case "InvalidGroup.NotFound":
+			if isAWSErr(err, "InvalidGroup.NotFound", "") {
 				return nil
-			case "DependencyViolation":
+			}
+			if isAWSErr(err, "DependencyViolation", "") {
 				// If it is a dependency violation, we want to retry
 				return resource.RetryableError(err)
-			default:
-				// Any other error, we want to quit the retry loop immediately
-				return resource.NonRetryableError(err)
 			}
+			resource.NonRetryableError(err)
 		}
-
 		return nil
 	})
+	if isResourceTimeoutError(err) {
+		_, err = conn.DeleteSecurityGroup(input)
+		if isAWSErr(err, "InvalidGroup.NotFound", "") {
+			return nil
+		}
+	}
+	if err != nil {
+		return fmt.Errorf("Error deleting security group: %s", err)
+	}
+	return nil
 }
 
 // Revoke all ingress/egress rules that a Security Group has

diff --git a/aws/resource_aws_security_group_rule.go b/aws/resource_aws_security_group_rule.go
@@ -221,18 +221,18 @@ information and instructions for recovery. Error message: %s`, sg_id, awsErr.Mes
 			ruleType, autherr)
 	}
 
+	var rules []*ec2.IpPermission
 	id := ipPermissionIDHash(sg_id, ruleType, perm)
 	log.Printf("[DEBUG] Computed group rule ID %s", id)
 
-	retErr := resource.Retry(5*time.Minute, func() *resource.RetryError {
+	err = resource.Retry(5*time.Minute, func() *resource.RetryError {
 		sg, err := findResourceSecurityGroup(conn, sg_id)
 
 		if err != nil {
 			log.Printf("[DEBUG] Error finding Security Group (%s) for Rule (%s): %s", sg_id, id, err)
 			return resource.NonRetryableError(err)
 		}
 
-		var rules []*ec2.IpPermission
 		switch ruleType {
 		case "ingress":
 			rules = sg.IpPermissions
@@ -241,7 +241,6 @@ information and instructions for recovery. Error message: %s`, sg_id, awsErr.Mes
 		}
 
 		rule := findRuleMatch(perm, rules, isVPC)
-
 		if rule == nil {
 			log.Printf("[DEBUG] Unable to find matching %s Security Group Rule (%s) for Group %s",
 				ruleType, id, sg_id)
@@ -251,10 +250,26 @@ information and instructions for recovery. Error message: %s`, sg_id, awsErr.Mes
 		log.Printf("[DEBUG] Found rule for Security Group Rule (%s): %s", id, rule)
 		return nil
 	})
+	if isResourceTimeoutError(err) {
+		sg, err := findResourceSecurityGroup(conn, sg_id)
+		if err != nil {
+			return fmt.Errorf("Error finding security group: %s", err)
+		}
 
-	if retErr != nil {
-		return fmt.Errorf("Error finding matching %s Security Group Rule (%s) for Group %s",
-			ruleType, id, sg_id)
+		switch ruleType {
+		case "ingress":
+			rules = sg.IpPermissions
+		default:
+			rules = sg.IpPermissionsEgress
+		}
+
+		rule := findRuleMatch(perm, rules, isVPC)
+		if rule == nil {
+			return fmt.Errorf("Error finding matching security group rule: %s", err)
+		}
+	}
+	if err != nil {
+		return fmt.Errorf("Error finding matching %s Security Group Rule (%s) for Group %s", ruleType, id, sg_id)
 	}
 
 	d.SetId(id)