| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-11 | |||
| 11:40:05 | sean-k-mooney | the failure is definatly comign form the LOG.error call | |
| 11:40:15 | sean-k-mooney | but i cant see any patch that would casue this in those diffes | |
| 11:40:27 | elodilles | yes, 5.1.0..5.3.0, but it still not shows the reverts that we only merged on stable/2023.1 :/ | |
| 11:41:35 | elodilles | there are 3 patches reverted in 5.2.0 | |
| 11:41:45 | sean-k-mooney | just so we are on the same page https://paste.opendev.org/show/bHorJ0sp6UTAKfu2EbJ2/ passes | |
| 11:42:03 | sean-k-mooney | its the LOG.error on line 34 | |
| 11:42:22 | sean-k-mooney | that is causing the issue but the message it is loging and the assert that is fialing do not make sense | |
| 11:43:42 | sean-k-mooney | no matter what i log it causes Calls: [call(7, b'X')] on os.write | |
| 11:45:30 | elodilles | sean-k-mooney: these were reverted on 5.2.0, but not in master (5.3.0): https://paste.opendev.org/show/bRU8ommJhreoVpdiWMRV/ | |
| 11:45:46 | elodilles | one of them has to be related to the issue | |
| 11:46:51 | sean-k-mooney | so the eventlet fix was added and removed fith | |
| 11:46:54 | sean-k-mooney | *right | |
| 11:47:19 | sean-k-mooney | im going to cloen oslo.log and pip install -e it into the env | |
| 11:47:25 | sean-k-mooney | and i guess do a biset effectivly | |
| 11:48:51 | frickler | so this whole issue is 6 months old and got successfully ignored? cool | |
| 11:49:42 | sean-k-mooney | no idea really. its only causing two test to fail | |
| 11:49:51 | sean-k-mooney | so i dont think its actully breaking real code | |
| 11:50:15 | sean-k-mooney | however im not parcarlly happy with just modifying the test without understadnign why this is breakign | |
| 11:50:27 | sean-k-mooney | as it really not obvious why this is happeining | |
| 11:50:58 | sean-k-mooney | and looking at the patch delta i woudl not expect this type of change between 5.2.0 and 5.3.0 | |
| 11:53:25 | elodilles | stephenfin: do you have any idea what could be a solution for this oslo.log issue? ^^^ | |
| 11:56:34 | sean-k-mooney | so its broken by 94b9dc3 | |
| 11:56:40 | frickler | sean-k-mooney: 5.2.0 is a bugfix release only done on stable/2023.1, 5.1.0 is the initial release for 2023.1 | |
| 11:56:51 | sean-k-mooney | https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17 | |
| 11:56:52 | elodilles | yepp | |
| 11:57:08 | frickler | so 5.1.0..5.3.0 gives a better idea of what happened in master. though iiuc the issue is in 5.0.0..5.1.0 | |
| 11:58:29 | frickler | 94b9dc3 is 5.0.1, ack | |
| 11:59:20 | sean-k-mooney | so with that calls to LOG.error are internally using os.write | |
| 11:59:32 | sean-k-mooney | presumably its realted to the lock its taking | |
| 11:59:44 | sean-k-mooney | here https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17#diff-2c76d41c287653560e6e84f39ce877b4f9ba33c7b36db17194e7435cd54adfb0R41 | |
| 12:00:58 | sean-k-mooney | its this call on release | |
| 12:01:04 | sean-k-mooney | https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17#diff-2c76d41c287653560e6e84f39ce877b4f9ba33c7b36db17194e7435cd54adfb0R108 | |
| 12:01:09 | sean-k-mooney | line 108 | |
| 12:01:19 | sean-k-mooney | that is causing teh test failure sepcifically | |
| 12:01:50 | sean-k-mooney | the next commit maks this conditionaly | |
| 12:01:52 | sean-k-mooney | https://github.com/openstack/oslo.log/commit/de615d9370681a2834cebe88acfa81b919da340c | |
| 12:02:12 | sean-k-mooney | so i guess there are two ways to fix this | |
| 12:02:25 | sean-k-mooney | i could proably disable this fix in this test | |
| 12:02:35 | sean-k-mooney | or i might be able to mock the lock | |
| 12:03:32 | sean-k-mooney | the issue is we have a StandardLogging fixture in our base test clase which handles all or most of the loging config | |
| 12:03:50 | sean-k-mooney | i can also mock the LOG var on the moduel i guess | |
| 12:04:28 | sean-k-mooney | we cocationally do that so i would just replace LOG with a mock explictly | |
| 12:05:13 | elodilles | so this issue only comes up in nova's tests, but could not cause any problem outside the test code, you mean? | |
| 12:07:03 | sean-k-mooney | its not breaking any real code | |
| 12:07:25 | sean-k-mooney | its just breaking the test case as we are mocking os.write and testign a function that uses it directly | |
| 12:07:40 | sean-k-mooney | and oslo.log is not ment ot driectly call os.write | |
| 12:07:47 | sean-k-mooney | or at least id did not previously | |
| 12:08:13 | sean-k-mooney | the "eventlet logging fix" patch add a loc that internally calls os.write | |
| 12:10:14 | elodilles | hmmmm, i see | |
| 12:11:32 | sean-k-mooney | self.useFixture(fixtures.MonkeyPatch("nova.privsep.utils.LOG", mock.Mock())) | |
| 12:11:36 | sean-k-mooney | that shoudl be the fix | |
| 12:11:55 | sean-k-mooney | ya that works | |
| 12:12:04 | sean-k-mooney | ill push that up as a patch | |
| 12:27:06 | elodilles | sean-k-mooney: thanks! \o/ | |
| 13:24:02 | opendevreview | sean mooney proposed openstack/nova master: adapt to oslo.log changes https://review.opendev.org/c/openstack/nova/+/894538 | |
| 13:24:36 | sean-k-mooney | frickler: elodilles: bauzas: ^ that should fix that specific issue | |
| 13:24:56 | bauzas | just saw the patch | |
| 13:25:50 | sean-k-mooney | there may be more generic ways to adress this but i dont really want do do anything more invaisve or global right now | |
| 13:26:22 | bauzas | sean-k-mooney: I wish oslo.log would have had an upgrades relnote | |
| 13:26:50 | sean-k-mooney | well they were not awre i guss that we were sentiive to things like calls to os.write | |
| 13:26:57 | sean-k-mooney | it does not rbeak runtime code | |
| 13:27:09 | sean-k-mooney | with that said i need to test this with our functional tests as well | |
| 13:27:15 | sean-k-mooney | so im going to do that now | |
| 13:28:54 | bauzas | cool | |
| 13:53:33 | opendevreview | Merged openstack/python-novaclient stable/2023.2: Update TOX_CONSTRAINTS_FILE for stable/2023.2 https://review.opendev.org/c/openstack/python-novaclient/+/894076 | |
| 14:22:02 | sean-k-mooney | bauzas: so the functional test also work find with 5.3.0 | |
| 14:22:16 | sean-k-mooney | so it really was just thowse two speicifc unit tests that were impacted | |
| 14:50:41 | bauzas | I voted +2 | |
| 14:51:24 | bauzas | folks (esp. cores), I have a lot of changes that need to be merged before RC1 : https://etherpad.opendev.org/p/nova-bobcat-rc-potential | |
| 14:52:19 | bauzas | at least : https://review.opendev.org/c/openstack/nova/+/893742 https://review.opendev.org/c/openstack/nova/+/893744 https://review.opendev.org/c/openstack/nova/+/893749 | |
| 14:53:27 | bauzas | dansmith: sean-k-mooney: gmann: ^ | |
| 14:56:07 | dansmith | bauzas: yep I saw, I'll work through the etherpad when I'm done with my current thing | |
| 14:56:19 | bauzas | ++ | |
| 15:10:05 | sean-k-mooney | stephenfin: can you review https://review.opendev.org/c/openstack/nova/+/894538 | |
| 15:10:13 | stephenfin | sure | |
| 15:11:10 | sean-k-mooney | bauzas: ill start look at those now to | |
| 15:11:32 | stephenfin | sean-k-mooney: is the print intentional? Do we need it? | |
| 15:11:33 | sean-k-mooney | did we really have no api microversion this cycle | |
| 15:11:39 | sean-k-mooney | ok i tought we did | |
| 15:12:02 | bauzas | sean-k-mooney: no, we haven't, neither any RPC change | |
| 15:12:17 | bauzas | it was a very smooth release in terms of upgrade | |
| 15:12:59 | sean-k-mooney | this is both good and bad for the first slurp release | |
| 15:13:53 | sean-k-mooney | stephenfin: no it was not good catch | |
| 15:14:17 | stephenfin | cool, can +2/+W once it's respin | |
| 15:14:20 | stephenfin | *respun | |
| 15:14:26 | bauzas | sean-k-mooney: I thought your print was needed | |
| 15:14:39 | sean-k-mooney | i can add a log for it | |
| 15:14:44 | sean-k-mooney | that was me just debuging things | |
| 15:15:02 | sean-k-mooney | the way we use pbr does not work with the debugger in vscode | |
| 15:15:05 | bauzas | cool, just respin then | |
| 15:15:17 | bauzas | hah | |
| 15:15:37 | bauzas | I personnally rather pdb with an entrypoint I add | |
| 15:15:48 | bauzas | but I understand your ask for walking thru | |
| 15:16:12 | opendevreview | sean mooney proposed openstack/nova master: adapt to oslo.log changes https://review.opendev.org/c/openstack/nova/+/894538 | |
| 15:17:45 | stephenfin | thanks | |
| 15:26:47 | sean-k-mooney | bauzas: i mentioned this to you before but can we also try to land https://review.opendev.org/c/openstack/nova/+/860829 today | |
| 15:33:03 | bauzas | sean-k-mooney: this is just changing the way we query, right? | |
| 15:33:39 | bauzas | my only concern is how much it would impact the SQL query plan | |
| 15:33:53 | sean-k-mooney | we need to do it anyway | |
| 15:33:57 | bauzas | and what would be the impact in terms of performance | |