| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-08 | |||
| 14:24:11 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova-specs master: Re-propose the spec to allow specifying a list of CPU models https://review.openstack.org/642030 | |
| 14:34:17 | sean-k-mooney | speaking of spec i need to write and review a few | |
| 14:44:27 | bauzas | sean-k-mooney: closing OSC gap is one thing, swap to OSC is a totally different thing | |
| 14:44:43 | bauzas | sean-k-mooney: I'm just talking of the next TC goal which is somehow close to the former | |
| 14:45:02 | bauzas | at least at the last consensus point we had | |
| 14:57:05 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Dont' wait for VIF plugging during revert resize https://review.openstack.org/639396 | |
| 14:57:18 | artom | Alright, I think the tests should pass on ^^ now | |
| 14:57:37 | artom | mriedem, can I ask you to hit that when you get a moment? You're neck-deep in the resize flow anyways ;) | |
| 14:58:32 | dansmith | mriedem: fried_rice: related failure to a novaclient release? http://logs.openstack.org/92/624592/11/gate/nova-live-migration/06aea60/job-output.txt.gz#_2019-03-08_12_45_00_250123 | |
| 14:59:22 | artom | stephenfin, how much do you know about resize? Can I pick you as the lucky RH core for ^^ ? | |
| 15:00:54 | mriedem | dansmith: yes | |
| 15:00:59 | mriedem | dansmith: https://review.openstack.org/#/c/641986/ | |
| 15:01:12 | dansmith | ah okay | |
| 15:01:31 | mriedem | artom: did you sort that out with dansmith? | |
| 15:02:25 | dansmith | mriedem: he's just removing a wait for events on resize, which is more a you think than me | |
| 15:02:46 | dansmith | it doesn't break the context manager for waiting for events, so I'm happy(er) | |
| 15:03:29 | artom | mriedem, I think so. I also checked with slaweq who's in our Neutron team, and he confirmed that the VIF remains plugged on the source host | |
| 15:03:47 | dansmith | artom: um, what? :) | |
| 15:04:01 | artom | dansmith, what what? | |
| 15:04:10 | artom | Is plugged the right word? Wired? | |
| 15:04:13 | mriedem | i feel like i just had a patch like this | |
| 15:04:17 | mriedem | but was maybe for confirm | |
| 15:04:30 | dansmith | artom: that makes no sense for what you're saying here | |
| 15:04:43 | artom | dansmith, entirely plausible, but explain how :) | |
| 15:04:54 | dansmith | oh, this is finish_revert, I see | |
| 15:05:09 | dansmith | I thought it was finish-finish | |
| 15:05:23 | artom | Swedish finish? | |
| 15:05:27 | aspiers | sean-k-mooney: strong +1 for renaming evacuate, my preferred term would probably be "resurrect" | |
| 15:05:30 | dansmith | but still, you're not asserting this was always broken right? | |
| 15:05:36 | artom | dansmith, always racy | |
| 15:05:44 | dansmith | artom: what's it racing with? | |
| 15:05:54 | artom | Neutron with the virt driver | |
| 15:06:19 | mriedem | artom: so you're reverting this essentially right? https://review.openstack.org/#/q/I9e0cffb889c94713c7f28812918103a5d97cefeb | |
| 15:06:19 | aspiers | sean-k-mooney: "revive" might also work | |
| 15:06:45 | artom | mriedem, err, yes. WTF. | |
| 15:08:31 | mriedem | ok -1 until we have a good explanation of why https://review.openstack.org/#/c/595069/ was wrong | |
| 15:08:42 | mriedem | but maybe that explains a regression you're seeing downstream? | |
| 15:08:44 | dansmith | artom: okay, so tickling from compute manager will cause that event to start heading towards us because there are still interfaces available for neutron agent to find.. I kinda buy that, I guess... | |
| 15:08:48 | artom | mriedem, fair enough | |
| 15:09:21 | dansmith | I also -1d for the drunk speak in the commit message | |
| 15:09:27 | artom | ... | |
| 15:09:36 | artom | Well now you're just picking on em :( | |
| 15:09:38 | artom | *me | |
| 15:09:42 | dansmith | but yeah, I'd like to hear from lyarwood I guess | |
| 15:09:52 | mriedem | dansmith: it was my change :) | |
| 15:09:55 | mriedem | lyarwood backported it | |
| 15:10:24 | mriedem | and https://review.openstack.org/#/c/179228/ was your change :) | |
| 15:10:27 | mriedem | it's great | |
| 15:10:33 | artom | In a meeting now, I think I'll have to play around with it afterwards | |
| 15:10:37 | artom | Until now I was relying on logs | |
| 15:11:02 | artom | Yo-yo patches... | |
| 15:11:05 | stephenfin | oh, that's interesting | |
| 15:15:37 | dansmith | oh | |
| 15:15:40 | mriedem | artom: dansmith: ok so i see the race in https://review.openstack.org/#/c/595069/ | |
| 15:15:43 | mriedem | the dest triggers the event, | |
| 15:15:46 | mriedem | the source is waiting for it, | |
| 15:15:53 | mriedem | but it might come before the source is registered for the callback | |
| 15:16:13 | dansmith | so wait, we've gone back and forth twice now? | |
| 15:16:16 | mriedem | dest unplugs vifs when it calls driver.destroy | |
| 15:16:17 | mriedem | yes | |
| 15:16:40 | mriedem | when the original change was made by dansmith we didn't have the code in the API which routes events to both the source and host if the instance had a migration context | |
| 15:16:49 | mriedem | so when i made my change, the source will get the event routed to it, | |
| 15:17:00 | mriedem | but, we might not be registered for the callback on the source by the time the event arrives | |
| 15:17:39 | mriedem | as you can see from my comments in https://review.openstack.org/#/c/179228/ it's all very confusing | |
| 15:18:09 | mriedem | the sequence of events is kind of tribal knowledge with only 1.5 people in the tribe nowadays | |
| 15:19:45 | mriedem | so we should probably (1) revert my change and (2) add a comment in the libvirt driver finish_revert_migration code about why we don't wait for the event (like there is a comment in finish_migration) | |
| 15:20:06 | mriedem | because the event does come to the source, but we are racing to catch it | |
| 15:21:16 | mriedem | actually...finish_revert_migration does plug vifs, | |
| 15:21:22 | mriedem | so why wouldn't we get an event for that on the source? | |
| 15:21:43 | mriedem | dest unplugs vifs on revert_resize because of driver.destroy, | |
| 15:21:58 | mriedem | source plugs vifs because of finish_revert_migration which re-spawns the guest | |
| 15:22:21 | mriedem | artom: are you seeing this downstream with OVS or linuxbridge? | |
| 15:22:57 | artom | mriedem, ovs | |
| 15:23:13 | artom | mriedem, we do get the event on the source, just before we wait for it | |
| 15:23:30 | artom | Because as soon as Neutron gets the API call, it's able to wire them and send us the events | |
| 15:23:49 | mriedem | the network-vif-plugged event? | |
| 15:23:52 | artom | Yeah | |
| 15:24:20 | mriedem | ok that's from something else then | |
| 15:24:39 | mriedem | because when the source calls plug_vifs during finish_revert_migration, it's doing it within the context to wait for the event | |
| 15:25:21 | artom | mriedem, do you have access to slaweq's notes on our downstream BZ? | |
| 15:25:47 | artom | mriedem, https://bugzilla.redhat.com/show_bug.cgi?id=1678681#c8 | |
| 15:25:48 | openstack | bugzilla.redhat.com bug 1678681 in openstack-nova "REVERT_RESIZE stuck for 300s: "VirtualInterfaceCreateException: Virtual Interface creation failed" [Medium,On_dev] - Assigned to alifshit | |
| 15:26:35 | mriedem | artom: then i'm pretty sure it's what we've talked about before, finish_revert_resize on the source host calls migrate_instance_finish which updates the port binding to point at the source host, which triggers an event | |
| 15:26:40 | mriedem | *before* the driver registers the callback | |
| 15:27:28 | artom | mriedem, right, but there's another thing at play here - Neutron will only wire the ports after we plug them | |
| 15:27:33 | artom | ... I think | |
| 15:28:35 | mriedem | yes i can read his comments and i think they align with what i just said, | |
| 15:28:56 | mriedem | "Nova then asks neutron to bind port on compute-0 again, at it happens:" - that's the migrate_instance_finish call on the source finish_revert_resize to update the port binding | |
| 15:29:15 | mriedem | "And after that, L2 agent on compute-0 wire port again:" - i assume that's the plug_vifs call from the driver's finish_revert_migration on the source host | |
| 15:30:35 | mriedem | artom: how recreateable is this? it would be a lot more clear if we just had some logging on the source that said, "updating port bindings to point at the source host" and then "finish revert migration in the driver which will plug vifs" | |
| 15:30:53 | mriedem | because if we saw the event come between those 2 messages we know it's the port binding change that is triggering the event before we're ready to wait for it | |
| 15:30:54 | artom | mriedem, like, 70% in our CI? | |
| 15:31:06 | mriedem | are you able to test with patches from upstream? | |
| 15:31:21 | artom | I'd need to check with the CI guys, but I could provide them test builds, yeah | |
| 15:31:55 | kashyap | aspiers: You about...? | |
| 15:32:43 | kashyap | aspiers: When you are -- at the risk of adding more work ... wonder if we should simply split out the addition of getDomainCapablities() method into its own patch. | |
| 15:33:06 | mriedem | artom: ew do you have to patch an rpm or something? | |
| 15:33:13 | kashyap | It is just my OCD of "one logical change per-patch thing". And it allows quicker merge, too. As it'll be easier on the reviewers eyes | |
| 15:33:14 | artom | mriedem, scratch build | |
| 15:33:35 | artom | Backport a patch, build RPMs with that | |