| *** ysandeep|out is now known as ysandeep | 04:56 | |
| *** ysandeep is now known as ysandeep|afk | 07:19 | |
| *** ysandeep|afk is now known as ysandeep | 08:04 | |
| opendevreview | Tom Weininger proposed openstack/octavia master: Documentation updates https://review.opendev.org/c/openstack/octavia/+/832509 | 09:00 |
|---|---|---|
| *** ysandeep is now known as ysandeep|lunch | 09:35 | |
| *** ysandeep|lunch is now known as ysandeep | 10:51 | |
| *** nilushko is now known as Niko_iL | 11:33 | |
| gthiemonge | #startmeeting Octavia | 16:00 |
| 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 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 16:00 |
| opendevmeet | The meeting name has been set to 'octavia' | 16:00 |
| gthiemonge | Hi Folks | 16:00 |
| johnsom | o/ | 16:00 |
| spharmon` | o/ | 16:00 |
| tweining | hi | 16:00 |
| gthiemonge | #topic Announcements | 16:02 |
| gthiemonge | * Yoga release schedule - Final RC | 16:02 |
| gthiemonge | FYI This week is the Final RCs (we don't have any additional RCs after RC1), next week is the Yoga Release! | 16:03 |
| johnsom | Wahoo | 16:04 |
| gthiemonge | Do you have any other announcements? | 16:05 |
| gthiemonge | Just a reminder: Zed PTG - Octavia | 16:05 |
| gthiemonge | #link https://etherpad.opendev.org/p/zed-ptg-octavia | 16:05 |
| gthiemonge | I need to work on the proposed topics/schedule | 16:05 |
| gthiemonge | #topic Brief progress reports / bugs needing review | 16:07 |
| gthiemonge | I have updated the huge "Fix plugging member subnets"/"Multi-VIP" patchchain | 16:07 |
| gthiemonge | I still need to sort out the dependencies between octavia and octavia-tempest-plugin | 16:07 |
| gthiemonge | (I still don't know if the octavia patch should depend on the tempest tests, any thoughts?) | 16:08 |
| johnsom | Ha, the old chicken and egg issue. I think tests should depend on the code that implements. IMO | 16:09 |
| 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 |
| *** ysandeep is now known as ysandeep|out | 16:09 | |
| gthiemonge | #link https://review.opendev.org/c/openstack/octavia/+/660239/ | 16:09 |
| gthiemonge | I think I've mixed both approaches in the patch chain :D | 16:09 |
| johnsom | It happens | 16:10 |
| gthiemonge | oh no I need to update the 2 main patches to reverse the depends-on | 16:10 |
| spharmon` | (Hopefully I'm not out of turn) I was hoping to discuss "Add rpc notification for load balancers" | 16:11 |
| spharmon` | https://review.opendev.org/c/openstack/octavia/+/831051 | 16:11 |
| gthiemonge | spharmon`: hi! and welcome! | 16:12 |
| spharmon` | Hi! Thanks! Nice to meet you :) | 16:13 |
| spharmon` | Yall keep going, but lmk when it's my turn to go. I have a specific question related to one comment. | 16:13 |
| gthiemonge | spharmon`: please, go ahead | 16:14 |
| spharmon` | Awesome. The comment is this one: | 16:14 |
| spharmon` | https://review.opendev.org/c/openstack/octavia/+/831051 | 16:14 |
| spharmon` | oops | 16:14 |
| spharmon` | This one: https://review.opendev.org/c/openstack/octavia/+/831051/6..10/octavia/common/rpc.py#b46 | 16:14 |
| 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 |
| gthiemonge | "they specifically asked"... johnsom, was it you? | 16:16 |
| johnsom | No, it looks like Erik | 16:17 |
| johnsom | I have not reviewed this patch | 16:17 |
| 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:18 |
| tweining | IMO adding an assert seems more pythonic than raising an equivalent AssertionError directly | 16:19 |
| spharmon` | I think the advantage of raising the assertion is to use the translation library, right? | 16:20 |
| 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 |
| gthiemonge | I'm not sure that translating "'TRANSPORT' must not be None" would help the user ;-) | 16:21 |
| tweining | assert TRANSPORT is None, _("'TRANSPORT' must not be None") should be equivalent | 16:21 |
| gthiemonge | I don't know it either | 16:22 |
| 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 |
| spharmon` | Ah yeah ok. I can refactor for the assersion. I agree it's more pythonic. | 16:22 |
| 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:24 |
| johnsom | I think their intent was to create a singleton for those | 16:25 |
| johnsom | With lazy init | 16:25 |
| 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 |
| 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:27 |
| gthiemonge | spharmon`: BTW Have you seen the other proposed commit for notification support in Octavia? https://review.opendev.org/c/openstack/octavia/+/784628 | 16:28 |
| 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:30 |
| gthiemonge | spharmon`: how is your patch now (WIP? almost ready?)? can I test it in my env? | 16:32 |
| 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 |
| 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 |
| gthiemonge | cool, I would like to test it ;-) | 16:33 |
| spharmon` | Awesome! I'd love any feedback! :) | 16:34 |
| tweining | I started continuing the work on the failover stop threshold patch here: https://review.opendev.org/c/openstack/octavia/+/656811 | 16:35 |
| gthiemonge | tweining: cool, I replied to your comment in the patch | 16:37 |
| 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 |
| johnsom | There are relational protections in the database for those states. Modifying the state machine is a big deal really. | 16:37 |
| gthiemonge | :/ | 16:37 |
| johnsom | Is there a description of why we need a new state? | 16:38 |
| johnsom | I.e. the commit message or spec? | 16:38 |
| gthiemonge | #link https://review.opendev.org/c/openstack/octavia/+/656811/13/octavia/db/repositories.py#1680 | 16:38 |
| gthiemonge | johnsom: ^ | 16:38 |
| tweining | I don't think we must have a new state. I just thought it would help. | 16:39 |
| johnsom | It's certainly possible to do, but some work. I will have to read through these comments and code to understand. | 16:39 |
| 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:40 |
| johnsom | Oh, if this is amp state, that is less risky to change. I was talking about provisioning status which has wide implications. | 16:41 |
| 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:42 |
| tweining | so a constant in octavia-lib which gets used in octavia.common.constants. | 16:43 |
| gthiemonge | there's a provisioning_status table: https://opendev.org/openstack/octavia/src/branch/master/octavia/db/models.py#L45 | 16:43 |
| johnsom | The states have a database relational constraint to make sure they are consistent. | 16:43 |
| gthiemonge | https://opendev.org/openstack/octavia/src/branch/master/octavia/db/models.py#L620-L623 | 16:43 |
| johnsom | I also have a topic for open discussion when we get there. | 16:45 |
| 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 |
| gthiemonge | no objections from me ;-) | 16:46 |
| gthiemonge | tweining: I can explain offline how to update an existing table in Octavia (add a migration script) | 16:47 |
| tweining | thanks | 16:48 |
| gthiemonge | #topic Open Discussion | 16:48 |
| gthiemonge | johnsom: o/ | 16:48 |
| johnsom | I wanted to raise that we have a new bug: | 16:48 |
| johnsom | #link https://storyboard.openstack.org/#!/story/2009942 | 16:48 |
| gthiemonge | yeah I've just received the notification email | 16:48 |
| johnsom | CentOS 9 Stream recently updated OpenSSL and removed a number of features. SHA1 for sure, but it also looks like TLSv1. | 16:49 |
| johnsom | We have tests that cover the various TLS versions (because we have code that allows you to allow/deny them). | 16:49 |
| gthiemonge | johnsom: so the bug is in the test? | 16:50 |
| johnsom | So, we need to think about how to handle tests that will no longer pass on certain platforms. | 16:50 |
| gthiemonge | johnsom: can we set the minimum_tls_version for this job and check what the API returns? | 16:51 |
| johnsom | In this case, yes it's a test issue. | 16:51 |
| 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:52 |
| 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 |
| gthiemonge | could the work query the capabalities of an amphora? | 16:53 |
| johnsom | We could also just change the defaults to remove TLSv1 from the default available list and make the assumptions it's dead | 16:53 |
| johnsom | Hmm, yeah, via a DIB image build time something... I guess | 16:54 |
| gthiemonge | I'm adding a topic to the agenda of the PTG ;-) | 16:54 |
| johnsom | +1 | 16:55 |
| johnsom | Just wanted to raise awareness on this. It's a tricky one | 16:55 |
| gthiemonge | 2 topics: TLS default settings, and amphora capabilities | 16:55 |
| johnsom | In theory they are going to want a fix back to wallaby too. sigh | 16:55 |
| gthiemonge | hmm | 16:56 |
| gthiemonge | yeah | 16:56 |
| johnsom | Ok, that was all I had today | 16:56 |
| gthiemonge | thanks for raising it | 16:56 |
| gthiemonge | any other topics folks? | 16:57 |
| tweining | no | 16:57 |
| spharmon` | Nothing from me | 16:58 |
| gthiemonge | ok | 16:58 |
| gthiemonge | thank you Folks! | 16:58 |
| gthiemonge | #endmeeting | 16:58 |
| opendevmeet | Meeting ended Wed Mar 23 16:58:41 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 16:58 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/octavia/2022/octavia.2022-03-23-16.00.html | 16:58 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/octavia/2022/octavia.2022-03-23-16.00.txt | 16:58 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/octavia/2022/octavia.2022-03-23-16.00.log.html | 16:58 |
| spharmon` | Thanks everyone! | 16:58 |
| spatel | johnsom just to follow up with you.. after setting up health TCP it works and allow to spin up everything cleanly | 19:05 |
| johnsom | Hmm, well, I'm glad you are working now. | 19:06 |
| spatel | Sorry i was busy so not able to get back to you | 19:06 |
| johnsom | You just did! grin | 19:06 |
| spatel | haha | 19:07 |
| spatel | I have one more question related k8s with octavia | 19:07 |
| johnsom | Ok, I may or may not have an answer | 19:08 |
| spatel | I have working Magnum and i have created k8s cluster for testing. Now i want to use Octavia for Ingress controller for my containers | 19:08 |
| spatel | I found this doc but little complicated to understand https://superuser.openstack.org/articles/guide-octavia-ingress-controller-for-kubernetes/ | 19:09 |
| spatel | what is step 2.1 in that doc for? what is the use for Certificate here? | 19:10 |
| johnsom | That document is about all I know about it. I have not done a deployment like that. | 19:10 |
| johnsom | I don't know. Maybe the K8S API is protected with mutual TLS authentication and requires a cert? | 19:12 |
| spatel | hmm | 19:12 |
| johnsom | Sorry, not an area I have worked with. | 19:12 |
| spatel | no worry! i will keep poking :) | 19:13 |
| opendevreview | Merged openstack/octavia-tempest-plugin master: Fix incorrect subnet_id for ipv6 member servers https://review.opendev.org/c/openstack/octavia-tempest-plugin/+/810660 | 19:58 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!