Skip to content
This repository was archived by the owner on Feb 11, 2022. It is now read-only.

Added support for config parameters elastic_ip => '<ip>', allocate_elast...#129

Open
madhurranjan wants to merge 22 commits into
mitchellh:masterfrom
madhurranjan:master
Open

Added support for config parameters elastic_ip => '<ip>', allocate_elast...#129
madhurranjan wants to merge 22 commits into
mitchellh:masterfrom
madhurranjan:master

Conversation

@madhurranjan

Copy link
Copy Markdown

We needed support to add elastic ips to instances on AWS. I've added a couple of config params:

  1. elastic_ip : 'ip' - user explicitly specifies the elastic ip to be associated with the instance
  2. allocate_elastic_ip : 'standard/vpc' - since these are the 2 options. This allocates a new IP and associates it with the instance.

Please let me know your thoughts. I am ready to modify these to get my pull request accepted by such an awesome project!
Thanks

@dldinternet

Copy link
Copy Markdown

Does this duplicate #65 (comment) ?

@madhurranjan

Copy link
Copy Markdown
Author

Damn . I didn't see this earlier . Yes it looks like it but approach is different. Also mine seems to cover most of the cases that the users have pointed out there . But I am guessing its only fair that the earlier commit be merged.

@dldinternet

Copy link
Copy Markdown

Hi,
Is there opportunity to combine the best of both solutions?

@tralamazza

Copy link
Copy Markdown
Collaborator

Yup I was thinking about merging both ;)
I will work tonight on some merges.

@madhurranjan

Copy link
Copy Markdown
Author

@tralamazza - Great. Let me know if you need anything from my side. I can help in anyway you need.

@bwhaley

bwhaley commented Aug 28, 2013

Copy link
Copy Markdown
Contributor

Hmm, it seems to me that this are in direct conflict. Both cannot be merged since they provide the same functionality in slightly different ways. I think my PR #65 should be merged, then the code to just associate an EIP from this PR could be added in a new PR.

As an aside, neither PR supports Amazon's new feature (within the past week or so) of assigning a public IP in a VPC during launch. I was thinking of adding this functionality as another PR after #65 was merged, but since it has been more than 4 months since I first wrote it I wasn't too keen on getting it done quickly.

@madhurranjan

Copy link
Copy Markdown
Author

