Tuesday, 2024-03-26

opendevreviewXiang Wang proposed openstack/oslo.messaging master: Implement ConsulOperator as a service broker to support HTTP Driver  https://review.opendev.org/c/openstack/oslo.messaging/+/91249901:08
opendevreviewVasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91419010:38
opendevreviewVasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91419010:49
opendevreviewVasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91419012:02
hberaudhey oslo folks, for your information, we recently observed failures related to a manual monkey patch of the stdlib threading module, made by oslo.log https://github.com/eventlet/eventlet/issues/948 surely related to https://opendev.org/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17. Context is available in the github link12:45
hberaudVasyl is patch ^ seems related to the observed failure too13:23
fricklerah, that looks similar to what sean-k-mooney saw in devstack, nice13:30
sean-k-mooneyso what was concerning about the devstack run13:31
sean-k-mooneywas that keystone does not use eventlet13:32
sean-k-mooneyand seting the env var for teh ascyio hub caused issues with eventlet13:32
sean-k-mooneyso defining EVENTLET_HUB seam to both enabled eventlet to some degree and set the default hub to use13:33
sean-k-mooneyim not sure if that i intentional but it seams like an eventlet bug to me13:33
sean-k-mooneyoh13:34
sean-k-mooneyactully reading that patch13:34
sean-k-mooneyya oslo is broken13:34
sean-k-mooney eventlet = sys.modules.get('eventlet')13:34
sean-k-mooney    if eventlet:13:34
sean-k-mooneythat just tells you that eventlet is installed but it does not check if its in use in the current process13:35
sean-k-mooneylet me see if i can hack something quickly to chekc if eventlet monkey patching is actully in use13:36
hberaudsean-k-mooney: see my comments on https://github.com/eventlet/eventlet/issues/94813:41
hberaudI think the condition based on the result of `sys.module.get('eventlet')` is a bad thing and trigger the patching of the threading module. Patched threading module who broke the asyncio hub.13:43
opendevreviewsean mooney proposed openstack/oslo.log master: only patch threading if monkey patched  https://review.opendev.org/c/openstack/oslo.log/+/91424613:44
sean-k-mooneyi think ^ would fix it right13:44
hberaudthis condition should be based on if we already patched the env, and not if the module is available in sys moduke (else importing eventlet will make appear eventlet in sys.module)13:45
sean-k-mooneyright so thats what i did13:45
sean-k-mooney import eventlet.patcher13:45
sean-k-mooney        if not eventlet.patcher.is_monkey_patched('thread'):13:45
sean-k-mooney            return13:45
sean-k-mooneyi kept that code in the orgianal if as eventlet may not be isntalled if this is deployed in a container13:46
hberaudand eventlet 0.36.0 already contains patch to ensure that the asyncio hub do not use a patched threading module https://github.com/eventlet/eventlet/commit/520c6e243774aeb9a80c712e1eacfb5957d472d013:46
hberaudunfortunately eventlet in 0.35.1 do not contains that fix13:47
opendevreviewVasyl Saienko proposed openstack/oslo.log master: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91419013:47
hberaudstars were not aligned 13:48
sean-k-mooneyso https://review.opendev.org/c/openstack/devstack/+/914108 should be running with eventlet 0.36.0 by the way13:49
hberaudsean-k-mooney: I think your patch is duplicated with Vasyl his one. Also I'd suggest to use eventletutils as I suggested early today to Vasyl https://review.opendev.org/c/openstack/oslo.log/+/91419013:50
sean-k-mooneyyep  https://zuul.opendev.org/t/openstack/build/29ba9328076440f997f6096cdabc0045/log/job-output.txt#701813:51
sean-k-mooneyyep so ill abandon my oslo patch 13:51
sean-k-mooneyand then ill make my devstack patch depend on Vasyl's patch13:51
hberaudcool, thanks13:51
sean-k-mooneyill also disbale some of the zuul jobs to waste less resoucs untill this work13:51
hberaudyeah sure13:52
sean-k-mooneyok we can check back on https://review.opendev.org/c/openstack/devstack/+/914108 in an hour if its still running it likely got past the inital failure in oslo.log13:57
sean-k-mooneyotherwise we would expect it to fail in 25 mins or so13:58
hberaudack13:59
sean-k-mooneyhum this time the bootstrap commadn seams to run but we hit another atribute error in keystone14:18
sean-k-mooneyhttps://zuul.opendev.org/t/openstack/build/b56da78c8f974d8c9d683a191a6c2265/log/job-output.txt#1302014:19
sean-k-mooneythat however does not look like its related to olo or evnetlet14:19
hberaudindeed, seems related to an other problem14:20
sean-k-mooneythere is a second devstack job in the buildset 14:21
sean-k-mooneyhttps://zuul.opendev.org/t/openstack/stream/148ef12138f24e7086475fe55e9d8ce6?logfile=console.log14:21
hberaudhttps://github.com/pyca/bcrypt/issues/68414:21
sean-k-mooneyand that is currently installing neutron14:21
sean-k-mooneyso lets wait and see14:21
sean-k-mooneywe might need to pin that buggy version in uc and one of the jobs just got unlucky14:22
hberaudpasslib is mostly abandoned and should be removed at somepoint14:22
hberaudbut that's an other topic14:22
sean-k-mooneywell it has not had an update since october 202014:23
sean-k-mooneyso i assume they are not going to do a quick release for the bcrypt change14:23
sean-k-mooneyso unless bcrypt fix the issue we might be force too do something about htat14:23
hberaudyep14:23
sean-k-mooneyhttps://github.com/pyca/bcrypt/issues/684#issuecomment-1902590553 we proably need to copy that14:25
hberaudyes14:27
sean-k-mooneyso it looks like keystone is the only thing that imports it and it does not use it for much14:27
sean-k-mooneyactully trove improts it to14:28
hberaudAFAIK the maintenance activity on keystone seems a quite low, so we may find other similar issues related to outdated things14:29
sean-k-mooneyya so im current lookign at this rabbithole. i dont really have time to fix all the thigns but ill see what i can do in 30 mins14:30
hberaudtkajinam, damani: please can you review Vasyl's patch? https://review.opendev.org/c/openstack/oslo.log/+/91419014:32
hberaud(see the discussion above)14:33
sean-k-mooneyim wokring on a patch to remove passlib form trove now14:40
sean-k-mooneythat looks trivial14:40
sean-k-mooneykeystone will likely take longer then i have right now14:40
sean-k-mooneybut proably not that much longer14:41
hberaudack14:41
hberaudthank you sean-k-mooney for all your great works!14:41
sean-k-mooneyim on pto for the next 10 days starting thurday so i dont really have time to start anythign new14:41
sean-k-mooneyso hacking on small things like this is proably as useful as anything else14:42
hberaudnp14:42
hberaudyes14:42
hberaudI proposed a patch to bump U.C to the latest version of eventlet https://review.opendev.org/c/openstack/requirements/+/91425614:46
tkajinamhberaud, +2+A-ed that patch. probably we should close https://bugs.launchpad.net/oslo.messaging/+bug/2043661 and see if that oslo.log fix works14:46
hberaudindeed14:47
sean-k-mooneyhttps://review.opendev.org/c/openstack/trove/+/914257 i think is all that is needed14:47
sean-k-mooneyunit tests pass locally at least14:47
hberaudwe should also backport this patch to stable branches that contains the original code14:47
hberaudtkajinam: ^14:47
tkajinamyeah14:48
hberauddamani recently started to backport related parent patches14:48
hberauddamani: do you want to handle these backports?14:48
tkajinamI was looking into oslo.messaging after that bug was reported but I didn't find why we haven't seen this problem until recent versions but not it makes very clear sense14:48
hberaud(the new ones)14:48
tkajinamprobably we can use closes-bug instead14:49
sean-k-mooneyim pretty sure we have that backported downstream for redhat's osp product too so if we can do the backport upstream that helps everyone out14:49
opendevreviewTakashi Kajinami proposed openstack/oslo.log master: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91419014:50
tkajinamI can take care of it.14:50
tkajinamhberaud, I've replaced related-bug tag by closes-bug tag because we consider this is the actual fix. I've already added my +2+A but it'd be nice if you can re-add your +2.14:52
hberaudsure14:54
hberaudtkajinam: don't you want to also link the oslo.messaging but in the commit message?14:55
hberaudwe could kill two birds with one stone14:56
tkajinamhberaud, I don't need it now, because I've closed that oslo.messaging bug as duplicate of bug 203934614:57
hberaudindeed it also works :)14:57
damanihberaud, i can do it 14:59
hberauddamani: cool, I let you manage that point with Takashi14:59
hberaudthx14:59
opendevreviewDaniel Bengtsson proposed openstack/oslo.log stable/2024.1: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91426215:14
tkajinamhberaud, do you think we have to create a release with this fix before 2024.1 GA or can we create one after GA ? I'm unsure if this is causing problems frequently enough to be considered as a critical bug15:22
tkajinamI know that this is problematic, though.15:23
opendevreviewDaniel Bengtsson proposed openstack/oslo.log stable/2023.2: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91426615:33
opendevreviewDaniel Bengtsson proposed openstack/oslo.log stable/2023.1: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91426715:35
hberaudcreating a release ASAP would be better, though, I don't know the criticity of this bug too. IMO, this bug is more or less isolated in a special kind of env for now... but.. I wouldn't bet too much on it either.15:35
hberaudtkajinam: ^15:36
JayFSeems to me like it causes zero harm, and eliminates an unknown-unknown. I'd likely want the fix in a stable release as long as it doesn't create a hardship.15:36
JayFIMO I doubt it makes a significant difference if that is pre-or-post 2024.1 release actually happening :)15:37
hberaudsean-k-mooney: the same failure appear again in the latest grenade job run https://zuul.opendev.org/t/openstack/build/b93dcee5237e4fc2aaeade08b41d2def/log/job-output.txt#11862. Even with your latest PS changes. Does I missed something?15:40
hberaud(just to be sure)15:40
hberaudand same thing on the devstack job15:41
tkajinamlet's see how quickly we can merge the fix... we have no concern about backport. my main concern is that we are quite close to GA and I'm a bit hesitant to rush to cover a new release and also req bump at this timing.16:05
sean-k-mooneythe devstack job hit the bcrypt issue 16:05
sean-k-mooneyhttps://zuul.opendev.org/t/openstack/build/148ef12138f24e7086475fe55e9d8ce6/log/job-output.txt#1264916:05
sean-k-mooneyso thats not the oslo log issue16:06
tkajinamhberaud, I'm unsure what's your expectation about 2024.2 ? we don't have stable/2024.2 branch yet, apparently16:06
opendevreviewTakashi Kajinami proposed openstack/oslo.log stable/2023.1: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91426716:10
hberaudtkajinam: do you speak about => https://review.opendev.org/c/openstack/oslo.log/+/914266 ?16:53
hberaudthe stable branch seems to have been created https://review.opendev.org/q/project:openstack/oslo.log+branch:stable/2023.216:54
hberaudand my comment on Daniel's patch is about the fact that the commit message refer to 2 "cherry-pick" when this patch is the first one of the list, so I wonder why we have 2 cherry-pick listed there16:56
hberaudsean-k-mooney: yes, but, if I'm right we apparently still obseve the same issue with oslo.log, but those seems ignored (Exception ignored in: <function _removeHandlerRef at 0x7f07a17a4040>) https://zuul.opendev.org/t/openstack/build/148ef12138f24e7086475fe55e9d8ce6/log/job-output.txt#1234516:59
hberaudsean-k-mooney: sorry, that's not exactly the same error, but it looks like it. Eventlet seems not involved17:02
sean-k-mooneyah ok its what i tought it was17:08
sean-k-mooneyLIBS_FROM_GIT=cinder,devstack,glance,keystone,neutron,nova,placement,requirements,swift17:08
sean-k-mooneyhberaud: os oslo.log is not in requried projects for that job so while zuul is cloneing it devstack is not using it17:09
hberaudok17:20
opendevreviewMerged openstack/oslo.log master: Fix eventlet detection  https://review.opendev.org/c/openstack/oslo.log/+/91419018:27
sean-k-mooneyhberaud: this time https://review.opendev.org/c/openstack/devstack/+/914108 to running swift19:59
sean-k-mooneyso improvement but still proably a long way off19:59
sean-k-mooneyhberaud: https://zuul.opendev.org/t/openstack/build/f29ddb7b451b425db9f60cc1d5940c96/log/controller/logs/screen-s-account.txt#3219:59
sean-k-mooney  raise RuntimeError("Multiple readers are not yet supported by asyncio hub")20:00
sean-k-mooneyhberaud: i could turn off swift for now. it is one of the hevier users of eventlet details20:01
sean-k-mooneyits still in logging code but i belive swift does not use oslo log20:02
*** zbitter is now known as zaneb23:22

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!