Add custom timeouts for aws_route_table_association and aws_main_route_table_association

televi commented 2 years ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Currently, the provider supports custom timeout values for creating AWS route tables but not for associating them - those are all hard coded. This feature would add customizable timers for create/update/delete for aws_route_table_association and aws_main_route_table_association.

The impetus for this is failures we're seeing in AWS (us-east-1, specifically) for route associations.

New or Affected Resource(s)

aws_route_table_association - add timeout values using the same basic format as aws_route_table aws_main_route_table_association - add timeout values using the same basic format as aws_route_table

Docs for the above also need to be updated.

Potential Terraform Configuration

# Existing structure for AWS Routes
resource "aws_route" "d" {
  provider = aws.target
  route_table_id = aws_route_table.rt.id
  destination_cidr_block = "0.0.0.0/0"
  gateway_id = aws_internet_gateway.igw.id
  timeouts {
    create = "5m"
    delete = "5m"
  }
}

resource "aws_main_route_table_association" "rta" {
  provider = aws.target
  depends_on = [
    aws_route.d,
  ]
  vpc_id = aws_vpc.vpc.id
  route_table_id = aws_route_table.rt.id
# This would be new
  timeouts {
    create = "10m"
    delete = "10m"
}

resource "aws_route_table_association" "rta" {
  provider = aws.target
  depends_on = [
    aws_route.d,
  ]
  vpc_id = aws_vpc.vpc.id
  route_table_id = aws_route_table.rt.id
# This would be new
  timeouts {
    create = "10m"
    delete = "10m"
}

References

None found.

0000

televi commented 2 years ago

With a bit of help (likely around how to test it), I can probably implement this. I looked at the code and it doesn't seem to be too difficult a change but I'm not a) a Go guy and b) a Terraform provider guy so I may be missing some important things.

It looks like I just need to:

add a Timeouts structure to the ResourceRouteTableAssociation function in route_table_association.go and set the defaults to the values used wait.go (essentially just repeating what I see implemented in route_table.go)
add Timeouts structure to the ResourceMainRouteTableAssociation in main_route_table_association.go
update route_table_association.go to use the timeouts added above for ResourceRouteTableAssociation and to pass the relevant timeout to the WaitRouteTableAssociation{Created|Deleted|Updated} functions
update main_route_table_association.go to use the timeouts added above for ResourceMainRouteTableAssociation and to pass the relevant timeout to WaitRouteTableAssociation{Created|Deleted|Updated} functions
update the WaitRouteTableAssociation{Created|Deleted|Updated} to accept a timeout parameter and use it when instantiating the StateChangeConf structure (instead of the hardcoded values used currently)

I have no idea (yet) how to test it beyond needing to do something in main_route_table_association_test.go and route_table_association_test.go. I'm guessing one or more new functions that somehow simulate waiting for something to work within a defined interval and one or more new functions where it fails to complete within a defined interval but see a) above (not a Go guy) so I'm not sure how to do that in Go's testing framework.

michalschott commented 2 years ago

Indeed this is required. With a lot of terratesting in this region I can see errors about reading route table associations all the time!

thatderek commented 2 years ago

I think the need for this might be ameliorated by the above PR which introduces retry-logic into the provider for aws_route_table_associations when newly-created ones appear to be erroring out otherwise.

Taino19xx commented 1 year ago

22927

@thatderek As of 12/15/2022 in v4.46.0, we are still seeing the errors. So the retry logic pull/22927, did not completely resolve this. It would be most welcome to have the custom timeouts.

ellisroll-b commented 1 year ago

Please... Times out all the time at 2m45sec on us-east-1. we need control.

thatderek commented 1 year ago

Oh interesting. I worked on this when I was at a different job. I vaguely remember the fix in #22927 though and it's surprising (albeit, I have very limited experience with the provider) that this is still an issue.

If someone still seeing this could post a debug log, I imagine that would be super helpful to whoever reviews this. Also, if you want to add your 👍 to the issue at the top, that's a mechanism I've used with some success to get attention to my issues in the past. =)

hashicorp / terraform-provider-aws