Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-04
16:41:43 dansmith artom: ++
16:41:59 mriedem btw i do a lot of ^ type stuff in the cross-cell resize functional testing to make sure the db is what i expect it to be
16:42:11 mriedem since i'm farting around with 2 dbs
16:42:17 gibi bauzas: cool, then your current -1 on the patch is just procedural while you reviewing the rest of the patch?
16:43:59 artom mriedem, I think the DB asserting part will be relatively easy, it's the running 2 services part that's finicky
16:44:43 artom dansmith, while I have you, can we context switch to your thoughts on https://review.openstack.org/#/c/639396/?
16:45:57 dansmith artom: ugh, I really don't want to... I know that is going to be ugly because of whatever has moved out of that scope, but I'm really (really) not in favor of changing how that works.. like REALLY.
16:46:04 bauzas gibi: I left a new comment as a soft -1
16:46:18 gibi bauzas: looking
16:46:21 bauzas gibi: but I'm continuing to review so I can +1 once I'm done
16:46:26 gibi bauzas: sure
16:46:30 bauzas gibi: just askign docd
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

Earlier   Later