From c738bf04778e1b920b41fccd20bb1d980399142a Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Thu, 18 Dec 2014 06:21:30 +0100 Subject: [PATCH] builtin/providers/aws: remove unreachable code and skip unnecessary remote call When DeleteInternetGateway is successful it returns a nil error value. However, for a nil error value, the RetryFunc returns an error yielding a unnecessary second call to DeleteInternetGateway in the retry logic. The logic works because DeleteInternetGateway eventually returns an ec2.Error with error code InvalidInternetGatewayID.NotFound since the internet gateway has been deleted in the previous call. The return value of nil breaks the retry logic and the deletion is deemed successful. Fix the unnecessary second call to DeleteInternetGateway by short circuiting with a nil error value when deletion of the internet gateway is successful on the first try. Add an acceptance test for internet gateway deletion and remove unreachable code while here. --- .../aws/resource_aws_internet_gateway.go | 46 ++++++------------- .../aws/resource_aws_internet_gateway_test.go | 36 +++++++++++++++ 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/builtin/providers/aws/resource_aws_internet_gateway.go b/builtin/providers/aws/resource_aws_internet_gateway.go index bdb9a4905..af2c0f6d7 100644 --- a/builtin/providers/aws/resource_aws_internet_gateway.go +++ b/builtin/providers/aws/resource_aws_internet_gateway.go @@ -97,40 +97,24 @@ func resourceAwsInternetGatewayDelete(d *schema.ResourceData, meta interface{}) return resource.Retry(5*time.Minute, func() error { _, err := ec2conn.DeleteInternetGateway(d.Id()) - if err != nil { - ec2err, ok := err.(*ec2.Error) - if !ok { - return err - } - - switch ec2err.Code { - case "InvalidInternetGatewayID.NotFound": - return nil - case "DependencyViolation": - return err // retry - default: - return resource.RetryError{err} - } + if err == nil { + return nil } - return fmt.Errorf("Error deleting internet gateway: %s", err) + ec2err, ok := err.(*ec2.Error) + if !ok { + return err + } + + switch ec2err.Code { + case "InvalidInternetGatewayID.NotFound": + return nil + case "DependencyViolation": + return err // retry + } + + return resource.RetryError{err} }) - - // Wait for the internet gateway to actually delete - log.Printf("[DEBUG] Waiting for internet gateway (%s) to delete", d.Id()) - stateConf := &resource.StateChangeConf{ - Pending: []string{"available"}, - Target: "", - Refresh: IGStateRefreshFunc(ec2conn, d.Id()), - Timeout: 10 * time.Minute, - } - if _, err := stateConf.WaitForState(); err != nil { - return fmt.Errorf( - "Error waiting for internet gateway (%s) to destroy: %s", - d.Id(), err) - } - - return nil } func resourceAwsInternetGatewayAttach(d *schema.ResourceData, meta interface{}) error { diff --git a/builtin/providers/aws/resource_aws_internet_gateway_test.go b/builtin/providers/aws/resource_aws_internet_gateway_test.go index 47a57c6a4..f5b1bda25 100644 --- a/builtin/providers/aws/resource_aws_internet_gateway_test.go +++ b/builtin/providers/aws/resource_aws_internet_gateway_test.go @@ -54,6 +54,36 @@ func TestAccAWSInternetGateway(t *testing.T) { }) } +func TestAccAWSInternetGateway_delete(t *testing.T) { + var ig ec2.InternetGateway + + testDeleted := func(r string) resource.TestCheckFunc { + return func(s *terraform.State) error { + _, ok := s.RootModule().Resources[r] + if ok { + return fmt.Errorf("Internet Gateway %q should have been deleted", r) + } + return nil + } + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckInternetGatewayDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccInternetGatewayConfig, + Check: resource.ComposeTestCheckFunc(testAccCheckInternetGatewayExists("aws_internet_gateway.foo", &ig)), + }, + resource.TestStep{ + Config: testAccNoInternetGatewayConfig, + Check: resource.ComposeTestCheckFunc(testDeleted("aws_internet_gateway.foo")), + }, + }, + }) +} + func testAccCheckInternetGatewayDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn @@ -113,6 +143,12 @@ func testAccCheckInternetGatewayExists(n string, ig *ec2.InternetGateway) resour } } +const testAccNoInternetGatewayConfig = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" +} +` + const testAccInternetGatewayConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16"