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

[LIBCLOUD-716] Fix port type error #533

Closed
wants to merge 1 commit into from

Conversation

aviweit
Copy link
Contributor

@aviweit aviweit commented Jun 3, 2015

@Kami
Copy link
Member

Kami commented Jun 5, 2015

Thanks, the change looks good to me.

It looks like things would only break if the API or auth service was on a non-standard port, right?

@Kami
Copy link
Member

Kami commented Jun 5, 2015

On a related note - some tests would also be good.

@Kami
Copy link
Member

Kami commented Jun 5, 2015

Thinking more about it - this code has been like that since pretty much the beginning and I worked with many OpenStack installations which ran on a custom port and I never encountered any issues.

Curious if we have introduced a regression somewhere recently or there is something else going on (e.g. some Python versions don't accept a string).

@aviweit
Copy link
Contributor Author

aviweit commented Jun 5, 2015

My libcloud runs on the following python version:
Python 2.7.6 (default, Mar 22 2014, 22:59:56)
[GCC 4.8.2] on linux2

in an ubuntu 14.04 system. My devstack is from a latest version.

I am using the following snippet to run openstack operations which fail, as described in the above jira issue link.

from libcloud.compute.types import Provider
from libcloud.compute.providers import get_driver
OpenStack = get_driver(Provider.OPENSTACK)
driver = OpenStack('demo', '******',
                   ex_tenant_name='demo',
                   ex_force_auth_url='http://1.2.3.4:5000',
                   ex_force_auth_version='2.0_password')
print driver.list_sizes()

My python2.7 socket.py create_connection complains that port is not an int or str. Looking into this, I can see that it receives a port from unicode type and that is probably the reason it fails.

Adding traces in _tuple_from_url to print netloc and its type, I can see that netloc for the nova service is unicode.

1.2.3.4:5000, <type 'str'>
1.2.3.4:5000, <type 'str'>
1.2.3.4:8774, <type 'unicode'>

From looking into libcloud code it seems to me that port can be a string if it is fetched out from a url (_tuple_from_url).
It seems that libcloud -> openstack communication is done through urls: connecting into keystone first (passing keystone url through ex_force_auth_url) followed by connecting into the proper openstack service API (e.g. nova) which is retrieved from the catalog returned by the keystone service, which is a url as well. The later is parsed as a unicode which leads to a unicode port which again, probably leads to my socket.py failure.

Is that correct?

I would like to raise a question:
Looking into _tuple_from_url, I can see:

         if ":" in netloc:
             netloc, port = netloc.rsplit(":")
             port = port

I am curious whether the 2nd port assignment is somewhat redundant.
Perhaps port can be consistent and be set as an integer here, as I can see in _tuple_from_url that port can be set with integers of 80 or 443 (if not set before).

@aviweit
Copy link
Contributor Author

aviweit commented Jun 5, 2015

This one line change, fixed my issue and I had to adapt only three openstack tests (committed here) for the unit tests to pass.

@aviweit
Copy link
Contributor Author

aviweit commented Jun 6, 2015

Hi,
I noticed someone else reported the exact issue I have and suggested a very similar solution:
http://mail-archives.apache.org/mod_mbox/libcloud-dev/201504.mbox/%3CCAFvoK-0B_PXYL7RNJ2_+pc4wSYMN2TJizzueXLJgL7r9DG3iQw@mail.gmail.com%3E

@Kami
Copy link
Member

Kami commented Jun 6, 2015

@aviweit Thanks for the additional details and clarification.

I added some additional casting to int inside the connect method to make sure port is always an int there and merged PR into trunk. Thanks.

aviweit added a commit that referenced this pull request Jun 6, 2015
Closes #533

Signed-off-by: Tomaz Muraus <tomaz@tomaz.me>
@aviweit
Copy link
Contributor Author

aviweit commented Jun 7, 2015

The merged fix works for me. I tested with libcloud managing OpenStack, Amazon and Softlayer.

I am closing this PR. Thanks.

@aviweit aviweit closed this Jun 7, 2015
@Kami
Copy link
Member

Kami commented Jun 7, 2015

Great. Thanks.

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