| *** rcernin has joined #openstack-lbaas | 00:14 | |
| *** rcernin has quit IRC | 00:22 | |
| *** rcernin has joined #openstack-lbaas | 00:22 | |
| *** ramishra has quit IRC | 02:17 | |
| *** sapd1 has joined #openstack-lbaas | 02:19 | |
| *** psachin has joined #openstack-lbaas | 03:22 | |
| *** rcernin has quit IRC | 03:22 | |
| *** rcernin has joined #openstack-lbaas | 04:02 | |
| *** ramishra has joined #openstack-lbaas | 04:02 | |
| *** rcernin has quit IRC | 04:07 | |
| *** rcernin has joined #openstack-lbaas | 04:48 | |
| *** rcernin has quit IRC | 04:48 | |
| *** sapd1 has quit IRC | 04:53 | |
| *** vishalmanchanda has joined #openstack-lbaas | 05:11 | |
| *** rcernin has joined #openstack-lbaas | 05:26 | |
| *** ccamposr has joined #openstack-lbaas | 07:35 | |
| *** rpittau|afk is now known as rpittau | 08:19 | |
| *** rcernin has quit IRC | 08:23 | |
| *** sapd1 has joined #openstack-lbaas | 08:29 | |
| *** ramishra has quit IRC | 08:31 | |
| *** ramishra has joined #openstack-lbaas | 08:36 | |
| *** rcernin has joined #openstack-lbaas | 08:58 | |
| *** sapd1 has quit IRC | 09:12 | |
| openstackgerrit | Gregory Thiemonge proposed openstack/octavia-tempest-plugin master: DNM test adding delays between least_connection requests https://review.opendev.org/c/openstack/octavia-tempest-plugin/+/781461 | 09:25 |
|---|---|---|
| *** rcernin has quit IRC | 09:37 | |
| *** ataraday has quit IRC | 09:44 | |
| *** sapd1 has joined #openstack-lbaas | 10:04 | |
| *** rcernin has joined #openstack-lbaas | 11:13 | |
| *** rcernin has quit IRC | 11:18 | |
| *** sapd1 has quit IRC | 11:38 | |
| *** rcernin has joined #openstack-lbaas | 11:44 | |
| *** servagem has joined #openstack-lbaas | 11:59 | |
| *** rcernin has quit IRC | 12:31 | |
| *** rcernin has joined #openstack-lbaas | 12:31 | |
| *** jamesdenton has quit IRC | 12:48 | |
| *** jamesdenton has joined #openstack-lbaas | 12:49 | |
| *** psachin has quit IRC | 13:04 | |
| openstackgerrit | Bodo Petermann proposed openstack/octavia master: Fix LB failover for amphorav2: set security group https://review.opendev.org/c/openstack/octavia/+/782255 | 13:33 |
| *** rcernin has quit IRC | 14:05 | |
| *** luksky has joined #openstack-lbaas | 14:16 | |
| openstackgerrit | Gregory Thiemonge proposed openstack/octavia-tempest-plugin master: DNM test adding delays between least_connection requests https://review.opendev.org/c/openstack/octavia-tempest-plugin/+/781461 | 14:27 |
| *** sapd1 has joined #openstack-lbaas | 14:31 | |
| *** __ministry1 has joined #openstack-lbaas | 15:23 | |
| openstackgerrit | Gregory Thiemonge proposed openstack/octavia master: Fix using subnets with host_routes in amphorav2 driver https://review.opendev.org/c/openstack/octavia/+/782279 | 15:32 |
| *** __ministry1 has quit IRC | 15:40 | |
| *** xgerman has joined #openstack-lbaas | 16:10 | |
| *** vishalmanchanda has quit IRC | 16:11 | |
| *** rpittau is now known as rpittau|afk | 17:11 | |
| openstackgerrit | Bodo Petermann proposed openstack/octavia master: Fix LB failover for amphorav2: set security group https://review.opendev.org/c/openstack/octavia/+/782255 | 17:14 |
| *** jamesdenton has quit IRC | 17:24 | |
| *** jamesdenton has joined #openstack-lbaas | 17:24 | |
| *** sapd1 has quit IRC | 19:36 | |
| *** rcernin has joined #openstack-lbaas | 20:25 | |
| *** rcernin has quit IRC | 20:38 | |
| *** ccamposr has quit IRC | 21:00 | |
| *** ccamposr has joined #openstack-lbaas | 21:00 | |
| *** rcernin has joined #openstack-lbaas | 21:03 | |
| *** rcernin has quit IRC | 21:17 | |
| rm_work | johnsom: was this related to the issue before? https://review.opendev.org/c/openstack/octavia/+/711275 recall we couldn't backport that | 21:48 |
| johnsom | From a quick read of the commit message there, no they are not related. | 21:48 |
| rm_work | fnpanic / johnsom: yeah, that multi-az patch was really good to merge I think at the time, but needs rebasing pretty heavily now. it would work fine as an "experimental" (non-default) feature | 21:48 |
| johnsom | Yeah, that patch is already there and it is failing under this new scenario | 21:49 |
| rm_work | ah hmm k | 21:49 |
| johnsom | rm_work Do you have time to look at that? I'm grooming the priority review list at the moment and planning to start working down it. | 21:50 |
| fnpanic | thanks for the update | 21:50 |
| johnsom | It look like we have ~15 open bug patches | 21:51 |
| rm_work | may have time this week | 21:51 |
| *** rcernin has joined #openstack-lbaas | 21:51 | |
| *** rcernin has quit IRC | 21:51 | |
| johnsom | +1 I also have a fix I need to make on Designate | 21:52 |
| *** rcernin has joined #openstack-lbaas | 21:52 | |
| rm_work | i also kinda want to pick up https://review.opendev.org/c/openstack/octavia/+/634302 again <_< | 22:01 |
| johnsom | Ha, yeah, that just drives me a bit nuts | 22:02 |
| rm_work | yeah such dumb cruft from old n-lbaas API | 22:07 |
| rm_work | was that because we were considering allowing members to exist on multiple pools? | 22:08 |
| rm_work | like to prevent multi-healthchecks or something? | 22:08 |
| johnsom | Well, the current model stops them from being shared IMO | 22:09 |
| rm_work | yes | 22:31 |
| rm_work | but was that our excuse back then? | 22:31 |
| johnsom | v2 api design pre-dated me, just barely | 22:32 |
| rm_work | ah, forgot that | 22:32 |
| rm_work | thought you were there from day 1 | 22:33 |
| johnsom | I think it was just modeling too close to the appliances, etc... | 22:33 |
| johnsom | I was for Octavia, but not v2 API | 22:33 |
| rm_work | yeah | 22:33 |
| rm_work | I believe I was there for v2 as well... but didn't know enough to argue TOO much | 22:33 |
| johnsom | neutron-lbaas existed before I joined the party | 22:33 |
| rm_work | well yeah, v1 predated me | 22:34 |
| johnsom | I think Mark was the only one really involved in v1 | 22:34 |
| rm_work | yes | 22:41 |
| rm_work | johnsom: do you have that bug handy? I could look at it right now | 22:46 |
| johnsom | https://storyboard.openstack.org/#!/story/2008731 | 22:46 |
| johnsom | Really I think we just need to unlock the objects if we don't dispatch to the queue | 22:48 |
| rm_work | yeah hmm | 22:57 |
| rm_work | deciding how to do that | 22:57 |
| rm_work | or whether to actually just dispatch to the queue rather than special-casing :D | 22:57 |
| johnsom | Or you could move the check back up to the API and just roll back the DB transaction | 22:58 |
| johnsom | Not sure if you need to be in the driver context to make that decision or not | 22:58 |
| johnsom | It would be the inverse of https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/member.py#L180 | 22:59 |
| rm_work | yeah | 22:59 |
| rm_work | could catch and rollback there | 23:00 |
| rm_work | johnsom: are drivers *allowed* to raise exceptions besides those in the octavia_lib.api.drivers.exceptions file? | 23:07 |
| johnsom | No | 23:07 |
| rm_work | the selection there is ... slim | 23:07 |
| rm_work | I guess I could do it based on a *return*? | 23:08 |
| johnsom | Probably not | 23:08 |
| rm_work | or just hijack something like UnsupportedOption to mean "please rollback" | 23:08 |
| johnsom | I would really not do that.... | 23:08 |
| rm_work | then I can't get back up to the top-level to trigger a rollback | 23:09 |
| rm_work | unless I trigger a rollback IN THE DRIVER which seems gross | 23:09 |
| johnsom | No, you would have to move the validation code up to the API | 23:09 |
| rm_work | that's *all the logic* lol | 23:09 |
| rm_work | not possible I think | 23:09 |
| rm_work | but will look closer | 23:10 |
| johnsom | It may not be possible. I haven't really looked at it. If not, it might just be easier to call a DB unlock method. | 23:10 |
| johnsom | Not that one that clean exists today, but.... | 23:10 |
| rm_work | yeah if rollback is possible, that's definitely cleanest | 23:11 |
| johnsom | Ah, I gave the wrong line before: https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/member.py#L357 | 23:12 |
| rm_work | err right i think i understood that | 23:12 |
| rm_work | anyway, i could add one more octavia-lib exception | 23:13 |
| rm_work | like | 23:13 |
| johnsom | It does look like a return value might pass up the driver chain. Maybe that is the right answer... | 23:13 |
| rm_work | NoAction or something | 23:13 |
| rm_work | yeah i will try that first, it should WORK | 23:13 |
| rm_work | it was more like "is that acceptable" | 23:13 |
| rm_work | you wrote this driver model stuff mostly | 23:14 |
| johnsom | Yeah, it is ... questionable | 23:14 |
| johnsom | I did | 23:14 |
| rm_work | fff I really really like all the new type-hint syntax in py3.??? | 23:15 |
| rm_work | it's so good | 23:15 |
| rm_work | i wish I could use it all the time everywhere | 23:15 |
| johnsom | I haven't played with it much yet. Does python actually enforce it in some useful way? | 23:16 |
| rm_work | turns out I didn't actually want quite as loosely typed language as I thought | 23:16 |
| johnsom | Oh, I am firmly in the "strong typed" came | 23:16 |
| rm_work | it'll definitely warn you... not sure if it enforces because I try not to violate it :D | 23:16 |
| johnsom | camp | 23:16 |
| rm_work | def member_batch_update(self, pool_id: str, members: str) -> bool: | 23:17 |
| rm_work | err | 23:17 |
| rm_work | def member_batch_update(self, pool_id: str, members: list) -> bool: | 23:17 |
| rm_work | feels pretty natural as far as extensions go | 23:18 |
| rm_work | I appreciate that it is optional per codebase | 23:18 |
| rm_work | or per def, really, but i'd enforce it i think if i was gonna use it at all | 23:18 |
| johnsom | I guess I would say adding an exception is probably the "right" answer. Like NoActionRequired or something makes sense. | 23:19 |
| johnsom | But just looking at the API code I struggle to see how it can't figure out that it's a no-op call and just not call. | 23:19 |
| rm_work | yeah it MIGHT be possible to move it up | 23:20 |
| rm_work | would be hilarious in that the driver function would just basically be a passthrough | 23:20 |
| rm_work | not necessarily a bad thing? | 23:20 |
| rm_work | might change the signature | 23:20 |
| rm_work | so ... not backportable tho | 23:21 |
| rm_work | trying to also optimize for that | 23:21 |
| johnsom | Pretty much what the driver should be really... Very light weight | 23:21 |
| rm_work | yeah so ... we can do that change in the future | 23:21 |
| johnsom | +1 on that backport-ability | 23:21 |
| rm_work | but I'd like to make a fix that is easily backportable | 23:21 |
| rm_work | which might require you being willing to stretch what you're ok with | 23:21 |
| rm_work | IE, reusing an exception | 23:21 |
| rm_work | which is never used in another context in this part of the API | 23:22 |
| rm_work | like `UnsupportedOptionError` :D | 23:22 |
| rm_work | then I can follow-up with moving the logic entirely | 23:22 |
| rm_work | and we can backport just the first one | 23:22 |
| johnsom | DriverError seems closer to a bad choice, but not horrible | 23:23 |
| rm_work | hmm | 23:23 |
| rm_work | was looking for something slightly less generic | 23:23 |
| johnsom | You could move the code out of the driver and still backport. This is all control plane side. | 23:23 |
| rm_work | that's the base class | 23:23 |
| rm_work | moving the code out of the driver would mean changing the signature | 23:24 |
| johnsom | No | 23:24 |
| rm_work | unless ... I guess if you just want to run literally the same code twice | 23:24 |
| rm_work | it seems like there is no reason to not just pass the data that'd come out of the validation, straight to the cast() | 23:24 |
| rm_work | kinda lulz | 23:25 |
| rm_work | hmm i wonder if it's actually already doing 90% of this duplicated >_< | 23:26 |
| johnsom | Yeah, I think so. There is marshaling games, but basically the same game, different formats. | 23:27 |
| johnsom | I just don't understand why you can check if they are all empty here: https://github.com/openstack/octavia/blob/master/octavia/api/v2/controllers/member.py#L427 | 23:27 |
| johnsom | Like it is doing here: https://github.com/openstack/octavia/blob/master/octavia/api/drivers/amphora_driver/v1/driver.py#L291 | 23:27 |
| johnsom | If empty, rollback, if not dispatch | 23:28 |
| rm_work | yeah maybe i can | 23:28 |
| rm_work | didn't realize it was already duplicating so much of this work | 23:28 |
| rm_work | which is painful because I wrote this | 23:28 |
| rm_work | me from two years ago is dumb and I hate him | 23:28 |
| rm_work | (unless I figure out why he did this and it makes sense, but then he's still dumb for not documenting it) | 23:29 |
| rm_work | but I'm leaning towards that guy just being a jerk | 23:29 |
| johnsom | I think most of it is going API->provider->controller worker schemas | 23:30 |
| rm_work | yeah <_< | 23:30 |
| johnsom | Ideally, at some point in the future, v2 will consume provider format directly | 23:30 |
| johnsom | You know, when we have that tech debt/refactor time to burn | 23:31 |
| johnsom | lol | 23:31 |
| rm_work | heh yeah so I have a patch ready but need to test this somehow | 23:35 |
| rm_work | unit tests are ... hmm | 23:35 |
| rm_work | hmmm did we change our functional tests at some point to actually require config? :/ | 23:36 |
| johnsom | No | 23:36 |
| rm_work | getting this error on like 900+ tests: octavia.common.exceptions.ProviderNotFound: Provider 'amphora' was not found | 23:38 |
| rm_work | looking into it i guess, since I want to see if I can write a functional test for this | 23:39 |
| haleyb | rm_work: i guess you'll have to run the ovn provider :p | 23:40 |
| johnsom | haleyb Certainly simplifies the case. assert (True, NotImplemented) | 23:41 |
| * johnsom Thinks you walked into that one | 23:41 | |
| rm_work | :P | 23:41 |
| haleyb | less tests == more time for [snoozing, drinking, ...] | 23:42 |
| johnsom | So I just did a fresh check out: | 23:43 |
| johnsom | functional: commands succeeded | 23:43 |
| johnsom | congratulations :) | 23:43 |
| rm_work | less tests == less time for [snoozing, drinking, ...] because I am debugging weird shit that someone broke with their patch that I didn't notice because there were no tests | 23:43 |
| johnsom | I wonder if you have a config file and some test(s) is picking it up instead of using a fixture? | 23:43 |
| rm_work | yeah maybe | 23:43 |
| rm_work | looking | 23:43 |
| rm_work | johnsom: the only thing i'll say is, this removes the provider's choice as to whether to make gratuitous calls on "no perceived change" | 23:45 |
| rm_work | we decide not to in Amphora, but maybe some provider actually would want to send all those through (though the major reason that comes to mind is their system being buggy in the backend and needing to allow that to force through again) | 23:46 |
| johnsom | Hmm, that is a good point | 23:46 |
| rm_work | but -- I also DON'T REALLY CARE THAT MUCH sooooo | 23:49 |
| rm_work | I would rather close this hole and let providers request we allow forcing it through or something -- which would be a change we wouldn't need to backport? :P | 23:49 |
| johnsom | Those buggy drivers can add a "repush all" feature. lol | 23:49 |
| johnsom | Which, I guess we have, called failover | 23:50 |
| rm_work | lol | 23:51 |
| rm_work | yeah | 23:51 |
| rm_work | hmm looks like there was a failover bug in ampv2 | 23:51 |
| rm_work | i hit the merge button a bit ago on that | 23:51 |
| rm_work | i am about to swap to v2 this week... hoping there aren't more of those lurking :/ | 23:52 |
| rm_work | debating putting that off | 23:52 |
| rm_work | though I just committed to it in our sprint planning :D | 23:52 |
| johnsom | Ah, was going to look at that right after I finished with the priority list (which is about now) | 23:52 |
| rm_work | i double-checked to make sure it did really match what we had in v1, and that it looked like it could matter | 23:53 |
| johnsom | I am kind of wondering why the v2 tempest job didn't catch that | 23:53 |
| rm_work | would failover an amp, one amp would be reachable? | 23:54 |
| rm_work | or does it do an entire failover WITH a traffic check now? | 23:54 |
| rm_work | last I checked our failover didn't have traffic checks, but maybe that was added (I remember I had an outstanding CR for a long time on that) | 23:55 |
| johnsom | Well, the base scenario job is standalone. | 23:55 |
| johnsom | Maybe that is the case, and probably should be fixed... | 23:55 |
| rm_work | hmm | 23:55 |
| johnsom | How do you test failover if you don't test traffic????? | 23:55 |
| rm_work | /shrug | 23:57 |
| rm_work | new amp comes up? :P | 23:58 |
| rm_work | also need to figure out why this fails: https://review.opendev.org/c/openstack/octavia/+/740432 | 23:58 |
| johnsom | Hmm, maybe the only failover test we have with standalone is in the API suite | 23:58 |
| rm_work | because that is what i'd like to merge downstream ahead of time | 23:58 |
| johnsom | Yeah, I think that is the case. Only in API which runs no-op. Or only in the active/standby suite. | 23:59 |
| johnsom | So... | 23:59 |
| johnsom | No test joy | 23:59 |
| johnsom | I wonder if I have an old dusty patch with that in it???? | 23:59 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!