service/ec2: Increase default VPN Gateway detach timeout to 30 mins (#…

…15201) * r/aws_vpn_gateway_attachment: Refactor using internal 'finder' and 'waiter' packages. Increase detachment timeout to 30m. Acceptance test output: $ make testacc TEST=./aws TESTARGS='-run=TestAccAWSVpnGatewayAttachment_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSVpnGatewayAttachment_ -timeout 120m === RUN TestAccAWSVpnGatewayAttachment_basic === PAUSE TestAccAWSVpnGatewayAttachment_basic === RUN TestAccAWSVpnGatewayAttachment_deleted === PAUSE TestAccAWSVpnGatewayAttachment_deleted === CONT TestAccAWSVpnGatewayAttachment_basic === CONT TestAccAWSVpnGatewayAttachment_deleted resource_aws_vpn_gateway_attachment_test.go:40: [INFO] Got non-empty plan, as expected --- PASS: TestAccAWSVpnGatewayAttachment_deleted (33.77s) --- PASS: TestAccAWSVpnGatewayAttachment_basic (38.51s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 38.552s * r/aws_vpn_gateway: Refactor test sweeper to call aws_vpn_gateway and aws_vpn_gateway_attachment Delete methods. Acceptance test output: $ TEST=./aws SWEEP=us-west-2,us-east-1 SWEEPARGS=-sweep-run=aws_vpn_gateway make sweep WARNING: This will destroy infrastructure. Use only in development accounts. go test ./aws -v -sweep=us-west-2,us-east-1 -sweep-run=aws_vpn_gateway -timeout 60m 2020/09/17 14:01:43 [DEBUG] Running Sweepers for region (us-west-2): 2020/09/17 14:01:43 [DEBUG] Sweeper (aws_vpn_gateway) has dependency (aws_dx_gateway_association), running.. 2020/09/17 14:01:43 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:43 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-west-2) 2020/09/17 14:01:43 [INFO] AWS Auth provider used: "EnvProvider" 2020/09/17 14:01:43 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:44 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:45 [DEBUG] Running Sweeper (aws_dx_gateway_association) in region (us-west-2) 2020/09/17 14:01:46 [DEBUG] Running Sweeper (aws_vpn_gateway) in region (us-west-2) 2020/09/17 14:01:46 [INFO] Deleting VPN Gateway (vgw-0b8054188ab62b680) Attachment (vpc-f2e16e8b) 2020/09/17 14:01:47 [DEBUG] Waiting for state to become: [detached] 2020/09/17 14:01:47 [DEBUG] Not detaching VPN Gateway 'vgw-0b8054188ab62b680' as no VPC ID is set 2020/09/17 14:01:47 [INFO] Deleting VPN gateway: vgw-0b8054188ab62b680 2020/09/17 14:01:47 [DEBUG] Waiting for state to become: [success] 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-west-2) 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association) already ran in region (us-west-2) 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-west-2) 2020/09/17 14:01:48 Sweeper Tests ran successfully: - aws_vpn_gateway - aws_dx_gateway_association_proposal - aws_dx_gateway_association 2020/09/17 14:01:48 [DEBUG] Running Sweepers for region (us-east-1): 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_vpn_gateway) has dependency (aws_dx_gateway_association), running.. 2020/09/17 14:01:48 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:48 [DEBUG] Running Sweeper (aws_dx_gateway_association_proposal) in region (us-east-1) 2020/09/17 14:01:48 [INFO] AWS Auth provider used: "EnvProvider" 2020/09/17 14:01:48 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:48 [DEBUG] Trying to get account information via sts:GetCallerIdentity 2020/09/17 14:01:48 [DEBUG] Running Sweeper (aws_dx_gateway_association) in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] Running Sweeper (aws_vpn_gateway) in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] No VPN Gateways to sweep 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association) has dependency (aws_dx_gateway_association_proposal), running.. 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association) already ran in region (us-east-1) 2020/09/17 14:01:49 [DEBUG] Sweeper (aws_dx_gateway_association_proposal) already ran in region (us-east-1) 2020/09/17 14:01:49 Sweeper Tests ran successfully: - aws_dx_gateway_association_proposal - aws_dx_gateway_association - aws_vpn_gateway ok github.com/terraform-providers/terraform-provider-aws/aws 5.308s * r/aws_vpn_gateway: Refactor using internal 'waiter' package. Increase detachment timeout to 30m. Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpnGateway_' ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSVpnGateway_ -timeout 120m === RUN TestAccAWSVpnGateway_basic === PAUSE TestAccAWSVpnGateway_basic === RUN TestAccAWSVpnGateway_withAvailabilityZoneSetToState === PAUSE TestAccAWSVpnGateway_withAvailabilityZoneSetToState === RUN TestAccAWSVpnGateway_withAmazonSideAsnSetToState === PAUSE TestAccAWSVpnGateway_withAmazonSideAsnSetToState === RUN TestAccAWSVpnGateway_disappears === PAUSE TestAccAWSVpnGateway_disappears === RUN TestAccAWSVpnGateway_reattach === PAUSE TestAccAWSVpnGateway_reattach === RUN TestAccAWSVpnGateway_delete === PAUSE TestAccAWSVpnGateway_delete === RUN TestAccAWSVpnGateway_tags === PAUSE TestAccAWSVpnGateway_tags === CONT TestAccAWSVpnGateway_basic === CONT TestAccAWSVpnGateway_reattach --- PASS: TestAccAWSVpnGateway_basic (87.01s) === CONT TestAccAWSVpnGateway_tags --- PASS: TestAccAWSVpnGateway_reattach (123.40s) === CONT TestAccAWSVpnGateway_delete --- PASS: TestAccAWSVpnGateway_delete (60.46s) === CONT TestAccAWSVpnGateway_withAmazonSideAsnSetToState --- PASS: TestAccAWSVpnGateway_tags (97.73s) === CONT TestAccAWSVpnGateway_disappears resource_aws_vpn_gateway_test.go:195: [INFO] Got non-empty plan, as expected --- PASS: TestAccAWSVpnGateway_disappears (40.79s) === CONT TestAccAWSVpnGateway_withAvailabilityZoneSetToState --- PASS: TestAccAWSVpnGateway_withAmazonSideAsnSetToState (52.12s) --- PASS: TestAccAWSVpnGateway_withAvailabilityZoneSetToState (47.02s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 272.597s * 'TestAccAWSVpnGatewayAttachment_deleted' -> 'TestAccAWSVpnGatewayAttachment_disappears' (#13826, #13527).
hashicorp · Sep 18, 2020 · 5b496db · 5b496db
1 parent df23954
commit 5b496db
Show file tree

Hide file tree

Showing 9 changed files with 232 additions and 271 deletions.
diff --git a/aws/internal/service/ec2/errors.go b/aws/internal/service/ec2/errors.go
@@ -28,3 +28,8 @@ const (
 	InvalidSecurityGroupIDNotFound = "InvalidSecurityGroupID.NotFound"
 	InvalidGroupNotFound           = "InvalidGroup.NotFound"
 )
+
+const (
+	InvalidVpnGatewayAttachmentNotFound = "InvalidVpnGatewayAttachment.NotFound"
+	InvalidVpnGatewayIDNotFound         = "InvalidVpnGatewayID.NotFound"
+)
diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go
@@ -71,3 +71,43 @@ func SecurityGroupByID(conn *ec2.EC2, id string) (*ec2.SecurityGroup, error) {
 
 	return result.SecurityGroups[0], nil
 }
+
+// VpnGatewayVpcAttachment returns the attachment between the specified VPN gateway and VPC.
+// Returns nil and potentially an error if no attachment is found.
+func VpnGatewayVpcAttachment(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) {
+	vpnGateway, err := VpnGatewayByID(conn, vpnGatewayID)
+	if err != nil {
+		return nil, err
+	}
+
+	if vpnGateway == nil {
+		return nil, nil
+	}
+
+	for _, vpcAttachment := range vpnGateway.VpcAttachments {
+		if aws.StringValue(vpcAttachment.VpcId) == vpcID {
+			return vpcAttachment, nil
+		}
+	}
+
+	return nil, nil
+}
+
+// VpnGatewayByID returns the VPN gateway corresponding to the specified identifier.
+// Returns nil and potentially an error if no VPN gateway is found.
+func VpnGatewayByID(conn *ec2.EC2, id string) (*ec2.VpnGateway, error) {
+	input := &ec2.DescribeVpnGatewaysInput{
+		VpnGatewayIds: aws.StringSlice([]string{id}),
+	}
+
+	output, err := conn.DescribeVpnGateways(input)
+	if err != nil {
+		return nil, err
+	}
+
+	if output == nil || len(output.VpnGateways) == 0 {
+		return nil, nil
+	}
+
+	return output.VpnGateways[0], nil
+}
diff --git a/aws/internal/service/ec2/id.go b/aws/internal/service/ec2/id.go
@@ -3,6 +3,8 @@ package ec2
 import (
 	"fmt"
 	"strings"
+
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode"
 )
 
 const clientVpnAuthorizationRuleIDSeparator = ","
@@ -68,3 +70,7 @@ func ClientVpnRouteParseID(id string) (string, string, string, error) {
 		fmt.Errorf("unexpected format for ID (%q), expected endpoint-id"+clientVpnRouteIDSeparator+
 			"target-subnet-id"+clientVpnRouteIDSeparator+"destination-cidr-block", id)
 }
+
+func VpnGatewayVpcAttachmentCreateID(vpnGatewayID, vpcID string) string {
+	return fmt.Sprintf("vpn-attachment-%x", hashcode.String(fmt.Sprintf("%s-%s", vpcID, vpnGatewayID)))
+}
diff --git a/aws/internal/service/ec2/waiter/status.go b/aws/internal/service/ec2/waiter/status.go
@@ -204,3 +204,27 @@ func SecurityGroupStatus(conn *ec2.EC2, id string) resource.StateRefreshFunc {
 		return group, SecurityGroupStatusCreated, nil
 	}
 }
+
+const (
+	attachmentStateNotFound = "NotFound"
+	attachmentStateUnknown  = "Unknown"
+)
+
+// VpnGatewayVpcAttachmentState fetches the attachment between the specified VPN gateway and VPC and its state
+func VpnGatewayVpcAttachmentState(conn *ec2.EC2, vpnGatewayID, vpcID string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		vpcAttachment, err := finder.VpnGatewayVpcAttachment(conn, vpnGatewayID, vpcID)
+		if tfec2.ErrCodeEquals(err, tfec2.InvalidVpnGatewayIDNotFound) {
+			return nil, attachmentStateNotFound, nil
+		}
+		if err != nil {
+			return nil, attachmentStateUnknown, err
+		}
+
+		if vpcAttachment == nil {
+			return nil, attachmentStateNotFound, nil
+		}
+
+		return vpcAttachment, aws.StringValue(vpcAttachment.State), nil
+	}
+}
diff --git a/aws/internal/service/ec2/waiter/waiter.go b/aws/internal/service/ec2/waiter/waiter.go
@@ -199,3 +199,43 @@ func SecurityGroupCreated(conn *ec2.EC2, id string, timeout time.Duration) (*ec2
 
 	return nil, err
 }
