From 10750e3b1e216aec5ed356ee75853fc5b796c57c Mon Sep 17 00:00:00 2001 From: Phil Frost Date: Thu, 2 Apr 2015 16:21:29 -0400 Subject: [PATCH 1/3] Revert "provider/aws: Fix dependency violation with subnets and security groups" This reverts commit 3d8005729d3e307535ea505c63fc238aae7beb87. Rationale: There is no guarantee that waiting will resolve the DependencyViolation. It's possible, perhaps likely, that the thing preventing the subnet deletion was created by some other means -- by another user, by a dynamic application, in another manifest, or that it was created by this manifest but for some other reason failed to be deleted. In these cases, the retry behavior makes the user wait 5 minutes only to receive the misleading error: Error deleting subnet: timeout while waiting for state to become 'destroyed' The obvious response to this is to try again, which yields another 5 minutes of waiting. The previous behavior was to fail quickly with an error which exactly described the problem. While this is mildly annoying, it's at least apparent what is happening. The situation the original commit was intended to address could be more elegantly addressed by: - running Terraform a second time, or - explicitly declaring dependencies via the `depends_on` attribute. --- builtin/providers/aws/resource_aws_subnet.go | 37 +++++--------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/builtin/providers/aws/resource_aws_subnet.go b/builtin/providers/aws/resource_aws_subnet.go index cc0ddec19566..7417f7e0aa64 100644 --- a/builtin/providers/aws/resource_aws_subnet.go +++ b/builtin/providers/aws/resource_aws_subnet.go @@ -159,38 +159,17 @@ func resourceAwsSubnetDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn log.Printf("[INFO] Deleting subnet: %s", d.Id()) - req := &ec2.DeleteSubnetInput{ - SubnetID: aws.String(d.Id()), - } - wait := resource.StateChangeConf{ - Pending: []string{"pending"}, - Target: "destroyed", - Timeout: 5 * time.Minute, - MinTimeout: 1 * time.Second, - Refresh: func() (interface{}, string, error) { - _, err := conn.DeleteSubnet(req) - if err != nil { - if apiErr, ok := err.(aws.APIError); ok { - if apiErr.Code == "DependencyViolation" { - // There is some pending operation, so just retry - // in a bit. - return 42, "pending", nil - } - - if apiErr.Code == "InvalidSubnetID.NotFound" { - return 42, "destroyed", nil - } - } - - return 42, "failure", err - } + _, err := conn.DeleteSubnet(&ec2.DeleteSubnetInput{ + SubnetID: aws.String(d.Id()), + }) - return 42, "destroyed", nil - }, - } + if err != nil { + ec2err, ok := err.(aws.APIError) + if ok && ec2err.Code == "InvalidSubnetID.NotFound" { + return nil + } - if _, err := wait.WaitForState(); err != nil { return fmt.Errorf("Error deleting subnet: %s", err) } From 8426e32c7f0117b521ff00c12df1210096dc6c2b Mon Sep 17 00:00:00 2001 From: Phil Frost Date: Tue, 28 Apr 2015 12:01:23 -0400 Subject: [PATCH 2/3] Don't retry deleting security groups on DependencyViolation Related: commit 7c89fb06ccb1acdb0c77ea8bd480254d121580c9. --- .../aws/resource_aws_security_group.go | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 044d0afaaa3e..8d5b556455cb 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -261,30 +261,19 @@ func resourceAwsSecurityGroupDelete(d *schema.ResourceData, meta interface{}) er log.Printf("[DEBUG] Security Group destroy: %v", d.Id()) - return resource.Retry(5*time.Minute, func() error { - _, err := conn.DeleteSecurityGroup(&ec2.DeleteSecurityGroupInput{ - GroupID: aws.String(d.Id()), - }) - if err != nil { - ec2err, ok := err.(aws.APIError) - if !ok { - return err - } + request := &ec2.DeleteSecurityGroupInput{ + GroupID: aws.String(d.Id()), + } - switch ec2err.Code { - case "InvalidGroup.NotFound": - return nil - case "DependencyViolation": - // If it is a dependency violation, we want to retry - return err - default: - // Any other error, we want to quit the retry loop immediately - return resource.RetryError{Err: err} - } + _, err := conn.DeleteSecurityGroup(request) + if err != nil { + ec2err, ok := err.(aws.APIError) + if ok && ec2err.Code == "InvalidGroup.NotFound" { + return nil } + } - return nil - }) + return err } func resourceAwsSecurityGroupRuleHash(v interface{}) int { From 0efa3e6889fcc580ee5dba294735a4e53f6640bb Mon Sep 17 00:00:00 2001 From: Phil Frost Date: Tue, 28 Apr 2015 12:41:48 -0400 Subject: [PATCH 3/3] Don't retry on DependencyViolation for internet gateways Logically reverts 043a4848ee88fe06c58235a1f07d5787a11a993f. --- builtin/providers/aws/resource_aws_internet_gateway.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin/providers/aws/resource_aws_internet_gateway.go b/builtin/providers/aws/resource_aws_internet_gateway.go index 5a91b4dd3ca8..949e82f12143 100644 --- a/builtin/providers/aws/resource_aws_internet_gateway.go +++ b/builtin/providers/aws/resource_aws_internet_gateway.go @@ -128,8 +128,6 @@ func resourceAwsInternetGatewayDelete(d *schema.ResourceData, meta interface{}) switch ec2err.Code { case "InvalidInternetGatewayID.NotFound": return nil - case "DependencyViolation": - return err // retry } return resource.RetryError{Err: err} @@ -232,10 +230,11 @@ func detachIGStateRefreshFunc(conn *ec2.EC2, instanceID, vpcID string) resource. return nil, "Not Found", err } else if ec2err.Code == "Gateway.NotAttached" { return "detached", "detached", nil - } else if ec2err.Code == "DependencyViolation" { - return nil, "detaching", nil } } + + log.Printf("Error on detachIGStateRefreshFunc: %#v", err) + return nil, "", err } // DetachInternetGateway only returns an error, so if it's nil, assume we're // detached