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

Ignore propagated routes during table import #5100

Merged

Conversation

richievos
Copy link
Contributor

@richievos richievos commented Jul 5, 2018

Similar to how they're ignored when performing a
resourceAwsRouteTableRead.

Relates to #5099

This syncs the continue logic from resourceAwsRouteTableImportState and resourceAwsRouteTableRead. Really these should be using a helper function, but given there's already 3 other places with this code copy-pasted, I wasn't sure what the preferred way of doing this would be. Writing a test for this seems like it'd be pretty difficult, since a full test would require a gateway connected that has routes propagating into it. I assume that's why the existing resourceAwsRouteTableRead doesn't have a comparable test.

Output from test runs below. Apologies I don't have AWS creds in place that I can run this test against. The change proposed here is rather trivial though, and this at least shows it compiles / passes existing normal tests.

$ make test
==> Checking that code complies with gofmt requirements...
go test ./... -timeout=30s -parallel=4
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
ok  	github.com/terraform-providers/terraform-provider-aws/aws	2.009s
$ make testacc TESTARGS='-run=TestAccAWSAvailabilityZones'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSAvailabilityZones -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAvailabilityZones_basic
--- FAIL: TestAccAWSAvailabilityZones_basic (1.44s)
	provider_test.go:75: No valid credential sources found for AWS Provider.
			Please see https://terraform.io/docs/providers/aws/index.html for more information on
			providing credentials for the AWS Provider
=== RUN   TestAccAWSAvailabilityZones_stateFilter
--- FAIL: TestAccAWSAvailabilityZones_stateFilter (0.96s)
	provider_test.go:75: No valid credential sources found for AWS Provider.
			Please see https://terraform.io/docs/providers/aws/index.html for more information on
			providing credentials for the AWS Provider
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	2.437s
make: *** [testacc] Error 1

@ghost ghost added the size/XS Managed by automation to categorize the size of a PR. label Jul 5, 2018
Similar to how they're ignored when performing a
`resourceAwsRouteTableRead`.

Relates to hashicorp#5099
@richievos richievos force-pushed the route_table_import_propagation_fix branch from fb38dd8 to dc2d3a7 Compare July 5, 2018 23:12
@ghost ghost added the size/XS Managed by automation to categorize the size of a PR. label Jul 5, 2018
@bflad bflad added bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. labels Jul 6, 2018
@bflad bflad added this to the v1.27.0 milestone Jul 6, 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 for this fix @richievos! 🚀

=== RUN   TestAccAWSRouteTable_panicEmptyRoute
--- PASS: TestAccAWSRouteTable_panicEmptyRoute (9.42s)
=== RUN   TestAccAWSRouteTable_ipv6
--- PASS: TestAccAWSRouteTable_ipv6 (15.32s)
=== RUN   TestAccAWSRouteTable_vgwRoutePropagation
--- PASS: TestAccAWSRouteTable_vgwRoutePropagation (25.96s)
=== RUN   TestAccAWSRouteTable_importBasic
--- PASS: TestAccAWSRouteTable_importBasic (27.57s)
=== RUN   TestAccAWSRouteTable_tags
--- PASS: TestAccAWSRouteTable_tags (30.36s)
=== RUN   TestAccAWSRouteTable_vpcPeering
--- PASS: TestAccAWSRouteTable_vpcPeering (33.27s)
=== RUN   TestAccAWSRouteTableAssociation_basic
--- PASS: TestAccAWSRouteTableAssociation_basic (37.42s)
=== RUN   TestAccAWSRouteTable_basic
--- PASS: TestAccAWSRouteTable_basic (40.67s)
=== RUN   TestAccAWSRouteTable_complex
--- PASS: TestAccAWSRouteTable_complex (157.34s)
=== RUN   TestAccAWSRouteTable_instance
--- PASS: TestAccAWSRouteTable_instance (263.61s)

@bflad bflad merged commit 6bc4259 into hashicorp:master Jul 6, 2018
bflad added a commit that referenced this pull request Jul 6, 2018
@bflad
Copy link
Contributor

bflad commented Jul 11, 2018

This has been released in version 1.27.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/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants