Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-09-05
17:13:27 mgagne and about the UX in case of failure.
17:13:41 sean-k-mooney one thing i was not aware of and maybe you can clarify. do we today allow you to attach an interface to an ironic node after its deployed?
17:13:49 mgagne yes
17:14:15 sean-k-mooney mgagne: and if we execeed the available interfaces we get an error or silent failure today?
17:15:11 mgagne if you have flat networks, Nova will fail with a 500 error without much information about the reason. This is what I understood from reading the code.
17:15:42 sean-k-mooney and you would like to chage that to a vifplug exception
17:15:55 mgagne attach/detach for Ironic was added in Pike: https://docs.openstack.org/releasenotes/nova/pike.html#new-features (2nd item)
17:15:55 sean-k-mooney well VirtualInterfacePlugException
17:16:27 mgagne sean-k-mooney: it would be a much better UX if the user got a 400 instead of a 500
17:18:13 sean-k-mooney so you want to make it a 400 calls bad request rather then a 500 server error as its an enduser error to try to attach more interface then phyically avaialble
17:18:37 mgagne yes, anything in the 4XX range
17:19:17 mgagne because I don't think it's a server side error from the user perspective
17:19:25 sean-k-mooney i mean that seam reasonable. i think i reivewd or partly reviewd code from you on this topic
17:20:11 mgagne could be 406, 409. I'm not an expert.
17:20:36 mgagne so you are the person I want to be friend with =)
17:21:40 sean-k-mooney im not an expert either but i dont think 409 or 406 is correct
17:22:03 sean-k-mooney 406 is for content type mismatches e.g. server say i speak json and you give it xml
17:22:09 openstackgerrit melanie witt proposed openstack/nova stable/pike: Add functional test for affinity with multiple cells https://review.openstack.org/599840
17:22:10 openstackgerrit melanie witt proposed openstack/nova stable/pike: Make scheduler.utils.setup_instance_group query all cells https://review.openstack.org/599841
17:22:38 sean-k-mooney 409 is for rases e.g. you tried to update something but someone else also did please retry
17:22:47 mgagne so 400 looks fine
17:23:26 mgagne or 402 if you want to make them pay for that feature =)
17:25:00 sean-k-mooney mgagne: i would use 400 or 418 if you are felling exotic or british
17:26:12 sean-k-mooney i would be tempted by 412 but i would have to read rfc 7232 so see if its correct or not
17:26:28 mgagne hehe so my question is how to raise that exception from Ironic to Nova API without losing much details.
17:27:41 mgagne because Ironic returns a 400 too but the error message would need to be parsed to find the reason and map it to something else in Nova.
17:28:09 mgagne and I'm not sure it's the right way to do it
17:28:20 sean-k-mooney i have to runn but a 400 with an embeded error code in the body might be the best option. this has been a topic dhellmann might be able to advise on. i think he had a session in vancouver on having consitent behavor in our error handling
17:29:10 sean-k-mooney if the api does not return a 400 already then that is proably enough
17:29:53 mgagne can we just assume that all 400 returned by Ironic are user errors at the Nova API level?
17:31:15 sean-k-mooney not all responces but perhaps for that specific endpoint + http method
17:31:33 mgagne +1
17:36:52 jroll mgagne: you need to go through scheduling to be able to determine if the node you pick has enough NICs, so I don't think it can ever be a synchronous error in the API
17:37:12 mgagne jroll: we are talking about the hotplug feature ;)
17:37:20 jroll ah
17:37:39 jroll you still need to reach down into the virt driver, right? which is always async
17:37:55 melwitt it's not always async -- depends on whether it's a cast or a call
17:38:01 melwitt it's usually async though
17:38:06 mgagne this is not what I found? or I misunderstood?
17:38:20 jroll well, so much for being well-informed :)
17:38:37 melwitt for example, when we do an attach_volume, we call down to the driver synchronously first to see if there's room, and fail in the API if not
17:38:54 melwitt something I learned recently
17:38:57 mgagne https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/attach_interfaces.py#L131-L154
17:40:02 mgagne https://github.com/openstack/nova/blob/master/nova/compute/rpcapi.py#L465-L474
17:40:07 mgagne call() is used
17:40:14 jroll mgagne: ah, cool, so I guess what I would do is add some sort of "NoNicsAvailable" exception that the ironic driver can return in this case
17:40:29 melwitt thanks, was just looking for that
17:41:43 melwitt indeed, call is synchronous
17:42:23 dansmith this would be a call to compute which does an http call to ironic, yeah?
17:42:28 mgagne jroll: there is already an exception for that (NoFreePhysicalPorts which is mapped to Invalid) https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/network/common.py#L170-L174
17:43:46 melwitt dansmith: I think they'd add something to the already-existing synchronous attach_interface call to call ironic
17:43:56 mgagne compute does a call to virt driver which performs the HTTP call: https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L1450-L1451
17:44:42 jroll mgagne: oh, I meant add that exception to nova, so we can return it here: https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L1474
17:44:54 dansmith yeah the unfortunate bit there is that if the second call takes a long time, we'll time out, report to the user that it failed, but eventually it succeeded
17:45:17 dansmith but, maybe this would be a good application of my long-call stuff
17:45:18 jroll or add VirtualInterfacePlugException to the list here: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/attach_interfaces.py#L134
17:45:19 mgagne jroll: yes! just need a way to detect this kind of error from Ironic API response and map accordingly
17:45:56 melwitt dansmith: ack
17:45:57 jroll mgagne: yep, this is where someone says all API errors should have a "code" :)
17:46:41 mgagne jroll: it's mapped to InterfaceAttachFailed in the compute manager https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L5956
17:46:51 mgagne which is handled already here https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/attach_interfaces.py#L149
17:47:29 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix formatting in changes-since guide https://review.openstack.org/600150
17:47:56 jroll mgagne: ah, ok. so we should detect this specific case and make a new exception with better info for the user eh?
17:48:03 mgagne jroll: yep =)
17:48:52 mgagne I'm just not sure how to do it properly without parsing error message which could theoretically be translated to a different language than english
17:49:03 mgagne I have to run right now, brb in ~1h
17:49:11 jroll gotcha
17:49:37 jroll mgagne: might be more of a topic for the ironic channel when you're back
17:53:36 sean-k-mooney jroll: i was away but i did mention having a 400 with and embded code :P
17:54:10 jroll sean-k-mooney: see, someone said it before I even predicted it :P
17:54:23 sean-k-mooney but if my memory is correct there was some converstation about makeing the apis more consitent in vancouver
17:54:38 sean-k-mooney e.g. between different services
17:54:47 sean-k-mooney i dont know hat the outcome of that was
17:55:40 sean-k-mooney the only thing i do remember is never embed html in and error responce
17:55:56 jroll yes, there was
17:56:05 jroll people agree we should do it, nobody has set aside the time afaik
17:56:50 sean-k-mooney is it a topic at the ptg. i assumed it would continue to be discussed in that api working group or in one of the cross project sessions
18:02:52 jroll sean-k-mooney: dunno, haven't been following it closely
18:15:35 mriedem i hope not
18:17:23 mriedem but maybe it'll be on tuesday when i might have to sit in the kata containers qemu room all day because someone from my company has to know what's going on in that "community"
18:21:32 cdent sean-k-mooney: feel free to come to api-sig session on monday if you wanna talk about that. the agenda is pretty light at the moment (although we intend to flesh it out more during tomorrow's meeting): https://etherpad.openstack.org/p/api-sig-stein-ptg
19:04:13 sean-k-mooney cdent: am i might, i think "starardising how we report errors" is a good goal. not sure i know enough about the options to add much to the conversation
19:08:01 cdent sean-k-mooney: there's an existing api guideline for errors: http://specs.openstack.org/openstack/api-wg/guidelines/errors.html but it is not implemented by much (placement does a bit of it, mostly to use 'code' to distinguish different 409 responses
19:13:54 sean-k-mooney oh ok i should read that.
19:14:24 sean-k-mooney by the way im glad ye used status code 418 in the example :)
19:18:10 sean-k-mooney mgagne: you proably should check out the link cdent posted http://specs.openstack.org/openstack/api-wg/guidelines/errors.html
19:18:34 sean-k-mooney anyway i need to go sleep/pack. talk to everyone tomorrow
19:19:01 mgagne sean-k-mooney: looks like a very cool spec =)
20:20:08 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix evacuate logging https://review.openstack.org/593055
20:53:51 mriedem UUIDs or accept ValueErrors for invalid UUIDs. See https://docs.openstack.org/oslo.versionedobjects/latest/reference/fields.html#oslo_versionedobjects.fields.UUIDField for further details
20:53:51 mriedem FutureWarning: ImageMeta(checksum=<?>,container_format=<?>,created_at=<?>,direct_url=<?>,disk_format=<?>,id=<?>,min_disk=<?>,min_ram=<?>,name=<?>,owner=<?>,properties=ImageMetaProps,protected=<?>,size=<?>,status=<?>,tags=<?>,updated_at=<?>,virtual_size=<?>,visibility=<?>) is an invalid UUID. Using UUIDFields with invalid UUIDs is no longer supported, and will be removed in a future release. Please update your code to input va
20:53:51 mriedem ugh i thought we had squashed this
20:54:28 mriedem i guess an ImageMeta object is certainly not a uuid
20:58:30 melwitt yeah... is it an error in a test or something? why is an ImageMeta object being treated as a UUIDField
20:59:19 mriedem missing mock i think
21:06:19 cdent Is "thin provisioning" of disk mostly a vmware thing, or does it also happen when using some other hypervisor+storage things?
21:08:16 dansmith cdent: other things have it
21:08:30 dansmith not everything amazing is made by vmware. JEEZ
21:09:04 dansmith a unix sparse file is kinda thing provisioning and pre-dates a lot of stuff
21:09:10 dansmith like 3.5" floppies

Earlier   Later