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

Allow forced replacement of route table associated with subnet #6999

Merged
merged 5 commits into from
Jul 31, 2019

Conversation

YakDriver
Copy link
Member

@YakDriver YakDriver commented Dec 28, 2018

Fixes #73

UPDATE 7/19/19: Rather than force replacement, allow the import of route table associations allows the associations to be updated. See below.

Changes proposed in this pull request:

  • r/aws_route_table_association: Add new argument force_replace allowing a route table to be associated with a subnet even if another route table is already associated. Add corresponding docs & tests.
  • r/aws_route_table_association: Implement import so that associations can be updated after they are imported.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSRouteTableAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSRouteTableAssociation_ -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSRouteTableAssociation_basic
=== PAUSE TestAccAWSRouteTableAssociation_basic
=== RUN   TestAccAWSRouteTableAssociation_replace
=== PAUSE TestAccAWSRouteTableAssociation_replace
=== CONT  TestAccAWSRouteTableAssociation_basic
=== CONT  TestAccAWSRouteTableAssociation_replace
--- PASS: TestAccAWSRouteTableAssociation_replace (63.32s)
--- PASS: TestAccAWSRouteTableAssociation_basic (63.59s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	63.696s

@ghost ghost added size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Dec 28, 2018
@YakDriver
Copy link
Member Author

YakDriver commented Dec 28, 2018

Here is a usage example.

Assume that subnet-04ff24b9b39f3822b was already associated with a route table. Normally, attempting to associate another route table, you would receive an error like this:

* aws_route_table_association.new_assoc: Resource.AlreadyAssociated: the specified association for route table rtb-05c7d040bf39801b0 conflicts with an existing association

However, with this PR, you can set the force_replace argument to true, and the association will replace the prior association.

resource "aws_route_table_association" "new_assoc" {
  route_table_id = "${aws_route_table.new_guy.id}"
  subnet_id      = "subnet-04ff24b9b39f3822b"
  force_replace  = true
}
$ terraform apply
aws_route_table.new_guy: Creation complete after 0s (ID: rtb-0833a30c3ba11dc03)
aws_route_table_association.new_assoc: Creating...
  force_replace:  "" => "true"
  route_table_id: "" => "rtb-0833a30c3ba11dc03"
  subnet_id:      "" => "subnet-04ff24b9b39f3822b"
aws_route_table_association.new_assoc: Creation complete after 1s (ID: rtbassoc-08cbb18f6a3d7a975)

Apply complete! Resources: 2 added, 0 changed, 0 destroyed.

@bflad bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Jan 9, 2019
@yves-vogl
Copy link

I'm hitting this issue too and would like to thank you for this patch.

@janjur
Copy link

janjur commented Feb 28, 2019

Please, merge.

@aleksap
Copy link

aleksap commented Mar 5, 2019

Please merge! We are hitting a major blocker with this issue.
Thanks!

@ibehren1
Copy link

ibehren1 commented Mar 5, 2019

Please merge -- need this as well.

@pit
Copy link

pit commented Mar 12, 2019

Waiting for merge also!

My case is importing existing resources into terraform state. And it's not possible to import aws_route_table_association (#561) and not possible to force old route table recreation.

@lorengordon
Copy link
Contributor

Anything we can do to promote this patch? Keep having to pop into the console to deal with this one, when people re-associate a route table improperly, since terraform fails to override this association. :(

@aleksap
Copy link

aleksap commented May 13, 2019

Any good soul out there that could merge this? ^^

@aeschright aeschright requested a review from a team June 25, 2019 21:45
@bflad bflad self-assigned this Jun 26, 2019
@YakDriver YakDriver force-pushed the reassociate_subnet branch 3 times, most recently from 2ad5c27 to 2dab22f Compare July 19, 2019 22:55
@YakDriver
Copy link
Member Author

YakDriver commented Jul 19, 2019

@bflad As discussed, I've implemented import for route table associations and taken out the force_replace argument. I've updated the docs and acceptance tests to test imports and updates.

example

A simplified example of using this.

Environment 1:

resource "aws_subnet" "client-network" {
...
}

resource "aws_route_table" "client-routes" {
...
}

resource "aws_route_table_association" "client-routes" {
  route_table_id = aws_route_table.client-routes.id
  subnet_id      = aws_subnet.client-network.id
}
$ terraform apply

Environment 2:

resource "aws_subnet" "new_sn" {
...
}

resource "aws_route_table_association" "replace" {
  route_table_id = var.route_table_id
  subnet_id      = aws_subnet. new_sn.id
}
$ terraform import aws_route_table_association.assoc subnet-6777656e646f6c796e/rtb-656c65616e6f72
$ terraform apply

Voilà. Association updated.

acceptance tests

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSRouteTableAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSRouteTableAssociation_ -timeout 120m
=== RUN   TestAccAWSRouteTableAssociation_basic
=== PAUSE TestAccAWSRouteTableAssociation_basic
=== RUN   TestAccAWSRouteTableAssociation_replace
=== PAUSE TestAccAWSRouteTableAssociation_replace
=== CONT  TestAccAWSRouteTableAssociation_basic
=== CONT  TestAccAWSRouteTableAssociation_replace
--- PASS: TestAccAWSRouteTableAssociation_basic (84.47s)
--- PASS: TestAccAWSRouteTableAssociation_replace (87.10s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	87.162s

@YakDriver YakDriver force-pushed the reassociate_subnet branch from 2dab22f to 9e0f878 Compare July 19, 2019 23:06
@YakDriver
Copy link
Member Author

YakDriver commented Jul 19, 2019

@aleksap @lorengordon @pit @ibehren1 @janjur @yves-vogl Please speak up if there are scenarios where import -> modify won't work and where you'd still need to force replace an association. My thinking is that having import capability (this PR) should cover most/all scenarios.

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @YakDriver 👋 Thanks so much for the updates and sticking through this one. Overall looking really good, just a few minor things. Please reach out with any questions or if you do not have time to implement the feedback. 👍

aws/resource_aws_route_table_association.go Outdated Show resolved Hide resolved
aws/resource_aws_route_table_association.go Outdated Show resolved Hide resolved
aws/resource_aws_route_table_association.go Outdated Show resolved Hide resolved
aws/resource_aws_route_table_association.go Outdated Show resolved Hide resolved
@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jul 31, 2019
@YakDriver YakDriver force-pushed the reassociate_subnet branch from 9cd1fe0 to 880448a Compare July 31, 2019 17:31
@YakDriver
Copy link
Member Author

@bflad I made the changes you requested. Let me know if there is anything else you see.

$ make testacc TESTARGS='-run=TestAccAWSRouteTableAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSRouteTableAssociation_ -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSRouteTableAssociation_basic
=== PAUSE TestAccAWSRouteTableAssociation_basic
=== RUN   TestAccAWSRouteTableAssociation_replace
=== PAUSE TestAccAWSRouteTableAssociation_replace
=== CONT  TestAccAWSRouteTableAssociation_basic
=== CONT  TestAccAWSRouteTableAssociation_replace
--- PASS: TestAccAWSRouteTableAssociation_basic (69.87s)
--- PASS: TestAccAWSRouteTableAssociation_replace (70.10s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	70.190s

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 31, 2019
@YakDriver YakDriver force-pushed the reassociate_subnet branch from 880448a to 07a1091 Compare July 31, 2019 17:39
@bflad bflad self-requested a review July 31, 2019 19:59
@bflad bflad added this to the v2.22.0 milestone Jul 31, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, @YakDriver, thanks! 🚀

--- PASS: TestAccAWSRouteTableAssociation_replace (27.43s)
--- PASS: TestAccAWSRouteTableAssociation_basic (27.60s)

@bflad bflad merged commit e2cadb3 into hashicorp:master Jul 31, 2019
bflad added a commit that referenced this pull request Jul 31, 2019
@YakDriver YakDriver deleted the reassociate_subnet branch July 31, 2019 21:24
@ghost
Copy link

ghost commented Aug 1, 2019

This has been released in version 2.22.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Nov 2, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_route_table_association fails if a different association already exists
9 participants