Skip to content

openstack: put floating ips in public_ips#291

Closed
bouttier wants to merge 1 commit intoapache:trunkfrom
bouttier:trunk
Closed

openstack: put floating ips in public_ips#291
bouttier wants to merge 1 commit intoapache:trunkfrom
bouttier:trunk

Conversation

@bouttier
Copy link
Copy Markdown

This patch use OS-EXT-IPS:type introduced in Nova 2013.1 ("Grizzly") but stay compatible with older version.

This is useful in testing environment where floating ips can be private but we want see them as public.
I'm not sure that you want that merged but I think it could be nice so I propose ...

This patch use OS-EXT-IPS:type introduced in Nova 2013.1 ("Grizzly").
@Kami
Copy link
Copy Markdown
Member

Kami commented May 12, 2014

@bouttier Looks good, but can you please add some test cases for this change (OS-EXT-IPS:type not being present, OS-EXT-IPS:type being set to floating, OS-EXT-IPS:type being set to a value different than floating)?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the other possible values for this attribute besides floating?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bouttier
Copy link
Copy Markdown
Author

I'm not sure of how add test cases, theses values depends of openstack version (rackspace used in tests files apparently). I think floating and fixed cases are already tested by existing tests ; if OS-EXT-IPS:type is not set, I use get function so ip_type = None != 'floating'.

@Kami
Copy link
Copy Markdown
Member

Kami commented May 12, 2014

@bouttier For testing we use response fixtures - you can find a bunch of examples in libcloud/test/compute/.

@Kami
Copy link
Copy Markdown
Member

Kami commented May 13, 2014

@bouttier Actually, I just checked at your pull description and OpenStack docs again and I don't actually think we want this in trunk - why would you want to see private IP as public (it seems like this should be handled in the code which uses Libcloud and not in the Libcloud itself)?

Also, addresses should already be correctly split in the "private" and "public" attribute.

@bouttier
Copy link
Copy Markdown
Author

In my case, I had an issue with salt-cloud who want remove the public key from .ssh/know_host on vm deletion. For that, salt-cloud search the first public ip (and I used an old version of libcloud that I fix in my way). I understand your point of view and I will search an other way in salt-cloud to select the right ip.

@bouttier bouttier closed this May 13, 2014
@Kami
Copy link
Copy Markdown
Member

Kami commented May 14, 2014

@bouttier One thing we could do to make this easier for you it to store information about floating and fixed IP addresses in the Node extra attribute.

@bouttier
Copy link
Copy Markdown
Author

I haven't had time to find the best way to modify salt yet, but yes this can be useful, and not only for me apparently (#296).

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.

2 participants