-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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 network_interface to aws_instance #12933
Conversation
All instance tests.
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Special thanks to the many authors and contributors to the multiple pull requests and issues referenced here. Thank you for your help and patience here while we sorted this out. You are all awesome ❤️ |
Test output after adding private_ip_addresses:
The two failing tests are failing for unrelated causes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 question within on updating - also, do we need to replicate this for spot_instances / spot_fleet_requests
Set: func(v interface{}) int { | ||
var buf bytes.Buffer | ||
m := v.(map[string]interface{}) | ||
buf.WriteString(fmt.Sprintf("%d-", m["device_index"].(int))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, so the only thing that can cause an update here is a change to device_index
?
Thanks for this. I would like to see this functionality merged. I am not sure what your expectations are, but I did some manual tests and the behavior did not align with my expectations. Change Primary Network Interface
Add An Additional Network Interface
Change a Secondary Network Interface
Remove a Secondary Network Interface
|
Another observation. The primary network interface (eth0, device_index=0) is different from the other network interfaces in AWS. It can't be changed without instance replacement. Secondary network interfaces can be added or removed without recreating the instances. This is similar to EBS volumes. The root EBS volume can't be detached from an instance, while other volumes can. The Terraform configuration parameters for Can we not differentiate between the primary and secondary network interfaces in the Terraform configuration parameters? We could use the existing network interface configuration parameters ( Or would you prefer the semantics to be that any changes to |
Hey @peteromoon, thanks for the observations! This is still very much a WIP and a lot of things still need to be fleshed out before it's ready-to-go.
In response to your second comment: The PR spawned from multiple issues on users not being able to replace the root network interface with one of their own choosing. The AWS API (and subsequently the SDK) don't differentiate between the primary network interface (eth0), and any secondary network interfaces. That is definitely good information to have though, and we'll be discussing this internally to come up with the best solution for this. I'll also update the title of the PR to |
e1f39ee
to
10ddf60
Compare
Great addition - Great work on getting this merged! |
I've upgraded terraform to 0.9.4 and all my spot instances are going to be recreated:
How can I fix that? |
Hey @epetrovich, apologies for this! Would you mind creating a separate issue to track this, and I'll get to fixing this as soon as possible. Thanks! |
Hey @epetrovich, I'm getting a
|
@grubernaut Thank for your attention. lifecycle { |
I am having an issue with specifying How to get around this? My main purpose is to tag network interfaces via tf |
@cannonba11 from what you've written it seems that you want to use a network interface which was created elsewhere and yet have your instance resource delete it on termination, which doesn't make sense - either you've created it elsewhere so it needs to be managed elsewhere, or you're creating it as part of instance creation, and hence want to delete it on termination? |
@edmundcraske
If I don't add the network_interface inside the runtime-instance block, then the instance comes up with an interface without tags. The only way I can have a tagged network interface come up with this instance is how I have it above (correct me if I am wrong). But this throws an error as delete_on_termination is set to true. If I remove |
@cannonba11 you need to remove the ‘delete_on_termination’ setting for the network interface - logically how can terraform delete it as part of the instance if it is actually managed under its own separate resource? What are you hoping to achieve by having it set? It is unnecessary and would cause an inconsistency by trying to manage the existence of the network interface in more than one place. |
@edmundcraske - Thanks for the explanation, I understand what you are saying but say someone terminates the instance from the aws console, the network interface will still be lying around. It will only be deleted via a |
That’s not really a terraform problem though - if you let AWS create a network interface for you, it doesn’t let you specify tags for the network interface, or tell you what the network interface ID is, you have to go and make separate calls to do that. |
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. |
Adds the
network_interface
schema object toaws_instance
resources.Would preferably want to remove the custom
Set
function, but without the hash ondevice_index
, we incur state diffs on every apply.Fixes: #2989, #2998, #3105, #1557, #5765
Closes: #12694, #10244, #7096, #10516
Related to: #4231, #1149, #10593, #3205