Skip to content

Floating ip support#14

Merged
hartmantis merged 7 commits intotest-kitchen:masterfrom
hufman:floating_ip
Oct 1, 2013
Merged

Floating ip support#14
hartmantis merged 7 commits intotest-kitchen:masterfrom
hufman:floating_ip

Conversation

@hufman
Copy link
Copy Markdown
Contributor

@hufman hufman commented Sep 9, 2013

Our Openstack deployment does not allow access by default, so this code tells kitchen-openstack to associate a floating IP address automatically.

@jgawor
Copy link
Copy Markdown
Contributor

jgawor commented Sep 27, 2013

We need that as well. Please consider merging this in.

@ramonskie
Copy link
Copy Markdown

running in to the same problem

@hartmantis
Copy link
Copy Markdown
Contributor

@hufman Sorry for letting these sit for so long. The OpenStack environments I normally test against don't support some of these features and DevStack is being annoying. A couple of questions but, otherwise, I love the idea of supporting this...

The linter this project uses is unhappy about a few style things in the PR. If you have the time, would it be possible to adjust those and add RSpec tests for the new functionality? If you're busy with other things now, let me know; I should have some time I could work on it this weekend.

Anyone with more experience with OpenStack's floating IPs than I (read: not a lot), what do you expect as a starting point when you want to assign a floating IP to a server? The couple people I spoke to said they'd have a pool with IPs already allocated to it, and not expect a driver like this to do the allocation for them.

If falling back to assigning a new IP to the pool is the appropriate behavior, should there be a way for teardown of the server to pull it back out of the pool?

Thanks!

@hartmantis hartmantis mentioned this pull request Sep 27, 2013
@ramonskie
Copy link
Copy Markdown

the floating ip gets assigned when the vm is up and running.
see vagrant-openstack plugin
https://github.com/cloudbau/vagrant-openstack-plugin/blob/master/lib/vagrant-openstack-plugin/action/create_server.rb#L86

and when the vm is destroyed the the floating ip gets released to the pool of the user

so the user should retrieve the floating ip
and then needs to put that floating ip in to the config file

@hufman
Copy link
Copy Markdown
Contributor Author

hufman commented Sep 30, 2013

When I was first making this ability, it was easier to allocate a new IP address all the time. Then I had to add support to reuse the old IP addresses. I removed that section of the code, which should hopefully bring the complexity down to the required level.
I don't know how to write RSpec tests.

@hartmantis
Copy link
Copy Markdown
Contributor

@ramonskie Is that specifically the feature you require, or just an observation of how others are handling floating IPs?

This change allows you to provide the name of an IP pool, but not request a specific IP address. That feature can certainly be added separately, though, if people need it too.

@ramonskie
Copy link
Copy Markdown

just a observation

@hartmantis
Copy link
Copy Markdown
Contributor

@ramonskie Cool, cool. Thank you.

@hufman
Copy link
Copy Markdown
Contributor Author

hufman commented Sep 30, 2013

I don't see how else I can reduce the complexity of the attach_ip function, I have that loop in there to try to avoid any race conditions that may happen if an IP is stolen away between the time it lists the available IPs and try to allocate one.

@hartmantis
Copy link
Copy Markdown
Contributor

@hufman Gotcha. Thanks again for your work on both this and #15. I'll try to beat the build back into line and get these both merged today.

hartmantis added a commit that referenced this pull request Oct 1, 2013
@hartmantis hartmantis merged commit ae067f1 into test-kitchen:master Oct 1, 2013
@hufman hufman deleted the floating_ip branch October 15, 2013 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants