16:00:25 <gthiemonge> #startmeeting Octavia
16:00:25 <opendevmeet> Meeting started Wed Mar 23 16:00:25 2022 UTC and is due to finish in 60 minutes.  The chair is gthiemonge. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:25 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:25 <opendevmeet> The meeting name has been set to 'octavia'
16:00:32 <gthiemonge> Hi Folks
16:00:33 <johnsom> o/
16:00:37 <spharmon`> o/
16:00:42 <tweining> hi
16:02:21 <gthiemonge> #topic Announcements
16:02:27 <gthiemonge> * Yoga release schedule - Final RC
16:03:30 <gthiemonge> FYI This week is the Final RCs (we don't have any additional RCs after RC1), next week is the Yoga Release!
16:04:07 <johnsom> Wahoo
16:05:14 <gthiemonge> Do you have any other announcements?
16:05:31 <gthiemonge> Just a reminder: Zed PTG - Octavia
16:05:38 <gthiemonge> #link https://etherpad.opendev.org/p/zed-ptg-octavia
16:05:47 <gthiemonge> I need to work on the proposed topics/schedule
16:07:12 <gthiemonge> #topic Brief progress reports / bugs needing review
16:07:35 <gthiemonge> I have updated the huge "Fix plugging member subnets"/"Multi-VIP" patchchain
16:07:59 <gthiemonge> I still need to sort out the dependencies between octavia and octavia-tempest-plugin
16:08:18 <gthiemonge> (I still don't know if the octavia patch should depend on the tempest tests, any thoughts?)
16:09:03 <johnsom> Ha, the old chicken and egg issue. I think tests should depend on the code that implements. IMO
16:09:12 <gthiemonge> anyway, I have removed the WIP from the Multi-VIP patch because I've completed the last concerns I had with it (improved coverage in the unit tests)
16:09:22 <gthiemonge> #link https://review.opendev.org/c/openstack/octavia/+/660239/
16:09:46 <gthiemonge> I think I've mixed both approaches in the patch chain :D
16:10:17 <johnsom> It happens
16:10:36 <gthiemonge> oh no I need to update the 2 main patches to reverse the depends-on
16:11:53 <spharmon`> (Hopefully I'm not out of turn) I was hoping to discuss "Add rpc notification for load balancers"
16:11:54 <spharmon`> https://review.opendev.org/c/openstack/octavia/+/831051
16:12:22 <gthiemonge> spharmon`: hi! and welcome!
16:13:01 <spharmon`> Hi! Thanks! Nice to meet you :)
16:13:37 <spharmon`> Yall keep going, but lmk when it's my turn to go. I have a specific question related to one comment.
16:14:09 <gthiemonge> spharmon`: please, go ahead
16:14:35 <spharmon`> Awesome. The comment is this one:
16:14:36 <spharmon`> https://review.opendev.org/c/openstack/octavia/+/831051
16:14:41 <spharmon`> oops
16:14:51 <spharmon`> This one: https://review.opendev.org/c/openstack/octavia/+/831051/6..10/octavia/common/rpc.py#b46
16:16:28 <spharmon`> Essentially, I'm wondering whether anyone has concerns with raising here rather than rerunning init(). Other projects raise in a similar fashion which is why I've done that here (My main concern is opening additional rpc connections and running out of file descriptors).
16:16:54 <gthiemonge> "they specifically asked"... johnsom, was it you?
16:17:13 <johnsom> No, it looks like Erik
16:17:57 <johnsom> I have not reviewed this patch
16:18:26 <spharmon`> It was Erik who raised the concern because of some historical knowledge, but he also clarified that he thinks it's appropriate to raise here. I think I'd like to resolve if no one has concerns with raising. In another channel, we discussed possibly changing these to assertions.
16:19:56 <tweining> IMO adding an assert seems more pythonic than raising an equivalent AssertionError directly
16:20:23 <spharmon`> I think the advantage of raising the assertion is to use the translation library, right?
16:21:22 <johnsom> I guess my concern would be do we assume that get_client will init the transport no matter what. Ugh, I don' t know this code very well.
16:21:41 <gthiemonge> I'm not sure that translating "'TRANSPORT' must not be None" would help the user ;-)
16:21:55 <tweining> assert TRANSPORT is None, _("'TRANSPORT' must not be None")   should be equivalent
16:22:05 <gthiemonge> I don't know it either
16:22:08 <spharmon`> I don't need an answer now. My assumption is no: TRANSPORT should be set elsewhere and never be None in get_client, but I'd conceed if that's not right.
16:22:42 <spharmon`> Ah yeah ok. I can refactor for the assersion. I agree it's more pythonic.
16:24:43 <spharmon`> As far as running init(), I think there's a risk if you don't cleanup the prior connections, right? Then, if you do run cleanup, like Erik mentions, there's a risk of race conditions with calls to the prior TRANSPORT, NOTIFIER, et c..
16:25:43 <johnsom> I think their intent was to create a singleton for those
16:25:51 <johnsom> With lazy init
16:27:23 <spharmon`> Ok, I see. I'll look more into create_transport to see if there's a safe way I can figure to do that.
16:27:29 <spharmon`> Anyway, that's all. Thanks for the insight. If yall get some cycles to drop a comment, I'd appreciate it. I'll be hanging around in here as well, so reach out any time.
16:28:45 <gthiemonge> spharmon`: BTW Have you seen the other proposed commit for notification support in Octavia? https://review.opendev.org/c/openstack/octavia/+/784628
16:30:55 <spharmon`> Yes, I have. I started off testing that one and landed here, in an effort to fix especially the non-singleton problem of reinitializing the rpc stuff here. There were also issues with placing the logic in with the database_tasks.
16:32:55 <gthiemonge> spharmon`: how is your patch now (WIP? almost ready?)? can I test it in my env?
16:33:06 <spharmon`> I also like having separate tasks for notifications. In my case, I'm testing with a third-party driver. This way, I can import these tasks in the flows in the third party driver.
16:33:32 <spharmon`> I'd say almost ready. We're running this in our dev environment. It's been in the lab for some time as well.
16:33:58 <gthiemonge> cool, I would like to test it ;-)
16:34:13 <spharmon`> Awesome! I'd love any feedback! :)
16:35:54 <tweining> I started continuing the work on the failover stop threshold patch here: https://review.opendev.org/c/openstack/octavia/+/656811
16:37:01 <gthiemonge> tweining: cool, I replied to your comment in the patch
16:37:06 <tweining> I added a new provisioning state FAILOVER_STOPPED for the amp, but get "foreign key constraint fails" errors. not sure why yet.
16:37:48 <johnsom> There are relational protections in the database for those states. Modifying the state machine is a big deal really.
16:37:57 <gthiemonge> :/
16:38:05 <johnsom> Is there a description of why we need a new state?
16:38:16 <johnsom> I.e. the commit message or spec?
16:38:35 <gthiemonge> #link https://review.opendev.org/c/openstack/octavia/+/656811/13/octavia/db/repositories.py#1680
16:38:38 <gthiemonge> johnsom: ^
16:39:05 <tweining> I don't think we must have a new state. I just thought it would help.
16:39:48 <johnsom> It's certainly possible to do, but some work. I will have to read through these comments and code to understand.
16:40:42 <gthiemonge> tweining: when you said you "added a new provisioning state FAILOVER_STOPPED", do you mean in the code? because the states are also defined in the DB
16:41:02 <johnsom> Oh, if this is amp state, that is less risky to change. I was talking about provisioning status which has wide implications.
16:42:00 <tweining> I added a new constant for that. I thought no DB schema change would be needed as it is an existing string field.
16:43:02 <tweining> so a constant in octavia-lib which gets used in octavia.common.constants.
16:43:32 <gthiemonge> there's a provisioning_status table: https://opendev.org/openstack/octavia/src/branch/master/octavia/db/models.py#L45
16:43:55 <johnsom> The states have a database relational constraint to make sure they are consistent.
16:43:57 <gthiemonge> https://opendev.org/openstack/octavia/src/branch/master/octavia/db/models.py#L620-L623
16:45:59 <johnsom> I also have a topic for open discussion when we get there.
16:46:21 <tweining> ok, I will try to make it work with that new state then if there are no objections or we can talk later about it.
16:46:40 <gthiemonge> no objections from me ;-)
16:47:35 <gthiemonge> tweining: I can explain offline how to update an existing table in Octavia (add a migration script)
16:48:02 <tweining> thanks
16:48:14 <gthiemonge> #topic Open Discussion
16:48:19 <gthiemonge> johnsom: o/
16:48:27 <johnsom> I wanted to raise that we have a new bug:
16:48:29 <johnsom> #link https://storyboard.openstack.org/#!/story/2009942
16:48:51 <gthiemonge> yeah I've just received the notification email
16:49:15 <johnsom> CentOS 9 Stream recently updated OpenSSL and removed a number of features. SHA1 for sure, but it also looks like TLSv1.
16:49:47 <johnsom> We have tests that cover the various TLS versions (because we have code that allows you to allow/deny them).
16:50:08 <gthiemonge> johnsom: so the bug is in the test?
16:50:20 <johnsom> So, we need to think about how to handle tests that will no longer pass on certain platforms.
16:51:10 <gthiemonge> johnsom: can we set the minimum_tls_version for this job and check what the API returns?
16:51:14 <johnsom> In this case, yes it's a test issue.
16:52:37 <johnsom> Yeah, I'm not sure how the controller/amp side is going to work on CentOS 9 Stream if the user selects TLSv1. Obviously the operator can block TLSv1 if they know it's broken in CentOS 9 STream, but...  Maybe we need to come up with a strategy for dealing with this
16:53:03 <johnsom> It's not like an API version will tell you, etc. It's purely trying the OS and see if it works I guess.
16:53:37 <gthiemonge> could the work query the capabalities of an amphora?
16:53:47 <johnsom> We could also just change the defaults to remove TLSv1 from the default available list and make the assumptions it's dead
16:54:49 <johnsom> Hmm, yeah, via a DIB image build time something... I guess
16:54:55 <gthiemonge> I'm adding a topic to the agenda of the PTG ;-)
16:55:03 <johnsom> +1
16:55:15 <johnsom> Just wanted to raise awareness on this. It's a tricky one
16:55:24 <gthiemonge> 2 topics: TLS default settings, and amphora capabilities
16:55:52 <johnsom> In theory they are going to want a fix back to wallaby too.  sigh
16:56:02 <gthiemonge> hmm
16:56:03 <gthiemonge> yeah
16:56:43 <johnsom> Ok, that was all I had today
16:56:57 <gthiemonge> thanks for raising it
16:57:26 <gthiemonge> any other topics folks?
16:57:49 <tweining> no
16:58:22 <spharmon`> Nothing from me
16:58:28 <gthiemonge> ok
16:58:35 <gthiemonge> thank you Folks!
16:58:41 <gthiemonge> #endmeeting