Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-07
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 gibi sean-k-mooney whaaat?
15:19:30 sean-k-mooney or was it swift object uuid
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 gmann until we check the regex of bool values in schema which will be very complex
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: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 | config_drive | |
15:32:25 gibi currently client side (novaclient) use empty value
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
15:46:18 gibi thanks
15:54:01 sean-k-mooney gibi: this is what i was thining of by the way https://bugs.launchpad.net/python-novaclient/+bug/1314395 https://github.com/openstack/python-novaclient/commit/adf6c351a05d205573be4f00db1e53aa2615a593
15:54:03 openstack Launchpad bug 1314395 in python-novaclient "Documentation for config_drive boot parameter is incorrect" [Undecided,Fix released] - Assigned to Jason Dunsmore (jasondunsmore)
15:54:42 sean-k-mooney gibi: many many many moons ago e.g. like essex it used to take a volume id too
15:54:52 artom stephenfin, so, I know I said the NUMA LM tests can wait until after FF
15:54:59 artom But with https://review.opendev.org/#/c/715362/4, I'd kinda want to see them merge first?
15:55:02 gibi sean-k-mooney: thanks for the reference
15:55:18 artom I'd be the only place where we test the fancier live migration stuff
15:57:34 stephenfin Hmm, that's a fair point. I'll try dig back into it tomorrow morning
15:58:30 sean-k-mooney ah the uuid was only for nova v1 api and was droped in https://review.opendev.org/#/c/10934/ damb that was a long time ago. im surprise we only update the docs in rocky

Earlier   Later