Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-04
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 dansmith okay, I'm even less concerned then :)
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 bauzas gibi: left yet another comment https://review.openstack.org/#/c/623543/39
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 bauzas gibi: that's the problem, I don't think so
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: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,
17:10:26 bauzas it's a bit sad to query it twice
17:10:27 mriedem because it has to clear it on sighup
17:10:41 gibi mriedem: ooh
17:10:41 mriedem dansmith: i don't know the answer to that offhand
17:10:47 artom dansmith, ah, OK. It's still only a bandaid tho...
17:10:59 mriedem gibi: bauzas: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L551
17:11:02 artom dansmith, thanks, that'll allow me to eat :P
17:11:10 gibi bauzas: sure your concern is valid either I would like to use a ProviderTree or add a local cache
17:12:00 gibi mriedem: looking
17:12:13 mdbooth mriedem: The purpose of the timestamp stuff IIUC is to assert that the contents of the volume are preserved across the migration. I personally think it's an important test, but if it's problematic in practise we'd need to be pragmatic, e.g. removing or guarding it. NP if you don't have a strong opinion, lyarwood and I can fling fedoras at each other internally :)
17:13:26 mriedem well, give a justifiable reason in the commit message to remove it
17:13:26 gibi bauzas, mriedem: so here in the compute/manager we only need the name of the RP and we know the UUID of it. So I guess I should be able to use the ProvideTree cache for that
17:14:27 mriedem need another core on the 2.71 novaclient change https://review.openstack.org/#/c/640657/ so we can get that into the final release
17:14:43 bauzas gibi: the cache is a non-public information from the reportclient
17:14:51 bauzas gibi: at the moment, I mean
17:15:10 bauzas mriedem: on it

Earlier   Later