Skip to content

Commit

Permalink
Merge pull request #21161 from msahihi/b-r/aws_route-increase_timeout
Browse files Browse the repository at this point in the history
r/route_table: Use configurable timeouts in aws_route
  • Loading branch information
ewbankkit authored Oct 6, 2021
2 parents 651c0e2 + 65e4963 commit d6488ff
Show file tree
Hide file tree
Showing 13 changed files with 295 additions and 185 deletions.
15 changes: 15 additions & 0 deletions .changelog/21161.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
```release-note:bug
resource/aws_route: Use custom `timeouts` values
```

```release-notes:enhancement
resource/aws_route_table: Add [custom `timeouts`](https://www.terraform.io/docs/language/resources/syntax.html#operation-timeouts) block
```

```release-notes:enhancement
resource/aws_default_route_table: Add [custom `timeouts`](https://www.terraform.io/docs/language/resources/syntax.html#operation-timeouts) block
```

```release-notes:enhancement
resource/aws_vpn_gateway_route_propagation: Add [custom `timeouts`](https://www.terraform.io/docs/language/resources/syntax.html#operation-timeouts) block
```
43 changes: 31 additions & 12 deletions aws/internal/service/ec2/finder/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,7 @@ func RouteTable(conn *ec2.EC2, input *ec2.DescribeRouteTablesInput) (*ec2.RouteT
}

if output == nil || len(output.RouteTables) == 0 || output.RouteTables[0] == nil {
return nil, &resource.NotFoundError{
Message: "Empty result",
LastRequest: input,
}
return nil, tfresource.NewEmptyResultError(input)
}

return output.RouteTables[0], nil
Expand All @@ -426,7 +423,9 @@ func RouteByIPv4Destination(conn *ec2.EC2, routeTableID, destinationCidr string)
}
}

return nil, &resource.NotFoundError{}
return nil, &resource.NotFoundError{
LastError: fmt.Errorf("Route in Route Table (%s) with IPv4 destination (%s) not found", routeTableID, destinationCidr),
}
}

// RouteByIPv6Destination returns the route corresponding to the specified IPv6 destination.
Expand All @@ -444,7 +443,9 @@ func RouteByIPv6Destination(conn *ec2.EC2, routeTableID, destinationIpv6Cidr str
}
}

return nil, &resource.NotFoundError{}
return nil, &resource.NotFoundError{
LastError: fmt.Errorf("Route in Route Table (%s) with IPv6 destination (%s) not found", routeTableID, destinationIpv6Cidr),
}
}

// RouteByPrefixListIDDestination returns the route corresponding to the specified prefix list destination.
Expand All @@ -461,7 +462,9 @@ func RouteByPrefixListIDDestination(conn *ec2.EC2, routeTableID, prefixListID st
}
}

return nil, &resource.NotFoundError{}
return nil, &resource.NotFoundError{
LastError: fmt.Errorf("Route in Route Table (%s) with Prefix List ID destination (%s) not found", routeTableID, prefixListID),
}
}

// SecurityGroupByID looks up a security group by ID. Returns a resource.NotFoundError if not found.
Expand Down Expand Up @@ -758,10 +761,7 @@ func VpcEndpoint(conn *ec2.EC2, input *ec2.DescribeVpcEndpointsInput) (*ec2.VpcE
}

if output == nil || len(output.VpcEndpoints) == 0 || output.VpcEndpoints[0] == nil {
return nil, &resource.NotFoundError{
Message: "Empty result",
LastRequest: input,
}
return nil, tfresource.NewEmptyResultError(input)
}

return output.VpcEndpoints[0], nil
Expand Down Expand Up @@ -801,7 +801,7 @@ func VpcEndpointSubnetAssociationExists(conn *ec2.EC2, vpcEndpointID string, sub
}

return &resource.NotFoundError{
LastError: fmt.Errorf("VPC Endpoint Subnet Association (%s/%s) not found", vpcEndpointID, subnetID),
LastError: fmt.Errorf("VPC Endpoint (%s) Subnet (%s) Association not found", vpcEndpointID, subnetID),
}
}

Expand All @@ -824,6 +824,25 @@ func VpcPeeringConnectionByID(conn *ec2.EC2, id string) (*ec2.VpcPeeringConnecti
return output.VpcPeeringConnections[0], nil
}

// VpnGatewayRoutePropagationExists returns NotFoundError if no route propagation for the specified VPN gateway is found.
func VpnGatewayRoutePropagationExists(conn *ec2.EC2, routeTableID, gatewayID string) error {
routeTable, err := RouteTableByID(conn, routeTableID)

if err != nil {
return err
}

for _, v := range routeTable.PropagatingVgws {
if aws.StringValue(v.GatewayId) == gatewayID {
return nil
}
}

return &resource.NotFoundError{
LastError: fmt.Errorf("Route Table (%s) VPN Gateway (%s) route propagation not found", routeTableID, gatewayID),
}
}

