| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-04 | |||
| 16:46:43 | gibi | bauzas: I will let you go through it before I start updating the patch | |
| 16:46:57 | artom | dansmith, yeah :/ | |
| 16:47:20 | artom | dansmith, as I said in the review, I don't see a way around *some* change, because we race on setting up the event waiter | |
| 16:47:21 | gibi | bauzas: would you accep the documentation in a separate patch? | |
| 16:47:32 | artom | If we don't get to the driver code by the time Neutron sends us the event, it's lost | |
| 16:48:11 | dansmith | artom: yeah, but neutron is sending the event in response to things we're doing, which just means whatever we do to trigger it needs to happen within the context | |
| 16:48:26 | bauzas | gibi: yup | |
| 16:48:32 | gibi | bauzas: coolio | |
| 16:48:36 | bauzas | I can turn into a +1 if you prefer | |
| 16:48:55 | gibi | bauzas: don't worry, I will respin anyhow | |
| 16:49:06 | artom | dansmith, yeah - but we're poking Neutron in the compute manager, and setting up the waiter in the virt driver... | |
| 16:49:12 | gibi | bauzas: just let me know if you spot any other issues | |
| 16:49:19 | dansmith | artom: I understand | |
| 16:49:21 | artom | From the virt driver, I should say. It's a call to the compute manager | |
| 16:49:46 | artom | Hrmm, there's no RPC, mind you | |
| 16:49:56 | artom | Just... a whole bunch of intermediary steps | |
| 16:50:07 | bauzas | gibi: I'm on it | |
| 16:50:13 | gibi | bauzas: thanks a lot | |
| 16:50:29 | bauzas | I wish I could help better, that's it | |
| 16:51:45 | artom | dansmith, anyways, I'll try to come up with a least bad way of doing it, we'll have a strawman to pick apart | |
| 16:52:03 | artom | At least you're on board with the need for change ;) | |
| 16:54:06 | mriedem | artom: so we likely need 2 context managers to wait for separate events right? | |
| 16:54:15 | mriedem | one for the port binding host change, and one for the vif plug | |
| 16:54:29 | mriedem | that port binding host change happens in several move operations we'd care about (at least resize and evacuate) | |
| 16:54:37 | mriedem | and unshelve i think... | |
| 16:54:58 | artom | mriedem, hrmm, you lost me, how would 2 context managers solve this? | |
| 16:55:14 | mriedem | couldn't we create a helper method for that neutronv2.api method that updates the host in the port binding to register an event callback | |
| 16:55:22 | mriedem | artom: we have 2 events to wait for yeah? | |
| 16:55:40 | mriedem | so you wait for the first (port binding host change), then call the driver to spawn or whatever which does the vif plug | |
| 16:55:45 | mriedem | which triggers the event we've always waited for | |
| 16:56:14 | artom | mriedem, uhh, I thought it was Nova that PUT'ed the post binding host change? | |
| 16:56:28 | mriedem | it does | |
| 16:56:38 | artom | And that got us a vif-plugged as a "response"? | |
| 16:56:42 | mriedem | and then we get an event from neutron that we're not waiting for | |
| 16:57:08 | mriedem | and then we actually plug vifs which produces another event yeah? | |
| 16:57:11 | dansmith | artom: was it not updating instance_extra.migration_context, or was it not persisting the applied-from-context instance attributes? | |
| 16:57:13 | artom | We are, sort us, we just start waiting after it gets to us, but yeah, you're basically right | |
| 16:57:15 | dansmith | artom: I had assumed the former | |
| 16:57:29 | artom | dansmith, the latter | |
| 16:57:32 | dansmith | ah | |
| 16:57:42 | dansmith | artom: which attributes of those specifically? | |
| 16:58:00 | artom | dansmith, so I haven't looked into it in detail, but I'm told the new NUMA topology | |
| 16:58:02 | dansmith | numa_topology? | |
| 16:58:11 | dansmith | you probably need to do something to make those dirty, | |
| 16:58:16 | artom | ... | |
| 16:58:23 | dansmith | since they're nested objects, they probably only save if the object itself is dirty | |
| 16:58:35 | dansmith | artom: you need to give them "the talk" | |
| 16:59:09 | artom | Right, tell them they've been naughty and need to be saved from their sins? | |
| 17:00:05 | mdbooth | lyarwood: Ok, I can see in the tempest logs that tempest receives a 404 for GET volume *before* deleting the type, and I can see the same in the cinder logs. However, c-vol doesn't log the completion of the delete until some time later, which is *after* the attempted deletion of the type. | |
| 17:00:23 | dansmith | they must not be marked as changed in the migration context, so I'm thinking just applying them to the instance and then calling save doesn't have them included in obj_what_changed() | |
| 17:00:31 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Microversion 2.71 - show server group https://review.openstack.org/640657 | |
| 17:00:36 | artom | mriedem, wait, plugging the vifs produces *another* event? | |
| 17:01:06 | artom | mriedem, I need to flow through the code again in that case, because it means I don't have a complete picture | |
| 17:01:08 | sean-k-mooney | artom: binding the vif produces a network-chaged event | |
| 17:01:13 | mriedem | artom: that was the original reason for adding the context manager in the virt driver | |
| 17:01:20 | artom | Oh, so binding -> network-changed | |
| 17:01:27 | artom | vig plugging -> vif-plugged | |
| 17:01:34 | mriedem | i don't think that's correct | |
| 17:01:36 | sean-k-mooney | when the neutron agent plugs the vif we get a network-vif-plugged event | |
| 17:01:57 | mriedem | if they were different events, we wouldn't be missing the 2nd event | |
| 17:02:31 | dansmith | the problem is the vif shows up on the destination, neutron notices and immediately sends the vif-plugged event right? | |
| 17:02:32 | mdbooth | mriedem: lyarwood was having trouble with the timestamp tests being flaky and removed them in https://review.openstack.org/#/c/639331/ . I vaguely recall you previously expressing an opinion on these tests. | |
| 17:02:33 | sean-k-mooney | i dont think we need to move the handeling of the network-chaged event | |
| 17:02:35 | dansmith | before nova is ready to start it | |
| 17:02:43 | mriedem | as far as i understand the problem, we change the port binding host and then plug vifs, which normally happens close enough that neutron sends a single network-vif-plugged event which we're waiting for by the time we plugged vifs and registered a callback in the virt driver | |
| 17:02:49 | sean-k-mooney | dansmith: yep basically | |
| 17:03:31 | dansmith | mriedem: and this is happening in the migrate case in a different place than it happens during spawn and that's the issue, right? | |
| 17:03:56 | sean-k-mooney | neutron sees the port on the sorce because we have not clean it up yet so when the rollback of the migration happens and we set the host back to the source neturon send the vif-plugged event right away | |
| 17:04:51 | dansmith | oh, it's the rollback case only? | |
| 17:05:28 | artom | dansmith, revert resize is the downstream bug | |
| 17:05:38 | artom | Hrmm, do we actually have 2 problems here? | |
| 17:05:46 | bauzas | gibi: left yet another comment https://review.openstack.org/#/c/623543/39 | |
| 17:05:46 | sean-k-mooney | dansmith: artom is fixing the rollback case. there is another race with sdn controler that send vif-plugged when you bind the port that this will also fix | |
| 17:05:46 | dansmith | okay, I'm even less concerned then :) | |
| 17:05:57 | bauzas | gibi: can't we do all of this on the libvirt driver anyway? | |
| 17:06:12 | dansmith | the rollback case could just not wait for events since it's already on the source, I would think.. that would be a band-aid worth doing I would think | |
| 17:06:39 | bauzas | gibi: but I guess it's not possible because you wanna claim() this | |
| 17:06:59 | gibi | bauzas: looking | |
| 17:07:39 | artom | dansmith, that'd be a weird bit of into to pass down to the virt driver, no? | |
| 17:07:47 | artom | ... cuz we just call spawn there | |
| 17:08:03 | dansmith | we already pass a similar flag in places don't we? | |
| 17:08:04 | mriedem | mdbooth: i don't really remember that, but i'm assuming that timestamp stuff was copied from other scenario tests, and the commit message doesn't really explain the problem with the timestamp stuff | |
| 17:08:12 | mriedem | just "it's bad" isn't very good | |
| 17:08:23 | artom | dansmith, I'd have to check | |
| 17:08:44 | dansmith | maybe I'm thinking about within the driver, but still | |
| 17:08:47 | mriedem | dansmith: upstream it's at least known to be happening in evacuate | |
| 17:08:58 | mriedem | http://status.openstack.org/elastic-recheck/#1813789 | |
| 17:09:01 | gibi | bauzas: yeah we need the info to drive the pci claim | |
| 17:09:08 | dansmith | mriedem: on evacuate, not evacuate and then restart-on-original-host right? | |
| 17:09:13 | sean-k-mooney | ya i was going to say the same. but dan is right we could skip waiting in this case but i kindo of like not have another flag to pass down | |
| 17:09:18 | gibi | bauzas: does compute/manager have access to the ProvideTree? | |
| 17:09:19 | bauzas | gibi: but you understand my concern, right? | |
| 17:09:32 | dansmith | mriedem: the evac case seems ripe for just moving where we wait, but I haven't gone digging | |
| 17:09:39 | sean-k-mooney | the flag would only fix the rollback case. artom change would fix the cisco aci/old odl case | |
| 17:09:39 | bauzas | gibi: that's the problem, I don't think so | |
| 17:09:41 | gibi | bauzas: I understand that we call placement a lot | |
| 17:09:47 | sean-k-mooney | in addtion to rollbacks | |
| 17:10:12 | dansmith | artom: thinking of this: vifs_already_plugged=False, | |
| 17:10:18 | bauzas | gibi: and say you have two pci requests for the same interface... | |
| 17:10:22 | mriedem | gibi: bauzas: pretty sure the compute manager has a handle to the provider tree, | |