From be385b83f823d4351a3fc8ea4b82a88fa5baa7a2 Mon Sep 17 00:00:00 2001 From: Clint Date: Tue, 5 Apr 2016 10:18:03 -0500 Subject: [PATCH] provider/aws: Fix issue with retrying deletion of Network ACLs Fix retry after removing associations by correctly checking and returning an error. This should patch the VPC/Resource leak in our nightly acceptance tests. --- .../providers/aws/resource_aws_network_acl.go | 17 +++++-- .../aws/resource_aws_network_acl_test.go | 46 ++++++++++++++++++- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/builtin/providers/aws/resource_aws_network_acl.go b/builtin/providers/aws/resource_aws_network_acl.go index 7d24c07d7..b0c370d5b 100644 --- a/builtin/providers/aws/resource_aws_network_acl.go +++ b/builtin/providers/aws/resource_aws_network_acl.go @@ -410,7 +410,7 @@ func resourceAwsNetworkAclDelete(d *schema.ResourceData, meta interface{}) error conn := meta.(*AWSClient).ec2conn log.Printf("[INFO] Deleting Network Acl: %s", d.Id()) - return resource.Retry(5*time.Minute, func() *resource.RetryError { + retryErr := resource.Retry(5*time.Minute, func() *resource.RetryError { _, err := conn.DeleteNetworkAcl(&ec2.DeleteNetworkAclInput{ NetworkAclId: aws.String(d.Id()), }) @@ -440,18 +440,24 @@ func resourceAwsNetworkAclDelete(d *schema.ResourceData, meta interface{}) error associations = append(associations, a) } } + + log.Printf("[DEBUG] Replacing network associations for Network ACL (%s): %s", d.Id(), associations) defaultAcl, err := getDefaultNetworkAcl(d.Get("vpc_id").(string), conn) if err != nil { return resource.NonRetryableError(err) } for _, a := range associations { - _, err = conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ + _, replaceErr := conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ AssociationId: a.NetworkAclAssociationId, NetworkAclId: defaultAcl.NetworkAclId, }) + if replaceErr != nil { + log.Printf("[ERR] Non retryable error in replacing associtions for Network ACL (%s): %s", d.Id(), replaceErr) + return resource.NonRetryableError(replaceErr) + } } - return resource.NonRetryableError(err) + return resource.RetryableError(fmt.Errorf("Dependencies found and cleaned up, retrying")) default: // Any other error, we want to quit the retry loop immediately return resource.NonRetryableError(err) @@ -460,6 +466,11 @@ func resourceAwsNetworkAclDelete(d *schema.ResourceData, meta interface{}) error log.Printf("[Info] Deleted network ACL %s successfully", d.Id()) return nil }) + + if retryErr != nil { + return fmt.Errorf("[ERR] Error destroying Network ACL (%s): %s", d.Id(), retryErr) + } + return nil } func resourceAwsNetworkAclEntryHash(v interface{}) int { diff --git a/builtin/providers/aws/resource_aws_network_acl_test.go b/builtin/providers/aws/resource_aws_network_acl_test.go index 57c843e70..bce803a96 100644 --- a/builtin/providers/aws/resource_aws_network_acl_test.go +++ b/builtin/providers/aws/resource_aws_network_acl_test.go @@ -358,6 +358,9 @@ func testAccCheckSubnetIsNotAssociatedWithAcl(acl string, subnet string) resourc const testAccAWSNetworkAclIngressConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" + tags { + Name = "TestAccAWSNetworkAcl_OnlyIngressRules" + } } resource "aws_subnet" "blob" { cidr_block = "10.1.1.0/24" @@ -388,6 +391,9 @@ resource "aws_network_acl" "foos" { const testAccAWSNetworkAclIngressConfigChange = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" + tags { + Name = "TestAccAWSNetworkAcl_OnlyIngressRules" + } } resource "aws_subnet" "blob" { cidr_block = "10.1.1.0/24" @@ -411,6 +417,9 @@ resource "aws_network_acl" "foos" { const testAccAWSNetworkAclEgressConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.2.0.0/16" + tags { + Name = "TestAccAWSNetworkAcl_OnlyEgressRules" + } } resource "aws_subnet" "blob" { cidr_block = "10.2.0.0/24" @@ -464,6 +473,9 @@ resource "aws_network_acl" "bond" { const testAccAWSNetworkAclEgressNIngressConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.3.0.0/16" + tags { + Name = "TestAccAWSNetworkAcl_EgressAndIngressRules" + } } resource "aws_subnet" "blob" { cidr_block = "10.3.0.0/24" @@ -494,6 +506,9 @@ resource "aws_network_acl" "bar" { const testAccAWSNetworkAclSubnetConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" + tags { + Name = "TestAccAWSNetworkAcl_SubnetChange" + } } resource "aws_subnet" "old" { cidr_block = "10.1.111.0/24" @@ -518,6 +533,9 @@ resource "aws_network_acl" "bar" { const testAccAWSNetworkAclSubnetConfigChange = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" + tags { + Name = "TestAccAWSNetworkAcl_SubnetChange" + } } resource "aws_subnet" "old" { cidr_block = "10.1.111.0/24" @@ -539,20 +557,29 @@ const testAccAWSNetworkAclSubnet_SubnetIds = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" tags { - Name = "acl-subnets-test" + Name = "TestAccAWSNetworkAcl_Subnets" } } resource "aws_subnet" "one" { cidr_block = "10.1.111.0/24" vpc_id = "${aws_vpc.foo.id}" + tags { + Name = "acl-subnets-test" + } } resource "aws_subnet" "two" { cidr_block = "10.1.1.0/24" vpc_id = "${aws_vpc.foo.id}" + tags { + Name = "acl-subnets-test" + } } resource "aws_network_acl" "bar" { vpc_id = "${aws_vpc.foo.id}" subnet_ids = ["${aws_subnet.one.id}", "${aws_subnet.two.id}"] + tags { + Name = "acl-subnets-test" + } } ` @@ -560,25 +587,37 @@ const testAccAWSNetworkAclSubnet_SubnetIdsUpdate = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" tags { - Name = "acl-subnets-test" + Name = "TestAccAWSNetworkAcl_Subnets" } } resource "aws_subnet" "one" { cidr_block = "10.1.111.0/24" vpc_id = "${aws_vpc.foo.id}" + tags { + Name = "acl-subnets-test" + } } resource "aws_subnet" "two" { cidr_block = "10.1.1.0/24" vpc_id = "${aws_vpc.foo.id}" + tags { + Name = "acl-subnets-test" + } } resource "aws_subnet" "three" { cidr_block = "10.1.222.0/24" vpc_id = "${aws_vpc.foo.id}" + tags { + Name = "acl-subnets-test" + } } resource "aws_subnet" "four" { cidr_block = "10.1.4.0/24" vpc_id = "${aws_vpc.foo.id}" + tags { + Name = "acl-subnets-test" + } } resource "aws_network_acl" "bar" { vpc_id = "${aws_vpc.foo.id}" @@ -587,5 +626,8 @@ resource "aws_network_acl" "bar" { "${aws_subnet.three.id}", "${aws_subnet.four.id}", ] + tags { + Name = "acl-subnets-test" + } } `