// 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) {
Expand Down
17 changes: 17 additions & 0 deletions aws/internal/service/ec2/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,20 @@ func VpcEndpointSubnetAssociationCreateID(vpcEndpointID, subnetID string) string
func VpnGatewayVpcAttachmentCreateID(vpnGatewayID, vpcID string) string {
return fmt.Sprintf("vpn-attachment-%x", hashcode.String(fmt.Sprintf("%s-%s", vpcID, vpnGatewayID)))
}

const vpnGatewayRoutePropagationIDSeparator = "_"

func VpnGatewayRoutePropagationCreateID(routeTableID, gatewayID string) string {
parts := []string{gatewayID, routeTableID}
id := strings.Join(parts, vpnGatewayRoutePropagationIDSeparator)
return id
}

func VpnGatewayRoutePropagationParseID(id string) (string, string, error) {
parts := strings.Split(id, vpnGatewayRoutePropagationIDSeparator)
if len(parts) == 2 && parts[0] != "" && parts[1] != "" {
return parts[1], parts[0], nil
}

return "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected vpn-gateway-id%[2]sroute-table-id", id, vpnGatewayRoutePropagationIDSeparator)
}
26 changes: 11 additions & 15 deletions aws/internal/service/ec2/waiter/waiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const (

// General timeout for EC2 resource creations to propagate
PropagationTimeout = 2 * time.Minute

RouteTableNotFoundChecks = 1000 // Should exceed any reasonable custom timeout value.
RouteTableAssociationCreatedNotFoundChecks = 1000 // Should exceed any reasonable custom timeout value.
)

