| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-07 | |||
| 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. | |
| 15:35:15 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: (WIP) Functional test with pGPUs https://review.opendev.org/717975 | |
| 15:35:36 | bauzas | gibi: FWIW, I'm almost done with the functional test, including for multiple types https://review.opendev.org/717975 | |
| 15:36:07 | gmann | gibi: and with your fix (allow '' in schema) does give correct servers list ? | |
| 15:36:08 | gibi | I can do a True or False query filter but then we break rackspace if they depend on passing through a uuid in the filter | |
| 15:36:24 | bauzas | gibi: you can look at it to see that we don't support some specific pGPU if the asked type by the configuration is not actually used by the pGPU | |
| 15:36:25 | gibi | gmann: yes, the WIP patch works for me | |
| 15:36:44 | gibi | bauzas: ack, I will get back to that tomorrow. | |
| 15:37:42 | gmann | gibi: ok, apart from rackspace usecase, if we allow empty string in query which is disallowed now still need microversion as it change the interface | |
| 15:38:12 | gmann | for interoperability | |
| 15:38:13 | gibi | gmann: we cannot filter for servers without config drive today, it is a bug | |
| 15:38:45 | gmann | humm | |
| 15:39:09 | gibi | as that filter would look like: ?config_drive= | |
| 15:39:21 | gibi | as the db contains empty value for the no config drive case | |
| 15:40:18 | gmann | i was thinking to fix it with GET reponse and DB to have false in case of no config drive(empty string) | |
| 15:41:05 | gibi | gmann: that is DB migration + microversion on the API (due to the rsp change) so that is someting I wouldn't start at the week of FF | |
| 15:41:22 | gibi | also it has a rackspace crazyness to it | |
| 15:41:53 | gibi | but on the long term this might be the good way | |
| 15:42:02 | gibi | for API consistency | |
| 15:42:11 | gibi | so I agree to add this to the API cleanup etherpad | |
| 15:42:24 | gmann | but i am thinking if changing query things is ok without microversion as it can create interop issue | |
| 15:43:03 | gibi | yeah, so I have to find a way now to unblock the 2.83 client patch without the need to change the query string semantic | |
| 15:43:59 | gibi | I can say that we have a nova bug but the client patch does not make it worse it just keep passing what it gets from the user to the nova API, and as soon as we fix the nova API we can stricten the client code too | |
| 15:44:12 | gmann | ok. I will check that, i think we can keep error there as it get from API | |
| 15:44:16 | gmann | yeah, ditto | |
| 15:44:28 | gmann | i will add that on etherpad and we can discuss the best possible solution. | |
| 15:44:34 | gibi | gmann: cool. thanks. | |
| 15:45:00 | gmann | thanks. we might have few more field like that | |
| 15:46:11 | gmann | gibi: done, L86 - https://etherpad.openstack.org/p/nova-api-cleanup | |
| 15:46:17 | gibi | ack | |