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

aws:Allow reassignment of EIP instances appropriately #7686

Merged
merged 1 commit into from
Oct 11, 2016

Conversation

jrnt30
Copy link
Contributor

@jrnt30 jrnt30 commented Jul 18, 2016

Recreated from PR #6682 because I fubared the merge and it was easier just to cherry-pick

fixes #6076

As atward described, when using the instance_id attribute for the association, in the background AWS is also assigning the network_interface_id. The current way of checking would essentially eliminate any possibility of reallocating the route to a new instance id due to the numTargets check always returning 2 in that case and fast failing.

Update order of precedence to ensure that allowedTargets is evaluated so that instance_id comes after network_interface_id to allow for check
If there are multiple but they are instance_id and network_interface_id is the other let AWS handle the network_interface_id
NOTE: One thing I would have liked to add a test for but had a family emergency come up is tainting the original instance and reapplying. I did not see any direct example of doing this and was considering simply standing up 2 instances in the test and running a re-apply to associate the 2nd instance. If I get a chance before this is merged I will try and do so however I do not have cycles immediately.

@liquid-sky
Copy link

Would it be possible to have this fix merged? This issue blocks us from using the latest features of terraform and we are stuck with 0.6.12.

@catsby
Copy link
Contributor

catsby commented Oct 11, 2016

Thank you @jrnt30 – I ran the tests and this seems to checkout!

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRoute_ -timeout 120m
=== RUN   TestAccAWSRoute_basic
--- PASS: TestAccAWSRoute_basic (27.85s)
=== RUN   TestAccAWSRoute_changeCidr
--- PASS: TestAccAWSRoute_changeCidr (38.93s)
=== RUN   TestAccAWSRoute_noopdiff
--- PASS: TestAccAWSRoute_noopdiff (205.06s)
=== RUN   TestAccAWSRoute_doesNotCrashWithVPCEndpoint
--- PASS: TestAccAWSRoute_doesNotCrashWithVPCEndpoint (30.87s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    302.740s
Test:

@catsby catsby merged commit be523d3 into hashicorp:master Oct 11, 2016
raylu pushed a commit to raylu/terraform-provider-aws that referenced this pull request Nov 17, 2017
raylu pushed a commit to raylu/terraform-provider-aws that referenced this pull request Nov 17, 2017
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.
@ghost
Copy link

ghost commented Apr 21, 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 21, 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.

aws_route Error: more than 1 target specified.
4 participants