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: Add support for ENI as Instance Primary Network Interface #12694

Closed
wants to merge 4 commits into from

Conversation

peteromoon
Copy link

@peteromoon peteromoon commented Mar 14, 2017

This PR is an implementation of issue 2998.

I am aware to two other open PRs that attempt to address this issue, 10516 and 10244.

The PR changes the network_interface_id parameter of an aws_instance resource to be an optional parameter that conflicts with the following parameters: vpc_security_group_ids, subnet_id and associate_public_ip_address. This follows the same patterns as the two PR mentioned above.

Additionally, the network_interface_id parameter is configured with “ForceNew=true” as the primary network interface of an AWS instance can’t be changed without recreating the instance.

A new acceptance test case (TestAccAWSInstance_withNetworkInterface) is also included in the PR. It checks that specified network interface is attached to created instance and the network interface is changed (by recreating the instance) should the network_interface_id parameter be changed.

I run the acceptance tests for AWSInstance as follows:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSInstance_withNetworkInterface’
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/14 08:45:24 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSInstance_with -timeout 120m
=== RUN   TestAccAWSInstance_withNetworkInterface
--- PASS: TestAccAWSInstance_withNetworkInterface (309.01s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	309.033s
$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSInstance_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/14 09:11:35 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSInstance_ -timeout 120m
=== RUN   TestAccAWSInstance_importBasic
--- PASS: TestAccAWSInstance_importBasic (179.53s)
=== RUN   TestAccAWSInstance_basic
--- PASS: TestAccAWSInstance_basic (240.96s)
=== RUN   TestAccAWSInstance_GP2IopsDevice
--- PASS: TestAccAWSInstance_GP2IopsDevice (112.66s)
=== RUN   TestAccAWSInstance_blockDevices
--- PASS: TestAccAWSInstance_blockDevices (119.65s)
=== RUN   TestAccAWSInstance_rootInstanceStore
--- PASS: TestAccAWSInstance_rootInstanceStore (119.37s)
=== RUN   TestAccAWSInstance_sourceDestCheck
--- PASS: TestAccAWSInstance_sourceDestCheck (328.66s)
=== RUN   TestAccAWSInstance_disableApiTermination
--- PASS: TestAccAWSInstance_disableApiTermination (279.02s)
=== RUN   TestAccAWSInstance_vpc
--- PASS: TestAccAWSInstance_vpc (181.95s)
=== RUN   TestAccAWSInstance_ipv6_supportAddressCount
--- PASS: TestAccAWSInstance_ipv6_supportAddressCount (185.19s)
=== RUN   TestAccAWSInstance_multipleRegions
--- FAIL: TestAccAWSInstance_multipleRegions (121.37s)
	testing.go:268: Step 0 error: Error applying: 1 error(s) occurred:

		* aws_instance.foo: 1 error(s) occurred:

		* aws_instance.foo: Error launching source instance: InvalidAMIID.NotFound: The image id '[ami-4fccb37f]' does not exist
			status code: 400, request id: 41674ce3-0e12-4a98-af4c-9f2778c67fae
=== RUN   TestAccAWSInstance_NetworkInstanceSecurityGroups
--- PASS: TestAccAWSInstance_NetworkInstanceSecurityGroups (206.71s)
=== RUN   TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs
--- PASS: TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs (206.80s)
=== RUN   TestAccAWSInstance_tags
--- PASS: TestAccAWSInstance_tags (189.57s)
=== RUN   TestAccAWSInstance_instanceProfileChange
--- PASS: TestAccAWSInstance_instanceProfileChange (197.11s)
=== RUN   TestAccAWSInstance_privateIP
--- PASS: TestAccAWSInstance_privateIP (190.93s)
=== RUN   TestAccAWSInstance_associatePublicIPAndPrivateIP
--- PASS: TestAccAWSInstance_associatePublicIPAndPrivateIP (185.40s)
=== RUN   TestAccAWSInstance_keyPairCheck
--- PASS: TestAccAWSInstance_keyPairCheck (104.64s)
=== RUN   TestAccAWSInstance_rootBlockDeviceMismatch
--- PASS: TestAccAWSInstance_rootBlockDeviceMismatch (195.65s)
=== RUN   TestAccAWSInstance_forceNewAndTagsDrift
--- PASS: TestAccAWSInstance_forceNewAndTagsDrift (300.71s)
=== RUN   TestAccAWSInstance_changeInstanceType
--- PASS: TestAccAWSInstance_changeInstanceType (205.14s)
=== RUN   TestAccAWSInstance_withNetworkInterface
--- PASS: TestAccAWSInstance_withNetworkInterface (331.57s)
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/aws	4182.595s
make: *** [testacc] Error 1

Please note that the TestAccAWSInstance_multipleRegions test fails. This failure occurs without the PR being applied to the master branch.

From my analysis the commit that introduced this regression is 1eb9a2d.

Thank you for considering this PR.

@grubernaut grubernaut self-assigned this Mar 16, 2017
@catsby
Copy link
Contributor

catsby commented Mar 16, 2017

Thanks! We'll take a look here. I can confirm TestAccAWSInstance_multipleRegions is failing on the master branch, and is unrelated to this change.

@ghost
Copy link

ghost commented Apr 13, 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 unassigned grubernaut Apr 13, 2020
@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants