Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-14
19:27:17 cfriesen yep
19:28:07 cfriesen it'll query the vswitch API
19:29:03 sean-k-mooney cfriesen: ya the issue is that bar seeting external-ids:iface-status=active which is an ovs agent specif thing there is no way to tell for ovs in general.
19:31:51 sean-k-mooney when odl first intregrated support into the ml2 framework this gap was discussed back in icehouse but they had no way syncronis state in odl with neutron without a intoducing an sdn constrolller specific api, b having odl emit notifcation on the rabbitmq bus or be haveing the neutron server poll odl for state change
19:32:27 sean-k-mooney the chose to do non of the above an just report vif_plugged on port binding instead and its been that way ever since
19:32:59 mnaser sean-k-mooney: maybe this is a networking-odl bug? i know that right now it creates a websocket that listens for state updates
19:33:01 mnaser https://github.com/openstack/networking-odl/blob/master/networking_odl/ml2/port_status_update.py
19:33:28 mnaser https://bugs.launchpad.net/networking-odl/+bug/1686023
19:33:29 openstack Launchpad bug 1686023 in networking-odl "networking-odl dynamic port status update full support missing" [Low,In progress]
19:33:32 mriedem jaypipes: replied in https://review.openstack.org/#/c/539605/ - need bauzas to probably elaborate at this point since i don't remember this being talked about at the ptg
19:33:42 mnaser i guess they support DOWN => ACTIVE but not ACTIVE=>DOWN
19:34:44 mnaser i wonder if there's an easy way to let neutron know that the port has gone down in there
19:34:45 mnaser and avoid the revert
19:34:48 sean-k-mooney mnaser: well i think its a ml2 framework feature request. e.g. allow agentless backend notify neutron of state changes
19:35:15 mriedem if we can blame this all on an incomplete neutron backend then that works for me
19:35:19 sean-k-mooney mnaser: that bug seams to focus on admin state which is different
19:35:19 mnaser lolol
19:36:01 mriedem https://review.openstack.org/#/c/465463/
19:36:04 mnaser sean-k-mooney: https://github.com/openstack/networking-odl/blob/master/networking_odl/ml2/port_status_update.py#L91-L95 ever that little bit? the bug talks about admin state but it seems to get the actual port state
19:37:20 sean-k-mooney mnaser: yest that should adress this but it seams that odl is not detecting the removal of the port and readding it
19:38:22 sean-k-mooney mnaser: perhaps the networking-odl ml2 driver is just not sending the notification to nova when it recives the notification from odl
19:38:41 mriedem shouldn't the driver go through the normal notificatoin code in neutron?
19:39:05 mnaser sean-k-mooney: its certainly not sending it when its being unplugged, but it looks like the code to watch for state doesnt even update it to 'down' or 'unplugged' or whatever state it should be in neutron
19:39:17 sean-k-mooney mriedem: that notification code is triggered by port status updates on the rpc bus
19:39:28 sean-k-mooney mriedem: these update from odl are from the websocket
19:41:19 sean-k-mooney mriedem: since we are not storing the state changes from https://github.com/openstack/networking-odl/blob/master/networking_odl/ml2/port_status_update.py#L91-L95 in the db i dont think https://github.com/openstack/neutron/blob/master/neutron/db/db_base_plugin_v2.py#L155-L169 will be invoked
19:41:52 mnaser sean-k-mooney: is there an example of a plugin that is storing/updating state changes in db which might be good to reference
19:42:58 sean-k-mooney maybe look at the ovs agent code.
19:43:05 mnaser https://github.com/openstack/networking-ovn/blob/master/networking_ovn/ml2/mech_driver.py#L713-L729
19:43:13 mnaser i guess i can use that as a reference and try submitting something to networking-odl
19:44:27 sean-k-mooney i dont think we actully need to store it in the db. we just need to send the notification to nova
19:45:07 mnaser i guess one thing though is talking about the revert is... if this is going to affect other drivers too
19:45:16 mnaser aka we fix odl but find 4 other vif types that are affected by that issue
19:46:17 sean-k-mooney mnaser: we are already special casing lb and we need to skip waithing i think for sriov too so basically we can only wait for ovs with agents today
19:46:54 sean-k-mooney maybe we can wait for ovn? but not sure.
19:49:47 mriedem unless the port profile/details add some specific flag that says we can expect events on replug, i don't think it's safe to just have the blanket wait that we have today
19:50:12 openstackgerrit Ed Leafe proposed openstack/nova master: Add 'member_of' param to GET /allocation_candidates https://review.openstack.org/552098
19:50:15 edleafe jaypipes: cdent: ^^ Addressed your comments
19:50:42 sean-k-mooney mriedem: that is true. i think this is something that first needs to made a requirement for all neutron backend before nova can start to depend on it.
19:51:13 sean-k-mooney mriedem: one option is add a nova config opetion to the compute node? we wait there correct? but not ideal
19:52:42 mriedem it's an option yes but not a good one imo
19:52:53 mriedem we already have vif_plugging_timeout and vif_plugging_is_fatal
19:53:11 mriedem we'd have to add a vif_plugging_reboot_if_hard_and_libvirt_and_thursday_timeout
19:54:46 sean-k-mooney hehe well it could be vif_wait_for_network but ya only ovs + reference agents could set it to true
19:55:11 sean-k-mooney it would have to default to false and proably should be deprecated from the start
19:56:01 mnaser i guess a revert seems like the cleanest possible choice? :x
19:56:10 sean-k-mooney mnaser: ya
19:56:10 jaypipes edleafe: +2 from me.
19:56:30 mnaser i'm not sure how a revert would happen because this seems to be 2 patches. a patch that reverts both referencing both commits?
19:56:36 mnaser unless someone else volunteers to do this
19:57:34 sean-k-mooney what were the 2 commits again. it will be next week if i do it at the earliest.
19:57:56 mriedem i think it's just this one https://review.openstack.org/#/q/Ib08afad3822f2ca95cfeea18d7f4fc4cb407b4d6
19:58:21 mriedem that's the one that adding the wait back in
19:58:23 mnaser what about https://review.openstack.org/#/q/Ib0cf5d55750f13d0499a570f14024dca551ed4d4 ?
19:58:39 mriedem that's what you want to happen, not wait
19:58:56 mriedem there were 3 changes:
19:59:21 mriedem 1. full hard reboot blasting everything away and re-plug; that caused issues because we were waiting for something that wouldn't happen
19:59:29 mriedem 2. to workaround ^, we stopped waiting
19:59:46 mriedem 3. because not waiting when it will work for some (ovs but not lb), the final patch was added which added the bridge conditional
19:59:53 mriedem but as we've now seen, that doesn't work for ovs (odl)
20:00:04 mriedem so revert #3 to go back to not waiting
20:00:15 mriedem which is https://review.openstack.org/#/c/541442/
20:00:25 sean-k-mooney mriedem: right https://review.openstack.org/#/c/541442/6/nova/virt/libvirt/driver.py skip waiting for lb and then larwood was extending it for sriov then odl came up
20:00:45 mriedem yeah so it's whack-a-mole at this point
20:01:05 mriedem heh, we could just revert all of them
20:01:06 mnaser so revert master and then cherry pick the reverts once they land?
20:01:08 mriedem going back to https://review.openstack.org/#/c/400384/
20:01:18 mriedem mnaser: cherry picking reverts is weird,
20:01:22 sean-k-mooney mriedem: so one patch to just remove waithing on reboot and backporting that is proable the simpelst thing
20:01:24 mriedem i'd just revert individually on all branches
20:01:26 mnaser ok i see
20:01:37 sean-k-mooney mriedem: or that.
20:02:21 mriedem i was never fully comfortable with https://review.openstack.org/#/c/400384/
20:02:34 mriedem it was trying to fix a problem with encrypted volumes and overshot probably to include vifs
20:02:41 mnaser related-bug or closes-bug to https://bugs.launchpad.net/nova/+bug/1755890 ?
20:02:41 openstack Launchpad bug 1755890 in OpenStack Compute (nova) "Instances fail to hard reboot when using OpenDaylight" [Undecided,New]
20:02:49 mnaser (for the revert)
20:02:52 mriedem closes
20:03:48 openstackgerrit Mohammed Naser proposed openstack/nova master: Revert "Refine waiting for vif plug events during _hard_reboot" https://review.openstack.org/553035
20:04:14 sean-k-mooney mriedem: we have to be carful not to reintroduce https://bugs.launchpad.net/nova/+bug/1724573 by reverting https://review.openstack.org/#/c/400384
20:04:15 openstack Launchpad bug 1724573 in OpenStack Compute (nova) "encrypted volumes are directly attached to instances after a compute host reboot" [Medium,Fix released] - Assigned to Matthew Booth (mbooth-9)
20:04:31 openstackgerrit Mohammed Naser proposed openstack/nova stable/queens: Revert "Refine waiting for vif plug events during _hard_reboot" https://review.openstack.org/553037
20:04:58 openstackgerrit Mohammed Naser proposed openstack/nova stable/pike: Revert "Refine waiting for vif plug events during _hard_reboot" https://review.openstack.org/553038
20:05:08 mnaser https://review.openstack.org/#/c/542767/
20:05:11 mnaser can someone 'block' that from merging
20:05:17 mnaser dansmith: maybe remove your +W?
20:05:50 dansmith ack
20:05:52 sean-k-mooney mnaser: it has a zuul -2 because its in merge conflict
20:06:16 mnaser sean-k-mooney: figure just in case someone doesnt notice it :)
20:06:53 mnaser https://review.openstack.org/#/q/topic:bug/1744361+is:open+owner:%22Mohammed+Naser+%253Cmnaser%2540vexxhost.com%253E%22
20:07:03 mriedem sean-k-mooney: yeah if we reverted all the way, we'd have to do a partial revert of https://review.openstack.org/#/c/400384 and only remove the changes for the vif plugging part
20:08:41 sean-k-mooney well looking at https://review.openstack.org/#/c/553035/ i think it solve the imidiate odl issue at least for now
20:09:07 sean-k-mooney that seams like the minimal change at the cost of never waiting
20:10:27 openstackgerrit Matt Riedemann proposed openstack/nova master: Make nova-cells-v1 run with neutron https://review.openstack.org/549789
20:14:17 mriedem if we wanted to go back to https://review.openstack.org/#/c/400384/,
20:14:28 mriedem we could pass destroy_vifs=False through self.destroy -> self.cleanup
20:14:39 mriedem because that was the change in behavior in that patch that caused a lot of the trouble
20:15:03 mriedem as i said, https://review.openstack.org/#/c/400384/ was meant to fix an encrypted volume issue and turned into "let's just fully destroy the damn thing and everything associated with it except disks"
20:21:03 sean-k-mooney im going to head off soon. FYI ill be online intermitely tommorw and then off until tuesday.
20:31:59 efried You don't like me anymore, jaypipes?

Earlier   Later