| gmaan | dansmith: bauzas0 not sure if we forgot it or any rational behind to continue the support the compute RPC 5x version, it think it is safe to remove now (https://review.opendev.org/q/topic:%22drop-compute-rpc-5x%22). one question for you about releasenotes https://review.opendev.org/c/openstack/nova/+/1012318/comment/c84cdbc4_174a3446/ | 02:03 |
|---|---|---|
| opendevreview | Michael Still proposed openstack/placement master: Import migrations only when syncing on startup https://review.opendev.org/c/openstack/placement/+/1012173 | 07:23 |
| opendevreview | Michael Still proposed openstack/placement master: Prefer the threading oslo.service backend https://review.opendev.org/c/openstack/placement/+/1012174 | 07:23 |
| opendevreview | Martin André proposed openstack/nova master: Fix volume attachment pagination https://review.opendev.org/c/openstack/nova/+/1012429 | 08:12 |
| zigo | melwitt: Hi, could you look into https://review.opendev.org/c/openstack/nova/+/998681 and let me know your thoughts? | 08:40 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Clear the leaked time override in notifications tests https://review.opendev.org/c/openstack/nova/+/1012432 | 08:42 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Stop HypervisorsTestV253.setUp() mutating shared dict https://review.opendev.org/c/openstack/nova/+/1012433 | 08:43 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Restore the resource tracker fixtures after each test https://review.opendev.org/c/openstack/nova/+/1012434 | 08:45 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Stop QuotaClassSetsTestV257 mutating shared state https://review.opendev.org/c/openstack/nova/+/1012435 | 08:46 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Restore manager.LOG after the reverts_task_state test https://review.opendev.org/c/openstack/nova/+/1012436 | 08:46 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Stop CastAsCallFixture leaving a phantom cast entry https://review.opendev.org/c/openstack/nova/+/1012437 | 08:47 |
| zigo | As promised, 6 patches fixing 6 test leaks above. | 08:51 |
| zigo | That last one was kind of hard to find, having to bissect to find who was the leaker (victim was obviously easier to find). | 08:51 |
| zigo | Hopefully, this will improve the gate, moving foward! :) | 08:51 |
| zigo | Though it looks like I'm not even finished. | 08:52 |
| gibi | zigo: nice catches | 08:58 |
| gibi | I left feedback on them | 08:59 |
| zigo | Cool, thanks ! I'll attend the 2 negative reviews. | 08:59 |
| gibi | thanks | 08:59 |
| Uggla | zigo thanks! | 09:03 |
| opendevreview | Merged openstack/nova master: Speed up graceful shutdown functional tests https://review.opendev.org/c/openstack/nova/+/1012349 | 09:31 |
| opendevreview | Lucas HENRY proposed openstack/nova master: Remove compute RPC 5.x compatibility from the client https://review.opendev.org/c/openstack/nova/+/1012318 | 10:35 |
| opendevreview | Lucas HENRY proposed openstack/nova master: Remove the compute RPC 5.x server endpoint https://review.opendev.org/c/openstack/nova/+/1012320 | 10:37 |
| opendevreview | Kamil Sambor proposed openstack/nova master: Fix pickling error that forces oslo.service to fall back to fork https://review.opendev.org/c/openstack/nova/+/1000956 | 11:42 |
| *** ykarel_ is now known as ykarel | 13:06 | |
| opendevreview | Thomas Goirand proposed openstack/nova master: Greenthread graceful shutdown runner in eventlet mode https://review.opendev.org/c/openstack/nova/+/1012488 | 13:23 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Greenthread delayed task scheduler in eventlet mode https://review.opendev.org/c/openstack/nova/+/1012490 | 13:25 |
| *** jgilaber_ is now known as jgilaber | 14:02 | |
| opendevreview | Thomas Goirand proposed openstack/nova master: Greenthread graceful shutdown runner in eventlet mode https://review.opendev.org/c/openstack/nova/+/1012488 | 14:05 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Greenthread delayed task scheduler in eventlet mode https://review.opendev.org/c/openstack/nova/+/1012490 | 14:05 |
| dansmith | sambork: gibi: gmaan: I'm thinking we don't want to be adding new eventlet stuff at this point, right? I think maybe just not do things like graceful shutdown in non-native mode? ^ | 14:38 |
| zigo | dansmith: The issue is that this needs to be backported. | 14:39 |
| zigo | These are real problems in Hibiscus and below. | 14:39 |
| zigo | If there's a way to just push this to Hibiscus, to Epoxy, then I'm happy to do that. | 14:40 |
| sambork | I wouldn't put any more effort into supporting eventlet, since we've deprecated it in this release. | 14:41 |
| zigo | sambork: How do we fix Hibiscus ? | 14:41 |
| zigo | And Gazpacho? | 14:41 |
| zigo | My only option would be to skip these tests, though I would strongly recommend to actually remove them from Nova, because they are broken. | 14:42 |
| sambork | Oh, if that's the case, then it's definitely outside my area of expertise. I'm not sure what the process should be here | 14:42 |
| zigo | Using my patch for stestr: https://github.com/mtreinish/stestr/pull/400 | 14:44 |
| zigo | I've been able to uncover nearly a dozen issue in Neutron, currently up to 7 in Nova, and not finished, plus 5 in Mistral, and that's just "so far" ... :) | 14:44 |
| sean-k-mooney | test fixes are vaild to backport | 14:44 |
| sean-k-mooney | but graceful shutdwon suprpot without threading is debatable | 14:44 |
| sean-k-mooney | skipping when monkeypatched is perhaps soemthign we coudl do | 14:45 |
| dansmith | I suggested that on the first one | 14:45 |
| dansmith | that'd be my preference, because it applies to master and stable alike | 14:45 |
| gibi | zigo: if these are test only changes then push the fix directly to stable/hibiscus with [stable-only] tag in the commit header | 14:45 |
| zigo | gibi: It's not test only, there's real issues in the graceful shutdown. | 14:46 |
| gibi | I see | 14:46 |
| zigo | I'd understand if we don't want to fix, but then, let's remove the tests completely ? | 14:46 |
| sean-k-mooney | the tests run in both modes | 14:46 |
| sean-k-mooney | with and without eventlet | 14:47 |
| sean-k-mooney | so removing the test woudl remove coverage for the treahded behvior | 14:47 |
| zigo | Yes, and that's the issue: the eventlet mode is actually broken. | 14:47 |
| dansmith | I'm confused.. you're just trying to fix unit/functional tests that run in eventlet mode? not real found issues? | 14:47 |
| zigo | So we should remove the eventlet part of the tests. | 14:47 |
| zigo | dansmith: I'm trying to get all of Nova pass unit testing using stestr dynamic mode as per my patch here: https://github.com/mtreinish/stestr/pull/400 | 14:48 |
| zigo | So far, it uncovered at least 6 leaking tests (a test that breaks another one), and I could see that the eventlet test for graceful shutdown leaves tasks that never stop. | 14:48 |
| sean-k-mooney | so the tests dont really have an eventlet part they are writen using the moduels that woudl be monkey patches | 14:48 |
| gibi | I see a relevant questiont from dansmith in https://review.opendev.org/c/openstack/nova/+/1012488/2/nova/service.py I also thought that threading.Thread would create a GreenThread in eventlet mode | 14:48 |
| zigo | So tests are never finished. | 14:48 |
| zigo | That's very annoying, because I can't finish my campain of fixing leaking tests. | 14:49 |
| sean-k-mooney | gibi: it shoudl yes | 14:49 |
| sean-k-mooney | if threading is monkey patched | 14:49 |
| zigo | Yeah, as sean-k-mooney says. | 14:49 |
| gibi | so why do we need the code change there? | 14:49 |
| sean-k-mooney | we shoudl not | 14:49 |
| sean-k-mooney | at least not for that | 14:50 |
| sean-k-mooney | zigo: the way we have been wrighn the test is tot update them to never use teh eventlet verions | 14:50 |
| sean-k-mooney | and rely on the monkey patching | 14:50 |
| zigo | We have 2 choices: either make it so that the tests do not run with eventlet, or we fix the issue. | 14:50 |
| sean-k-mooney | we can fix the issue without calling utils.get_eventlet().greenthread.spawn(_run) | 14:51 |
| zigo | Though this leaves something like the vncproxy of Hibiscus broken. | 14:51 |
| zigo | (not sure it's doing graceful shutdown though) | 14:51 |
| zigo | Ok, I'll try to do that. | 14:51 |
| zigo | Thanks for the input. | 14:51 |
| gibi | if we end up just fixing the tests to avoid leaks then I'm happy to review those on master too and then backport as normal. | 14:53 |
| gibi | that would be cleaner even if we rip some of those test out later in 2027.2 | 14:53 |
| zigo | I don't mind either way, but IMO, it's nicer to actually do something about the current bad state. Doing what sean-k-mooney says seems reasonable too, so which path do I take? :) | 14:54 |
| sean-k-mooney | zigo: for what its worth we (mainly gibi and gmaan) have been trackign down and fixing these races in the tests for the last 6-12 months | 14:56 |
| stephenfin | gmaan: would be interested in your opinion on https://review.opendev.org/c/openstack/nova/+/1012429 | 14:57 |
| sean-k-mooney | the gracefuly shutdown ones have been soemwhat problematic because of how coupled that feature is to the concrancy | 14:57 |
| zigo | sean-k-mooney: Happy to read that you're also on this effort. Agreed that it doesn't feel easy. | 14:58 |
| sean-k-mooney | well as i said gibi has been doing a lot fo the heavy lifting btu this has been part fo stablising the gate | 14:59 |
| sean-k-mooney | there are a lot of non production issues in the unit test that are races in threaded mode that have been fixed in the last cycle | 15:00 |
| zigo | I probably will end up just abandonning these patches, blacklist them in my package, and move to less controvertial stuff then. | 15:01 |
| zigo | I'm not super comfortable on having tests in both modes and how to figure out what to do in case they are running at the same time. | 15:02 |
| zigo | It's anyway a dead end if we're removing the code. | 15:02 |
| zigo | Though again: it's probably time to get rid of these tests upstream then ... :P | 15:03 |
| sean-k-mooney | again these test test treaded mode as well | 15:04 |
| sean-k-mooney | its the same test for both modes | 15:04 |
| sean-k-mooney | so deletign the test remove the treading coverage | 15:04 |
| sean-k-mooney | you cant test with eventlet on and off in teh same process by the way | 15:05 |
| sean-k-mooney | i.e. you have to do two invokations of tox or stestr so that you use clean worker proceses | 15:05 |
| sean-k-mooney | and set the env var to enabel/disbale eventlet | 15:06 |
| zigo | Got ya. | 15:07 |
| dansmith | gibi: in case you didn't see it: https://review.opendev.org/c/openstack/grenade/+/1011350 | 15:40 |
| dansmith | gibi: I'll propose the devstack change to stop forcing a template | 15:41 |
| gibi | no I haven't so thanks for the pointer | 15:43 |
| dansmith | I should have followed up with the actual devstack change, I was just kinda in the mindset that I wanted to see it change mid-upgrade for the more realistic case of someone taking our default and it changing | 15:45 |
| gibi | yeah I'm just testing that manually locally | 15:49 |
| gibi | with some live migration in the mix | 15:49 |
| gmaan | I was in same impression that threading.Thread is monkey patched. anyways zigo i agree to fix the test instead of routing the code to greenthread explicitly | 15:50 |
| zigo | Feel free to take over then, because I already abandonned the patch, and finding new leaking tests. | 15:52 |
| gmaan | stephenfin: ack, will check after morning meeting | 15:53 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Use a private ConfigOpts in the conf dynamic-opts tests https://review.opendev.org/c/openstack/nova/+/1012541 | 16:08 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Do not mutate the shared fake_vcpumodel in a test https://review.opendev.org/c/openstack/nova/+/1012542 | 16:09 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Reset the guestfs forceTCG flag in the vfs tests setUp https://review.opendev.org/c/openstack/nova/+/1012543 | 16:10 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Guard the live migration monitor tests against the jobStats flag https://review.opendev.org/c/openstack/nova/+/1012544 | 16:21 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Do not mutate the shared fake_vcpumodel in the tests https://review.opendev.org/c/openstack/nova/+/1012545 | 16:22 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Give the quota class sets tests their own shared state https://review.opendev.org/c/openstack/nova/+/1012547 | 16:36 |
| opendevreview | Thomas Goirand proposed openstack/nova master: Mock manager.LOG in the reverts_task_state test https://review.opendev.org/c/openstack/nova/+/1012549 | 16:39 |
| opendevreview | Merged openstack/nova master: Do not mutate the shared fake_vcpumodel in a test https://review.opendev.org/c/openstack/nova/+/1012542 | 18:28 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!