Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-02
15:52:19 sean-k-mooney mriedem: is this all in relation to https://bugs.launchpad.net/nova/+bug/1323658
15:52:20 openstack Launchpad bug 1323658 in neutron "Nova resize/restart results in guest ending up in inconsistent state with Neutron" [Medium,Fix released] - Assigned to Dariusz Smigiel (smigiel-dariusz)
15:52:36 mriedem sean-k-mooney: no, https://review.openstack.org/#/c/540168
15:57:16 bauwser damn, interesting convo about vif pluging and neutron agents
15:57:24 giblet figleaf: Locally it raises for me if I pull the oslo ovo patch into my env (Depends-On: https://review.openstack.org/540344)
15:57:38 giblet figleaf: I mean it raised before the rebase
15:57:55 giblet figleaf: let me try with the new patch
15:58:22 bauwser mriedem: yeah, I guess the crux of the problem is how linuxbridge notifies the events
15:58:34 bauwser since that bug report is all about it
15:59:07 bauwser I mean for Quantum
15:59:08 sean-k-mooney welll so i think we are conflating too things here
15:59:16 sean-k-mooney vif pluging and vif binding
15:59:27 bauwser vif binding is the allocation, right?
15:59:28 sean-k-mooney vif plugging is the act of adding the tap to the backend
15:59:40 bauwser vif plugging is the fact to attach the vif to the guest ?
16:00:03 sean-k-mooney vif binding id that ack of the neutron ml2 direver populating the port details with a vif_type and vif_binding_details
16:00:25 mriedem this might be closer to the issue https://review.openstack.org/#/c/540168/1/nova/virt/libvirt/driver.py@5372
16:00:38 mriedem for one, we're operating on a stale cache to determine which vifs to wait for
16:00:46 mriedem b/c we never wait for the vifs to be unplugged and refresh the nw info cache
16:01:02 mriedem based on that, it's likely better to yes just ignore the vif plugged event,
16:01:19 mriedem in the server create case, we have the async network info wrapper that is waiting for the nw_info cache updates during port binding,
16:01:24 mriedem and then the virt driver spawn is waiting on those
16:01:40 mriedem so the nw info cache is up to date in the server create case, but we have nothing like that in the case of hard reboot
16:02:26 melwitt yeah, IIUC we get the network_info fresh at the beginning of a reboot and then that's it
16:02:33 sean-k-mooney mriedem: well one thing im not clear on. when we do a hard reboot do we redo the neutron port binding or jsut reuse the old one
16:02:43 mriedem re-use the existing one
16:03:38 sean-k-mooney well the neutron event is sent when neutron finish wirieing up a prot for the first time after portbinding. i dont know if neutron will send it again if we dont rebind
16:04:06 mriedem it must, otherwise this would be 100% fail in the gate
16:04:09 mriedem what we have is a race right now
16:04:25 sean-k-mooney mriedem: oh thats good to know
16:05:37 mriedem melwitt: i think we should probably not copy the comment from finish_migration
16:06:11 mriedem i bet this is primarily an issue based on (1) which network backend you're using and (2) the fact we're operating on a stale nw info cache, which is different from how spawn() works and waits for vif plug events
16:06:20 bauwser mriedem: melwitt: good point, the nw_info is only accurate when we call hard_reboot, not when it comes to _create_domain_and_nw()
16:06:24 melwitt okay
16:06:26 mriedem and therefore we should assume the vifs are already plugged and we don't need to wait for them
16:06:43 mriedem if we wanted to be thorough,
16:07:00 mriedem we'd (1) wait for the vif-unplugged event, and then (2) refresh the nw-info cache before moving on to plug the vifs
16:07:08 bauwser right
16:07:27 mriedem i'm not advocating that we do that in this patch, since we have to backport this
16:07:34 mriedem melwitt: so if you wanted to just update the comments, i'd be cool with that
16:07:56 melwitt I was scrutinizing the logs of a failed run yesterday
16:07:56 bauwser fair enough
16:10:10 sean-k-mooney mriedem: when you say the vif are already plugged you mean from a neutron point of view correct. not that the vif is added to the backend which is done by os-vif
16:10:50 mriedem sean-k-mooney: i'm talking about this vifs_already_plugged kwarg in the libvirt driver code
16:11:16 openstackgerrit Eric Fried proposed openstack/nova master: New-style _set_inventory_for_provider https://review.openstack.org/537648
16:11:17 openstackgerrit Eric Fried proposed openstack/nova master: SchedulerReportClient.update_from_provider_tree https://review.openstack.org/533821
16:11:18 openstackgerrit Eric Fried proposed openstack/nova master: Use update_provider_tree from resource tracker https://review.openstack.org/520246
16:11:19 openstackgerrit Eric Fried proposed openstack/nova master: Move refresh time from report client to prov tree https://review.openstack.org/535517
16:11:19 openstackgerrit Eric Fried proposed openstack/nova master: Fix nits in update_provider_tree series https://review.openstack.org/531260
16:11:20 openstackgerrit Eric Fried proposed openstack/nova master: Make generation optional in ProviderTree https://review.openstack.org/539324
16:11:21 sean-k-mooney mriedem: yes but im trying to figure out if this opens a security hole
16:11:46 sean-k-mooney mriedem: if that arge is set to True we will not wait for neutron
16:12:00 bauwser sean-k-mooney: well, all other server actions do the same, y'kno<
16:12:05 bauwser know*
16:12:14 sean-k-mooney os-vif will add the port when we call plug
16:12:32 bauwser sean-k-mooney: we're only able to correctly wait for Neutron in the boot case only because we have a fresh network info
16:12:37 sean-k-mooney but we wait for neutron to make sure the firewal rules are active on the port before unpasing the vm
16:12:45 bauwser fixing that in other server action calls would require far more
16:13:04 bauwser we == os-vif ?
16:13:15 sean-k-mooney we == nova
16:13:29 melwitt mriedem: he's the run I was looking at yesterday. there are no network-vif-unplugged events for the instance for the failed test (the hard reboot) http://logs.openstack.org/42/525842/11/check/neutron-tempest-linuxbridge/2502b64/logs/screen-q-svc.txt.gz#_Jan_29_16_01_28_170964
16:13:30 sean-k-mooney os-vif does not talk to neutron ever
16:13:53 melwitt I linked to a network-changed but if you look further in the log there are no unplug events
16:14:14 melwitt so I don't expect things would work if we tried to wait for an unplug event because neutron never sends one
16:16:14 sean-k-mooney melwitt: correct but wen we delete the tap any iprules create by the neutron security group driver are removed. if neutron never sees the device is unplugged and plugged again i dont think it will ever recreate them
16:18:18 melwitt sean-k-mooney: os-vif unplug deletes the tap, is what you are saying?
16:18:22 mriedem giblet: thanks for reviewing the nova-multiattach job change, replies inline https://review.openstack.org/#/c/532689/
16:18:51 sean-k-mooney melwitt: actully qemu doing a hard reboot does but os-vif will also remove it form the linux bridge
16:19:06 sean-k-mooney or ovs bridge depending on your life choices
16:19:32 melwitt sean-k-mooney: okay. and then the os-vif plug call will create *not* create the iprules again?
16:19:57 sean-k-mooney melwitt: the ip routes are created by the neutron security group driver not os-vif
16:20:19 melwitt hm
16:20:25 giblet mriedem: cool. looking in a minute
16:20:35 sean-k-mooney so if nova does not notify neutron i dont think it will reinstall them
16:20:57 sean-k-mooney melwitt: we may be able to simply rebind the port to the same host as part of the reboot
16:21:29 melwitt sean-k-mooney: if that's the case, I think that would be a latent bug because the behavior before this "re-initialize everything" approach still destroyed and undefined the libvirt domain. it just didn't do any os-vif unplug/plug
16:21:36 mriedem should we just not destroy_vifs on hard reboot and avoid this mess?
16:22:27 bauwser mriedem: well, looks like we somehow decided in the past to destroy everything
16:22:33 sean-k-mooney mriedem: the tap will still be delete by qemu/kvm/libvirt so its not an os-vif issue
16:22:49 mriedem bauwser: decided in the last 6 months
16:22:58 mriedem bauwser: this is a regression introduced in queens,
16:22:59 bauwser interesting
16:23:06 mriedem which is why i'm -W on the ocata backport
16:23:11 melwitt no, the handling of the libvirt domain did not change
16:23:14 openstackgerrit Balazs Gibizer proposed openstack/nova master: Escalate UUID validation warning to error in test https://review.openstack.org/540386
16:23:31 giblet figleaf: now your new unit test also passes in my local env ^^
16:23:50 giblet figleaf: btw thanks for that unit test. I did not now that we have tests for the fixtures
16:25:35 sean-k-mooney so i think we dont see this on ovs because of the ovsdb monitor which will see the port remved and readded by libvirt/os-vif. the gap seams to be that we dont have a reliable way to notify neutron that this happened for linuxbridge
16:26:35 melwitt if we're losing iptables rules upon reboot, we've always been losing them, if it has to do with destroying the libvirt domain because we have always been destroying the domain on a reboot and we also destroy it for a stop, etc
16:27:34 bauwser yeah, stop/start calls hard_reboot
16:27:35 bauwser fun
16:27:36 sean-k-mooney mlavalle: but only with linuxbridge when the polling intergave misses the removal and addtion
16:28:43 sean-k-mooney /intergave/interval/ that was bad even for me
16:29:04 melwitt my point is that is not new, that was not changed in the last 6 months. the change was doing os-vif unplug/plug and volume detach/attach
16:29:38 mriedem melwitt: i think you pointed this out before but this is likely a bogus debug message https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L346
16:29:49 mriedem _events being empty could just mean we already processed the event
16:29:53 sean-k-mooney melwitt: oh ok
16:29:58 mriedem _events == None means we are shutting down compute
16:30:20 bauwser melwitt: ping me when you're done with your new rev and I'll review it when I'm back
16:31:36 sean-k-mooney mriedem: you are correct the os-vif is new but the code in the linuxbidge pluging was copied directly from nova so i think the logic should be the same.

Earlier   Later