| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-16 | |||
| 19:18:13 | armax | http://logs.openstack.org/25/465125/1/experimental/gate-tempest-dsvm-neutron-pg-full-ubuntu-xenial/2a7a080/ | |
| 19:18:32 | mriedem | armax: sweet | |
| 19:19:07 | armax | mriedem: nice one | |
| 19:19:14 | armax | mriedem: you need to add a test now ;) | |
| 19:19:24 | mriedem | armax: yup, working on it | |
| 19:19:28 | armax | mriedem: you rock | |
| 19:19:40 | mriedem | smcginnis distracted me with all of his NFS love | |
| 19:22:14 | armax | mriedem: do you want to rerun the job to confirm if there’s another race lurking? | |
| 19:22:40 | armax | mriedem: I thought you learned the craft of multitasking when you became PTL | |
| 19:23:41 | mriedem | armax: i can run it again yeah while i work on tests | |
| 19:24:17 | armax | mriedem: does Zuul listens to one whilst there’s still another ongoing run? | |
| 19:24:33 | armax | oh | |
| 19:24:35 | armax | never mind | |
| 19:24:44 | mriedem | armax: i didn't see that you checked experimental before i did | |
| 19:24:50 | mriedem | so i'll have to do it again once that run is done i think | |
| 19:24:56 | armax | didn’t notice that jenkins came back, we’re good | |
| 19:24:59 | mriedem | artom_: let me know if i'm off base here https://review.openstack.org/#/c/415520/ | |
| 19:34:23 | mriedem | armax: do we need to worry about the subnet being gone too? | |
| 19:34:32 | mriedem | L450 | |
| 19:35:34 | openstackgerrit | sean mooney proposed openstack/nova master: do not set vcpu_pin elements when virt_type=qemu https://review.openstack.org/465160 | |
| 19:38:23 | artom_ | mriedem, will do, thanks for the comments! | |
| 19:39:45 | mriedem | artom: i think we should just get the device metadata first, then attach to the guest, and then if that's fine, do instance.save() to update the db | |
| 19:39:57 | mriedem | then we don't have to worry about rolling back the detach in the guest | |
| 19:43:01 | armax | mriedem: I’ll reply on the review | |
| 19:51:16 | artom | mriedem, do we have what we need to build the metadata if we do so before attaching? | |
| 19:51:30 | artom | I'm thinking of PCI and/or MAC addresses | |
| 19:52:23 | mriedem | artom: oh i guess i didn't dig into _build_device_metadata, but that reads from the guest xml doesn't it? | |
| 19:52:37 | mriedem | to correlate the tag with the device or something | |
| 19:52:49 | artom | mriedem, yeah | |
| 19:52:53 | mriedem | :( | |
| 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: Have scheduler claim resources https://review.openstack.org/463993 | |
| 20:32:50 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make the method to put allocations public https://review.openstack.org/464086 | |
| 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 | LOG.warning(_LW('Received unexpected event %(event)s for ' | |
| 20:35:08 | mriedem | 'instance'), | |
| 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 | bauzas | edleafe: not sure I understand why you're still working on my branch ? | |
| 20:55:22 | openstackgerrit | Hongbin Lu proposed openstack/nova master: [WIP] Handle exception on adding secgroup https://review.openstack.org/465173 | |
| 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. | |