+
+const (
+	VpnGatewayVpcAttachmentAttachedTimeout = 15 * time.Minute
+
+	VpnGatewayVpcAttachmentDetachedTimeout = 30 * time.Minute
+)
+
+func VpnGatewayVpcAttachmentAttached(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) {
+	stateConf := &resource.StateChangeConf{
+		Pending: []string{ec2.AttachmentStatusDetached, ec2.AttachmentStatusAttaching},
+		Target:  []string{ec2.AttachmentStatusAttached},
+		Refresh: VpnGatewayVpcAttachmentState(conn, vpnGatewayID, vpcID),
+		Timeout: VpnGatewayVpcAttachmentAttachedTimeout,
+	}
+
+	outputRaw, err := stateConf.WaitForState()
+
+	if output, ok := outputRaw.(*ec2.VpcAttachment); ok {
+		return output, err
+	}
+
+	return nil, err
+}
+
+func VpnGatewayVpcAttachmentDetached(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) {
+	stateConf := &resource.StateChangeConf{
+		Pending: []string{ec2.AttachmentStatusAttached, ec2.AttachmentStatusDetaching},
+		Target:  []string{ec2.AttachmentStatusDetached},
+		Refresh: VpnGatewayVpcAttachmentState(conn, vpnGatewayID, vpcID),
+		Timeout: VpnGatewayVpcAttachmentDetachedTimeout,
+	}
+
+	outputRaw, err := stateConf.WaitForState()
+
+	if output, ok := outputRaw.(*ec2.VpcAttachment); ok {
+		return output, err
+	}
+
+	return nil, err
+}
diff --git a/aws/resource_aws_vpn_gateway.go b/aws/resource_aws_vpn_gateway.go
@@ -12,6 +12,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+	tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter"
 )
 
 func resourceAwsVpnGateway() *schema.Resource {
@@ -233,7 +235,7 @@ func resourceAwsVpnGatewayAttach(d *schema.ResourceData, meta interface{}) error
 	err := resource.Retry(1*time.Minute, func() *resource.RetryError {
 		_, err := conn.AttachVpnGateway(req)
 		if err != nil {
-			if isAWSErr(err, "InvalidVpnGatewayID.NotFound", "") {
+			if isAWSErr(err, tfec2.InvalidVpnGatewayIDNotFound, "") {
 				return resource.RetryableError(err)
 			}
 			return resource.NonRetryableError(err)
@@ -250,14 +252,10 @@ func resourceAwsVpnGatewayAttach(d *schema.ResourceData, meta interface{}) error
 
 	// Wait for it to be fully attached before continuing
 	log.Printf("[DEBUG] Waiting for VPN gateway (%s) to attach", d.Id())
-	stateConf := &resource.StateChangeConf{
-		Pending: []string{ec2.AttachmentStatusDetached, ec2.AttachmentStatusAttaching},
-		Target:  []string{ec2.AttachmentStatusAttached},
-		Refresh: vpnGatewayAttachmentStateRefresh(conn, vpcId, d.Id()),
-		Timeout: 15 * time.Minute,
-	}
-	if _, err := stateConf.WaitForState(); err != nil {
-		return fmt.Errorf("Error waiting for VPN gateway (%s) to attach: %s", d.Id(), err)
+	_, err = waiter.VpnGatewayVpcAttachmentAttached(conn, d.Id(), vpcId)
+
+	if err != nil {
+		return fmt.Errorf("error waiting for VPN Gateway (%s) Attachment (%s) to become attached: %w", d.Id(), vpcId, err)
 	}
 
 	return nil
@@ -282,40 +280,24 @@ func resourceAwsVpnGatewayDetach(d *schema.ResourceData, meta interface{}) error
 		d.Id(),
 		vpcId)
 
-	wait := true
 	_, err := conn.DetachVpnGateway(&ec2.DetachVpnGatewayInput{
 		VpnGatewayId: aws.String(d.Id()),
 		VpcId:        aws.String(vpcId),
 	})
-	if err != nil {
-		if isAWSErr(err, "InvalidVpnGatewayID.NotFound", "") {
-			err = nil
-			wait = false
-		}
-		if isAWSErr(err, "InvalidVpnGatewayAttachment.NotFound", "") {
-			err = nil
-			wait = false
-		}
 
-		if err != nil {
-			return err
-		}
+	if isAWSErr(err, tfec2.InvalidVpnGatewayAttachmentNotFound, "") || isAWSErr(err, tfec2.InvalidVpnGatewayIDNotFound, "") {
+		return nil
 	}
 
-	if !wait {
-		return nil
+	if err != nil {
+		return fmt.Errorf("error deleting VPN Gateway (%s) Attachment (%s): %w", d.Id(), vpcId, err)
 	}
 
 	// Wait for it to be fully detached before continuing
-	log.Printf("[DEBUG] Waiting for VPN gateway (%s) to detach", d.Id())
-	stateConf := &resource.StateChangeConf{
-		Pending: []string{ec2.AttachmentStatusAttached, ec2.AttachmentStatusDetaching, "available"},
-		Target:  []string{ec2.AttachmentStatusDetached},
-		Refresh: vpnGatewayAttachmentStateRefresh(conn, vpcId, d.Id()),
-		Timeout: 10 * time.Minute,
-	}
-	if _, err := stateConf.WaitForState(); err != nil {
-		return fmt.Errorf("Error waiting for vpn gateway (%s) to detach: %s", d.Id(), err)
+	_, err = waiter.VpnGatewayVpcAttachmentDetached(conn, d.Id(), vpcId)
+
+	if err != nil {
+		return fmt.Errorf("error waiting for VPN Gateway (%s) Attachment (%s) to become detached: %w", d.Id(), vpcId, err)
 	}
 
 	return nil