Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-07-30
12:36:21 sean-k-mooney it could be something funky with the sepeate tread we have for that but its definly and upstream bug
12:39:33 stephenfin Yeah, what sean-k-mooney said. The reason it's such a big deal for cloud-provider-openstack is that the tests (a) create and delete a lot of PVCs (which translates to a lot of attach/detach calls), and (b) run serially, so things compound
12:40:13 gibi https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L312-L319
12:40:34 gibi this is the code that finds which event waiter to unblock based on incoming events
12:41:01 gibi called from here https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L385
12:41:32 gibi https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L2512-L2542 and this is the place where we get the events from libvirt
12:41:39 gibi it has logs
12:41:49 gibi so if these logs are not visible it means nova is not receiving any events
12:47:37 sean-k-mooney im not sure if that loggin actully works, which thrad is that runign on
12:47:40 sean-k-mooney specificly https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L2512-L2542
12:47:58 sean-k-mooney is that on the libvirt pthread or on the nova main thread
12:48:17 sean-k-mooney there is some issue with mixing real thread and oslo.log
12:50:43 sean-k-mooney for what its worht the matching code looks corect
12:50:44 gibi yeah that is likely running on the libvirt pthread
12:50:45 sean-k-mooney https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L316-L319
12:51:28 gibi I thought we can still log from the libvirt pthread
12:51:49 sean-k-mooney it was "fixed" by adding a pipemutex in oslo logs
12:52:01 sean-k-mooney but im not sure i really trust that entilry
12:52:07 gibi :)
12:52:38 sean-k-mooney im thinkging we add some good old prints or perhaps writes to a file that we can tail
12:52:55 sean-k-mooney although not right now.
12:56:28 gibi yeah that is always an option locally to troubleshoot it further
12:58:37 stephenfin https://bugs.launchpad.net/nova/+bug/2119114
12:59:15 stephenfin I marked it as confirmed since eharney also took a look at this and I've reproduced it across multiple environments. I did not set a priority though
12:59:32 stephenfin eharney and sean-k-mooney, naturally
13:00:16 gibi stephenfin: thank you!
13:00:21 gibi Uggla: ^^ for visibility
13:01:30 stephenfin I will dump all this is the bug, but fwiw, I see no entries for `Received event` messages in Caracal or later. I do see them on Bobcat
13:01:43 stephenfin (in the n-cpu logs, that is)
13:04:05 Uggla gibi, thanks, I'm not sure. Does it relates to the "gate" bug we discussed in the meeting yesterday ?
13:08:44 sean-k-mooney stephenfin: so its not actully breaking anything its just slow but i think tis in the medium to high range
13:09:16 sean-k-mooney i woudl proably set it to high but its been 3 release so medium woudl work for me too
13:19:07 gibi Uggla: no it is a new one, it does not block the gate
13:19:45 Uggla gibi, oh ok.
13:20:45 opendevreview Takashi Natsume proposed openstack/nova-specs master: Create specs directory for 2026.1 Gazpacho https://review.opendev.org/c/openstack/nova-specs/+/956166
13:22:23 opendevreview Takashi Natsume proposed openstack/nova master: Update contributor guide for 2025.2 Flamingo https://review.opendev.org/c/openstack/nova/+/944603
13:42:21 stephenfin So this is interesting: restarting the n-cpu services fixes things... 🤔
14:11:47 gibi stephenfin: maybe something blocks the libvirt thread receiving the notifications?
14:12:13 gibi and it gets unblocked by the restart until that something hits again and blocks it again
14:13:16 opendevreview Wesley Hershberger proposed openstack/nova stable/2024.1: Add ServersViewBuilderTestV296 unit test class https://review.opendev.org/c/openstack/nova/+/955304
14:13:17 opendevreview Wesley Hershberger proposed openstack/nova stable/2024.1: Use dict object for request_specs_dict in the _list_view https://review.opendev.org/c/openstack/nova/+/955305
14:17:36 dansmith bauzas: did you see my comments on the tpm series about the confirm flag?
14:18:10 dansmith you seemed to ask why they were changed.. I haven't looked at the change yet, but I asked to do it differently to avoid creating a mess we have to clean up or live with forever
14:18:41 opendevreview Merged openstack/nova master: Update contributor guide for 2025.2 Flamingo https://review.opendev.org/c/openstack/nova/+/944603
14:21:36 andrewbonney hi all. if there are any cores around I'd appreciate a review of https://review.opendev.org/c/openstack/nova/+/919961
14:21:51 andrewbonney that's a small bug fix which has been hanging around for 1 year+
14:26:23 gibi eventlet removal sync in 4 minutes https://meet.google.com/bcy-uqoz-hje
14:28:33 gibi andrewbonney: you are today's lucky one :) I approved the fix. Thanks for fixing that bug.
14:28:42 andrewbonney Thank you!
14:32:20 stephenfin gibi: Yeah, sean-k-mooney and I were thinking it might be a race, but it's weird that its happening consistently on every branch since bobcat
14:32:33 stephenfin restarting libvirtd didn't help either
15:03:49 opendevreview Ian Watson proposed openstack/nova master: Add migration conditional for resource tracker https://review.opendev.org/c/openstack/nova/+/955690
15:09:23 dansmith gibi: andrewbonney I had commented about some missing test coverage
15:09:40 dansmith looks like we raced and I lost
15:14:22 gibi I'm fine pulling it from the gate or looking at a fup with the test coverage
15:15:42 dansmith maybe andrewbonney can at least respond to my question and do a FUP if we just need an extra assert in an adjacent test
15:16:23 andrewbonney I'll take a look, but I've been away for a few months so need to context switch back in
15:18:03 sean-k-mooney oh the dns patch
15:19:17 sean-k-mooney dansmith: your asking is the a testcase where we do resest the dns name?
15:20:04 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for quota sets API https://review.opendev.org/c/openstack/nova/+/956141
15:20:05 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for migrations API https://review.opendev.org/c/openstack/nova/+/956142
15:20:05 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server migrations API https://review.opendev.org/c/openstack/nova/+/956143
15:20:06 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server tags API https://review.opendev.org/c/openstack/nova/+/956144
15:20:06 opendevreview Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server metadata APIs https://review.opendev.org/c/openstack/nova/+/956145
15:22:55 dansmith sean-k-mooney: no,
15:23:12 sean-k-mooney oh then i did not understand your question in the review
15:23:40 dansmith sean-k-mooney: the new test case added an assertion that we don't call reset when we're not supposed to, but I don't see an adjacent similar test case that asserts that we do call it when expected
15:23:59 dansmith I'm assuming there just needs to be an assert_called_once() somewhere in one of the nearby tests
15:24:26 dansmith point being, the patch is disabling calling that function in some situations, but I don't know that anything would break if it incorrectly disables the call in _all_ situations
15:26:48 sean-k-mooney oh ok am its only requrie if a prot is detached so the next test i think could have that mocked and assert its called
15:27:43 sean-k-mooney although in the detach flow
15:27:48 sean-k-mooney im not sure we need to do that
15:28:00 sean-k-mooney so we might be able to remove _reset_port_dns_name entirly
15:28:16 dansmith doesn't that make this patch totally different then?
15:28:28 sean-k-mooney i woudl have to lod context again
15:28:34 dansmith that's removing a line we were always calling.. this patch makes it conditional, but never checks the was-default-before case
15:28:41 dansmith gibi: andrewbonney ^
15:29:07 sean-k-mooney si the _reset_port_dns_name call if if we are using extenreal dns integration
15:29:31 sean-k-mooney if we are uisng neutron internl dns the its cleared vai port update on detact.
15:29:44 sean-k-mooney i think the code is still correct
15:29:48 dansmith that has nothing to do with my concern that we don't know if we're doing it or not
15:29:59 sean-k-mooney but we maybe jsut missing a test for that edge case
15:30:16 dansmith it's not an edge case, it's what we were *always* doing before
15:30:32 sean-k-mooney yes which was incorect
15:30:35 dansmith and if it wasn't problematic for andrewbonney's situation, this patch wouldn't be needed
15:30:42 sean-k-mooney we shoudl never have been doing it on unbind
15:30:44 sean-k-mooney only on detach
15:31:10 sean-k-mooney in both the internal and external dns case we were privously doing it unconditionally
15:31:21 sean-k-mooney now both path only do it if its a detach
15:32:23 dansmith none of that has anything to do with the tests I'm quoting.. I'm talking about the _unit_ test that checks that this method does what it should based on the various inputs, and there's now a conditional that we don't (AFAIK) verify does the right thing
15:34:07 sean-k-mooney detach defautls to true. so test_unbind_ports_reset_dns_name_by_admin whih is the next test calls unbiend iwth it set to ture
15:34:26 dansmith right, and where's the assert that reset_dns was called?
15:34:30 sean-k-mooney and the new test passes detach=false
15:34:37 sean-k-mooney it never existed before
15:34:41 dansmith I KNOW THAT
15:35:06 sean-k-mooney my point is the lack fo that assert is not related to this patch
15:35:18 sean-k-mooney we can add it
15:35:32 dansmith before, there was no question.. it was clearly a gap in the assertion, but at least there was no way for it to not be run.. now there *is* a way for it not to be run, and the test doesn't assert that it is called when we expect it to
15:37:11 sean-k-mooney sure we can add the assert. i have no issue with that
15:39:38 sean-k-mooney we can proably check this in the funtional test as well. we jsut need to replace shelve with detach

Earlier   Later