14:02:20 <ihrachys> #startmeeting neutron_upgrades
14:02:21 <openstack> Meeting started Thu Jan 11 14:02:20 2018 UTC and is due to finish in 60 minutes.  The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:02:22 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:02:24 <openstack> The meeting name has been set to 'neutron_upgrades'
14:02:35 <lujinluo> o/
14:02:41 <ihrachys> hey, sorry for late start
14:02:52 <lujinluo> not a problem at all!
14:02:54 <ihrachys> there were no action items
14:03:15 <ihrachys> as some of you may have noticed, gate is unstable to the point where we can't land much these days
14:03:57 <lujinluo> yeah, portbinding patch has gone through around 10 recheck without passing zuul, LOL
14:04:02 <TuanVu> Hi everybody :)
14:04:07 <annp> hi
14:04:22 <ihrachys> multiple reasons: meltdown CVE patched in infra slowed down some jobs; infra patching services triggered POST_FAILUREs and TIMEOUTs; there are duplicate jobs because while we are moving from project-config jobs to in-tree jobs for zuulv3...
14:04:46 <ihrachys> I don't think we will be able to land much until we at least fix gate setup for zuulv3
14:05:02 <lujinluo> ok, understood
14:05:06 <ihrachys> it doesn't stop us from working on patches and reviewing them, but sets expectations properly
14:05:21 <TuanVu> thanks for the info, Ihar
14:05:46 <ihrachys> oh good news - duplicate jobs are mostly gone: https://review.openstack.org/#/c/530500/
14:05:57 <ihrachys> so rechecks may be more effective now
14:06:49 <annp> +1
14:06:51 <ihrachys> anyway... let's dive into patches
14:07:01 <ihrachys> https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db
14:07:27 <ihrachys> before we go through those requiring reviews, let's (re)check those with W+1
14:07:53 <ihrachys> ok I see both patches are rechecked
14:08:02 <ihrachys> not sure if before or after the project-config change landed.
14:08:09 <ihrachys> so it may need another round of rechecks :)
14:08:23 <ihrachys> ok now top to bottom
14:08:37 <ihrachys> first is https://review.openstack.org/521797 "Use Router OVO in external_net_db"
14:10:12 <hungpv_> Is there anything needs to be concerned?
14:10:21 <ihrachys> I am looking right now
14:10:29 <ihrachys> a minor question: https://review.openstack.org/#/c/521797/16..21/neutron/db/external_net_db.py
14:10:41 <ihrachys> why do we need to call .all() if you iterate through it right away?
14:10:56 <ihrachys> I think when you iterate, it will already call .all() under the hood
14:11:04 <ihrachys> (or better, will fetch records lazily)
14:11:29 <hungpv_> Oh, i didn't know it
14:11:40 <hungpv_> I'll check again
14:11:54 <ihrachys> I think that's what will happen. it's a minor thing, not a bother really.
14:12:45 <hungpv_> Ok
14:14:35 <ihrachys> ok this is more serious: https://review.openstack.org/#/c/521797/21/neutron/objects/router.py
14:15:34 <ihrachys> I don't think the code is correct
14:16:28 <ihrachys> and I would say, at this point we should first fix test coverage for the code because it's clearly not covered properly if this didn't trigger an error.
14:16:36 <ihrachys> so I am with slaweq on adding a UT
14:17:38 <lujinluo> why is 'gw_port_id' not updatable?
14:18:56 <hungpv_> Hmm, let's me recall
14:19:41 <ihrachys> it's a foreign key so maybe that
14:20:03 <hungpv_> I'll check again tomorrow
14:20:10 <ihrachys> though it should be able to be None I guess?
14:20:17 <lujinluo> yes
14:20:26 <lujinluo> nullable is true
14:21:06 <ihrachys> good question
14:21:35 <ihrachys> lujinluo, please leave a comment for that one there
14:22:16 <lujinluo> done
14:22:42 <ihrachys> ok cool
14:22:48 <ihrachys> so clearly there is more work to do there
14:22:59 <ihrachys> moving on
14:23:04 <ihrachys> https://review.openstack.org/#/c/507772/ "Use Network OVO in db_base_plugin"
14:23:21 <TuanVu> I’ve already updated the patch as your recommendation and still continue working on it
14:23:35 <TuanVu> At this moment, I’m working on this failed unit test:
14:23:36 <TuanVu> “neutron.tests.unit.plugins.ml2.test_plugin.TestNetworksV2.test_update_network_set_not_shared_other_tenant_returns_409”
14:23:36 <TuanVu> It’s failing at create_port step
14:24:10 <TuanVu> 
14:24:17 <TuanVu> It looks like we have problem with “obj_context.session.refresh”
14:24:17 <TuanVu> I’m trying to apply the same solution from xujun’s patch set
14:24:17 <TuanVu> https://review.openstack.org/#/c/530182/17/neutron/objects/base.py
14:24:30 <TuanVu> 
14:24:32 <ihrachys> do you have any significant comments from previous patchsets not addressed yet?
14:24:34 <TuanVu> But I’m not sure if it’ll work, so any suggestion is welcome
14:25:07 <TuanVu> hmm, let me see ..
14:26:13 <TuanVu> ok, I think all of your comments have been addressed in the latest patch
14:26:21 <TuanVu> PS22
14:27:25 <TuanVu> my main concern at this moment is above failed unit test: "“neutron.tests.unit.plugins.ml2.test_plugin.TestNetworksV2.test_update_network_set_not_shared_other_tenant_returns_409”
14:28:51 <ihrachys> am I looking at the very latest patchset?
14:28:52 <ihrachys> 22
14:29:07 <ihrachys> because I see some replies that something is Done but it's the smae
14:29:09 <ihrachys> *same
14:29:22 <TuanVu> could you please point out which one?
14:29:37 <ihrachys> like https://review.openstack.org/#/c/507772/19..22/neutron/plugins/ml2/plugin.py line 888
14:29:43 <ihrachys> or 874 same file
14:30:22 <ihrachys> https://review.openstack.org/#/c/507772/19..22/neutron/objects/network.py line 313 seems wrong - you ripped off implementation; I think you should have it there, otherwise it won't detect existing tenants relying on a rbac entry
14:30:45 <ihrachys> also https://review.openstack.org/#/c/507772/19..22/neutron/db/db_base_plugin_common.py line 299
14:30:55 <ihrachys> it says you went with all Network but the code is same
14:31:58 <ihrachys> so I am not sure if it's latest / all comments handled.
14:32:10 <ihrachys> the missing implementation for bound tenants is especially concerning
14:32:22 <ihrachys> because it should have functional impact on the object
14:32:33 <ihrachys> we can't have this merged without rbac behavior fixed for the object
14:32:34 <TuanVu> hmm, lot's of concern, so let me answer one by one, is that ok?
14:33:01 <TuanVu> https://review.openstack.org/#/c/507772/19..22/neutron/plugins/ml2/plugin.py line 888
14:34:00 <ihrachys> yeah go ahead. maybe not every one but this one and bound tenants implementation may be worth a discussion right now
14:34:20 <ihrachys> or if you have legit answers to each, we can as well follow up in gerrit
14:34:28 <TuanVu> somehow I missed this one, thank you for pointing that out
14:34:39 <TuanVu> yeah, better to follow up on gerrit
14:35:01 <TuanVu> about bound tenants implementation, I'll update it as your recommendation
14:35:07 <TuanVu> thanks a lot, Ihar
14:35:13 <ihrachys> ok cool
14:35:35 <ihrachys> next in list is https://review.openstack.org/530182 "Use Router OVO in l3_db"
14:35:43 <ihrachys> I don't think I ever checked it
14:36:39 <ihrachys> this is probably the most controversial thing there: https://review.openstack.org/#/c/530182/17/neutron/objects/base.py
14:36:43 <ihrachys> (refresh OVO method)
14:37:53 <TuanVu> it looks like xujun is not online
14:38:05 <TuanVu> is that ok?
14:38:11 <ihrachys> though it kinda makes sense to reflect hacks we already have with sqlalchemy
14:39:11 <ihrachys> TuanVu, yeah I think we will move on. I will leave feedback there. there is another issue with 'description' field there (that is supposed to be introduced by standard attributes logic already)
14:40:11 <ihrachys> next is https://review.openstack.org/#/c/506037/ "Part II of Integrate Port OVO"
14:40:21 <ihrachys> lujinluo, I assume we were waiting for port binding to land
14:40:37 <ihrachys> which should hopefully be next days granted the gate works
14:40:52 <lujinluo> besides that, it also needs some extra work
14:40:58 <lujinluo> so no hurry ;)
14:41:09 <ihrachys> oh ok. would you benefit from a review iteration?
14:41:40 <lujinluo> no, maybe after i make it pass all the tests
14:42:10 <ihrachys> ok good
14:42:38 <ihrachys> there are no patches in queue that seem ready for a discussion otherwise
14:43:48 <ihrachys> anything that we missed? (like, patches with wrong topic?)
14:44:40 <ihrachys> I have this revert in trunk service to unbreak the way we use engine facades in scope of OVO: https://review.openstack.org/#/c/532343/
14:44:50 <ihrachys> that's similar to IPAllocation issue that I was talking a while back
14:46:11 <ihrachys> ok seems like nothing
14:46:36 <ihrachys> I walked through etherpad with remaining bits: http://etherpad.openstack.org/p/neutron-remaining-ovo and stroke through those patches that merged
14:47:30 <lujinluo> oh thanks Ihar
14:47:52 <ihrachys> port binding section should be mostly gone this week
14:47:59 <TuanVu> thanks for your update, Ihar
14:48:00 <ihrachys> there is lots of work still though
14:48:08 <lujinluo> yeah
14:48:11 <ihrachys> ok, anything else you want to discuss?
14:48:24 <lujinluo> none from me
14:48:31 <TuanVu> me either
14:49:01 <ihrachys> ok
14:49:21 <ihrachys> if you have cycles and have no patches on your plate, etherpad would be a good place to look for next thing
14:49:27 <ihrachys> thanks everyone for joining
14:49:31 <lujinluo> thanks everyone!
14:49:34 <ihrachys> we meet next week!
14:49:34 <ihrachys> #endmeeting