Cinder microversion 3.40 not matching 3.27, treated as 3.4

Bug #1705093 reported by Steve Noyes
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
python-cinderclient
Fix Released
Undecided
Unassigned

Bug Description

Using change in https://review.openstack.org/#/c/330285/:

Using the above new Cinder v3.27 API change, nova (in nova.volume.cinder.API.attachment_create()) is creating a client via:

attachment_ref = cinderclient(context, '3.27').attachments.create(
                volume_id, connector, instance_id)

This is throwing an exception CinderAPIVersionNotAvailable. It's not a very visible error, because the code just retries using the cinder v2 api.

The problem is in python-cinderclient.cinderclient.api_versions. The current max version is:

line 32: MAX_VERSION = "3.40"

I believe this is happening because the max version is getting interpreted as 3.4, not 3.40 (the end zero is getting dropped) and when that is compared to 3.27, the match is failing.

Some detail:

nova/volume/cinder.check_microversion:
(Pdb) url
u'http://192.168.0.181:8776/v3/f20f94c164584263b88a1a55020276cc'
(Pdb) microversion
'3.27
max_api_version = cinder_client.get_highest_client_server_version(url)
(Pdb) max_api_version
3.4
max_api_version = cinder_api_versions.APIVersion(str(max_api_version))
(Pdb) max_api_version
<APIVersion: 3.4>
(Pdb) max_api_version.ver_minor
4

Revision history for this message
Steve Noyes (steve-noyes) wrote :

it's definitely a cinder client issue. I changed MAX_VERSION to "3.4" (from "3.40") and cinder_client.get_highest_client_server_version(url) still returns a float 3.4.

Revision history for this message
Steve Noyes (steve-noyes) wrote :

$ git show
commit 23c452ce1a7d7cf772bad725247b7aa71e7062ac
Merge: 6a00d30 d11b105
Author: Jenkins <email address hidden>
Date: Wed Jul 12 17:59:19 2017 +0000

Revision history for this message
Scott DAngelo (scott-dangelo) wrote :

Yes, there are two ways to fix this:
1) pass a string back from cinderclient.get_highest_client_server_version() and change Nova to use that instead of the float.
https://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L71 would change from:
- max_api_version = cinder_client.get_highest_client_server_version(url)
- # get_highest_client_server_version returns a float which we need to cast
- # to a str and create an APIVersion object to do our version comparison.
- max_api_version = cinder_api_versions.APIVersion(str(max_api_version))
to something like:
+ max_api_version = cinder_client.get_highest_client_server_version(url)
+ max_api_version = cinder_api_versions.APIVersion(max_api_version)
(maybe do in all one line, but would be >80 char, etc...

2) Change the logic in cinderclient to look at the number of decimal places in both versions, and do all manner of gymnastics to return properly, given version 3.400 > 3.40 > 3.4 .
https://github.com/openstack/python-cinderclient/blob/master/cinderclient/client.py#L98

Revision history for this message
Gorka Eguileor (gorka) wrote :

In my opinion this should be fixed in Cinder client, I'll post a patch to fix it.

Changed in python-cinderclient:
assignee: nobody → Gorka Eguileor (gorka)
Revision history for this message
Gorka Eguileor (gorka) wrote :

I have the fix for this, but I need to fix our unit tests before submitting it: https://bugs.launchpad.net/python-cinderclient/+bug/1705249

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to python-cinderclient (master)

Fix proposed to branch: master
Review: https://review.openstack.org/485198

Changed in python-cinderclient:
status: New → In Progress
Revision history for this message
Gorka Eguileor (gorka) wrote :

Now method returns a string instead of a float, so there will be no longer a need to do a string conversion in Nova: `max_api_version = cinder_api_versions.APIVersion(str(max_api_version))` once the next cinderclient version becomes the minimum required version.

Changed in python-cinderclient:
assignee: Gorka Eguileor (gorka) → Steve Noyes (steve-noyes)
assignee: Steve Noyes (steve-noyes) → nobody
Revision history for this message
Steve Noyes (steve-noyes) wrote :

reassigned bug by mistake.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-cinderclient (master)

Reviewed: https://review.openstack.org/485198
Committed: https://git.openstack.org/cgit/openstack/python-cinderclient/commit/?id=52cc5c6cb3856dcddd455122742a5bf8de0fe834
Submitter: Jenkins
Branch: master

commit 52cc5c6cb3856dcddd455122742a5bf8de0fe834
Author: Gorka Eguileor <email address hidden>
Date: Wed Jul 19 14:45:13 2017 +0200

    Fix highest version supported by client and server

    Current code mistakenly thinks that 3.40 is older than 3.27, because
    it's treating 3.40 as 3.4, thus returning the wrong maximum supported
    version by both server and client.

    It is also returning a 3.40 version as 3.4 because it's returning it as
    a float.

    This patch fixes both issues by not using float conversion but using the
    APIVersion object to do the comparison and by changing returned type to
    a string so it can be used to instantiate a client.

    Change-Id: Ica4d718b3de31c31da047f07c5154b242e122596
    Closes-Bug: #1705093

Changed in python-cinderclient:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to python-cinderclient (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/485805

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on python-cinderclient (master)

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.openstack.org/485805
Reason: I've got it sorted out now. The problem was never that the cinderclient method was returning the wrong thing, it was that when Nova cast it to a str, it turned a 3.40 float into a 3.4 string:

http://paste.openstack.org/show/616084/

So the commit message in Gorka's change was really confusing because it wasn't the cinderclient code's fault really. So I'll drop this and just push a release note for the API change.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to python-cinderclient (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/485817

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to python-cinderclient (master)

Reviewed: https://review.openstack.org/485817
Committed: https://git.openstack.org/cgit/openstack/python-cinderclient/commit/?id=72671fffe51898446c8671e122cd6ef11171fdb1
Submitter: Jenkins
Branch: master

commit 72671fffe51898446c8671e122cd6ef11171fdb1
Author: Matt Riedemann <email address hidden>
Date: Thu Jul 20 16:46:07 2017 -0400

    Add release note for get_highest_client_server_version return type change

    Change Ica4d718b3de31c31da047f07c5154b242e122596 changed the return
    type on the cinderclient.client.get_highest_client_server_version
    method from a float to a str. Since it's a public API we should
    document the change with a release note.

    Change-Id: I197c80ef657156261ecbf51cee6300268438b639
    Related-Bug: #1705093

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/python-cinderclient 3.0.0

This issue was fixed in the openstack/python-cinderclient 3.0.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.