| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-10-24 | |||
| 16:58:14 | bauzas | tl;dr: it requires reviews from myself | |
| 17:13:01 | opendevreview | Merged openstack/nova master: libvirt: Deprecate volume driver for unsupported volume backends https://review.opendev.org/c/openstack/nova/+/932247 | |
| 17:38:43 | tkajinam | I posted this in oslo channel accidentally but I have another oslo.utils patch to be used by nova so I'll create a release once it's merged https://review.opendev.org/c/openstack/oslo.utils/+/932335 | |
| 17:44:31 | tkajinam | As we discussed a lot about eventlet removal does anyone care to give quick review to https://review.opendev.org/c/openstack/nova/+/930726 which removes workaround to ancient eventlet versions ? | |
| 17:45:22 | opendevreview | Mengyang Zhang proposed openstack/nova-specs master: Add Burst Length Support to Cinder QoS https://review.opendev.org/c/openstack/nova-specs/+/932653 | |
| 17:58:24 | dansmith | tkajinam: sean-k-mooney should probably look at that | |
| 18:07:40 | sean-k-mooney | i dont recall if that is one of the things i was trying to clean up. | |
| 18:07:57 | sean-k-mooney | glancing at it if the bug in eventlet is already fixed then it soudn like an easy win | |
| 18:08:07 | sean-k-mooney | ill add myslef as a reviewer | |
| 18:11:35 | frickler | fyi, mentioned this in the oslo channel earlier: bumping to latest eventlet seems to break a couple of projects, including nova https://review.opendev.org/c/openstack/requirements/+/933257 | |
| 18:12:42 | frickler | hberaud mentioned a change that makes read/write calls actually non-blocking, which might trigger stuff | |
| 18:13:07 | frickler | https://github.com/eventlet/eventlet/issues/973 | |
| 18:13:52 | sean-k-mooney | os.read is ment to block but yeild execution fo the current greenthread | |
| 18:14:44 | JayF | that's going to be sneaky breaking for Ironic/IPA as well, I'll note | |
| 18:14:56 | JayF | likely would cause issues with image streaming error handling (timeouts/disk failures) | |
| 18:15:20 | JayF | stuff I wouldn't 100% expect to show up in happy path testing; but def. not unit tests | |
| 18:15:30 | JayF | oh, thought this was -requirements, either way, info is good :D | |
| 18:15:32 | sean-k-mooney | i have nto actully looked at the change in detail but reads and write need to be blocking and yeild | |
| 18:17:21 | frickler | I completely do not rule out the possibility that this is actually a bug in eventlet | |
| 18:17:25 | dansmith | um, is this a regression in eventlet? I was thinking it only monkeypatched the network IO bits | |
| 18:17:43 | sean-k-mooney | it monkeypatches file io too | |
| 18:17:46 | dansmith | like, is this change actually not part of the original intent of eventlet? | |
| 18:18:27 | sean-k-mooney | eventlet shoudl not make any standard lib fucntion that is normally blocking non blocking form the point of view of the caller fo the function | |
| 18:18:37 | frickler | best discuss in the oslo channel then where herve is actually around | |
| 18:19:01 | dansmith | right, and I think the change doesn't actually change the behavior, it just adds trampoline calls in between non-blocking reads | |
| 18:19:28 | sean-k-mooney | it may be doing somethign valid yes | |
| 18:19:39 | sean-k-mooney | frickler: do you have a like to the nova failure? | |
| 18:20:06 | sean-k-mooney | oh i see them | |
| 18:20:33 | frickler | sean-k-mooney: in the reqs patch I linked, both py311 and functional failing | |
| 18:21:18 | dansmith | sean-k-mooney: I don't see that it patches os.open, read, write: https://eventlet.readthedocs.io/en/latest/patching.html | |
| 18:22:01 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/monkey_patch.py#L44-L52 | |
| 18:22:20 | JayF | https://github.com/eventlet/eventlet/blob/master/eventlet/green/os.py#L22 | |
| 18:23:07 | dansmith | okay I guess I'm not sure that necessarily means it patches open() | |
| 18:23:16 | sean-k-mooney | os is one of the one it default patches | |
| 18:23:19 | JayF | it patches fdopen(), read(), write() and others outta tha library https://github.com/eventlet/eventlet/blob/master/eventlet/patcher.py#L533 | |
| 18:23:32 | dansmith | I say that because mocking open is different than other things and I thought it has (for quite a while) been a C function | |
| 18:24:00 | JayF | Yeah, eventlet overrides more than one C function and replaces it with a python-native version | |
| 18:24:10 | sean-k-mooney | by the way the unit and functional test patches dont obviously look like eventlet falirues | |
| 18:24:11 | JayF | that's how the major 3.12 blocker was resolved | |
| 18:24:19 | dansmith | I know it does, I just didn't think open() itself | |
| 18:25:35 | sean-k-mooney | https://github.com/eventlet/eventlet/blob/master/eventlet/green/builtin.py | |
| 18:25:46 | JayF | docstring says: Wrap os.open; This behaves identically, but collaborates with the hub's notify_opened protocol. ... which implies we're both right-ish? | |
| 18:26:16 | JayF | patched but no significant behavior change | |
| 18:26:34 | sean-k-mooney | ya it looks more or less the same | |
| 18:26:44 | sean-k-mooney | its just setting up a notifyier on the file descriptor | |
| 18:26:51 | sean-k-mooney | to know when the file is ready i guess | |
| 18:27:20 | JayF | sean-k-mooney: my knowledge of eventlet mostly stops at the patcher module :D hehe | |
| 18:27:46 | dansmith | idk, ages ago (like icehouse timeframe) when we were transitioning from the mysql C client to the python-native one, I thought that we encountered a situation where local file i/o was not patched and that surprised me, and some of the elders explained that it wasn't in scope | |
| 18:28:30 | sean-k-mooney | i mean we can disabel patchign of this | |
| 18:28:36 | dansmith | but that was a long time ago and we've shuffled our own monkey patching a lot since then, so maybe we weren't patching os at the time, or maybe I'm just misremembering | |
| 18:28:40 | sean-k-mooney | we coudl set os=false adn builtins=false | |
| 18:29:04 | sean-k-mooney | we disabel os on windows for posix reasons | |
| 18:29:30 | sean-k-mooney | so i can push a patch and see what that does but i need to look at the tempest job | |
| 18:29:51 | sean-k-mooney | the unit/fucniton test failrues dont really tell us a lot about what the actual effect of the chagne is | |
| 18:30:56 | sean-k-mooney | ok devstack only deploy nova-api neutron failed to start so we didnt finish stacking | |
| 18:31:19 | sean-k-mooney | nova api seams to be up | |
| 18:31:54 | sean-k-mooney | neutron.plugins.ml2.ovo_rpc RuntimeError: cannot notify on un-acquired lock | |
| 18:32:02 | sean-k-mooney | that is the neutron server error | |
| 18:32:38 | sean-k-mooney | oh thats coming form oslo messaging | |
| 18:32:42 | sean-k-mooney | https://5ce7fa2fbddee5b97128-6f49bdea8e90fe6675f1ae011c4f0f22.ssl.cf5.rackcdn.com/933257/1/check/tempest-full-py3/018987a/controller/logs/screen-q-svc.txt | |
| 18:36:36 | sean-k-mooney | frickler: i think its better to phase this as the new eventlet release is breaking oslo adn that is breakign everything else | |
| 18:36:58 | sean-k-mooney | the nova unit/funtional test failure might eb related or flaky tests so i woudl try at least one recheck to confirm that | |
| 18:39:31 | sean-k-mooney | frickler: the fact the devstack functional job worked is also odd | |
| 18:40:03 | dansmith | JayF: sean-k-mooney: https://github.com/openstack/nova/blob/icehouse-eol/nova/cmd/__init__.py | |
| 18:40:05 | dansmith | os=False | |
| 18:40:39 | sean-k-mooney | rigth so we still do that for winwos | |
| 18:40:42 | sean-k-mooney | *windows | |
| 18:40:58 | dansmith | but that was for every binary on every platform, AFAIK | |
| 18:41:00 | JayF | I think that was required to be added some point around t? u? v? era-ish for greendns to behave properly? My memory is very lossy but I remember having to back os=False outta IPA at some point | |
| 18:41:03 | dansmith | ...at the time | |
| 18:41:20 | sean-k-mooney | it was yes | |
| 18:41:28 | sean-k-mooney | im trying to see when we changed it | |
| 18:41:43 | sean-k-mooney | we sould be able to go back but i am not sure its breaking nova | |
| 18:42:22 | dansmith | it was that way through rocky at least | |
| 18:42:42 | dansmith | I lose the thread when the refactor to monkey_patch.py happened | |
| 18:42:48 | dansmith | anyway, | |
| 18:43:05 | dansmith | I'm not saying we should, I'm defending why I thought the os.open thing wouldn't affect us :) | |
| 18:43:08 | dansmith | and proving that I'm old | |
| 18:43:22 | sean-k-mooney | dansmith: the sdk devstack tests passed https://5355e5989bc73779bf10-cc682349d9014dd293929a9a1be5247d.ssl.cf5.rackcdn.com/933257/1/check/openstacksdk-functional-devstack/fad2996/testr_results.html | |
| 18:46:20 | sean-k-mooney | dansmith: https://github.com/openstack/nova/commit/4f2d4d03f3a1a0a277dd04ec2abcbee2d4018fa2 | |
| 18:47:27 | sean-k-mooney | so there was no reason given why it orginally was false and also no reason given why we shoudl remov it | |
| 18:47:38 | dansmith | yeah :) | |
| 18:47:55 | sean-k-mooney | dansmith: by the way i also remember os=false for what its worth | |
| 18:48:24 | sean-k-mooney | as in that what i expected to see on master | |
| 18:48:52 | sean-k-mooney | for now i just recheck the requiremnt patch to see if things are failing the same way consitently | |
| 18:49:51 | sean-k-mooney | hum we changed in stein | |
| 18:58:26 | opendevreview | Ghanshyam proposed openstack/osc-placement master: Update python classifier in setup.cfg https://review.opendev.org/c/openstack/osc-placement/+/931113 | |
| 19:12:03 | sean-k-mooney | frickler: i was able to repoduce the nova.tests.unit.test_service.TestWSGIServer.test_server_pool_waitall failure locally | |
| 19:13:30 | sean-k-mooney | nova.tests.unit.test_service.TestWSGIService has two failing unit tests for me too because we are not stubing calsl to bind but that happened with our without the new eventlet | |
| 19:24:15 | frickler | sean-k-mooney: maybe the results also vary between python versions/distros. the cross jobs run with py311 on bookworm | |
| 19:28:33 | sean-k-mooney | im using debian testing on 3.12 | |
| 19:28:46 | sean-k-mooney | frickler: the unit test fialure is because its too specific | |
| 19:29:03 | sean-k-mooney | the unit test is ment to assert that waitall is called when we shutdown the wsgi server | |
| 19:29:17 | sean-k-mooney | but its asserting its called once and now its called twice | |
| 19:29:25 | sean-k-mooney | so the fix for that is just change the assert | |
| 19:29:39 | sean-k-mooney | im repoducing the functional test failure now | |
| 19:40:38 | sean-k-mooney | the functionatl test that is failing is restarting the scheduler and that reulst in self.service_ref being None | |
| 19:40:55 | sean-k-mooney | i think this is an artifact of how we are doing that | |
| 19:41:45 | sean-k-mooney | restarting services in our functional tests is kind of a hack | |
| 19:42:19 | sean-k-mooney | so again i think this will be a case of a slight test tweak but not an actual regression | |