const (
Expand Down Expand Up @@ -266,12 +269,12 @@ const (
NetworkAclEntryPropagationTimeout = 5 * time.Minute
)

func RouteDeleted(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string) (*ec2.Route, error) {
func RouteDeleted(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string, timeout time.Duration) (*ec2.Route, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{RouteStatusReady},
Target: []string{},
Refresh: RouteStatus(conn, routeFinder, routeTableID, destination),
Timeout: PropagationTimeout,
Timeout: timeout,
ContinuousTargetOccurence: 2,
}

Expand All @@ -284,12 +287,12 @@ func RouteDeleted(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, d
return nil, err
}

func RouteReady(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string) (*ec2.Route, error) {
func RouteReady(conn *ec2.EC2, routeFinder finder.RouteFinder, routeTableID, destination string, timeout time.Duration) (*ec2.Route, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{},
Target: []string{RouteStatusReady},
Refresh: RouteStatus(conn, routeFinder, routeTableID, destination),
Timeout: PropagationTimeout,
Timeout: timeout,
ContinuousTargetOccurence: 2,
}

Expand All @@ -308,21 +311,14 @@ const (
RouteTableAssociationCreatedTimeout = 5 * time.Minute
RouteTableAssociationUpdatedTimeout = 5 * time.Minute
RouteTableAssociationDeletedTimeout = 5 * time.Minute

RouteTableReadyTimeout = 10 * time.Minute
RouteTableDeletedTimeout = 5 * time.Minute
RouteTableUpdatedTimeout = 5 * time.Minute

RouteTableNotFoundChecks = 40
RouteTableAssociationCreatedNotFoundChecks = 40
)

func RouteTableReady(conn *ec2.EC2, id string) (*ec2.RouteTable, error) {
func RouteTableReady(conn *ec2.EC2, id string, timeout time.Duration) (*ec2.RouteTable, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{},
Target: []string{RouteTableStatusReady},
Refresh: RouteTableStatus(conn, id),
Timeout: RouteTableReadyTimeout,
Timeout: timeout,
NotFoundChecks: RouteTableNotFoundChecks,
}

Expand All @@ -335,12 +331,12 @@ func RouteTableReady(conn *ec2.EC2, id string) (*ec2.RouteTable, error) {
return nil, err
}

func RouteTableDeleted(conn *ec2.EC2, id string) (*ec2.RouteTable, error) {
func RouteTableDeleted(conn *ec2.EC2, id string, timeout time.Duration) (*ec2.RouteTable, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{RouteTableStatusReady},
Target: []string{},
Refresh: RouteTableStatus(conn, id),
Timeout: RouteTableDeletedTimeout,
Timeout: timeout,
}

outputRaw, err := stateConf.WaitForState()
Expand Down
12 changes: 9 additions & 3 deletions aws/resource_aws_default_route_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"log"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
Expand All @@ -27,6 +28,11 @@ func resourceAwsDefaultRouteTable() *schema.Resource {
State: resourceAwsDefaultRouteTableImport,
},

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(2 * time.Minute),
Update: schema.DefaultTimeout(2 * time.Minute),
},

//
// The top-level attributes must be a superset of the aws_route_table resource's attributes as common CRUD handlers are used.
//
Expand Down Expand Up @@ -211,7 +217,7 @@ func resourceAwsDefaultRouteTableCreate(d *schema.ResourceData, meta interface{}
return fmt.Errorf("error deleting Route in EC2 Default Route Table (%s) with destination (%s): %w", d.Id(), destination, err)
}

_, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination)
_, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination, d.Timeout(schema.TimeoutCreate))

if err != nil {
return fmt.Errorf("error waiting for Route in EC2 Default Route Table (%s) with destination (%s) to delete: %w", d.Id(), destination, err)
Expand All @@ -223,7 +229,7 @@ func resourceAwsDefaultRouteTableCreate(d *schema.ResourceData, meta interface{}
for _, v := range v.(*schema.Set).List() {
v := v.(string)

if err := ec2RouteTableEnableVgwRoutePropagation(conn, d.Id(), v); err != nil {
if err := ec2RouteTableEnableVgwRoutePropagation(conn, d.Id(), v, d.Timeout(schema.TimeoutCreate)); err != nil {
return err
}
}
Expand All @@ -234,7 +240,7 @@ func resourceAwsDefaultRouteTableCreate(d *schema.ResourceData, meta interface{}
for _, v := range v.(*schema.Set).List() {
v := v.(map[string]interface{})

if err := ec2RouteTableAddRoute(conn, d.Id(), v); err != nil {
if err := ec2RouteTableAddRoute(conn, d.Id(), v, d.Timeout(schema.TimeoutCreate)); err != nil {
return err
}
}
Expand Down
44 changes: 15 additions & 29 deletions aws/resource_aws_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
Expand Down Expand Up @@ -49,6 +48,7 @@ func resourceAwsRoute() *schema.Resource {

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(2 * time.Minute),
Update: schema.DefaultTimeout(2 * time.Minute),
Delete: schema.DefaultTimeout(5 * time.Minute),
},

Expand Down Expand Up @@ -244,7 +244,7 @@ func resourceAwsRouteCreate(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error creating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err)
}

_, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination)
_, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination, d.Timeout(schema.TimeoutCreate))

if err != nil {
return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", routeTableID, destination, err)
Expand Down Expand Up @@ -385,7 +385,7 @@ func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error updating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err)
}

_, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination)
_, err = waiter.RouteReady(conn, routeFinder, routeTableID, destination, d.Timeout(schema.TimeoutUpdate))

if err != nil {
return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to become available: %w", routeTableID, destination, err)
Expand Down Expand Up @@ -425,42 +425,28 @@ func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error {
}

log.Printf("[DEBUG] Deleting Route: %s", input)
err = resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
_, err = conn.DeleteRoute(input)

if err == nil {
return nil
}

if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteNotFound) {
return nil
}

// Local routes (which may have been imported) cannot be deleted. Remove from state.
if tfawserr.ErrMessageContains(err, tfec2.ErrCodeInvalidParameterValue, "cannot remove local route") {
return nil
}

if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidParameterException) {
return resource.RetryableError(err)
}

return resource.NonRetryableError(err)
})
_, err = tfresource.RetryWhenAwsErrCodeEquals(
d.Timeout(schema.TimeoutDelete),
func() (interface{}, error) {
return conn.DeleteRoute(input)
},
tfec2.ErrCodeInvalidParameterException,
)

if tfresource.TimedOut(err) {
_, err = conn.DeleteRoute(input)
if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteNotFound) {
return nil
}

if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidRouteNotFound) {
// Local routes (which may have been imported) cannot be deleted. Remove from state.
if tfawserr.ErrMessageContains(err, tfec2.ErrCodeInvalidParameterValue, "cannot remove local route") {
return nil
}

if err != nil {
return fmt.Errorf("error deleting Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err)
}

_, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination)
_, err = waiter.RouteDeleted(conn, routeFinder, routeTableID, destination, d.Timeout(schema.TimeoutDelete))

if err != nil {
return fmt.Errorf("error waiting for Route in Route Table (%s) with destination (%s) to delete: %w", routeTableID, destination, err)
Expand Down
Loading

0 comments on commit d6488ff

Please sign in to comment.