Fair enough. I had one observation an hour back while working with the VM creation . If we create VMs in parallel and create new Elastic ips during this process( I was trying to create 2 to 4 VMs in parallel) , there seems to be a synchronization issue in Fog and the machine creation fails as terminate is called. However, if you pre-create the Elastic IPs and just do the association bit as part of VM creation things work fine ( This is by associating values directly with elastic_ip: "" . I plan to look at this issue but so far the latter approach has worked and the earlier approach has failed multiple times.

@podollb

podollb commented Oct 31, 2013

Copy link
Copy Markdown

So, just so I'm clear, is there currently on-going work in the form of a pull request that will soon be merged into the main code to allow associating a pre-allocated elastic IP to the VM rather than just the boolean flag that allocates and assigns one to the VM (as discussed in this issue)?

@madhurranjan

Copy link
Copy Markdown
Author

Hi,

I don't think that's going to happen. However, if you need the functionality , you can use this commit here - madhurranjan@5ef9f1f .

@maximmi

maximmi commented Dec 3, 2013

Copy link
Copy Markdown

Want to see this in main rep too

@goruha

goruha commented Dec 3, 2013

Copy link
Copy Markdown

subscribing

@dawogfather

Copy link
Copy Markdown

+1

@moodytux

moodytux commented Dec 7, 2013

Copy link
Copy Markdown

+1 for merging in madhurranjan@5ef9f1f . I have my elastic IP already allocated (and it needs to stay that way), so the ability to associate it without creating a new EIP is just what I've been looking for.

@andybons

Copy link
Copy Markdown

+1 Subscribe.

@fairchild

Copy link
Copy Markdown

this would be very handy. Reading thru the code, it looks like the elastic ip is always released on vagrant destroy.
I think it would be preferable if the ip was only dissociated from the destroyed instance, but not released. I might have read the code wrong, but that was what it looked like it would do from a quick skim.

@madhurranjan

Copy link
Copy Markdown
Author

The Ip isn't released . Its just disassociated - madhurranjan@5ef9f1f#diff-aa52ade045461d146cf02496d14c9371R22 . If you need any change around that I can do it but don't think this is going to be merged.

@goruha

goruha commented Jan 9, 2014

Copy link
Copy Markdown

We he the same cases with @mark-rushakoff
We created fork https://github.com/maximmi/vagrant-aws/tree/EIP and fix there a few bugs.
@madhurranjan let's merge our branches and continue develop together.

I suppose that @mitchellh do not merge this pull request, because there are a lot of bugs.
For example it does not work with cases when VM is halt and up (we fixed it for elastic ip that is not allocated).
We have errors on vagrant destroy.
From your last commit, I got that we do not have point if elastic ip should be released or just dissociated.

and so on.

@goruha

goruha commented Jan 9, 2014

Copy link
Copy Markdown

@mitchellh Please confirm that you will merge this pull request when we will have clean code and most bugs will be fixed.

Thanks.

@goruha

goruha commented Jan 9, 2014

Copy link
Copy Markdown

I suggest such behaviour.

Allocated IP

on up: Should be allocated and associated.
on halt: IP should be dissociated
on resume: IP should be associated
on destroy: IP should be dissociated and released.

Associte existed elastic IP

on up: Should be associated.
on halt: IP should be dissociated
on resume: IP should be associated
on destroy: IP should be dissociated

What do you think about?

@goruha

goruha commented Jan 9, 2014

Copy link
Copy Markdown

Also I suppose we need to do re-factoring to move IP related code to separated action class from "run_instance". And reuse the same code with "start_instance".

@fairchild

Copy link
Copy Markdown

I actually do not see the purpose of destroying an elastic ip. I view the purpose of an elastic up being to give long term consistency. If yu do nt need a persistent ip, thn there is no need t allocate, associat, dissociate and destroy- just don't use elastic ip. Is there a use case where this is a valuable flow, giving benefit over just using a normal ec2 ip?

~Michael
Sent from my iPad

On Jan 9, 2014, at 4:49 AM, Igor Rodionov [email protected] wrote:

I suggest such behaviour.

Allocated IP

on up: Should be allocated and associated.
on halt: IP should be dissociated
on resume: IP should be associated
on destroy: IP should be dissociated and released.

Associte existed elastic IP

on up: Should be associated.
on halt: IP should be dissociated
on resume: IP should be associated
on destroy: IP should be dissociated

What do you think about?


Reply to this email directly or view it on GitHub.

@madhurranjan

Copy link
Copy Markdown
Author

@goruha - I am fine to refactor and include the changes you've suggested and am fine to merge with your code. My use case was/is to spawn off environments , run tests and bring them down after a couple of days, so the code I had initially written suited that perfectly but obviously there are other use cases and I am happy to contribute.

@goruha

goruha commented Jan 9, 2014

Copy link
Copy Markdown

@fairchild we use vagrant to up instances live develop server and run provision on it.
This is good idea for me to have the same workflow for development and manage dev, stage and prod environments, or any other infrastructure servers.
Like vagrant up (test you local copy) and vagrant up --provider=aws (up real server for team use).
But we need the same, static ip to use DNS for that.

@mathomas

Copy link
Copy Markdown

First, thanks to the author and contributors for this great plugin. It has been a real lifesaver as I cycle through different AWS deployment strategies.

That said a big, huge +1 for being able to specify a pre-allocated EIP. I have to admit I assumed that's how this feature would work from the start, and was momentarily confused when it didn't do what I thought it would.

What is the benefit of the feature as it stands (dynamically-created EIP that changes with each "up/destroy" cycle)? I feel I could be missing something important.

Thanks again...

@loginx

loginx commented Apr 14, 2014

Copy link
Copy Markdown

👍

@tonglil

tonglil commented May 14, 2014

Copy link
Copy Markdown

I thought the use case for EIPs is that a domain name is pointed at an EIP that gets associated and dissociated with the instances that get spun up and down?
If a new ip is associated, isn't it the same as using the public IP for the instance?
Being able to set a specific EIP would be greatly appreciated!

@fonsecas72

Copy link
Copy Markdown

👍

@warrenseine

Copy link
Copy Markdown

Agreed with @mathomas, I also assumed I could specified a pre-allocated Elastic IP, and this is apparently not the case. I'm sorry I can't contribute the feature.

@dkinzer

dkinzer commented Aug 26, 2014

Copy link
Copy Markdown

This should have been the elastic_ip solution to get merged. A production environment needs to be able to generate the same IP address and the other solution just doesn't provide that.

@jasiedu

jasiedu commented May 3, 2015

Copy link
Copy Markdown

Is this feature still under consideration?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.