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

resource/aws_route: Support changing network_interface_id #2353

Closed
wants to merge 2 commits into from

Conversation

raylu
Copy link

@raylu raylu commented Nov 17, 2017

network_interface_id and instance_id are both optional and computed. If you specify one, the other is computed by resourceAwsRouteSetResourceData.

hashicorp/terraform#7686 made it so that if you specified instance_id, applied, changed the instance_id, and applied again, we would ignore the computed network_interface_id (an ENI on the old instance) and update the instance_id correctly. This broke the opposite case (specify network_interface_id, apply, change, apply again).

This PR checks which has changed instead of preferring one over the other, fixing #2270. It depends on hashicorp/terraform#16693.

The fix in hashicorp/terraform#7686 causes the
exact same bug in reverse (if you specify network_interface_id in
config, apply, and then change the network_interface_id, it will apply
the old instance_id, which is a no-op).

This checks which has changed from the instance state, allowing us to
change either network_interface_id or instance_id correctly.
@Ninir Ninir added the bug Addresses a defect in current functionality. label Nov 22, 2017
@radeksimko radeksimko added the service/ec2 Issues and PRs that pertain to the ec2 service. label Jan 16, 2018
@radeksimko radeksimko changed the title r/aws_route: Support changing network_interface_id resource/aws_route: Support changing network_interface_id Jan 16, 2018
@bcornils
Copy link
Contributor

looks like we have a check failing. Has anyone had time to assess or review criticality of the failure?

@bflad
Copy link
Contributor

bflad commented Nov 8, 2018

Hi folks 👋 It looks like this old PR has merge conflicts, fails TravisCI testing, and hasn't seen an update in a long time since last asked so I'm going to close it. If there is interest in this still, please feel free to create a new PR and we'll review it. 👍

@bflad bflad closed this Nov 8, 2018
@raylu
Copy link
Author

raylu commented Nov 8, 2018

sorry for missing bcornils' message from june. as mentioned in the description, this PR depends on hashicorp/terraform#16693 (which introduces TestResourceDataStateRaw which I use here)

@raylu
Copy link
Author

raylu commented Apr 25, 2019

@bflad / @bcornils could I get this re-opened? alternatively, could I get eyes on hashicorp/terraform#16693 ?

@ghost
Copy link

ghost commented Mar 30, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants