| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-05 | |||
| 16:05:43 | mriedem | pvc: let me go back to your original question: https://review.openstack.org/#/c/590188/ will not be backported upstream | |
| 16:05:45 | mriedem | because it's for a feature | |
| 16:06:04 | mriedem | even though it says it's a bug, it's a bug related to implementing a feature | |
| 16:06:31 | pvc | i see thank you :) | |
| 16:07:40 | mriedem | volume extend support was added in pike https://specs.openstack.org/openstack/nova-specs/specs/pike/implemented/nova-support-attached-volume-extend.html | |
| 16:08:26 | pvc | yes but on LVM driver only. for rbd is not yet supported | |
| 16:09:01 | mriedem | sure, that's not a bug though | |
| 16:09:36 | mriedem | more specifically, it's supported for iscsi and fibrechannel volume types | |
| 16:09:42 | mriedem | which is more than just lvm | |
| 16:09:58 | pvc | yes do you have idea when is the implementation for rbd? | |
| 16:10:12 | mriedem | the next patch in that series https://review.openstack.org/#/c/594273/ | |
| 16:10:27 | mriedem | but the blueprint isn't approved - it has to be discussed during the weekly nova meeting | |
| 16:11:45 | pvc | i see thank you so much for your time :) | |
| 16:15:42 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Fix nova-status "_check_resource_providers" check https://review.openstack.org/600113 | |
| 16:23:30 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Fix nova-status "_check_resource_providers" check https://review.openstack.org/600119 | |
| 16:32:19 | jaypipes | mgagne: no, it would not fail at the API level, AFAIK. it would fail during node provisioning in ironic virt driver. | |
| 16:33:29 | openstackgerrit | melanie witt proposed openstack/nova stable/rocky: Make scheduler.utils.setup_instance_group query all cells https://review.openstack.org/599732 | |
| 16:37:51 | openstackgerrit | melanie witt proposed openstack/nova stable/queens: Make scheduler.utils.setup_instance_group query all cells https://review.openstack.org/599766 | |
| 16:50:59 | openstackgerrit | Merged openstack/nova master: Fix nova-status "_check_resource_providers" check https://review.openstack.org/599875 | |
| 16:51:05 | openstackgerrit | Merged openstack/nova master: Fix DB archiver AttributeError due to wrong table name attribute used https://review.openstack.org/599878 | |
| 16:54:39 | mriedem | alex_xu: Kevin_Zheng: yikun: replied in https://review.openstack.org/#/c/591976/ re: the changes-since == changes-before debate | |
| 16:54:50 | mriedem | cdent: edleafe: from an API SIG pov you might have input on ^ | |
| 16:55:04 | mriedem | would be interesting to know if any other APIs in openstack have filtering capability like that | |
| 16:55:34 | cdent | mriedem: been a while since I looked at that so not caught up on the issues | |
| 16:56:06 | cdent | nova drove the existence of changes-since, yeah? | |
| 16:56:07 | mriedem | the big debate is what to do if changes-since == changes-before | |
| 16:56:27 | mriedem | umm, i'm assuming so... | |
| 16:57:35 | mriedem | glance has some funky filter-based filtering stuff using operators | |
| 16:59:53 | mgagne | jaypipes: I was referring to the attach-interface action, the hotplug feature | |
| 17:00:38 | mgagne | hmm I think I reworded my question before sending, might not have been clear then :-/ | |
| 17:09:34 | openstackgerrit | Elancheran S proposed openstack/nova master: Add exact match aggregate image properties matcher/filter https://review.openstack.org/593167 | |
| 17:10:27 | mgagne | ok, it seems it would fail on Ironic side with NoFreePhysicalPorts exception which would be mapped to a Bad Request at the Ironic API. This will be mapped to VirtualInterfacePlugException in Nova virt driver. There is a generic try catch in compute manager which will raise InterfaceAttachFailed. And API will map to HTTPInternalServerError | |
| 17:11:59 | openstackgerrit | Elancheran S proposed openstack/nova stable/pike: Add exact match aggregate image properties matcher/filter https://review.openstack.org/599870 | |
| 17:12:12 | sean-k-mooney | mgagne: not in all cases. i know cisco added a thing where you can attach more interfaces then avaialble to a ironic system via neutron turnk port extition | |
| 17:12:50 | mgagne | sean-k-mooney: yes, I'm concerned about "flat" networks where there is no trunk involved | |
| 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 | sean-k-mooney | well VirtualInterfacePlugException | |
| 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: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 | |