| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-12-03 | |||
| 17:16:15 | opendevreview | Dan Smith proposed openstack/nova master: Fix missing guest.migrate(parallel=) mock arg https://review.opendev.org/c/openstack/nova/+/969503 | |
| 17:16:45 | dansmith | and this ^ should fix the actual failure. We'll need to swap the order of those before merge of course, but in this sequence you can see the problem | |
| 17:26:15 | dansmith | I'll create a bug for this after my current call, btw | |
| 17:43:08 | gibi | dansmith: looking | |
| 17:49:59 | gibi | this is the eventlet change in that codepath https://review.opendev.org/c/openstack/nova/+/948072/17/nova/virt/libvirt/driver.py | |
| 17:50:52 | gibi | I'm wondering how the eventlet version raised the exception if if it is not waited for the spawned greenlet to finish | |
| 17:51:57 | gibi | I'm pretty fried already so I probably not seeing someting obvious here | |
| 17:56:56 | dansmith | I'm still on another call, but maybe a CastAsCall difference? perhaps that's the change here and not something that would be actually different at runtime? but also, it seems bad for us to not capture such an error at runtime | |
| 17:57:04 | dansmith | so perhaps the actual miss has been longer than eventlet removal | |
| 17:57:22 | dansmith | but it's definitely different with the new future stuff, and hides errors we were catching before (as demonstrated) | |
| 17:58:24 | gibi | yeah there is spawn synchronous fixture (like CastAsCall for RPC) so that can be involved | |
| 17:58:33 | dansmith | er, right | |
| 17:58:43 | gibi | hm that gave me a hunch | |
| 18:00:58 | gibi | there was a test fixture for this sync spanw for testing and it was actually broken (or I assumed at that time that it was broken) and I changed the behavior of it (thought fixing it) during the eventlet removal. This can be a fallout of that... | |
| 18:01:04 | gibi | now I just need to find it | |
| 18:02:44 | opendevreview | Merged openstack/nova-specs master: Spec for asynchronous volume-attach API https://review.opendev.org/c/openstack/nova-specs/+/958900 | |
| 18:05:57 | gibi | dansmith: I think this is the old code that made it work https://review.opendev.org/c/openstack/nova/+/948072/17/nova/tests/fixtures/nova.py#b1322 | |
| 18:06:23 | gibi | nope,ignore me | |
| 18:06:40 | gibi | link and add_done_callback is behaving the same | |
| 18:11:44 | gibi | OK I'm lost. I will try tomorrow with fresh minds | |
| 18:12:09 | dansmith | no problem, at least we know the problem now | |
| 18:21:27 | opendevreview | Will Szumski proposed openstack/nova master: WIP: Adds regression test for bug LP#2132984 https://review.opendev.org/c/openstack/nova/+/969380 | |
| 19:03:22 | opendevreview | Merged openstack/nova-specs master: Add backlog spec for Graceful shutodwn of nova services https://review.opendev.org/c/openstack/nova-specs/+/967469 | |
| 22:10:01 | opendevreview | melanie witt proposed openstack/nova master: WIP Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/969538 | |
| 22:53:47 | opendevreview | Ghanshyam proposed openstack/nova-specs master: Propose Spec 1 of Graceful shutodwn for 2026.1 https://review.opendev.org/c/openstack/nova-specs/+/969299 | |
| 23:47:10 | JayF | Unsure who all has core on nova stable; but it'd be really nice to get this landed: https://review.opendev.org/c/openstack/nova/+/968208 (and I'll take it back to next release when it is) | |
| 23:54:34 | opendevreview | Ghanshyam proposed openstack/nova-specs master: Add spec3 in graceful shutdown backlog spec https://review.opendev.org/c/openstack/nova-specs/+/969543 | |
| #openstack-nova - 2025-12-04 | |||
| 00:10:23 | gmaan | sean-k-mooney: gibi updated the graceful shurdown spec, please check - https://review.opendev.org/c/openstack/nova-specs/+/969299 | |
| 00:34:39 | opendevreview | melanie witt proposed openstack/nova master: WIP Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/969538 | |
| 00:44:10 | sean-k-mooney | gmaan: ack ill take a look in my morning | |
| 01:04:43 | opendevreview | sean mooney proposed openstack/nova-specs master: add spec for resource tracker notifications https://review.opendev.org/c/openstack/nova-specs/+/967712 | |
| 02:17:15 | opendevreview | Merged openstack/nova stable/2025.2: [ironic] Ensure unprovision happens for new states https://review.opendev.org/c/openstack/nova/+/968208 | |
| 02:36:20 | opendevreview | Jay Faulkner proposed openstack/nova stable/2025.1: [ironic] Ensure unprovision happens for new states https://review.opendev.org/c/openstack/nova/+/969547 | |
| 02:50:23 | opendevreview | zhou zhong proposed openstack/nova master: Create ephemeral and private secret when attaching encryptor https://review.opendev.org/c/openstack/nova/+/969019 | |
| 07:00:32 | opendevreview | Merged openstack/nova master: ensure correct cleanup of multi-attach volumes https://review.opendev.org/c/openstack/nova/+/916322 | |
| 07:59:48 | ratailor | gibi, sean-k-mooney bauzas could you please check this schema validation failure https://e88fada250d77d396ef5-a605dbf95134d478b50bec2dfa092555.ssl.cf1.rackcdn.com/openstack/146bb31174804bc5bebdba2b537c97db/testr_results.html related to https://review.opendev.org/q/topic:%22bp/show-instance-action-finish-time%22 | |
| 08:00:33 | bauzas | but I want to give a swing on the left specs I didn't read yet | |
| 08:00:48 | ratailor | In my local devstack env, I could get the instance action finish_time for create and attach_volume actions here https://paste.openstack.org/show/bWzmPfwfOZ9FQpfCpVPm/ | |
| 08:01:17 | ratailor | bauzas, ack. | |
| 08:01:31 | ratailor | gmaan, ^^ | |
| 08:06:19 | bauzas | ratailor: I'm not really a tempest expert about schema validations | |
| 08:06:50 | bauzas | I saw the patch that got -1 review.opendev.org/c/openstack/tempest/+/953331 but honestly I don't know why | |
| 08:07:15 | ratailor | bauzas, ack. I think stephenfin can help here. ^^ | |
| 08:07:23 | bauzas | I see you're depending on the nova patches | |
| 08:07:52 | bauzas | so everything looks correct to me at a first glance | |
| 08:08:02 | ratailor | bauzas, could you please review nova-spec and implementation and suggest if anything is missing. | |
| 08:08:35 | bauzas | as I said, I'll do another round of open specs today | |
| 08:09:02 | gibi | Details: HTTP response body is invalid (Additional properties are not allowed ('finish_time' was unexpected) | |
| 08:09:18 | gibi | so I assume your nova patch added a new field to the response | |
| 08:09:22 | gibi | that was not there before | |
| 08:09:26 | ratailor | gibi, yes | |
| 08:09:43 | gibi | and the tempest schema wasn't updated to accept that field in the new microvresion | |
| 08:21:24 | ratailor | gibi, this is what I have done to update tempest schema | |
| 08:21:26 | ratailor | https://review.opendev.org/c/openstack/tempest/+/953331/7/tempest/lib/api_schema/response/compute/v2_101/servers.py | |
| 09:00:23 | jkulik | ratailor: looks like you're overwriting your changes to show_instance_action in line 19 again in line 40 | |
| 09:13:47 | ratailor | jkulik, nice catch, thanks! | |
| 09:36:53 | zigo | https://ci.debian.net/packages/n/nova/testing/ppc64el/66782181/ | |
| 09:36:53 | zigo | Looks like I have 2 unit tests failing in Nova's autopkgtest because our ppc64el servers are too fast, no ? | |
| 09:36:53 | zigo | Hi there! | |
| 09:39:44 | zigo | gibi: ^ Your thoughts? | |
| 09:40:16 | zigo | Should we add a sleep() in these unit tests? | |
| 10:57:48 | opendevreview | Thomas Goirand proposed openstack/nova master: Fix race condition in DB updates https://review.opendev.org/c/openstack/nova/+/969745 | |
| 11:12:47 | opendevreview | Dominik proposed openstack/nova master: Regression test for Placement allocations remaining during failed schedule (bug #2132020) https://review.opendev.org/c/openstack/nova/+/969251 | |
| 11:19:23 | opendevreview | Dominik proposed openstack/nova master: Regression test for Placement allocations remaining during failed schedule https://review.opendev.org/c/openstack/nova/+/969251 | |
| 11:39:54 | sean-k-mooney | zigo: we try not ot add sleeps in gerneal so ideally no | |
| 11:40:38 | sean-k-mooney | zigo: we normally contol time in these test | |
| 11:41:24 | sean-k-mooney | so it looks like the test has a race condtion. if we add a sleep it woudl be somethign liks sleep(0.001) | |
| 11:42:09 | sean-k-mooney | zigo: i think gibi fixed a few cases likt this before when they started workign on the eventlet removal | |
| 12:02:34 | stephenfin | ratailor: bauzas: you need to define a new schema for tempest before the nova patch can merge. gmaan is a better person again to ask https://github.com/openstack/tempest/tree/8e2e854051b247ce914982ab0c3c0ed5e2f01b09/tempest/lib/api_schema/response/compute | |
| 12:02:58 | stephenfin | hopefully the duplication can be removed once the final patches merge against nova (hint hint) | |
| 12:03:07 | stephenfin | *final OpenAPI patches | |
| 12:06:12 | opendevreview | Thomas Goirand proposed openstack/nova master: Fix race condition in DB updates https://review.opendev.org/c/openstack/nova/+/969745 | |
| 12:12:06 | ratailor | stephenfin, ack. | |
| 12:19:00 | zigo | sean-k-mooney: You're wrong, IMO. | |
| 12:19:18 | zigo | sean-k-mooney: The issue is getting 2 timestamps in less than 1 second. | |
| 12:19:33 | zigo | Then they are equal, and later on, the test checks if they are NOT equal. | |
| 12:23:16 | sean-k-mooney | zigo: what im saying is we normlaly try to decouple the unit test entirly form teh infra | |
| 12:23:52 | sean-k-mooney | for example by mockign time.now() so that each call woudl increment by 1 second or some other way | |
| 12:24:44 | sean-k-mooney | zigo: for functional test we might uses sleeps as we try to avoid mockign if we cna btu for unit test we usually mock otu the tiem functions | |
| 12:37:24 | opendevreview | Merged openstack/nova stable/2025.1: [ironic] Ensure unprovision happens for new states https://review.opendev.org/c/openstack/nova/+/969547 | |
| 12:40:55 | zigo | sean-k-mooney: Oh ok, I get it now. Then I can just do that (ie: do timestamp2 = timestamp+1) | |
| 12:44:27 | zigo | Though it may be harder than just mocking the timestamp function, no? I don't know how the updated_at is generated. | |
| 12:51:08 | sean-k-mooney | we can likely mock time.now() and that shoudl be enough i think but i woudl have to look at the specific tests | |
| 12:55:34 | gibi | zigo: sean-k-mooney: we should have a fixture for the time.now() already in our tests maybe it is not used in those specific tests | |
| 12:55:49 | gibi | but overall I agree to make the test independent of the value of time.now | |
| 12:56:16 | zigo | I'd say: please, feel free to takeover my patch and make it better. | |
| 12:56:23 | zigo | I don't have enough available time to do so. | |
| 12:59:11 | gibi | I don't have time for that at the moment too. sorry | |
| 14:14:14 | sean-k-mooney | gibi: we do have a fixture for that yes but i didnt check if it was used either | |
| 14:14:40 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/test.py#L828 | |
| 14:15:29 | sean-k-mooney | that works a littel defently then i descrbed making all calsl return a fixed time | |
| 14:16:01 | sean-k-mooney | in this case the test logic is either incorect or we need one to monotmicly march forward to fit this test | |
| 14:16:34 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/test_nova_manage.py#L2172-L2175 | |
| 14:24:43 | opendevreview | Dominik proposed openstack/nova master: Regression test for Placement allocations remaining during failed schedule https://review.opendev.org/c/openstack/nova/+/969251 | |
| 14:39:43 | opendevreview | Dominik proposed openstack/nova master: Remove Placement allocations in the broken build cleanup https://review.opendev.org/c/openstack/nova/+/968446 | |
| 14:45:01 | opendevreview | Merged openstack/nova-specs master: Propose Spec 1 of Graceful shutodwn for 2026.1 https://review.opendev.org/c/openstack/nova-specs/+/969299 | |
| 15:08:05 | gibi | dansmith: sorry no progress yet with the utils.spawn issue. I was distracted with other things | |
| 15:08:44 | dansmith | gibi: no worries, I found it while doing other things, so once I found the issue I went back to those things | |
| 15:18:39 | gibi | dansmith: but now I found the change I had a hunch about https://review.opendev.org/c/openstack/nova/+/953121/2/nova/tests/fixtures/nova.py#b1248 so before this change the utils.spawn in untit test executed the passed in function and *raised back* any exception. But that is semantically wrong as only utils.spawn(f).wait() should raise it. So I changed the unit test fixture semantics to not raise from | |
| 15:18:45 | gibi | __init__ | |