| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-16 | |||
| 19:53:32 | artom | mriedem, you're basically trying to answer the question "what happens if _build_device_metadata fails?" right? | |
| 19:54:07 | artom | Because the try block it's in only handles libvirtError... | |
| 19:54:12 | mriedem | yeah | |
| 19:54:39 | mriedem | as noted, the compute manager will handle any NovaException that comes out of the driver.attach_interface method | |
| 19:55:02 | mriedem | but that only deallocates the port, which if it's a pre-existing port is an unbind of the host on the port, if nova created the port then nova deletes the port, | |
| 19:55:15 | mriedem | which would trigger a network-vif-deleted event, and nova would get that and detach the interface from the guest, | |
| 19:55:26 | mriedem | but i'm worried about the case the the port was provided to nova to attach to the instance, and we just unbind it, | |
| 19:55:39 | mriedem | i don't think we'll get a network-vif-deleted event from neutron in that case, because it wasn't deleted | |
| 19:55:49 | mriedem | and we wouldn't detach the interface from the guest | |
| 19:57:30 | artom | That raises a question about the semantics of tagged attach, namely, do we fail the whole thing if only the tagging part fails | |
| 19:59:54 | artom | In situations like this I really miss Java's stricter exception system | |
| 20:21:02 | mriedem | artom: well, | |
| 20:21:25 | mriedem | if i'm attaching an interface with a tag and the interface shows up in my guest but the tag doesn't, i'm going to wonder wtf happened and create a support ticket, | |
| 20:21:38 | mriedem | so i'm thinking if any of the operation fails, it all needs to fail | |
| 20:21:41 | mriedem | and rollback the attach | |
| 20:32:12 | artom | mriedem, yeah. If we had a way to communicate partial failure to the user it might have been different, but we don't | |
| 20:32:50 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make the method to put allocations public https://review.openstack.org/464086 | |
| 20:32:50 | openstackgerrit | Ed Leafe proposed openstack/nova master: Have scheduler claim resources https://review.openstack.org/463993 | |
| 20:32:51 | openstackgerrit | Ed Leafe proposed openstack/nova master: Pass a list of instance UUIDs to scheduler https://review.openstack.org/465171 | |
| 20:33:37 | artom | mriedem, I feel like the cleanest would be to catch _build_device_metadata failures in attach_interface and unplug the vif, like we currently do upon libvirtError | |
| 20:34:22 | mriedem | artom: yeah probably, | |
| 20:34:34 | mriedem | then we'll get the network-vif-unplugged event from neutron | |
| 20:35:01 | mriedem | but, i don't think we handle that | |
| 20:35:08 | mriedem | 'instance'), | |
| 20:35:08 | mriedem | LOG.warning(_LW('Received unexpected event %(event)s for ' | |
| 20:35:52 | mriedem | http://logs.openstack.org/25/465125/1/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/65c553c/logs/screen-n-cpu.txt.gz?level=WARNING#_May_16_18_05_12_035418 | |
| 20:36:10 | mriedem | we get a lot of those | |
| 20:36:23 | mriedem | and we don't actually detach from the guest | |
| 20:37:17 | mriedem | artom: really, i don't know why attach_interface just doesn't call detach_interface on failure | |
| 20:37:23 | mriedem | since that handles the things we care about | |
| 20:37:39 | mriedem | unplugs the vif and detaches the device from the guest | |
| 20:38:02 | artom | mriedem, for real, vif_driver.unplug ends up going to Neutron? | |
| 20:38:12 | mriedem | artom: it doesn't call neutron directly | |
| 20:38:24 | mriedem | but the neutron agent is polling for changes, which are introduced via the os-vif unplug | |
| 20:38:39 | mriedem | and the neutron agent notifies the neutron server which sends an event back to nova | |
| 20:38:53 | mriedem | but as noted, we don't handle network-vif-unplugged | |
| 20:38:58 | artom | Ah... Well, by default I assume code that's there is there for a reason, and works well enough | |
| 20:39:16 | artom | And it was apparently decided by past powers that be that vif_driver.unplug was enough cleanup, so... ;) | |
| 20:39:23 | mriedem | heh yeah idk | |
| 20:39:35 | mriedem | well, | |
| 20:39:38 | mriedem | i guess the thinking was, | |
| 20:39:49 | mriedem | you'd only get that libvirtError if failing to attach the device in the guest failed | |
| 20:39:54 | mriedem | so there would be nothing to detach from the guest, | |
| 20:39:59 | mriedem | that's no longer the case with your change | |
| 20:40:10 | mriedem | b/c we have a new failure domain after the interface is attached into the guest | |
| 20:40:29 | artom | Aha, smart. | |
| 20:40:42 | artom | And that actually clarified the plug vs attach distinction in my head | |
| 20:40:55 | artom | plug = to the network | |
| 20:40:58 | artom | attach = to the guest | |
| 20:41:01 | mriedem | yeah | |
| 20:41:12 | mriedem | stepping back, what could _build_device_metadata actually fail on besides db/rpc traffic stuff | |
| 20:41:15 | mriedem | or coding errors | |
| 20:41:41 | artom | I looked through it, I didn't see anything besides what you mentioned. | |
| 20:42:26 | cfriesen | mriedem: do you think there's any point in further pursuing the live migration resource-tracking-numa-xml patches to try and get them merged, or should we give up and wait for jaypipes to fix it with the new placement/allocation code in queens? | |
| 20:42:55 | mriedem | cfriesen: i haven't dug into those, | |
| 20:43:05 | mriedem | but in general i wouldn't count on anything | |
| 20:43:13 | artom | mriedem, I need to run to fetch offsprings from daycare. I'll try to submit a new patchset tonight that detach_interface'es if _build_device_metadata fails | |
| 20:43:23 | mriedem | artom: ok | |
| 20:48:55 | kashyap | cfriesen: Hey, was travelling today. Just opened IRC & saw your ping (it's late here). | |
| 20:49:36 | kashyap | cfriesen: Is it about the stability of 'cpu-throttle-initial' and 'cpu-throttle-increment'? | |
| 20:49:48 | kashyap | If so, I can't say yet without digging myself, which I could only do tomorrow :-) | |
| 20:49:59 | kashyap | (Unless DanPB has already saw the email thread and responded there) | |
| 20:52:47 | cfriesen | kashyap: no worries. yeah, that's it. tomorrow's fine, and nobody has responded. :) | |
| 20:55:22 | openstackgerrit | Hongbin Lu proposed openstack/nova master: [WIP] Handle exception on adding secgroup https://review.openstack.org/465173 | |
| 20:55:22 | bauzas | edleafe: not sure I understand why you're still working on my branch ? | |
| 20:55:29 | openstackgerrit | Ed Leafe proposed openstack/nova master: Remove selecting host based on host_subset_size https://review.openstack.org/465174 | |
| 20:55:45 | bauzas | edleafe: please stop doing that ! | |
| 20:56:03 | bauzas | I'm just uploading my series, and I'm working on it since yesterday ! | |
| 20:56:50 | bauzas | edleafe: is it because you want your name on the BP ? | |
| 20:56:58 | edleafe | There was still some interest in my approach. dansmith asked that I change the way that instance UUIDs are passed. | |
| 20:57:07 | edleafe | bauzas: dpm | |
| 20:57:09 | edleafe | ugh | |
| 20:57:22 | edleafe | don't be silly. I don't care about names on BPs | |
| 20:57:33 | bauzas | I'm seriously nerved, because you're duplicating my series | |
| 20:57:35 | dansmith | edleafe: no, I'm not interested in the scheduler approach, I was just saying that if we need to pass the uuids, we should do them as a param | |
| 20:57:53 | dansmith | edleafe: I said if you wanted to re-open the discussion about conductor vs. scheduler you could | |
| 20:58:18 | edleafe | dansmith: ok, and yes, I do want to re-open, because I think this is much cleaner | |
| 20:58:42 | edleafe | dansmith: the code is there and working OMM. | |
| 20:58:42 | dansmith | bauzas: have you replied to any of the feedback on your series? because I don't think I've seen that and so it looks like nothing has happened on that set for quite a while | |
| 20:58:44 | dansmith | since before summit or so | |
| 20:58:49 | bauzas | sorry to have a family life, folks | |
| 20:59:23 | bauzas | if you want to see code, then... | |
| 21:01:37 | cdent | I reallly think we can do more to move code along in a collaborative fashion | |
| 21:02:11 | cdent | I generally think that if feedback has not had a response for a couple days that code is up for grabs to be fixed. I know that's not a standard that we have formalized, but I wonder if we should. | |
| 21:02:21 | cdent | This stuff needs to move a lot faster than it is | |
| 21:02:26 | dansmith | agree | |
| 21:02:34 | edleafe | that's more of a free-for-all than collaboration, though | |
| 21:02:37 | openstackgerrit | Hongbin Lu proposed openstack/nova master: [WIP] Handle exception on adding secgroup https://review.openstack.org/465173 | |
| 21:02:40 | bauzas | cdent: sure, but when you're working on something since 2 days, and then seeing a concurrent patch, do you think your former 2 days were just done ? | |
| 21:02:58 | cdent | bauzas: the way to get around that is to respond to feedback first | |
| 21:03:17 | dansmith | bauzas: right, it's hard to know someone is working on feedback when there is no response :) | |
| 21:03:18 | cdent | edleafe: I agree there are ways to make it more clean, but in general principle... | |
| 21:03:20 | bauzas | for example, I said to edleafe that I was working on removing host_subset_size and then I see https://review.openstack.org/465174 | |
| 21:03:30 | delete | hi, I am getting this error | |
| 21:03:31 | delete | GlanceConnectionFailed: Connection to glance host 198.23.82.11:9292 failed: Error finding address for https://198.23.82.11:9292/v1/images/detail?is_public=none&limit=20: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:579) | |
| 21:03:37 | delete | glance is using a self-signd certificate | |
| 21:03:52 | edleafe | bauzas: and I told you that I had done it in a subsequent patch | |
| 21:03:54 | bauzas | dansmith: well, I pinged you and edleafe this morning explaining why I wasn't yet uploading my series | |
| 21:05:09 | dansmith | bauzas: yeah, but I hadn't seen any updates since then.. I guess I missed them | |
| 21:05:13 | delete | nevermind | |