| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-12 | |||
| 21:50:10 | dansmith | rpodolyaka: seems like enginefacade could manage this for us.. automatically doing savepoints or something, if we nest calls | |
| 21:50:14 | mriedem | ildikov: yeah so the "OpenStack-API-Version" header isn't in the paste from the nova-api call | |
| 21:50:29 | dansmith | rpodolyaka: but regardless, gotta handle it for now I guess | |
| 21:50:37 | mriedem | ildikov: but i don't think nova is supposed to be picking the microversion is it, cinderclient is, at least by default it would be 3.0 | |
| 21:51:04 | mriedem | ildikov: is there a difference in how cinderclient's shell creates the client object and makes requests for v3 vs how nova is doing it? | |
| 21:51:28 | mriedem | smcginnis: cinder is up to microversion 3.24 already? | |
| 21:51:39 | smcginnis | mriedem: Sounds right. | |
| 21:51:40 | rpodolyaka | dansmith: yeah, it's just that we do not create a SAVEPOINT for each nested call - so we've got a modifier - https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/enginefacade.py#L860 | |
| 21:51:53 | rpodolyaka | which should be used in such cases | |
| 21:51:55 | mriedem | smcginnis: where do you guys document your REST API version history? | |
| 21:52:02 | mriedem | not finding it here http://docs.openstack.org/developer/cinder/ | |
| 21:52:05 | rpodolyaka | although, we don't probably use it anywhere in nova | |
| 21:52:06 | smcginnis | Document? | |
| 21:52:09 | ildikov | mriedem: that's an amazing question, I have no clue :) | |
| 21:52:14 | smcginnis | mriedem: Kidding. One moment. | |
| 21:52:23 | mriedem | smcginnis: i guess here http://docs.openstack.org/developer/cinder/devref/api_microversion_history.html | |
| 21:52:35 | smcginnis | mriedem: Beat me to it. | |
| 21:52:48 | ildikov | mriedem: it's 3.23, I test jgriffith's new attach API patch, which adds the 3.24 | |
| 21:53:01 | jgriffith | 3.24 | |
| 21:53:06 | jgriffith | as of an hour ago | |
| 21:53:19 | melwitt | rpodolyaka: there are use of the savepoint modifier in nova/db/sqlalchemy/api for several things that catch/expect DBDuplicateEntry | |
| 21:53:31 | mriedem | ildikov: so my guess is, | |
| 21:53:42 | rpodolyaka | melwitt: ah, then I missed those | |
| 21:53:57 | mriedem | the cinderclient shell code does some version discovery with the cinder api and gets the latest available microversion from the server and negotiates that with what the client understands, | |
| 21:54:03 | mriedem | that's how novaclient works | |
| 21:54:45 | dansmith | rpodolyaka: cool, thanks | |
| 21:54:49 | mriedem | and right now the max version in cinderclient is 3.15 i guess? https://github.com/openstack/python-cinderclient/blob/master/cinderclient/api_versions.py#L32 | |
| 21:54:50 | rpodolyaka | np! | |
| 21:54:53 | ildikov | mriedem: that sounds reasonable | |
| 21:54:56 | mriedem | not sure how that works if you're getting it to use 3.24 | |
| 21:55:28 | ildikov | mriedem: I'm setting OS_VOLUME_API_VERSION=3.24 | |
| 21:56:07 | ildikov | mriedem: that's the only magic on the Cinder side to make it work | |
| 21:56:53 | rpodolyaka | dansmith: lol :) | |
| 21:57:56 | mriedem | wtf where is cinderclient.API_MIN_VERSION set? | |
| 21:58:08 | mriedem | and cinderclient.API_MAX_VERSION | |
| 21:59:40 | ildikov | mriedem: you're checking the cinderclient code now? | |
| 21:59:46 | stewie925 | hello guys | |
| 22:00:08 | mriedem | ildikov: yes | |
| 22:00:57 | ildikov | mriedem: ok, I've just started with that, maybe scottda can help us | |
| 22:01:28 | stewie925 | I'm setting up openstack (controller, compute) and I am following the setup steps to set up the nova compute component on the controller node. | |
| 22:02:04 | stewie925 | so far so good... but when the step calls for 'service nova-api restart' I am getting a 'nova-api:unrecognized service' | |
| 22:02:39 | mriedem | ildikov: if api_version and api_version.ver_minor != 0: | |
| 22:02:40 | mriedem | headers["OpenStack-API-Version"] = "volume " + api_version.get_string() | |
| 22:02:55 | mriedem | nova is constructing a base cinderclient.v3.client.Client class | |
| 22:03:07 | mriedem | which from the entry point is going to just have api_version set to '3.0' | |
| 22:03:17 | mriedem | so looking at that cinderclient code, it's not going to send the header for 3.0 | |
| 22:03:19 | mriedem | and kaboom | |
| 22:04:08 | mriedem | ildikov: so nova is going to create the client object using this https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/client.py#L711 | |
| 22:04:35 | mriedem | actually, | |
| 22:04:41 | mriedem | nova would be using this https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/client.py#L656 | |
| 22:04:48 | scottda | ildikov: are you using this patch? https://review.openstack.org/#/c/385682/3/nova/volume/cinder.py | |
| 22:04:54 | mriedem | ildikov: note that does " return importutils.import_class(client_path)" | |
| 22:05:01 | mriedem | which doesn't pass in the api_version kwarg | |
| 22:05:15 | mriedem | compared to https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/client.py#L735 which does pass in the api_version kwarg | |
| 22:05:25 | ildikov | scottda: no, I'm depending on another one already | |
| 22:05:27 | mriedem | and ^ is going to be using 3.0 | |
| 22:06:14 | mriedem | anyway that gets us here https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/v3/client.py#L72 | |
| 22:06:23 | mriedem | and then https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/v3/client.py#L112 | |
| 22:06:30 | scottda | ildikov: Does your patch call api_versions.get_highest_version(client) ? | |
| 22:06:56 | mriedem | and then we create a session client here https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/client.py#L607 | |
| 22:06:59 | mriedem | because nova is using a session | |
| 22:07:07 | scottda | ildikov: or point me to your patch(es) | |
| 22:07:16 | mriedem | when session client makes a request it updates the headers here https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/client.py#L109 | |
| 22:07:30 | mriedem | which gets us here https://github.com/openstack/python-cinderclient/blob/1.10.0/cinderclient/api_versions.py#L310 | |
| 22:07:32 | ildikov | scottda: I don't touch the client part in my patch | |
| 22:07:40 | mriedem | and for whatever reason, that only sets the header if the minor version is > 0 | |
| 22:07:44 | mriedem | which seems like a bug | |
| 22:07:56 | ildikov | scottda: so I don't call get_highest_version either | |
| 22:08:31 | mriedem | i bet you $100 that that's just a copy/paste from the novaclient update_headers code | |
| 22:08:31 | scottda | Well, the cinderclient doesn't auto-negotiate to the highest mutually supported version (yet) | |
| 22:08:43 | mriedem | because 2.0 is legacy compute api, 2.1 is new hotness with microversions | |
| 22:08:45 | mriedem | there is no nova 3.0 | |
| 22:08:51 | mriedem | but cinder decided to jump from 2.0 to 3.0 | |
| 22:09:03 | mriedem | scottda: ildikov: so yeah your client is busted | |
| 22:09:19 | mriedem | i'll take the $100 consulting fee in cash or check | |
| 22:10:07 | smcginnis | mriedem: Nova auto-negotiates to the highest? Don't you consider that a risk? | |
| 22:10:19 | mriedem | smcginnis: for the shell, | |
| 22:10:20 | ildikov | mriedem: does that contain the figuring out the endpoint not found issue too? :) | |
| 22:10:21 | mriedem | not the python API | |
| 22:10:26 | mriedem | ildikov: no | |
| 22:10:28 | mriedem | that's ksa | |
| 22:10:32 | smcginnis | mriedem: Still seems like a risk. | |
| 22:10:43 | mriedem | smcginnis: if you're coding to the shell...well... | |
| 22:10:50 | ildikov | mriedem: fair enough | |
| 22:11:00 | mriedem | if you're coding to the shell you deserve to get broken | |
| 22:11:24 | smcginnis | Well, if you're just running commands from the CLI, that seems a risk that suddenly you could get different behavior. | |
| 22:13:38 | ildikov | mriedem: thanks much for the help/explanation of the pain I'm experiencing with testing :) | |
| 22:14:31 | openstack | Launchpad bug 1656105 in python-cinderclient "api_versions.update_headers doesn't set the microversion header if version is 3.0" [Undecided,New] | |
| 22:14:31 | mriedem | ildikov: https://bugs.launchpad.net/python-cinderclient/+bug/1656105 | |
| 22:14:56 | ildikov | mriedem: thanks much! | |
| 22:15:01 | scottda | I'll put up a patch for that ^^^..When I get back from the bus stop. | |
| 22:15:38 | ildikov | scottda: sounds great, thanks | |
| 22:15:39 | mriedem | smcginnis: if you're coding to the shell you can override with --os-compute-api-version | |
| 22:15:43 | mriedem | to opt in or pin the version | |
| 22:15:57 | mriedem | which is exactly what you have to do if you're requesting specific microversions via the python API | |
| 22:15:59 | scottda | ildikov: But why are you using 3.0? You'll still need the version with the new APIs | |
| 22:16:06 | mriedem | microversions in the python API bindings is opt-in, | |
| 22:16:21 | mriedem | the shell is a convenience to use the latest available that the client understands and the server can provide, | |
| 22:16:34 | mriedem | if you're coding to the shell and worry about changes with microversions, then you also have to pin with --os-compute-api-version | |
| 22:16:34 | scottda | BBIAB | |