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

Data source: use aws_subnets over aws_subnet_ids #1113

Merged
merged 3 commits into from
May 10, 2022
Merged

Conversation

rhoboat
Copy link
Contributor

@rhoboat rhoboat commented May 4, 2022

Description

Addresses internal aws v4 compatibility.

This updates example code only. The provider version handles both old and new data sources, so users have no updates to make on their end as a result of this change. It is backward compatible. When we bump the provider version to at least 3.75 and/or remove the 4.x lock, that's when we have a potential backward incompatibility, but most likely, even that update will be functionally backward compatible, i.e., only require an update to the provider version, and no config changes, imports, or state migrations.

Documentation

N/A

TODOs

Related Issues

Addresses https://github.com/gruntwork-io/cloud-chasers/issues/20

yorinasub17
yorinasub17 previously approved these changes May 4, 2022
Copy link
Contributor

@yorinasub17 yorinasub17 left a comment

Choose a reason for hiding this comment

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

LGTM assuming the tests pass!

@rhoboat
Copy link
Contributor Author

rhoboat commented May 5, 2022

Test failures:

  1. This is not related to the AWS provider, so unrelated failure.
    --- FAIL: TestValidateAllTerraformModulesOnTerratest/home/circleci/project/examples/azure/terraform-azure-frontdoor-example (3.06s)
  1. Failed on fetching the syslog for the instances after the redeploy. I verified the test installs 4.12.1 of the AWS provider. Since it was able to deploy and validate the redeploy, I don't think this has to do with the code changes, like data.aws_subnets fetches failing.
=== CONT  TestTerraformRedeployExample
    ec2-syslog.go:81: 'Fetching syslog for Instance i-0ff7bf658328561de in us-west-1' unsuccessful after 120 retries
  1. This ran in eu-north-1. This page verifies that t2.micro is supported in Stockholm. The AWS docs list the possible reasons for this error.
    In any case, no files were changed that would affect this test.
=== CONT  TestGetInstanceIdsForAsg
    asg_test.go:82: 
        	Error Trace:	asg_test.go:82
        	            				asg_test.go:53
        	            				asg_test.go:43
        	Error:      	Received unexpected error:
        	            	Unsupported: The requested configuration is currently not supported. Please check the documentation for supported configurations.
        	            		status code: 400, request id: f6ffd647-52b3-4559-ab82-50a086088713
        	Test:       	TestGetInstanceIdsForAsg
=== CONT  TestGetInstanceIdsForAsg
    asg_test.go:154: 
        	Error Trace:	asg_test.go:154
        	            				asg_test.go:133
        	            				panic.go:642
        	            				testing.go:756
        	            				asg_test.go:82
        	            				asg_test.go:53
        	            				asg_test.go:43
        	Error:      	Received unexpected error:
        	            	ValidationError: AutoScalingGroup name not found - null
        	            		status code: 400, request id: de4e3814-36e7-48af-b33c-710a30001db6
        	Test:       	TestGetInstanceIdsForAsg
--- FAIL: TestGetInstanceIdsForAsg (2.82s)
  1. No files were changed that would affect this test. However, looking through the test, it might be flaky, so I changed it to make sure the test takes at least 25 seconds (greater than or equal to 25).
    assertion_compare.go:313: 
        	Error Trace:	apply_test.go:185
        	Error:      	"25" is not greater than "25"
        	Test:       	TestParallelism
        	Messages:   	[]
--- FAIL: TestParallelism (31.75s)

@rhoboat
Copy link
Contributor Author

rhoboat commented May 10, 2022

1 and 2 are still failing, but unrelated to this change.
3 and 4 have stopped failing.
Thanks for the review! Merging.

@rhoboat rhoboat merged commit d48fccc into master May 10, 2022
@rhoboat rhoboat deleted the feature/awsv4-cc20 branch May 10, 2022 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants