| *** dmitriis is now known as Guest8110 | 01:29 | |
| opendevreview | Takashi Kajinami proposed openstack/networking-sfc master: Switch back to attribute_mapped_collection https://review.opendev.org/c/openstack/networking-sfc/+/920868 | 07:43 |
|---|---|---|
| opendevreview | Merged openstack/neutron unmaintained/zed: [stable only] Do not fail on missing logical router ports https://review.opendev.org/c/openstack/neutron/+/920621 | 07:49 |
| slaweq | haleyb hi, FYI: I just added one small topic to the 'on demand' agenda for today's drivers meeting | 08:04 |
| opendevreview | Slawek Kaplonski proposed openstack/neutron-lib master: [api-ref] Add note about no validation of the target_tenant in RBAC API https://review.opendev.org/c/openstack/neutron-lib/+/920870 | 08:51 |
| opendevreview | Merged openstack/ovn-octavia-provider stable/2023.2: Remove leftover OVN LB HM port upon deletion of a member https://review.opendev.org/c/openstack/ovn-octavia-provider/+/917725 | 08:54 |
| opendevreview | Sahid Orentino Ferdjaoui proposed openstack/neutron master: ml2/ovs: avoid reconfig bridges/flows when reconnecting to OVS https://review.opendev.org/c/openstack/neutron/+/920819 | 09:09 |
| opendevreview | Merged openstack/ovn-octavia-provider stable/2023.1: Remove leftover OVN LB HM port upon deletion of a member https://review.opendev.org/c/openstack/ovn-octavia-provider/+/917726 | 09:45 |
| opendevreview | Merged openstack/ovn-octavia-provider stable/2024.1: Remove leftover OVN LB HM port upon deletion of a member https://review.opendev.org/c/openstack/ovn-octavia-provider/+/917724 | 09:45 |
| opendevreview | Merged openstack/ovn-octavia-provider stable/2023.1: FIX OVN LB Health Monitor checks for IPv6 members https://review.opendev.org/c/openstack/ovn-octavia-provider/+/919809 | 11:42 |
| opendevreview | Takashi Kajinami proposed openstack/networking-sfc master: Switch back to attribute_mapped_collection if needed https://review.opendev.org/c/openstack/networking-sfc/+/920868 | 11:53 |
| opendevreview | Takashi Kajinami proposed openstack/networking-sfc master: Switch back to attribute_mapped_collection https://review.opendev.org/c/openstack/networking-sfc/+/920868 | 13:11 |
| opendevreview | Takashi Kajinami proposed openstack/networking-sfc master: Switch back to attribute_mapped_collection https://review.opendev.org/c/openstack/networking-sfc/+/920868 | 13:23 |
| haleyb | slaweq: ack, just getting in but looks like a full meeting | 13:55 |
| slaweq | yeap | 13:55 |
| haleyb | #startmeeting neutron_drivers | 14:00 |
| opendevmeet | Meeting started Fri May 31 14:00:35 2024 UTC and is due to finish in 60 minutes. The chair is haleyb. Information about MeetBot at http://wiki.debian.org/MeetBot. | 14:00 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 14:00 |
| opendevmeet | The meeting name has been set to 'neutron_drivers' | 14:00 |
| haleyb | ping list: ykarel, mlavalle, mtomaska, slaweq, obondarev, tobias-urdin, lajoskatona, amotoki, haleyb, ralonsoh | 14:00 |
| slaweq | o/ | 14:00 |
| ihrachys | o/ | 14:00 |
| lajoskatona | o/ | 14:01 |
| mlavalle | \o | 14:01 |
| haleyb | i know one of the rfes was from seba but he's marked away | 14:02 |
| lajoskatona | seba is Christian Rohmann's nick? | 14:03 |
| haleyb | sebastian lohff - i had https://bugs.launchpad.net/neutron/+bug/2065198 on the list | 14:04 |
| haleyb | well, i didn't put it in the on-demand, its just been screened | 14:05 |
| lajoskatona | haleyb: ack | 14:05 |
| haleyb | what is Christian's nick? he didn't follow the (nick):topic format | 14:05 |
| mlavalle | we can discuss the RFE wiyhout him in the meeting, can't we? | 14:06 |
| haleyb | sure, just if we had questions | 14:06 |
| mlavalle | if we have questions we can ask in Launchpad or the patches | 14:07 |
| haleyb | alright, can get started | 14:07 |
| haleyb | #link https://bugs.launchpad.net/neutron/+bug/2064120 | 14:07 |
| haleyb | [RFE] BGP Dynamic Routing for IPv4 AND IPv6 routes not possible with just one dragent (really make use of MP-BGP or support multiple BGP speakers / sessions per dragent) | 14:07 |
| haleyb | i triaged this about a month ago | 14:08 |
| lajoskatona | racosta actually asked about this topic also (https://meetings.opendev.org/irclogs/%23openstack-neutron/%23openstack-neutron.2024-05-28.log.html ) | 14:09 |
| haleyb | and i see he had a comment on the ML | 14:10 |
| racosta | o/ | 14:11 |
| racosta | I'm just following the discussion... | 14:11 |
| lajoskatona | To tell the truth I am not so deeply faimiliar with n-d-r but based on the option in the RFE ,a spec would be good to dug deeper and select which option to select | 14:11 |
| lajoskatona | 1) Enabling a BGP speaker to have more than one address family | 14:12 |
| lajoskatona | 2) Somehow get more than one BGP speaker per dragent working by | 14:12 |
| haleyb | racosta: hi, you probably know more about this space than most | 14:12 |
| haleyb | 2a) extend os_ken to support more than one BGP session | 14:13 |
| racosta | yeah, the os-ken side is the biggest problem in my opinion... | 14:13 |
| haleyb | 2b) if extending os_ken is not an option, maybe the whole dragent can be built to run multiple instances of a single driver (os_ken) to then accept more than one bgp speaker scheduled to it. | 14:13 |
| haleyb | copy/paste from the email | 14:14 |
| slaweq | so this seems like clearly n-d-r specific RFE and because of that I think we may also ask frickler about his opinion as he is one of the most active maintainers of that stadium project | 14:14 |
| mlavalle | ++ | 14:14 |
| slaweq | regarding os-ken - do You know if maybe something like that was implemented in ryu after we forked it? | 14:15 |
| lajoskatona | true frickler hasgood backgound for n-d-r | 14:15 |
| slaweq | maybe we 'just' would need to port some changes | 14:15 |
| lajoskatona | I checked ryu a few weeks ago and there were no changes as I remember, and the last changes/fixes were ported by ralonsoh | 14:16 |
| lajoskatona | but it can be double checked | 14:16 |
| haleyb | yes, good idea. and it seems a spec would be good as lajoskatona mentioned so we have more info on the possible solutions | 14:16 |
| slaweq | lajoskatona ok, thx | 14:16 |
| haleyb | so i will add some comments based on above and ask for a spec so we can discuss options | 14:18 |
| lajoskatona | +1 | 14:18 |
| mlavalle | +1 | 14:19 |
| slaweq | before the spec shouldn't we vote if we accept that rfe in general? I'm ok with it and spec can help to choose correct solution to be implemented but just for formality I think we should approve rfe :) | 14:19 |
| mlavalle | the rfe is reasonable IMO | 14:20 |
| lajoskatona | +1 from me to the RFE | 14:20 |
| haleyb | slaweq: true, i was only thinking of getting more info, but it does seem reasonable to me as well | 14:21 |
| haleyb | i actually don't remember the rules, is 4 quorum? | 14:22 |
| haleyb | it might be the best we can do until rodolfo gets back | 14:23 |
| slaweq | +1 from me for the RFE | 14:23 |
| ihrachys | I can throw my +0.5 in (hides) | 14:23 |
| slaweq | and I think we already have 5 votes now | 14:23 |
| slaweq | 5.5 :) | 14:24 |
| mlavalle | lol 5.5 | 14:24 |
| haleyb | i only counted myself lajoskatona mlavalle slaweq (4) + ihrachys (.5) | 14:25 |
| racosta | I also think this RFE reasonable ;) | 14:25 |
| mlavalle | thanks for your opinion racosta :-) | 14:26 |
| haleyb | racosta: ack, thanks, i will approve then and ask for a spec | 14:26 |
| slaweq | yes, it's 4 | 14:26 |
| slaweq | sorry | 14:26 |
| * haleyb is only on first coffee and was out late so had to double check | 14:26 | |
| * slaweq had holiday yestarday and today work is hard :P | 14:27 | |
| haleyb | ok, next topic | 14:28 |
| haleyb | lajoskatona added a topic about sqlachemy, i've been looking at this too | 14:28 |
| lajoskatona | yes actually my goal was to make it visible to everybody and has a discussion | 14:29 |
| haleyb | i was in the process of creating a bug yesterday when i had to leave | 14:30 |
| haleyb | and i was testing a change to see if it helped with the cover job | 14:30 |
| ihrachys | is there more to do there except posting a sed /old/new/s patch + pass CI? are there hidden risks beyond it? | 14:30 |
| lajoskatona | so my understanding is that we currently have lazy=subquery which was fine for years with sqlalchemy<2 | 14:30 |
| haleyb | but let's talk, i have a link to the selectin page | 14:31 |
| lajoskatona | and even today we can live with it, but it is not effective | 14:31 |
| haleyb | #link https://docs.sqlalchemy.org/en/20/orm/queryguide/relationships.html#selectin-eager-loading | 14:31 |
| lajoskatona | ihrachys: I suppose no but never tried :-) | 14:31 |
| haleyb | What Kind of Loading to Use ? | 14:31 |
| haleyb | that's where i was reading | 14:31 |
| haleyb | One to Many / Many to Many Collection - The selectinload() is generally the best loading strategy to use | 14:32 |
| ihrachys | "The only scenario in which selectin eager loading is not feasible is when the model is using composite primary keys, and the backend database does not support tuples with IN, which currently includes SQL Server." does it imply we are good as long as we don't deploy on microsoft? | 14:32 |
| lajoskatona | "The subqueryload() eager loader is mostly legacy at this point, superseded by the selectinload() | 14:33 |
| lajoskatona | so it can happen even that it will be dropped in a future release I suppose | 14:34 |
| lajoskatona | ihrachys: does microsoft is a deploy destination for Neutron / openstack? | 14:34 |
| slaweq | IMO we are still in a pretty good moment in the cycle to try it - worst case we can revert the change, right? | 14:35 |
| ihrachys | won't be dropped; not until ms sql is gone I guess. but we don't care. | 14:35 |
| lajoskatona | :-) | 14:35 |
| lajoskatona | slaweq: +1 | 14:35 |
| ihrachys | lajoskatona I don't think a lot of people even try postgres... we are mysql shop, for better or worse. | 14:35 |
| slaweq | lajoskatona: no, we officially support only mysql/mariadb and postgresql | 14:35 |
| ihrachys | so let's just post a sed s/old/new/ and see if anything falls off :) | 14:35 |
| haleyb | i can create a bug today and propose a patch. i've only looked in neutron so far, not neutron-lib or other places | 14:35 |
| slaweq | ihrachys++ | 14:36 |
| mlavalle | yeap, let's give it a try | 14:36 |
| lajoskatona | +1, haleyb if you lack time next week I can jump in | 14:36 |
| ihrachys | haleyb probably a good idea to run neutron-lib change against master neutron test suite... which reminds me that we don't have such a job :) https://bugs.launchpad.net/neutron/+bug/2067515 | 14:36 |
| lajoskatona | I quiclky grepped stadiums and as I see directly no mention of lazy=subquery | 14:37 |
| haleyb | lajoskatona: i already have a patch locally, i was waiting for cover test to complete when i had to leave yesterday | 14:37 |
| lajoskatona | haleyb: cool, thanks | 14:38 |
| ihrachys | beyond the sql query type change... do we want to touch on memory consumption in general? | 14:38 |
| ihrachys | afair Mike said query change may save memory, but sqla2 should not have blown the memory in the first place | 14:39 |
| haleyb | we might need a change in neutron-lib though in _load_one_to_manys() will have to look | 14:39 |
| ihrachys | haleyb can probably switch in stages, no need to replace all of them at once if some pose unexpected work | 14:40 |
| racosta | will it be possible to see a reduction in the time spent on queries through tests in Zuul? | 14:40 |
| racosta | I remembered this discussion here: https://etherpad.opendev.org/p/oct2023-ptg-os-dbperf-crossproject | 14:40 |
| haleyb | ihrachys: my results show old and new versions of coverage use about the same amount of memory, so no smoking gun yet | 14:41 |
| ihrachys | :( | 14:41 |
| ihrachys | racosta I see select counters; will the SELECT IN switch show a reduction? | 14:41 |
| haleyb | and it's sitll unclear why the sqlachemy bump triggered this to me | 14:42 |
| ihrachys | haleyb I think I ran with sqla1 vs 2 locally (cover tox) and both seemed to have grown in memory consumed over time. so I couldn't reproduce locally. can you? | 14:43 |
| haleyb | ihrachys: i can occasionally get my VM to go sideways with sqla2, i need to move back to v1 and retry there | 14:45 |
| ihrachys | ack. "occasionally" is the worst. maybe I was just lucky then. | 14:46 |
| haleyb | the fact each coverage process can consume 30-40% memory is sad | 14:46 |
| ihrachys | thanks for the update. I guess we will learn more in the next weeks. we should probably move on? | 14:47 |
| haleyb | we have one more topic from slaweq | 14:47 |
| slaweq | thx haleyb, I wanted to ask about Your opinion about https://bugs.launchpad.net/neutron/+bug/2022043/ | 14:48 |
| slaweq | I was trying to explain it better in comment #5 in that LP | 14:48 |
| ihrachys | slaweq thanks for the last comment, helps a lot | 14:48 |
| slaweq | there was fix for that but apparently it broke some ci jobs | 14:48 |
| lajoskatona | tempest jobs also? | 14:49 |
| slaweq | so I am now not sure if we want/should try to fix it or maybe it is better to leave it as it is now and close the LP simply | 14:49 |
| ihrachys | I have a question about "Unfortunately, due to the "populate_project_id" method ... project_id is always added to every resource". AFAIU the patch (reverted) was accommodating for this behavior. Should this behavior be unconditional though in the first place? E.g. should populate_project_id populate for all dicts regardless whether API definition has the field?.. | 14:49 |
| slaweq | ihrachys that is good question which I asked myself yesterday too | 14:50 |
| ihrachys | you have an answer or also confused like me? | 14:50 |
| slaweq | maybe it should be fixed in different way and in https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/attributes.py#L259 it shouldn't be added at all to the res_dict if attributes don't have it | 14:50 |
| slaweq | ihrachys I don't have answer for that | 14:51 |
| slaweq | :) | 14:51 |
| ihrachys | on superfluous level, this SOUNDS like a reasonable thing to do - don't add fields that are not supported. I suspect there may be a risk of breakage if a plugin then relies on them present? | 14:52 |
| slaweq | ihrachys maybe but I am not aware of anything like that really. Also tenant_id is still in the context object always. If plugin relies on e.g. flavor['project_id'] then this is IMO mistake simply | 14:53 |
| haleyb | slaweq: so if these resources don't need a project_id, what is it set to? just the caller's project_id? | 14:54 |
| slaweq | haleyb yes, look in https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/attributes.py#L259 | 14:54 |
| slaweq | for every POST request, if there is no project_id in the res_dict it is added from the context | 14:54 |
| ihrachys | it is a bug in a plugin if it does rely on it. still a break that can go nasty. :) (gonna revert another one lol) | 14:54 |
| haleyb | ack | 14:55 |
| slaweq | for resources which belongs to project it makes sense | 14:55 |
| slaweq | but for some resources not really | 14:55 |
| slaweq | ihrachys and that's why I am asking here what we should do with it | 14:55 |
| slaweq | maybe it is better to leave it as it is simply | 14:55 |
| slaweq | :) | 14:55 |
| ihrachys | I'm always for trying to make the code correct. we can throw this change at the wall and see if it sticks, like with sql queries :) | 14:56 |
| ihrachys | if not, we should add a comment to api definitions explaining why these are part of the definitions. perhaps link to a bug. | 14:57 |
| mlavalle | yes, let's try to fix it | 14:58 |
| slaweq | ok, so I will try to propose another fix for it and make sure I don't break any ci this time | 14:58 |
| slaweq | :) | 14:58 |
| ihrachys | thanks! we should check against neutron suite at least. | 14:58 |
| mlavalle | like you said, we are still early in the cycle | 14:58 |
| slaweq | thank You for help with this | 14:59 |
| lajoskatona | +1 | 14:59 |
| haleyb | slaweq: ack, thanks, but someone beat you to breaking the CI, that job is failing again :( | 14:59 |
| slaweq | and sorry for breaking ci so You had to revert it :) | 14:59 |
| slaweq | LOL | 14:59 |
| * haleyb is crying | 15:00 | |
| ihrachys | np, it's part of the job. we learned ci is not testing something, so hopefully we'll plug the gap | 15:00 |
| haleyb | ihrachys: it might have been one of the netaddr changes... | 15:01 |
| ihrachys | I was going to ask about the logs after the meeting because of course - landed a bunch of these yesterday. | 15:01 |
| haleyb | alright, we can end meeting | 15:02 |
| ihrachys | o/ | 15:02 |
| mlavalle | \o | 15:03 |
| haleyb | #endmeeting | 15:03 |
| opendevmeet | Meeting ended Fri May 31 15:03:02 2024 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 15:03 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/neutron_drivers/2024/neutron_drivers.2024-05-31-14.00.html | 15:03 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/neutron_drivers/2024/neutron_drivers.2024-05-31-14.00.txt | 15:03 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/neutron_drivers/2024/neutron_drivers.2024-05-31-14.00.log.html | 15:03 |
| haleyb | thanks, and we'll need to look at gate | 15:03 |
| slaweq | o/ | 15:03 |
| ihrachys | haleyb link to failure? | 15:03 |
| lajoskatona | o/ | 15:04 |
| haleyb | ihrachys: pick any neutron patch? :P | 15:05 |
| haleyb | https://73c7e5d11add2b6adb65-e4098ecd70541ccfec6de5edaad12e3e.ssl.cf1.rackcdn.com/919844/2/check/openstack-tox-py311-with-sqlalchemy-master/aaf84fc/testr_results.html | 15:05 |
| ihrachys | haleyb ok I think that's because we no longer fail on None passed? let me see, this seems like a test case issue, will report back | 15:07 |
| haleyb | ihrachys: ack, thanks | 15:08 |
| ihrachys | we do now (as per your suggestion btw): if value is None: return [] | 15:08 |
| ihrachys | which I think is sane | 15:08 |
| ihrachys | if it's none, it now behaves as if it was not passed at all (allocation pools picked by generate_pools). haleyb this is sane, right? if so, I will need to adjust the test cases to not check what they check - though I wonder how we are supposed to do it except just removing the test cases (or disabling) until neutron-lib is bumped? | 15:14 |
| ihrachys | haleyb question btw - why is this job in neutron gate voting at all? shouldn't it be advisory? | 15:16 |
| haleyb | ihrachys: it's voting because it consistently passed, and we wanted to make sure we didn't regress with sqlalchemy2 | 15:16 |
| ihrachys | well... it passed until it didn't. as we started landing neutron-lib patches, we realized it's a gap in cross-gate deps. | 15:17 |
| haleyb | ihrachys: and i can't say i've looked at failure to know what's best, or if we broke the expected behavior | 15:17 |
| ihrachys | haleyb the behavior we broke is if subnet[allocation_pools] is passed as None, it no longer crashes with 404, but is accepted (handled as if nothing was passed for the field). we can make it crash again, but seems wrong. | 15:18 |
| ihrachys | s/404/400 | 15:19 |
| haleyb | ihrachys: ah, i hadn't noticed it's the same test via different paths | 15:20 |
| haleyb | i am fried for the week... | 15:21 |
| opendevreview | Ihar Hrachyshka proposed openstack/neutron master: Make openstack-tox-py311-with-sqlalchemy-master non-voting https://review.opendev.org/c/openstack/neutron/+/920896 | 15:22 |
| haleyb | ihrachys: so if we disable that job, then bump neutron-lib, then fix tests we'll be good? if we bump i think the jobs will just start failing | 15:22 |
| opendevreview | Ihar Hrachyshka proposed openstack/neutron master: tests: Don't except error 400 from allocation_pools=None https://review.opendev.org/c/openstack/neutron/+/920897 | 15:25 |
| ihrachys | haleyb I think we can just not validate this case in the test suite ^ | 15:25 |
| haleyb | ihrachys: ack, seems reasonable will watch it | 15:30 |
| ihrachys | yeah I haven't tested it lol | 15:30 |
| ihrachys | sec, will do to speed up things... | 15:30 |
| haleyb | ihrachys: 8-o - pre-commit hooks can't come fast enough :) | 15:31 |
| haleyb | slaweq: question on the pre-commit hooks, if you know the answer (unrelated to above change) - if i change a file, say neutron/worker.py, can a pre-commit hook force the running of neutron.tests.unit.test_worker ? | 15:34 |
| haleyb | i'm actually creating a short downstream presentation on saving the planet using pre-commit hooks :) | 15:35 |
| opendevreview | Ihar Hrachyshka proposed openstack/neutron master: Revert "[OVN] Prevent Trunk creation/deletion with parent port bound" https://review.opendev.org/c/openstack/neutron/+/919858 | 15:39 |
| opendevreview | Ihar Hrachyshka proposed openstack/neutron master: reno: ml2/ovn allows to create/delete trunks for bound ports https://review.opendev.org/c/openstack/neutron/+/919859 | 15:39 |
| -opendevstatus- NOTICE: Gerrit on review.opendev.org is being upgraded to version 3.9 and will be offline. We have allocated an hour for the outage window lasting until 1700 UTC | 16:01 | |
| ihrachys | haleyb not slaweq but you can collect changes files with git diff and then run test runner with just these files? a bit of scripting but should be doable. I don't think pre-commit hook itself receives any args for changed files. | 16:14 |
| ihrachys | what I hate is that tox itself is rather slow to prepare env (even to start running any tests), so putting it directly in pre-commit will end up with me skipping pre-commits completely to speed up work :) I am an obsessive git commit --amend'er and commit every second minute :) | 16:15 |
| haleyb | ihrachys: with pep8 you can just run it on the changes in the most recent commit, at least I have an alias for it, 'tox -e pep8 -- HEAD~1 $@' | 16:16 |
| haleyb | and i usually run the unit tests for the file in question, but manually | 16:16 |
| haleyb | and neutron pretty much requires a test_* file for every source file, so it should be doable, i just wondered if slaweq noticed something like that while adding it | 16:17 |
| ihrachys | you can build a script around it. pep8 target for pylint has a wrapper already, see run_pylint function in tools/coding-checks.sh | 16:17 |
| ihrachys | regardless of pre-commit having unit tox target support a revision would be very helpful | 16:19 |
| ihrachys | haleyb the fix passed zuul; need a second +2 now | 17:03 |
| ihrachys | who wants to be the gate hero? https://review.opendev.org/c/openstack/neutron/+/920897 | 17:03 |
| ihrachys | ah sorry; there is no +2 yet, misread my own Priority+2 | 17:04 |
| haleyb | ihrachys: I +2'd but am not the gate hero | 19:00 |
| opendevreview | Jay Jahns proposed openstack/neutron master: Add network dns_domain to dns assignment if present https://review.opendev.org/c/openstack/neutron/+/920459 | 19:03 |
| opendevreview | Brian Haley proposed openstack/neutron master: Change to use selectin for DB load strategy https://review.opendev.org/c/openstack/neutron/+/920933 | 20:19 |
| opendevreview | Brian Haley proposed openstack/neutron-lib master: [api-ref] Add note about no validation of the target_tenant in RBAC API https://review.opendev.org/c/openstack/neutron-lib/+/920870 | 20:30 |
| opendevreview | Brian Haley proposed openstack/neutron master: Change to use selectin for DB load strategy https://review.opendev.org/c/openstack/neutron/+/920933 | 21:28 |
| opendevreview | Brian Haley proposed openstack/neutron-lib master: Add 'selectin' as a supported lazy relationship attribute https://review.opendev.org/c/openstack/neutron-lib/+/920936 | 21:35 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!