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

provider/aws: Allow VPC Classic Linking in Autoscaling Launch Configs #7470

Merged

Conversation

andrewsykim
Copy link

@andrewsykim andrewsykim commented Jul 2, 2016

AWS provides EC2 ClassicLink which allows instances outside of a VPC to reach instances within it. More details here. The use case is mainly for transitioning existing infrastructure into a VPC. You can specify a default VPC id and an associated security group with an autoscaling launch config so that all instances within the autoscaling group are automatically linked to a desired VPC. From my current understanding, terraform does not have support yet for VPC linking in launch configurations.

I noticed that the acceptance tests require the use of real AWS resources. I've tested my changes against resources that we own but I've replaced those values with some of the values I've seen in other examples for obvious reasons. As a result I'm not entirely sure if they'll work using your AWS resources. I'd be interested to know how you guys manage resources for acceptance testing from outside contributors.

terraform andrewsykim$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSLaunchConfiguration_withVpcClassicLink'
==> Checking that code complies with gofmt requirements...
/Users/andrewsykim/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
2016/07/02 13:40:52 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLaunchConfiguration_withVpcClassicLink -timeout 120m
=== RUN   TestAccAWSLaunchConfiguration_withVpcClassicLink
--- PASS: TestAccAWSLaunchConfiguration_withVpcClassicLink (10.37s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    10.388s

@stack72
Copy link
Contributor

stack72 commented Jul 3, 2016

Fixes #2433

@stack72
Copy link
Contributor

stack72 commented Jul 15, 2016

Hi @andrewsykim

Thanks so much for the work here. The code looks good, but unfortunately the tests don't pass:

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSLaunchConfiguration_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLaunchConfiguration_ -timeout 120m
=== RUN   TestAccAWSLaunchConfiguration_importBasic
--- PASS: TestAccAWSLaunchConfiguration_importBasic (24.15s)
=== RUN   TestAccAWSLaunchConfiguration_basic
--- PASS: TestAccAWSLaunchConfiguration_basic (47.85s)
=== RUN   TestAccAWSLaunchConfiguration_withBlockDevices
--- PASS: TestAccAWSLaunchConfiguration_withBlockDevices (31.08s)
=== RUN   TestAccAWSLaunchConfiguration_withSpotPrice
--- PASS: TestAccAWSLaunchConfiguration_withSpotPrice (21.91s)
=== RUN   TestAccAWSLaunchConfiguration_withVpcClassicLink
--- FAIL: TestAccAWSLaunchConfiguration_withVpcClassicLink (12.79s)
    testing.go:264: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_launch_configuration.foo: Error creating launch configuration: ValidationError: The security group 'sg-6a4d2f0d' does not exist
            status code: 400, request id: d62645c6-4abf-11e6-89c5-554d8a5135c7
=== RUN   TestAccAWSLaunchConfiguration_withIAMProfile
--- PASS: TestAccAWSLaunchConfiguration_withIAMProfile (38.83s)
=== RUN   TestAccAWSLaunchConfiguration_withEncryption
--- PASS: TestAccAWSLaunchConfiguration_withEncryption (29.97s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    206.607s
make: *** [testacc] Error 1

This seems to be due to a hardcoded sg in the test for this work - can we try and create this with terraform as part of the test and pass it in that way? That way we would ensure that the test system would be able to work as expected

Thanks

Paul

@stack72 stack72 added enhancement waiting-response An issue/pull request is waiting for a response from the community labels Jul 15, 2016
@stack72 stack72 self-assigned this Jul 15, 2016
@andrewsykim
Copy link
Author

@stack72 sounds good, I saw a few acceptance test cases that had hard coded values so I assumed there was a set of values we can hard code for those acceptance tests to work (with some shared AWS account). For the tests to pass I can add a setup step that creates all the existing resources required for this acceptance test

@stack72
Copy link
Contributor

stack72 commented Jul 15, 2016

Hi @andrewsykim

Yeah, if you could have the acceptance test create the security group that would be great. That should allow it to pass then

Paul

@andrewsykim
Copy link
Author

andrewsykim commented Jul 15, 2016

@stack72 I've modified the acceptance tests to generate a VPC and a security group as part of its
setup. Passes for me

Andrews-MacBook-Pro-393:terraform andrewsykim$ git rev-parse HEAD
a624be9a63e7c945df2e685e960cb0f4b625437e
Andrews-MacBook-Pro-393:terraform andrewsykim$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSLaunchConfiguration_withVpcClassicLink'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
2016/07/15 17:56:12 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLaunchConfiguration_withVpcClassicLink -timeout 120m
=== RUN   TestAccAWSLaunchConfiguration_withVpcClassicLink
--- PASS: TestAccAWSLaunchConfiguration_withVpcClassicLink (16.15s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    16.177s

@stack72
Copy link
Contributor

stack72 commented Jul 16, 2016

Hi @andrewsykim

Just had to modify 1 thing in your acceptance test - change the ami id for one that works in our default region of us-west-2. The test results now look as follows:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSLaunchConfiguration_'                     2 ↵
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSLaunchConfiguration_ -timeout 120m
=== RUN   TestAccAWSLaunchConfiguration_importBasic
--- PASS: TestAccAWSLaunchConfiguration_importBasic (19.91s)
=== RUN   TestAccAWSLaunchConfiguration_basic
--- PASS: TestAccAWSLaunchConfiguration_basic (57.28s)
=== RUN   TestAccAWSLaunchConfiguration_withBlockDevices
--- PASS: TestAccAWSLaunchConfiguration_withBlockDevices (42.90s)
=== RUN   TestAccAWSLaunchConfiguration_withSpotPrice
--- PASS: TestAccAWSLaunchConfiguration_withSpotPrice (42.12s)
=== RUN   TestAccAWSLaunchConfiguration_withVpcClassicLink
--- PASS: TestAccAWSLaunchConfiguration_withVpcClassicLink (77.03s)
=== RUN   TestAccAWSLaunchConfiguration_withIAMProfile
--- PASS: TestAccAWSLaunchConfiguration_withIAMProfile (62.67s)
=== RUN   TestAccAWSLaunchConfiguration_withEncryption
--- PASS: TestAccAWSLaunchConfiguration_withEncryption (30.01s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    331.947s

Thanks for all the work on this :)

Paul

@ghost
Copy link

ghost commented Apr 24, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants