| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-25 | |||
| 13:04:50 | elodilles | sean-k-mooney: regarding yesterday's ping about backportability of the 3 patches till caracal ( https://review.opendev.org/c/openstack/nova/+/918089/3 ): to me it looked OK to backport, but reading dansmith's opinion - he has more insight in this, so - it's completely on nova team decision whether an exception to backport this should be granted or not o:) (based on the discussion, backporting to | |
| 13:04:56 | elodilles | prior version than Caracal is not possible in my opinion). | |
| 13:09:30 | ratailor | elodilles, sean-k-mooney, gibi could you please review these, one more +2 is required https://review.opendev.org/c/openstack/nova/+/933107 https://review.opendev.org/c/openstack/nova/+/873901 https://review.opendev.org/c/openstack/nova/+/938054 ? | |
| 13:14:06 | elodilles | ratailor: i don't have +2 right on master branch o:) | |
| 13:15:08 | ratailor | elodilles, ack. no problem. you can provide your suggestion, if you want to. Thanks! :) | |
| 13:15:36 | elodilles | :) | |
| 13:25:49 | sean-k-mooney | ratailor: sure, i need to context switch to somethign else shortly but +2w on https://review.opendev.org/c/openstack/nova/+/933107, im already +2 on https://review.opendev.org/c/openstack/nova/+/938054 so that needs someone else. ill quickly review https://review.opendev.org/c/openstack/nova/+/873901 | |
| 13:26:16 | ratailor | sean-k-mooney, sure. Thanks! | |
| 13:27:37 | sean-k-mooney | my commnets on test coverage are still open on https://review.opendev.org/c/openstack/nova/+/873901 but no one has rasied a concern in 3 months so im sort of inlcined to say we can add more later if needed | |
| 13:29:48 | sean-k-mooney | ratailor: what ill do is the following. ill elevate form +1 to +2w but it would be nice if you could submit a followup after this merges for the happy path test i called out and other nits. | |
| 13:30:15 | ratailor | sean-k-mooney, sure. I will do. | |
| 13:34:49 | opendevreview | ribaudr proposed openstack/nova master: FUP Remove unnecessary PCI check https://review.opendev.org/c/openstack/nova/+/944105 | |
| 13:34:50 | opendevreview | ribaudr proposed openstack/nova master: FUP Add a warning to make non-explicit live migration request debugging easier https://review.opendev.org/c/openstack/nova/+/944133 | |
| 13:34:50 | opendevreview | ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124 | |
| 13:34:51 | opendevreview | ribaudr proposed openstack/nova master: FUP: Improve libvirt fixture for hostdevs https://review.opendev.org/c/openstack/nova/+/945232 | |
| 13:34:51 | opendevreview | ribaudr proposed openstack/nova master: FUP Update pci-passthrough and virtual-gpu documentation https://review.opendev.org/c/openstack/nova/+/944153 | |
| 13:34:52 | opendevreview | ribaudr proposed openstack/nova master: FUP improve and add integration tests for PCI SR-IOV servers https://review.opendev.org/c/openstack/nova/+/944106 | |
| 13:38:34 | elodilles | btw, if any nova core is ready for a 10 sec review (of a bot generated patch): https://review.opendev.org/c/openstack/nova/+/944890 | |
| 13:39:05 | bauzas | elodilles: done | |
| 13:48:05 | elodilles | thanks \o/ | |
| 14:04:43 | dansmith | ...there must be some sort of magic to get failing gabbit tests to actually show me logs... | |
| 14:07:18 | gibi | dansmith: does OS_DEBUG=1 helps? | |
| 14:07:27 | dansmith | it does not | |
| 14:07:29 | gibi | :/ | |
| 14:07:52 | gibi | it was too long ago when last time I debugged gabbit to remember | |
| 14:08:01 | dansmith | yeah, I've been tearing through infrastructure trying to hack it to give me logs or see why it's swallowing it, | |
| 14:08:18 | dansmith | but it's just very focused on the gabbit result | |
| 14:08:37 | dansmith | glance functional tests are sort of similar.. really really hard to get the backend logs when there's a failure | |
| 14:08:39 | dansmith | super annoying | |
| 14:14:39 | gibi | I agree | |
| 14:16:48 | sean-k-mooney | i was going to suggest the same OS_DEBUG=1 i confirmed it is allowed in the tox config so its not that either | |
| 14:18:08 | sean-k-mooney | its usign the normal log fixtures but it also has 2 addtional ones for error and warning | |
| 14:18:10 | sean-k-mooney | https://github.com/openstack/placement/blob/master/placement/tests/functional/base.py#L54-L58 | |
| 14:18:22 | sean-k-mooney | so i think its those last two that are the problem | |
| 14:18:31 | dansmith | yeah, I'm about to just repro the gabbit that is failing in python so I can effing see the problem | |
| 14:18:49 | sean-k-mooney | i woudl ty comemntign out lines 57 and 58 | |
| 14:18:57 | sean-k-mooney | and see fi it woks with os_debug then | |
| 14:19:41 | dansmith | already tried :) | |
| 14:20:46 | sean-k-mooney | well you can do the tried and true Raise RuntimeError instead of logging | |
| 14:22:27 | dansmith | I have no idea where it's failing | |
| 14:22:33 | sean-k-mooney | https://github.com/openstack/placement/blob/master/placement/tests/functional/fixtures/capture.py#L40-L62 | |
| 14:22:35 | dansmith | it's a 500, which should be logging lots | |
| 14:24:38 | sean-k-mooney | oh gabbit also has its own fixtures with its own logging config. it does include that however | |
| 14:24:47 | sean-k-mooney | https://github.com/openstack/placement/blob/master/placement/tests/functional/fixtures/gabbits.py#L62 | |
| 14:24:57 | dansmith | there are multiple layers of log interception in placement, and also in gabbi yeah | |
| 14:25:16 | dansmith | I'm wondering if gabbi has changed in some way and left placement stuck | |
| 14:25:39 | dansmith | to be clear, I've tried to mangle that capture module and I'm not sure it's ever even getting hit | |
| 14:25:46 | dansmith | I can define error, warning, etc on there and raise and nothing changes | |
| 14:26:01 | dansmith | not sure if that's because it ends up with another nameless 500 or what | |
| 14:27:30 | sean-k-mooney | well if you are looging and its intercepted it should show up in one fo those fixtures | |
| 14:27:48 | dansmith | to be clear, I'm not logging | |
| 14:28:00 | dansmith | there's a gabbit test that now fails with 500 after a change I made | |
| 14:28:08 | dansmith | and I'm trying to figure out what the 500 is | |
| 14:28:15 | sean-k-mooney | oh you want ot see that ok | |
| 14:28:23 | dansmith | but I agree, it should be going through that fixture | |
| 14:28:58 | sean-k-mooney | so it has to be realted to this block https://github.com/openstack/placement/blob/master/placement/tests/functional/fixtures/gabbits.py#L62-L71 | |
| 14:30:09 | dansmith | I mean, you'd think, but there are similar fixtures also in functional/base | |
| 14:30:17 | dansmith | and in gabbi itself | |
| 14:31:04 | sean-k-mooney | oh you think that the fact we are inheriting form the gabbit testcase might be altering the behavior | |
| 14:31:16 | sean-k-mooney | looking at git blame https://github.com/openstack/placement/commit/fcd1e538d9b7d315cb3f74c83f1a9e4ddcab180e | |
| 14:31:41 | sean-k-mooney | it same like there was a lot of log output in the past and they added thsi handelign ot silance some of that | |
| 14:31:58 | dansmith | I removed all those fixture setups in gabbits.py and no change | |
| 14:32:25 | dansmith | right, I'm sure it's the attempt to make it pretty, but it has made it completely un-debuggable, AFAICT | |
| 14:32:28 | sean-k-mooney | i proably shoudl be doing somethign else bug you peaked my interest | |
| 14:32:44 | sean-k-mooney | do you have a patch i can pully or aplly and see if i can replicate it | |
| 14:33:18 | dansmith | I can push it up | |
| 14:34:44 | opendevreview | Dan Smith proposed openstack/placement master: Reproduce bug 2104040: allocate in over-capacity https://review.opendev.org/c/openstack/placement/+/945464 | |
| 14:34:45 | opendevreview | Dan Smith proposed openstack/placement master: Fix placement allocate while over-capacity https://review.opendev.org/c/openstack/placement/+/945465 | |
| 14:34:54 | dansmith | sean-k-mooney: ^ | |
| 14:35:09 | sean-k-mooney | the secodn one is causign the 500 ya | |
| 14:35:16 | dansmith | maybe the actual thing failing will get hit by a tempest test and I can see what the effing problem is :) | |
| 14:35:17 | dansmith | yeah | |
| 14:36:49 | sean-k-mooney | well placmeent does not use eventlet so in theory i can also run this under a debugger in vscoded and set a breakpoint | |
| 14:37:14 | sean-k-mooney | i have never tried that with placment but i have got that to work wiht some of watchers unit tests | |
| 14:37:19 | dansmith | if I knew where to set a breakpoint I could do the same :) | |
| 14:37:44 | sean-k-mooney | you can make the debugger tirgger on uncaught excetpions | |
| 14:38:18 | sean-k-mooney | im going ot make sure i can repoduce the fialure first and then ill try something more fancy | |
| 14:39:48 | dansmith | thanks, maybe I've got tunnel vision on something | |
| 14:44:23 | sean-k-mooney | ... we still have not merget the patch to move form psycopg2 to psycopg2-binary... | |
| 14:44:32 | sean-k-mooney | oh we didnt propose that ot placment | |
| 14:44:36 | sean-k-mooney | ill fix that later | |
| 14:45:10 | sean-k-mooney | if we use psycopg2-binary instead we dont need the postgress development headers to be able to compile it | |
| 14:47:41 | sean-k-mooney | dansmith: so i ran the functional tests and everything passed | |
| 14:47:52 | sean-k-mooney | oh nevermind | |
| 14:47:59 | sean-k-mooney | that with master i have not applied you change yet | |
| 14:48:39 | sean-k-mooney | i did see "TransactionFactory already started, not reconfiguring." pirnted once or twice which is a bit suss but it does nto seam to be breaking anything | |
| 14:49:07 | dansmith | master passes for me too :) | |
| 14:49:40 | dansmith | I think I've just guessed the thing it's failing on by trial and error, which is good, but it's like debugging in the dark | |
| 14:50:35 | sean-k-mooney | so i have output | |
| 14:50:41 | sean-k-mooney | and two failed testcases | |
| 14:51:01 | sean-k-mooney | https://paste.opendev.org/show/baZsKMeiO9mOxyGz2OlY/ | |
| 14:51:11 | dansmith | ...right | |
| 14:51:15 | dansmith | that's just gabbit output | |
| 14:51:30 | dansmith | not the logging from the wsgi app to show you where the 500 was | |
| 14:51:38 | sean-k-mooney | ah you want the api logs form the service right | |
| 14:51:51 | dansmith | yeah, so I can see, you know, the *actual* problem | |
| 14:53:15 | sean-k-mooney | right well now that i know which tests are failing i can see if i can get more output sicn ei an just target those specific tests | |
| 15:05:29 | Uggla | sean-k-mooney, do you think we should have a cross team nova-watcher session at the PTG ? | |
| 15:05:45 | sean-k-mooney | sigh... the way we dynmically load teh gabit tst means that they are not discoverd wehn we use pytest and the way we alias the global fixtures module with our own breaks unittests discoveray | |
| 15:06:59 | sean-k-mooney | Uggla: am we could the topics i wanted to raise are not only relvent to watcher but it does not need to be super formal | |