Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-14
19:35:19 sean-k-mooney mnaser: that bug seams to focus on admin state which is different
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 jaypipes edleafe: +2 from me.
19:56:10 sean-k-mooney mnaser: ya
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 openstack Launchpad bug 1755890 in OpenStack Compute (nova) "Instances fail to hard reboot when using OpenDaylight" [Undecided,New]
20:02:41 mnaser related-bug or closes-bug to https://bugs.launchpad.net/nova/+bug/1755890 ?
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?
20:32:11 dansmith efried: we took a vote
20:32:17 dansmith sad to say, you're off the island
20:32:20 efried And edleafe won???
20:32:23 efried Come ON!
20:32:27 dansmith hah
20:33:06 efried I mean, he does have seniority.
20:33:10 efried SERIOUS seniority.
20:33:41 jaypipes efried: your torch has been extinguished.
20:34:04 dansmith you must leave the tribal council area IMMEDIATELY
20:34:23 jaypipes dansmith: edleafe has won the microversion battle being currently waged.
20:34:34 dansmith ah heh
20:34:55 efried dansmith: It's not too late! You can saaaaave me!
20:35:05 jaypipes efried: that is correct. when a person is voted off the island in the survivor show, their torch is extinguished.
20:35:15 dansmith I wouldn't mind seeing a fire making challenge between the two of you
20:35:33 jaypipes efried: or at least, that's the way it was the last time I saw that show, which would have been around 2005.
20:35:45 dansmith efried: nah, you have to use flint and coconut husk

Earlier   Later