15:01:21 <ihrachys> #startmeeting neutron_upgrades
15:01:22 <openstack> Meeting started Mon Nov 28 15:01:21 2016 UTC and is due to finish in 60 minutes.  The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:01:23 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:01:25 <openstack> The meeting name has been set to 'neutron_upgrades'
15:01:26 <electrocucaracha> howdy
15:01:27 <korzen> hello
15:01:29 <sindhu> Hi
15:01:31 <johndperkins> hi
15:01:35 <ihrachys> giving a minute for everyone to join...
15:02:42 <korzen> thanks to Thanksgiving, I could catch up with reviews
15:02:46 <dasanind_> Hi
15:02:59 <ihrachys> sorry was pulled by someone evil!
15:03:02 <ihrachys> but now I am back
15:03:03 <ihrachys> #link https://wiki.openstack.org/wiki/Meetings/Neutron-Upgrades-Subteam Agenda
15:03:20 <ihrachys> #topic Announcements
15:03:46 <ihrachys> nothing to report here I believe. please register for PTG in Feb 2017 ;)
15:03:55 <ihrachys> #topic Partial Multinode Grenade
15:04:15 <ihrachys> I think jschwarz is still working on linuxbridge grenade job
15:04:40 <jschwarz> I am
15:04:50 <ihrachys> there is a patch that should have helped: https://review.openstack.org/#/c/400258/ but it does not seem like it fixed the failure as seen in http://logs.openstack.org/59/396659/2/experimental/gate-grenade-dsvm-neutron-linuxbridge-multinode-nv/f652b83/logs/testr_results.html.gz
15:05:22 <jschwarz> it's probably because the fix is for neutron-legacy and not for neutron
15:05:25 <ihrachys> jschwarz: for what I understand, it seems like nova tries to land a port on a compute node that is (no longer) registered in neutron-server
15:05:30 <jschwarz> I'm gonna port it to both ways and see if it works
15:05:37 <ihrachys> jschwarz: do we use lib/neutron for the job?
15:05:53 <ihrachys> jschwarz: you can check devstacklog to determine it
15:05:57 <jschwarz> ihrachys, that's what I wasn't sure of - I implemented the fix for lib/neutron-lib iirc
15:06:04 <jschwarz> ihrachys, will have a thorough look now
15:06:14 <ihrachys> jschwarz: you mean -legacy not -lib right?
15:06:20 <jschwarz> ihrachys, sorry yes
15:06:50 <ihrachys> jschwarz: seems -legacy: http://logs.openstack.org/59/396659/2/experimental/gate-grenade-dsvm-neutron-linuxbridge-multinode-nv/f652b83/logs/grenade.sh.txt.gz#_2016-11-23_11_12_09_237
15:07:00 <ihrachys> jschwarz: I am actually not sure if we use lib/neutron anywhere in our own gate
15:07:05 <ihrachys> which is a shame ;0
15:07:19 <jschwarz> ihrachys, ack
15:07:20 <ihrachys> like, how is it legacy if it's used for everything? :)
15:07:29 <ihrachys> jschwarz: so I believe your fix applies
15:07:32 <jschwarz> ihrachys, so I'm not sure why it didn't work for legacy
15:07:37 <ihrachys> jschwarz: but does not help the issue
15:07:45 <jschwarz> ihrachys, I'm gonna look if my code actually ran through
15:08:00 <ihrachys> jschwarz: I am sure it did. but why do you think it's enough to pass?
15:08:19 <ihrachys> jschwarz: yeah, sure, agent now starts; but it's neutron-server that fails to schedule binding to a (non-existing) agent
15:08:29 <jschwarz> ihrachys, the failure in the logfile mentions that the linuxbridge agent failed because br-ex didn't exist when it started
15:08:43 <jschwarz> ihrachys, the agent doesn't start
15:08:44 <ihrachys> in the job I posted a link to?
15:09:04 <jschwarz> ihrachys, yes
15:09:05 <jschwarz> ihrachys, http://logs.openstack.org/59/396659/2/experimental/gate-grenade-dsvm-neutron-linuxbridge-multinode-nv/f652b83/logs/subnode-2/old/screen-q-agt.txt.gz
15:09:20 <ihrachys> oh right
15:09:28 <ihrachys> I think I understand what's going on
15:09:34 <ihrachys> that's subnode, it runs 'old' code
15:09:38 <ihrachys> including old devstack
15:09:45 <jschwarz> ihrachys, how old?
15:09:48 <ihrachys> we would need to backport it to newton for devstack
15:09:59 <ihrachys> jschwarz: like newton old
15:10:11 <jschwarz> ihrachys, ah.
15:10:30 <jschwarz> ihrachys, so, I can start a backport and change the depends-on for the DNM patch and see if the fixes it :P
15:10:42 <ihrachys> jschwarz: so probably the path forward is to try to backport the fix (without landing master just yet), then retrigger DNM with both patches (master and newton) included
15:10:49 <jschwarz> ihrachys, or, rather, it wouldn't work because the subnode takes the stable/newton code no matter what?
15:10:51 <ihrachys> jschwarz: you don't need to change change-id
15:10:55 <ihrachys> jschwarz: since it's the same for backport
15:11:05 <ihrachys> jschwarz: and zuul should correctly capture both matching
15:11:24 <jschwarz> ihrachys, question is, will it use the new stable/newton patch for the subnode as well
15:11:35 <jschwarz> ihrachys, or is it hard-coded to always be stable/newton or something
15:11:35 <ihrachys> jschwarz: if you depend on both master fix and newton backport, zuul should correctly gate with both included on both nodes
15:11:40 <jschwarz> ok
15:11:42 <jschwarz> will do it now
15:11:47 <ihrachys> if not, that's a bug in zuul setup for the job
15:11:50 <ihrachys> jschwarz: cool
15:12:06 <jschwarz> ihrachys, https://review.openstack.org/#/c/403752/
15:12:25 <ihrachys> jschwarz: btw be aware I switched the job to xenial lately https://review.openstack.org/#/c/402488/
15:12:29 <jschwarz> ihrachys, and triggered a recheck experimental
15:12:38 <ihrachys> jschwarz: should not break anything, but I won't guarantee
15:12:40 <jschwarz> ihrachys, Aye I saw - that was a quick merge :)
15:12:47 <ihrachys> jschwarz: I think it's 'check experimental' not recheck
15:12:52 <jschwarz> ihrachys, worst case scenario I'll know who to yell at
15:12:57 <jschwarz> ihrachys, yeps, that's what I did :)
15:13:12 <ihrachys> jschwarz: the more friends in infra you get the quicker you merge patches ;)
15:13:21 <jschwarz> ihrachys, :D
15:13:30 <ihrachys> ok I guess we settled the path forward here. cool.
15:13:41 <jschwarz> happy to be of service
15:13:57 <ihrachys> on the other front, I believe all other grenade jobs in neutron gate switched to xenial already with no issues
15:14:06 <ihrachys> #topic Object implementation
15:14:18 <ihrachys> #link https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db
15:14:46 <ihrachys> some patches landed lately are:
15:14:47 <ihrachys> address scope integration: https://review.openstack.org/308005
15:14:56 <ihrachys> provider resource association: https://review.openstack.org/304322
15:15:18 <ihrachys> objects_exist API in the merge queue: https://review.openstack.org/395748
15:15:32 <ihrachys> also subnet service type should be ready to merge: https://review.openstack.org/375536
15:15:47 <ihrachys> rossella_s: ajo: blogan: ^ please review the latter one
15:16:07 <rossella_s> ihrachys, ack
15:16:29 <sshank> ihrachys, objects_exists is dependent on a OVO patch. It wont merge until the OVO gets merged?
15:16:29 <ihrachys> I was reviewing some more lately, but so far I haven't found anything more to land
15:16:36 <ihrachys> sshank: oh
15:17:05 <ihrachys> ok I will give that dep patch a go after the meeting, it has +1 for korzen so should be in good shape
15:17:19 <sshank> ihrachys, Thanks.
15:17:28 <ihrachys> of common interest, there is also a patch that modifies our UUIDField approach: https://review.openstack.org/#/c/393150/
15:17:36 <electrocucaracha> ihrachys: what about the integration of router route?
15:17:42 <ihrachys> it will affect all patches introducing UUIDField based objects once landed
15:18:47 <ihrachys> electrocucaracha: that's integration only right?
15:18:57 <ihrachys> will review right after
15:19:11 <electrocucaracha> ihrachys: yeah, the creation was merged couple of weeks ago
15:19:15 <electrocucaracha> ihrachys: thanks
15:19:35 <ihrachys> so folks, make note of that UUIDField patch I mentioned above
15:19:52 <ihrachys> once landed, it will break some of patches introducing objects with a UUIDField
15:20:09 <ihrachys> the fix is easy - replacing obj_fields.UUIDField with common_types.UUIDField
15:20:28 <ihrachys> but we will need to do that across the board for all new objects
15:21:20 <ihrachys> I see dasm also respinned the patch that switches all existing objects from tenant_id to project_id: https://review.openstack.org/#/c/382659/
15:21:28 <electrocucaracha> ihrachys: does that mean that we have to consider this new approach for current ovo implementations?
15:21:40 <ihrachys> electrocucaracha: you can't until we land the new type class
15:22:00 <ihrachys> electrocucaracha: so for now stick to what we have (obj_fields.UUIDField), and we will adopt when the patch actually lands
15:22:04 <dasm> ihrachys: yep. came back from vacations ;)
15:22:13 <ihrachys> it may take some time to land since the author is new to the team
15:22:26 <ihrachys> dasm: cool. I will have a look.
15:22:46 <dasm> ihrachys: it's still not ready, but it's getting better. hopefully sooner than later.
15:22:57 <ihrachys> also, some of you folks may have noticed that there is ongoing work in the tree to switch to a new oslo.db enginefacade
15:23:13 <dasm> ihrachys: when it'll be in good shape, i'll ping you, korzen and electrocucaracha to take a look at it
15:23:18 <ihrachys> that replaces all autonested_transactions context managers with specific reader/writer managers.
15:24:06 * ihrachys tries to find an example
15:24:08 <korzen> ihrachys, does enginefacade help with detached db_obj problem?
15:24:26 <electrocucaracha> ihrachys: there are plenty for rebasing merge conflicts
15:24:49 <korzen> the integration of subnet and network OVOs are dependent on db_obj being detached from the session
15:24:52 <ihrachys> korzen: no I don't think so. actually, Anna mentioned to me that we may need to rethink expunge calls because apparently they are not working with the new facade
15:25:08 <electrocucaracha> ihrachys: https://review.openstack.org/#/c/306685/21/neutron/db/flavors_db.py@109
15:26:07 <ihrachys> electrocucaracha: thanks!!!
15:26:10 <korzen> ihrachys, the expunge does not work either so
15:26:22 <korzen> it is good that we need to change it ;)
15:26:31 <ihrachys> yeah, so you see, in the patch, we remove the writer.using context manager, same way we could do for autonested_transaction
15:26:45 <ihrachys> we will probably need to switch our object classes to using the new context managers too
15:27:08 <ihrachys> meaning, use writer.using for create/update/delete and reader.using for get_object[s], count, objects_exist
15:27:34 <ihrachys> korzen: I am not filled in with details on why they are not compatible, but I trust Anna's judgement :)
15:28:24 <ihrachys> electrocucaracha: btw that flavour patch, does it seem like ready?
15:28:33 <ihrachys> I don't see any TODOs or WIP markers
15:28:54 <electrocucaracha> ihrachys: nope, I need to check the latest errors, it was fine before
15:29:37 <electrocucaracha> actually I have plans to review Qoutas first before that one https://review.openstack.org/#/c/338625/
15:30:23 <ihrachys> electrocucaracha: I see korzen already chimed in there with a -1. are they interdependable for some reason?
15:32:17 <electrocucaracha> ihrachys: I haven't have time to take a look, but the good thing about that patch is that contains four objecst
15:32:38 * ihrachys makes notes
15:32:45 * ihrachys opens more tabs
15:33:44 <ihrachys> ok let's move on to the next topic
15:33:49 <ihrachys> #topic Other patches on review
15:34:07 <ihrachys> ok one thing I wanted to mention
15:34:17 <ihrachys> docs team plans for a common openstack upgrades guide
15:34:20 <ihrachys> #link https://review.openstack.org/#/c/394261/
15:34:26 <ihrachys> atm it's in spec stage
15:34:55 <ihrachys> but it's still worth having a look, and I expect us to help folks later with the content on neutron
15:36:59 <ihrachys> anything more on your plate of common interest?
15:37:47 <electrocucaracha> ihrachys: I was thinking about the testing on null ids
15:38:10 <ihrachys> electrocucaracha: ah right. you mean the test for db_obj?
15:38:17 <korzen> I'm working on devref: https://review.openstack.org/#/c/336518
15:38:41 <korzen> anyone that still did not review it, please do
15:38:42 <electrocucaracha> ihrachys: the discussion that was started on the router patch
15:38:49 <ihrachys> electrocucaracha: we indeed can't create an object without foreign keys properly set, so we definitely need to create objects there.
15:39:07 <ihrachys> electrocucaracha: I need to digest the rationale and see if it means that in this case the test case is worthless
15:39:33 <ihrachys> korzen: electrocucaracha: I guess the explanation to skip the test case for the field that can't be null is that db_obj.<field> will always be non-null there?
15:39:40 <electrocucaracha> ihrachys: yeah, my only concern is that we are not testing the scenarios where the id is set as null
15:40:32 <korzen> ihrachys, yes
15:40:45 <korzen> ihrachys, well it is not skipping the test
15:40:48 <ihrachys> electrocucaracha: I think the scenario that is not covered is when you maybe decide to change the value of the foreign key value. it should be doable right?
15:41:03 <ihrachys> from general model perspective, not necessarily from business logic perspective
15:42:12 <electrocucaracha> well that's another one, I was thinking in the case where you have an optional foreign keys
15:42:35 <electrocucaracha> If I remember SecurityGroupRules has one
15:42:47 <ihrachys> yeah, we may want to work on the base test framework to add some test cases for those corner cases.
15:43:07 <electrocucaracha> more things to the TODO list :)
15:43:12 <ihrachys> then we should be safe to skip the existing test case for the fields
15:43:33 <ihrachys> electrocucaracha: I will revisit the vote though, I don't think it's worth a block
15:44:27 <electrocucaracha> ok
15:44:59 <ihrachys> #topic Open discussion
15:45:15 <sshank> ihrachys, korzen Regarding standard attribute, we started a WIP patch.
15:45:23 <ihrachys> sshank: link?
15:45:39 <sshank> ihrachys, korzen: https://review.openstack.org/#/c/400412/
15:45:59 <korzen> ok thanks
15:46:10 <sshank> not sure if the idea is correct.
15:47:19 <korzen> sshank, taking a quick look at it, it needs more work ;)
15:47:22 <ihrachys> sshank: the idea is but implementation is not correct. I will leave a comment.
15:47:32 <korzen> yea, exactly
15:47:51 <sshank> ihrachys, Ok. Thanks
15:47:58 <ihrachys> sshank: and we will need some test cases for the feature
15:48:05 <korzen> so from other news, the distributed port binding for live migration is going to extend the PortBinding with host_id and status fields, host_id being primary key
15:48:33 <ihrachys> korzen: ok that was the decision in the spec? I haven't checked since I left the comments last time.
15:48:49 <korzen> yes, it didn't change
15:49:09 <ihrachys> for those wondering, we are talking about https://review.openstack.org/#/c/309416/33/specs/ocata/portbinding_information_for_nova.rst@555
15:49:36 <korzen> the concern was that using DistributedPortBinding rework will take more time than extending
15:50:12 <ihrachys> korzen: ok, so from OVO/downtime-less upgrades perspective, status field should be easy (probably no actual work)
15:50:27 <dasanind_> ihrachys: korzen: is there any impact if we extend the primary key while expand migration?
15:50:37 <ihrachys> korzen: as for primary key extended to cover host, I believe it's also ok because we are migrating from 1 to many
15:51:31 <ihrachys> hm
15:51:39 <ihrachys> it makes me think that it may be actually a problem
15:52:01 <korzen> I hope that extending the primary keys will not block the DB for much time?
15:52:31 <ihrachys> I need to draw some lines on a piece of paper to make sense of it
15:52:42 <korzen> from no-downtime, it is the bigger problem
15:53:08 <korzen> is it backwar compatible?
15:53:26 <ihrachys> yea. the current newton code assumes that there is only a single object to return when fetching by port_id
15:53:41 <korzen> if you will do the insert/select without second primary key?
15:54:17 <ihrachys> korzen: I don't think we actually have code that would do that, but I suspect we may have code that does not pass host on fetch
15:54:46 <ihrachys> ok we need to make sense of the idea before it's too late :)
15:55:25 <korzen> if the change will not be backward compatible then we are doomed
15:55:43 <korzen> not mentioning many lines of code at OVO side
15:55:54 <korzen> to handle the empty host_id etc
15:55:59 <korzen> migrating data
15:57:13 <ihrachys> I will need to fetch newton code and read it through to understand how we use the table right now.
15:58:13 <ihrachys> korzen: thanks for raising the flag, it's a very important piece of the puzzle for Ocata
15:58:40 <ihrachys> btw we are still to look at CI setup for mixed API endpoint versions.
15:59:14 <ihrachys> ok, we have 1 min left, let's call it a day and spend the minute for reviews ;)
15:59:20 <ihrachys> thanks everyone
15:59:31 <ihrachys> I will walk thru patches mentioned during the meeting right now
15:59:33 <ihrachys> cheers
15:59:37 <ihrachys> #endmeeting