Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r/route_table: Use configurable timeouts in aws_route #21161

Merged
merged 4 commits into from
Oct 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -21,6 +21,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 @@ -264,12 +267,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 @@ -282,12 +285,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 @@ -306,21 +309,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 @@ -333,12 +329,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