| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-05-13 | |||
| 12:23:01 | jkulik | docs for the MultiDict stuff https://docs.pylonsproject.org/projects/webob/en/stable/reference.html#query-post-variables | |
| 12:28:12 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.moves https://review.opendev.org/727224 | |
| 12:29:25 | jkulik | should I just open a bug report if I fear something is broken instead of asking on IRC for someone to confirm? | |
| 12:30:16 | sean-k-mooney | jkulik: im not sure if this is actully supported | |
| 12:30:28 | jkulik | oh, down the line it's checked against a list | |
| 12:30:33 | jkulik | let me find that part again | |
| 12:31:43 | sean-k-mooney | jkulik: the servers endpoint only supprot one uuid https://docs.openstack.org/api-ref/compute/?expanded=show-server-details-detail#show-server-details | |
| 12:31:48 | sean-k-mooney | not a list of them | |
| 12:32:03 | sean-k-mooney | at least the server detials endpoint | |
| 12:32:41 | jkulik | index does support more in theory, though. it ends up here in the end https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L1849-L1854 | |
| 12:33:11 | jkulik | oh, I mean query params to /servers or /servers/detail | |
| 12:33:13 | sean-k-mooney | well the docs text is | |
| 12:33:34 | sean-k-mooney | right so its not supported for /servers/detail | |
| 12:33:49 | sean-k-mooney | well i guess it might be | |
| 12:34:03 | sean-k-mooney | the docs text reads | |
| 12:34:06 | sean-k-mooney | uuid (Optional) | |
| 12:34:08 | sean-k-mooney | ||
| 12:34:10 | sean-k-mooney | query | |
| 12:34:12 | sean-k-mooney | ||
| 12:34:14 | sean-k-mooney | string | |
| 12:34:16 | sean-k-mooney | ||
| 12:34:18 | sean-k-mooney | Filter the server list result by the UUID of the server. | |
| 12:34:20 | sean-k-mooney | This parameter is only valid when specified by administrators. If non-admin users specify this parameter, it is ignored. | |
| 12:34:23 | sean-k-mooney | so based on the docs we dont supprot it | |
| 12:34:35 | jkulik | it would explicitly state that's it supports a list? | |
| 12:34:41 | sean-k-mooney | it might be possibel but its not intended to work | |
| 12:35:13 | jkulik | (because for neutron ports "id", it looked the same but works) | |
| 12:35:13 | sean-k-mooney | it filters the list of servers by the uuid | |
| 12:35:20 | sean-k-mooney | it does not say it supports a list of uuids | |
| 12:35:40 | jkulik | sure, but the code has extra ifs for a list ;) | |
| 12:36:07 | jkulik | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2838-L2841 | |
| 12:36:11 | sean-k-mooney | jkulik: do we have api test for the list case | |
| 12:36:20 | jkulik | haven't checked. will do. | |
| 12:48:07 | jkulik | can't find any in the unit-tests at least. for a lot of other query-params, there are tests, but uuid doesn't seem to be in there. | |
| 12:49:21 | sean-k-mooney | jkulik: looking at the code if ti ever worke i suspect it was a nova v2 or nova v1 capablity that was never fully supproted and is not supproted anymore | |
| 12:50:07 | jkulik | looking at the code, it seems totally supported, if we would just convert the MultiDict's multiple entries to a list instead | |
| 12:50:41 | sean-k-mooney | https://github.com/openstack/nova/commit/3f35fe6a88672ea2ab7e080a55235c5cca45dc2c | |
| 12:51:07 | sean-k-mooney | that code is related to listing instance by ip | |
| 12:51:27 | sean-k-mooney | jkulik: its not for listing servers by uuid | |
| 12:51:59 | francoisp | lyarwood, bauzas hello, when you get a chance: https://review.opendev.org/#/c/726508/ (this time around for Queens) - thanks! | |
| 12:52:09 | bauzas | ack, will look | |
| 12:52:39 | sean-k-mooney | jkulik: we support a list there becasue you can have multiple servers with the same ip adress | |
| 12:52:56 | sean-k-mooney | but we dont support the uuid query arg being a list | |
| 12:53:13 | jkulik | sean-k-mooney, but is uses the uuid key out of search_opts which comes from the query-params | |
| 12:53:23 | jkulik | which cannot be a list because it's broken | |
| 12:53:39 | sean-k-mooney | jkulik: the uuid quey arg is not ment to be a list | |
| 12:53:55 | sean-k-mooney | it susing it here so you can filter by ip and uuid | |
| 12:53:59 | jkulik | my use-case would be retrieving infos for multiple servers I know the UUID of in one request btw. | |
| 12:54:03 | sean-k-mooney | the docs say uuid singualr | |
| 12:54:23 | sean-k-mooney | jkulik: yep which as far as im aware we do not support | |
| 12:54:28 | jkulik | sure. the docs say the same for "id" in ports, so I didn't think that's so important ;) | |
| 12:55:06 | jkulik | and since search_opts gets passed down to the db layer in the end, which also can support a list, we would just have to adjust that one line to make it work | |
| 12:55:09 | sean-k-mooney | gmann: your one of our api experst do you know if we have ever supported filtering server list by a list of uuids | |
| 12:55:32 | jkulik | but if it's not supposed to, that's fine, too. I just think the code would support it. | |
| 12:55:37 | sean-k-mooney | jkulik: there is a difference between things that work as a result of internal impemnstion details and what is actully supported | |
| 12:56:55 | jkulik | sean-k-mooney, thanks for looking at it :) | |
| 12:57:37 | sean-k-mooney | jkulik: this is the relevent spec for filtering https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/add-whitelist-for-server-list-filter-sort-parameters.html | |
| 13:00:48 | sean-k-mooney | jkulik: so i think we suport filterign with a singel value but not lists or any advance queries. | |
| 13:05:23 | jkulik | I don't see any mentioning of either in the spec, but I can live with the answer "works as designed". | |
| 13:07:09 | sean-k-mooney | it is a feature you could request or propose as a blueprit/spec. | |
| 13:07:21 | sean-k-mooney | i belive its working as designed currently | |
| 13:07:37 | sean-k-mooney | but i dont nessicaly thing it woudl be too contovertall to add support for what you want | |
| 13:07:56 | sean-k-mooney | its just not a bug since i dont think it was ever inteded to wrok | |
| 13:18:47 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.iteritems/itervalues/iterkeys https://review.opendev.org/727757 | |
| 13:21:07 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Don't delete disks on shared storage during evacuate https://review.opendev.org/578846 | |
| 13:21:07 | openstack | bug 1550919 in OpenStack Compute (nova) "[Libvirt]Evacuate fail may cause disk image be deleted" [Medium,In progress] https://launchpad.net/bugs/1550919 - Assigned to Lee Yarwood (lyarwood) | |
| 13:21:07 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add functional test for bug 1550919 https://review.opendev.org/631294 | |
| 13:39:14 | jkulik | functional tests don't support testing the "uuid as list" case either https://github.com/openstack/nova/blob/master/nova/tests/functional/api/client.py#L248-L250 | |
| 13:39:46 | jkulik | the lack of tests for 'uuid' in search_opts isn't good, though :D | |
| 13:45:45 | sean-k-mooney | jkulik: yep again i think that is becasue we did not orginally plan to expose this via the api and retroactivly had to try and fix it | |
| 13:46:15 | openstackgerrit | Jiri Suchomel proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.opendev.org/574301 | |
| 14:24:36 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.byte2int/int2byte https://review.opendev.org/727777 | |
| 14:34:02 | gmann | jkulik: sean-k-mooney yeah, multi filters things are not supported. for this case where you want to list multiple servers you can use some other query filter which matches multiple servers. like ?name=test so it will return all servers matching with 'test*' | |
| 14:35:58 | gmann | remember, multiple filters are with AND condition | |
| 14:38:34 | gmann | in current behaviour only last present item is being used even you are asking for multi dict | |
| 14:41:03 | gmann | jkulik: it should not be hard to support that, we just need to change the way we fetch filter from GET and query DB with OR condition. | |
| 14:41:35 | gmann | jkulik: anyways, all API change need spec process, please feel free to add BP and spec for the same | |
| 14:43:30 | gmann | but remember any other filter present with mutli dict filter will be with AND condition. so you would not be able to do 'server-uuid1 OR (server-uuid2 AND vm_state=active)' it will be '(server-uuid1 OR server-uuid2) AND vm_state=active)' | |
| 15:32:43 | lyarwood | we should start a club :) | |
| 15:33:37 | lyarwood | gibi: I've not had a chance to look at your caching change btw, I'll try to get to it tomorrow | |
| 15:34:09 | gibi | lyarwood: dont rush it is a very incomplete messy pile of boo | |
| 15:36:02 | gibi | lyarwood: I just figured out that the current imagebacked code my temporarily use double of the image size disk space. For example if an image is a qcow in glance but nova configured to force_raw_images then after image download we copy out the raw data from qcow and the delete the qcow file we downloaded :/ | |
| 15:36:17 | gibi | s/my/might/ | |
| 15:36:47 | gibi | who thought that we will have disk space for that operation?! | |
| 15:38:03 | gibi | I get to feel that I have no power to patch this code in a way I imagined | |
| 15:38:26 | dansmith | lyarwood: is it a club if everyone is a member? | |
| 15:38:34 | dansmith | I think that's called "a population" | |
| 15:39:10 | dansmith | gibi: did you catch the scrollback of our convo yesterday? | |
| 15:39:20 | lyarwood | dansmith: ^_^ | |
| 15:39:58 | lyarwood | gibi: yeah I don't think we can convert in-place tbh | |
| 15:40:01 | gibi | dansmith: yes. but honestly I have to go back to it as I already forget what was the pre-filter idea | |
| 15:40:43 | dansmith | gibi: so, I think it's not unreasonable to say "to boot an instance of $root from an image of $imgsize, the host needs to have $root+$imgsize free space, as a rule | |
| 15:41:21 | dansmith | although you know what.. | |
| 15:41:32 | dansmith | I think we were totally missing something yesterday, now that I re-state that with a fresh mind | |
| 15:42:29 | dansmith | for that to even help, we have to account for the base in the inventory or allocations somehow, which is what we were trying to avoid there | |
| 15:42:36 | dansmith | so, hrmph | |
| 15:43:07 | dansmith | gibi: I will say that although I understand the desire to disable the cache as a workaround, it's really not a useful solution for anything other than a very small subset of cases | |
| 15:43:29 | dansmith | i.e. where you expect only one of each image type to be booted in a disk-constrained place anyway | |
| 15:43:43 | gibi | and I think it is pretty impossible to do properly due to what assumption the code currently makes | |
| 15:43:49 | dansmith | so I wouldn't want to spend a bunch of effort or cause a bunch of destabilization in the image backend to do so | |