add support for create instance with eni

[automaticgiant]

pretty unfamiliar with golang and codebase, but i'll give it a shot. hopefully ppl will help.
hopefully this will finish #1149 for real at some point.

it looks like it's still not set up to send the eni in the request.
idk how to get request logs from the aws go sdk while it's being used by tf, but the sts message i got back is consistent - says I can't use default sg/subnet - it's not taking the eni's sg/subnet.

hard to say whether being in vpc affects it, but i'm not encouraged to set up in a personal aws account to test.

ima keep working on it.

[automaticgiant]

i'm pretty sure the important todo bits are in https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_instance.go#L1017 , but i'm not groking it all the way yet.

[automaticgiant]

all i have for now is that it seems to work. i think maybe i should add a forcenew for the new instance attribute. not super clear on whether my implementation method (if id set, skip auto stuff - subnet/publicip) is solid. and i guess there is a way to test that it does to aws what it should?

[Ninir]

Hi @automaticgiant
Thanks for the work here! There are other considerations to integrate on this: conflicts, . I made a work on this here and there is also #7096 for multiple ENIs.

I'll let Terraform team choose how to deal with all of this :)

[automaticgiant]

those both look more thorough than mine. i only needed a little bit more functionality. should we try to collaborate on these and turn them into one solid pr?

[Ninir]

@automaticgiant Which little functionality did you need? I think I missed it comparing our PRs :)
Let's collaborate, for sure!

[automaticgiant]

@Ninir the bit about exposing id from eni creation so you can create one and get the id from it to add to the new ec2 instance

[automaticgiant]

should we be organizing related PR conversation in the original issue or any particular place? i think buy-in/guidance/coordination help from core team would be really helpful. we need to get the right people connected and looking at these.
i was kindof selfish and just wanted to fix it for myself, but now i realized i don't want to maintain my own fork so i made a pr without checking existing work.
but we do need to work together and get these related features polished, tested, documented, and merged.
@catsby - i didn't mention mitchell, figured you'd be good first step

[automaticgiant]

#7096 #10593 #2998 #10244 and this (#10516) are what I could find that might need coordinated

[automaticgiant]

i almost want to use chef-provisioning, but it looks like it's on life-support, so hesitant. need this TF feature.

[Ninir]

@automaticgiant Ok I got it :) Instead of setting a new attribute, why not getting the id attribute directly?

For sure, this subject needs to be merged asap in order to prevent more and more PRs.
I would also want to add @stack72 to the reviewers/deciders here, and discuss how we can handle this in the better way.

If you want, I can work on the aggregation of everyone works! I might need to add a few acceptance tests :) this will close so much issues / PRs! 😄

[automaticgiant]

@Ninir i don't understand the question. could you link to a code line or do a line comment? i just made something that solved my specific problem, however naive it may have been.

do you think we should request an integration branch to pr pieces to?

[Ninir]

@automaticgiant Well, for instance, you can use aws_network_interface.test.id (i.e. the id attribute) which in your case is also the network_interface_id , since you alias it:

...

resource "aws_network_interface" "test" {
    subnet_id = "${aws_subnet.main.id}"
    private_ips = ["10.0.1.50"]
}
 
output "test" {
    value = "${aws_network_interface.test.id}"
}

Well, I think that we can put pieces together in 2 PRs: one for the network_interface_id addition, and one for network_interfaces for instance!

[automaticgiant]

oh! is that the same value? didn't realize that.

if we get HC to do a branch we can pr to, everyone gets credit. not sure how important that is, but don't really want to steal code from PRs without crediting them.

[Ninir]

Hi @automaticgiant

Yes it is the same value :)
Well, if we want to go for it, letting maintainers merge the most advanced /stable one and close related PRs/issues.

[automaticgiant]

@Ninir @freimer @pielu @choosy
would any of you mind pring your changes to my fork, or would any of you rather host this feature integration branch?

[pielu]

@automaticgiant I have pulled your branch and merged with mine, not sure how I can create PR against yours though. Here's the branch. Let me know if there's anything more I could do to move this forward :-)

[automaticgiant]

@pielu check it out yo automaticgiant#1

[pielu]

@automaticgiant I think it's sorted out now. Should you need more actions on my side please give me a shout.

[automaticgiant]

@Ninir @freimer @choosy
have two merged, mine and @pielu
pls check to see if yours can contribute.
volunteers for tests appreciated! i can start next week, but don't know what i'm doing :D

[freimer]

Last I checked they made significant changes that caused all changes to fail. Don't know when I'll have time to take a look at it again. Very busy...

…

Sent from my iPhone

On Jan 24, 2017, at 6:11 AM, Hunter Morgan ***@***.***> wrote: @Ninir @freimer @choosy have two merged, mine and @pielu pls check to see if yours can contribute. volunteers for tests appreciated! i can start next week, but don't know what i'm doing :D — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[grubernaut]

Closing via #12933

[ghost]

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.