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: Fix issue changing EIP Association #6521

Merged
merged 3 commits into from
May 6, 2016

Conversation

catsby
Copy link
Contributor

@catsby catsby commented May 6, 2016

Fix issue when moving an EIP from one Instance to another.
Currently, we provide the PrivateIpAddress in the AssociateAddress call, but we're sending the private IP of the current instance, not the instance we're targeting. Since private_ip is computed, on association we get the PrivateIp of the first Instance here, which incorrectly get's thrown in with the request.

Fixes #6436 and failing test TestAccAWSEIP_instance

Update

Chatted with @phinze about the possible/likely conflict with #6070 and we've got a plan for going forward. I will re-work this PR and update soon

More Update:

I've implemented what @phinze and I discussed:

  • remove private_ip as a user specified value: this value is once again computed and reflects the current private ip
  • added associate_with_private_ip: this is a private IP you designate to associate the EIP with.

Private IP is a computed attribute, meaning it will pick up it's value from the Instance or Network Interface that the EIP is associated with. The associate_with_private_ip allows the users to specify what value the EIP should take. Without distinguishing between the two cases, we're unable to move the EIP from Instance to Instance (see #6436 and TestAccAWSEIP_instance test)

@catsby catsby changed the title [WIP] provider/aws: Fix issue changing EIP Association provider/aws: Fix issue changing EIP Association May 6, 2016
@catsby
Copy link
Contributor Author

catsby commented May 6, 2016

Tests are green:

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEIP_ -timeout 120m
=== RUN   TestAccAWSEIP_basic
--- PASS: TestAccAWSEIP_basic (10.44s)
=== RUN   TestAccAWSEIP_instance
--- PASS: TestAccAWSEIP_instance (156.02s)
=== RUN   TestAccAWSEIP_network_interface
--- PASS: TestAccAWSEIP_network_interface (32.28s)
=== RUN   TestAccAWSEIP_twoEIPsOneNetworkInterface
--- PASS: TestAccAWSEIP_twoEIPsOneNetworkInterface (32.19s)
=== RUN   TestAccAWSEIP_associated_user_private_ip
--- PASS: TestAccAWSEIP_associated_user_private_ip (143.78s)
PASS
ok      github.com/has

@catsby catsby mentioned this pull request May 6, 2016
@phinze
Copy link
Contributor

phinze commented May 6, 2016

LGTM!

@catsby catsby merged commit 99e0aec into master May 6, 2016
@catsby catsby deleted the b-aws-eip-reassociate branch May 6, 2016 20:38
cristicalin pushed a commit to cristicalin/terraform that referenced this pull request May 24, 2016
provider/aws: Update EIP to use new associate_with_private_ip instead of private_ip
@ghost
Copy link

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

EIP reallocation failure
2 participants