| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-07 | |||
| 13:13:35 | hrw | lyarwood: it was copied from one place, adapted and reused. | |
| 13:13:38 | sean-k-mooney | hrw by convention in python if a varible is not used in the same function and is just returned we should just retrun the expression | |
| 13:14:02 | hrw | yeah | |
| 13:14:25 | sean-k-mooney | but its not important enough to actully repin a patch over as lyarwood said | |
| 13:14:40 | openstackgerrit | Luyao Zhong proposed openstack/nova master: partial support for live migration with specific resources https://review.opendev.org/715362 | |
| 13:14:41 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Support live migration with vpmem https://review.opendev.org/687856 | |
| 13:14:41 | hrw | just with amount of comments around that comment took me a while to notice which part of line it applied to ;D | |
| 13:15:19 | openstackgerrit | Lee Yarwood proposed openstack/nova master: fup: Add removal TODOs for disable_native_luksv1 and rbd_volume_local_attach https://review.opendev.org/718099 | |
| 13:16:48 | luyao | stephenfin: Thanks for review :), your comments are addressed. https://review.opendev.org/687856 | |
| 13:25:02 | stephenfin | cool | |
| 13:26:17 | stephenfin | lyarwood: could you take a look at https://review.opendev.org/715406 and its predecessor to close that out? | |
| 13:26:53 | lyarwood | stephenfin: yup looking | |
| 13:35:01 | stephenfin | sean-k-mooney: replied on https://review.opendev.org/#/c/700894/ | |
| 13:36:30 | sean-k-mooney | ok. ill take a look at it sortly. that said i tried doing it a few different ways and without a significant rewrite this was the cleanest way i could do it | |
| 13:40:22 | dansmith | brinzhang: are you able to work on that policy patch? | |
| 13:45:16 | sean-k-mooney | stephenfin: in anycase ill take a look again and rebase it ontop of your mock lib removal patch | |
| 13:45:46 | stephenfin | sean-k-mooney: don't rebase it - that mock removal patch is having teething issues and won't be finished this week | |
| 13:46:11 | stephenfin | specifically I'm hitting bugs in the stdlib mock libraries that I'm having to work around :( | |
| 13:46:39 | sean-k-mooney | stephenfin: ok | |
| 13:46:44 | sean-k-mooney | i didnt want it to conflict | |
| 13:46:58 | sean-k-mooney | stephenfin: or bugs in mock the lib | |
| 13:47:34 | sean-k-mooney | stephenfin: i have hit bugs with the mock lib before so its proably that we work around those initally now we need to return to the normal behavior | |
| 13:47:41 | stephenfin | afaict, mock the lib is a straight backport of the stdlib library | |
| 13:48:00 | stephenfin | so we'd presumably see the same issues if we downgraded to e.g. mock 2.0 | |
| 13:48:16 | sean-k-mooney | stephenfin: it is ment to be yes but im not sure that is always true | |
| 14:37:00 | gibi | nova api experts, how can I specify a config_drive filter of GET /server/details to get the servers that does not have a config drive | |
| 14:37:03 | gibi | ? | |
| 14:37:48 | gibi | GET /servers/detail?config_drive=& is rejected by the schema | |
| 14:38:19 | gibi | GET /servers/detail?config_drive=False returns empty list even if I have servers without config drive | |
| 14:39:21 | gibi | config_drive in the response is defined like | |
| 14:39:23 | gibi | "Indicates whether or not a config drive was used for this server. The value is True or an empty string. An empty string stands for False." | |
| 14:41:31 | gibi | https://bugs.launchpad.net/nova/+bug/1871409 | |
| 14:41:32 | openstack | Launchpad bug 1871409 in OpenStack Compute (nova) "cannot filter the list servers to show only those that does not have config drive" [Undecided,New] | |
| 14:51:22 | openstackgerrit | Arthur Dayne proposed openstack/nova master: libvirt:driver:Disallow AIO=native when 'O_DIRECT' is not available https://review.opendev.org/682772 | |
| 15:02:40 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: WIP: fix config_drive filter in GET /servers/details https://review.opendev.org/718140 | |
| 15:10:32 | gibi | stephenfin: in https://review.opendev.org/682772 do you mean accepting boolean like values or do you also mean defaulting config_drive to False if not present? | |
| 15:10:42 | stephenfin | just the former | |
| 15:10:48 | stephenfin | sorry, that wasn't clear | |
| 15:11:31 | gibi | stephenfin: no worries. I just discovered this bug while trying to make the 2.83 client patch to work | |
| 15:13:10 | gibi | stephenfin: so for boolean like True value we should translate the filter to True and for a boolean like False value translate it to empty string? or keep the empty string as False semantic as in the response today? | |
| 15:15:01 | stephenfin | so 'GET /servers/details?config_drive=' will get us servers without a config drive? We probably need to retain that behavior, if so | |
| 15:15:22 | gibi | stephenfin: currently 'GET /servers/details?config_drive=' is not accepted by the schema | |
| 15:15:42 | stephenfin | what do you mean by the empty string, in that case? | |
| 15:15:46 | gibi | stephenfin: the respons of GET servers/details use empty value in the config_drive field to indicate no config drive (crazy) | |
| 15:16:04 | stephenfin | Ah | |
| 15:16:21 | gibi | https://docs.openstack.org/api-ref/compute/?expanded=list-servers-detailed-detail#id21 | |
| 15:16:23 | stephenfin | I guess we've to keep that the same, but I don't think the filter needs to work like that | |
| 15:17:09 | stephenfin | We could change that API response in a future clean up microversion | |
| 15:17:22 | gibi | so the API response will keep using True or "", the filter in the query will use truelike or falselike values | |
| 15:17:44 | stephenfin | Yeah, that's sensible IMO | |
| 15:17:47 | gibi | OK, thanks | |
| 15:17:53 | gibi | I will do that then | |
| 15:18:04 | stephenfin | No point letting the crazy spread into the query string too | |
| 15:18:24 | gibi | gmann: Do we alredy have an API cleanup item for the crazy config drive value in https://docs.openstack.org/api-ref/compute/?expanded=list-servers-detailed-detail#id21 | |
| 15:18:27 | gibi | ? | |
| 15:18:48 | gibi | stephenfin: yeah, make sense | |
| 15:19:14 | sean-k-mooney | gibi: you mean where the config drive is a cinder volume uuid | |
| 15:19:22 | gmann | gibi: to dis-allow empty dict? | |
| 15:19:30 | sean-k-mooney | or was it swift object uuid | |
| 15:19:30 | gibi | sean-k-mooney whaaat? | |
| 15:19:42 | sean-k-mooney | it was a thing that rack space implemented | |
| 15:19:51 | gibi | gmann: to not have True or empty string, but have proper False value | |
| 15:20:15 | sean-k-mooney | and we used to allow in the api but i dont think the support is still nove or perhaps never was upstreamed | |
| 15:20:16 | gibi | sean-k-mooney: does it mean the api ref is incomplete for config_drive here https://docs.openstack.org/api-ref/compute/?expanded=list-servers-detailed-detail#id21 ? | |
| 15:21:30 | gibi | sean-k-mooney: or you only mean we allowed empty string there to let rackspace replace that with a uuid downstream? | |
| 15:22:33 | sean-k-mooney | gibi i need to go check the code but we used to accetp a uuid there at the api level but i dont know when we stopped i might even be a pre microverion thing | |
| 15:23:03 | gibi | the db schema allows for a string(255) for config drive | |
| 15:23:12 | sean-k-mooney | yes | |
| 15:23:24 | sean-k-mooney | because rackspace used to put a uuid in it | |
| 15:24:18 | gmann | empty string not allowed i think. schema allow bool only - https://github.com/openstack/nova/blob/master/nova/api/validation/parameter_types.py#L219 | |
| 15:25:27 | gmann | this is test verify it - https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_serversV21.py#L4657 | |
| 15:25:48 | gibi | gmann: at POST /serves, yes https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/servers.py#L193 but at the query string of GET /servers/details it is not boolean https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/servers.py#L607 | |
| 15:26:11 | gmann | ohk, you mean in response | |
| 15:26:27 | gibi | do we have schema for GET rsp? | |
| 15:27:16 | gmann | gibi: query schema are the only one for GET | |
| 15:27:38 | sean-k-mooney | gibi: in the current api it should always be a bool | |
| 15:27:43 | gmann | and eevrything on query param is string so we cannot restrict that in schema side. we need to add code check for that | |
| 15:28:10 | sean-k-mooney | we should just use the sting to bool thing form oslo | |
| 15:28:20 | gibi | gmann: the current schema does not allow empty string, but the db contains an empty string if there is no config drive attached | |
| 15:28:20 | gmann | until we check the regex of bool values in schema which will be very complex | |
| 15:28:32 | sean-k-mooney | e.g. to accpeth true True yes y 1 all as true | |
| 15:29:00 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: (WIP) Functional test with pGPUs https://review.opendev.org/717975 | |
| 15:29:01 | gibi | sean-k-mooney: in the current API config_drive is "True" or empty string' | |
| 15:29:19 | gmann | yeah, we can 1. always return bool in response (convert empty string to False) and 2. allow query for bool value only | |
| 15:29:25 | gibi | I don't want to change the GET rsp, but I need to provide filtering support | |
| 15:29:30 | gibi | in the query string | |
| 15:30:04 | gibi | gmann: changing GET rsp is an API cleanup that needs a microversion and outside of my bugfixing effort | |
| 15:30:26 | gibi | gmann: but I still need to support some way to filter for servers wihtout config drive | |
| 15:30:43 | gibi | stephenfin: suggested to use True and False like values there | |
| 15:31:06 | gibi | but if there is a hidden rackspace dependency for a uuid in the config_drive filed then this change will break them | |
| 15:31:20 | gmann | if no change in GET response then it might be confusing ? not sure client side will show black value for that. i think we had same issue on swap field in flavor | |
| 15:32:25 | gibi | currently client side (novaclient) use empty value | |
| 15:32:25 | gibi | | config_drive | | | |
| 15:32:34 | gmann | gibi: does query with empty string just ignore the filter or error ? | |
| 15:32:49 | gibi | gmann: error as per https://bugs.launchpad.net/nova/+bug/1871409 | |
| 15:32:50 | openstack | Launchpad bug 1871409 in OpenStack Compute (nova) "cannot filter the list servers to show only those that does not have config drive" [Medium,In progress] - Assigned to Balazs Gibizer (balazs-gibizer) | |
| 15:33:04 | gibi | the query string schema does not allow empty string | |
| 15:33:30 | gibi | gmann: I can fix that by https://review.opendev.org/#/c/718140/1/nova/api/openstack/compute/schemas/servers.py | |
| 15:33:59 | gibi | but then stephenfin had a point that by this we spread the True or "" crazyness from the GET rsp to the query sting | |
| 15:35:00 | gmann | yeah that is my concern also. | |