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

Force new route when changing the route table #4946

Merged

Conversation

tomelliff
Copy link
Contributor

Before this change, changing the route table of a route showed an in place modification for Terraform to perform.
AWS actually creates a new route in the new route table with the ReplaceRoute API call but this left the old route lying around and no longer managed.

The acceptance test provided in this commit was failing before because Terraform was attempting to update the new route's 10.3.0.0/16 route to point to the new target but the 10.3.0.0/16 route didn't exist in that route table and instead wanted Terraform to call the CreateRoute API instead:

There is no route defined for '10.3.0.0/16' in the route table. Use CreateRoute instead.

Instead we should just use ForceNew to make Terraform create a new route in the new route table and then delete the old one.

Fixes #1841

Changes proposed in this pull request:

  • Changing the route table of a route should force a new resource

Output from acceptance testing:

$ make testacc TESTARGS="-run=TestAccAWSRoute_changeRouteTable"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSRoute_changeRouteTable -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSRoute_changeRouteTable
--- PASS: TestAccAWSRoute_changeRouteTable (161.83s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	161.850s

Before this change, changing the route table of a route showed an in place modification for Terraform to perform.
AWS actually creates a new route in the new route table with the ReplaceRoute API call but this left the old route lying around and no longer managed.

The acceptance test provided in this commit was failing before because Terraform was attempting to update the new route's 10.3.0.0/16 route to point to the new target but the 10.3.0.0/16 route didn't exist in that route table and instead wanted Terraform to call the CreateRoute API instead:

> There is no route defined for '10.3.0.0/16' in the route table. Use CreateRoute instead.

Instead we should just use ForceNew to make Terraform create a new route in the new route table and then delete the old one.
@ghost ghost added the size/M Managed by automation to categorize the size of a PR. label Jun 22, 2018
@bflad bflad added bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. labels Jun 22, 2018
@bflad bflad added this to the v1.25.0 milestone Jun 22, 2018
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.

LGTM -- thanks @tomelliff! 🚀

(oops forgot the _ after Route when running this, but more testing is better, right? 😅 )

65 tests passed (all tests)
=== RUN   TestAccAWSRoute53HealthCheck_CloudWatchAlarmCheck
=== RUN   TestAccAWSRoute53HealthCheck_withChildHealthChecks
=== RUN   TestAccAWSRoute53HealthCheck_IpConfig
=== RUN   TestAccAWSRouteTable_importBasic
--- PASS: TestAccAWSRouteTable_importBasic (22.33s)
=== RUN   TestAccAWSRoute53DelegationSet_importBasic
--- PASS: TestAccAWSRoute53DelegationSet_importBasic (22.53s)
=== RUN   TestAccAWSRoute53HealthCheck_importBasic
--- PASS: TestAccAWSRoute53HealthCheck_importBasic (24.66s)
=== RUN   TestAccAWSRoute53HealthCheck_withHealthCheckRegions
=== RUN   TestAccAWSRoute53HealthCheck_withSearchString
=== RUN   TestAccAWSRoute53DelegationSet_basic
--- PASS: TestAccAWSRoute53DelegationSet_basic (28.60s)
=== RUN   TestAccAWSRoute53HealthCheck_withSNI
=== RUN   TestAccAWSRoute53HealthCheck_basic
=== RUN   TestAccAWSRoute53QueryLog_Import
--- PASS: TestAccAWSRoute53QueryLog_Import (85.72s)
=== RUN   TestAccAWSRoute53DelegationSet_withZones
--- PASS: TestAccAWSRoute53DelegationSet_withZones (90.30s)
=== RUN   TestAccAWSRoute53Zone_importBasic
--- PASS: TestAccAWSRoute53Zone_importBasic (110.38s)
=== RUN   TestAccAWSRoute53QueryLog_Basic
--- PASS: TestAccAWSRoute53QueryLog_Basic (135.91s)
=== RUN   TestAccAWSRouteTable_complex
--- PASS: TestAccAWSRouteTable_complex (167.99s)
=== RUN   TestAccAWSRoute53Record_basic
--- PASS: TestAccAWSRoute53Record_basic (189.10s)
=== RUN   TestAccAWSRoute53Record_importUnderscored
--- PASS: TestAccAWSRoute53Record_importUnderscored (198.65s)
=== RUN   TestAccAWSRoute53Record_basic_fqdn
--- PASS: TestAccAWSRoute53Record_basic_fqdn (199.02s)
=== RUN   TestAccAWSRoute53Record_caaSupport
--- PASS: TestAccAWSRoute53Record_caaSupport (187.84s)
=== RUN   TestAccAWSRoute53Record_generatesSuffix
--- PASS: TestAccAWSRoute53Record_generatesSuffix (187.39s)
=== RUN   TestAccAWSRoute53Record_spfSupport
--- PASS: TestAccAWSRoute53Record_spfSupport (192.61s)
=== RUN   TestAccAWSRoute53Record_txtSupport
--- PASS: TestAccAWSRoute53Record_txtSupport (207.21s)
=== RUN   TestAccAWSRoute53Record_alias
--- PASS: TestAccAWSRoute53Record_alias (216.84s)
=== RUN   TestAccAWSRoute53Record_s3_alias
--- PASS: TestAccAWSRoute53Record_s3_alias (208.61s)
=== RUN   TestAccAWSRoute53Record_importBasic
--- PASS: TestAccAWSRoute53Record_importBasic (247.64s)
=== RUN   TestAccAWSRoute53Record_failover
--- PASS: TestAccAWSRoute53Record_failover (227.11s)
=== RUN   TestAccAWSRoute53Record_weighted_basic
--- PASS: TestAccAWSRoute53Record_weighted_basic (247.62s)
=== RUN   TestAccAWSRoute53Record_aliasUppercase
--- PASS: TestAccAWSRoute53Record_aliasUppercase (244.86s)
=== RUN   TestAccAWSRoute53Record_latency_basic
--- PASS: TestAccAWSRoute53Record_latency_basic (205.56s)
=== RUN   TestAccAWSRouteTable_basic
--- PASS: TestAccAWSRouteTable_basic (24.79s)
=== RUN   TestAccAWSRouteTableAssociation_basic
--- PASS: TestAccAWSRouteTableAssociation_basic (25.74s)
=== RUN   TestAccAWSRoute53Record_wildcard
--- PASS: TestAccAWSRoute53Record_wildcard (284.76s)
=== RUN   TestAccAWSRouteTable_ipv6
--- PASS: TestAccAWSRouteTable_ipv6 (9.10s)
=== RUN   TestAccAWSRouteTable_tags
--- PASS: TestAccAWSRouteTable_tags (14.26s)
=== RUN   TestAccAWSRouteTable_panicEmptyRoute
--- PASS: TestAccAWSRouteTable_panicEmptyRoute (6.16s)
=== RUN   TestAccAWSRouteTable_vpcPeering
--- PASS: TestAccAWSRouteTable_vpcPeering (19.74s)
=== RUN   TestAccAWSRoute_basic
--- PASS: TestAccAWSRoute_basic (18.49s)
=== RUN   TestAccAWSRoute53Record_geolocation_basic
--- PASS: TestAccAWSRoute53Record_geolocation_basic (248.45s)
=== RUN   TestAccAWSRoute53Zone_updateComment
--- PASS: TestAccAWSRoute53Zone_updateComment (92.09s)
=== RUN   TestAccAWSRoute53Zone_private_basic
--- PASS: TestAccAWSRoute53Zone_private_basic (88.07s)
=== RUN   TestAccAWSRouteTable_vgwRoutePropagation
--- PASS: TestAccAWSRouteTable_vgwRoutePropagation (24.77s)
=== RUN   TestAccAWSRoute_ipv6Support
--- PASS: TestAccAWSRoute_ipv6Support (10.65s)
=== RUN   TestAccAWSRoute_ipv6ToPeeringConnection
--- PASS: TestAccAWSRoute_ipv6ToPeeringConnection (10.64s)
=== RUN   TestAccAWSRoute_ipv6ToInternetGateway
--- PASS: TestAccAWSRoute_ipv6ToInternetGateway (19.11s)
=== RUN   TestAccAWSRoute53Zone_private_region
--- PASS: TestAccAWSRoute53Zone_private_region (99.69s)
=== RUN   TestAccAWSRoute_changeCidr
--- PASS: TestAccAWSRoute_changeCidr (29.54s)
=== RUN   TestAccAWSRoute53Zone_basic
--- PASS: TestAccAWSRoute53Zone_basic (145.12s)
=== RUN   TestAccAWSRoute_changeRouteTable
--- PASS: TestAccAWSRoute_changeRouteTable (31.91s)
=== RUN   TestAccAWSRoute53Record_empty
--- PASS: TestAccAWSRoute53Record_empty (185.14s)
=== RUN   TestAccAWSRoute53ZoneAssociation_region
--- PASS: TestAccAWSRoute53ZoneAssociation_region (162.43s)
=== RUN   TestAccAWSRoute_doesNotCrashWithVPCEndpoint
--- PASS: TestAccAWSRoute_doesNotCrashWithVPCEndpoint (25.75s)
=== RUN   TestAccAWSRoute53Record_TypeChange
--- PASS: TestAccAWSRoute53Record_TypeChange (269.41s)
=== RUN   TestAccAWSRoute53ZoneAssociation_basic
--- PASS: TestAccAWSRoute53ZoneAssociation_basic (170.77s)
=== RUN   TestAccAWSRoute53Record_SetIdentiferChange
--- PASS: TestAccAWSRoute53Record_SetIdentiferChange (265.86s)
=== RUN   TestAccAWSRoute53Record_multivalue_answer_basic
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (210.71s)
=== RUN   TestAccAWSRoute53Record_AliasChange
--- PASS: TestAccAWSRoute53Record_AliasChange (262.06s)
=== RUN   TestAccAWSRoute53Record_longTXTrecord
--- PASS: TestAccAWSRoute53Record_longTXTrecord (236.10s)
=== RUN   TestAccAWSRoute_ipv6ToInstance
--- PASS: TestAccAWSRoute_ipv6ToInstance (107.29s)
=== RUN   TestAccAWSRoute53Record_weighted_alias
--- PASS: TestAccAWSRoute53Record_weighted_alias (413.32s)
=== RUN   TestAccAWSRoute53Record_allowOverwrite
--- PASS: TestAccAWSRoute53Record_allowOverwrite (272.71s)
=== RUN   TestAccAWSRoute_ipv6ToNetworkInterface
--- PASS: TestAccAWSRoute_ipv6ToNetworkInterface (158.62s)
=== RUN   TestAccAWSRoute_noopdiff
--- PASS: TestAccAWSRoute_noopdiff (170.90s)
=== RUN   TestAccAWSRouteTable_instance
--- PASS: TestAccAWSRouteTable_instance (246.92s)
=== RUN   TestAccAWSRoute53Zone_forceDestroy
--- PASS: TestAccAWSRoute53Zone_forceDestroy (434.17s)

@bflad bflad merged commit caa2fa0 into hashicorp:master Jun 22, 2018
bflad added a commit that referenced this pull request Jun 22, 2018
@bflad
Copy link
Contributor

bflad commented Jun 27, 2018

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

@ghost
Copy link

ghost commented Apr 4, 2020

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 Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. size/M Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_route thinks route_table_id is a modifyable resource - its not
2 participants