| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-08-26 | |||
| 16:11:15 | gibi | Uggla: I'm currently try to squeez in https://review.opendev.org/c/openstack/nova/+/957088 before FF | |
| 16:11:27 | gibi | the rest of the eventlet series can wait post FF / G | |
| 16:11:57 | bauzas | my only concern is that AMD SEV-ES series requires a reshape | |
| 16:12:03 | Uggla | gibi, thanks that was one of the question I had. | |
| 16:12:14 | dansmith | gibi: you said you were fine with the content on that one, but...no unit tests? | |
| 16:12:17 | bauzas | this requires a lot of caution | |
| 16:12:18 | dansmith | are those coming? | |
| 16:12:26 | sean-k-mooney | Uggla: techihnaly teh parla clodu migration chnag eis for 2026.1 | |
| 16:12:45 | bauzas | the what ? | |
| 16:12:47 | sean-k-mooney | or did we approve it late for this cycle | |
| 16:12:50 | gibi | dansmith: it is a refactor like the scatter gather change | |
| 16:13:01 | gibi | dansmith: I don't see what extra unit test we need | |
| 16:14:19 | bauzas | well, yes and no | |
| 16:14:21 | dansmith | okay I guess I thought something has to change with those mechanics, but I'll go look at what is testing this to see | |
| 16:14:37 | bauzas | there is a refactor but there is also a slight behavioral change | |
| 16:15:47 | dansmith | the lock obviously isn't verified, but if it's being run over then maybe that's okay | |
| 16:17:02 | gibi | the change is the lock and I want to verify that locally. If you need a functional test to ensure that lock is valid then sure we can do that too | |
| 16:17:22 | gibi | I don't see any other change in logic | |
| 16:18:58 | bauzas | I guess having such an invasive change without any single line of nova/tests made me afraid and I missed the point of the refactor | |
| 16:19:12 | gibi | invasive? you mean a lock? | |
| 16:19:30 | gibi | I'm confused | |
| 16:19:56 | gibi | I thought the goal of the refactor is not to change behavior so if the existing test does not need to be touched that is actaully good | |
| 16:20:23 | gibi | as it proves no behavior change happened | |
| 16:21:23 | dansmith | gibi: I guess I expected to see at least something running this code in both modes, but perhaps I'm jumping the gun there. I know we don't have that for everything we're thread-ifying, but this is also sort of a self-contained example | |
| 16:21:49 | sean-k-mooney | so it would be nice to have the zuul change to enable it in the condutore in the patch | |
| 16:21:49 | bauzas | gibi: I agree with the fact that until some env var is set, nothing changes | |
| 16:22:10 | sean-k-mooney | but i dont consider that an invasive change either for what its worth | |
| 16:22:11 | gibi | dansmith: yeah we did not transformed the unit test to both modes as part of the changes but we did it separately. | |
| 16:22:24 | bauzas | but yeah, I'd have appreciated some guideline to temper my guts | |
| 16:22:32 | sean-k-mooney | its adding a lock that shoudl techenically have been there even under eventlet + swapting the executor we use | |
| 16:22:34 | dansmith | unlike general thread spawning, this is a self-contained create/destroy of a single executor pool, which means we could test both modes here and now which is kinda what I was expecting | |
| 16:23:01 | dansmith | but that's fine | |
| 16:23:22 | gibi | dansmith: I will check my unit test enalbing series if it enables the related tests already... | |
| 16:23:42 | gibi | with threading in a separate tox target | |
| 16:24:37 | sean-k-mooney | what was that job called by the way | |
| 16:24:45 | sean-k-mooney | i tough we already enabled that in ci | |
| 16:24:53 | sean-k-mooney | is that still pending? | |
| 16:24:57 | gibi | nova-tox-py312-threading | |
| 16:25:00 | gibi | it is pending | |
| 16:25:04 | gibi | https://review.opendev.org/c/openstack/nova/+/955791/9 | |
| 16:25:06 | sean-k-mooney | ah ok | |
| 16:25:26 | sean-k-mooney | sorry i tought that was earlier in the series | |
| 16:25:36 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/953475/20 that one right | |
| 16:25:45 | Uggla | assuming gibi is working on the eventlet refactor any chance to land something in the above list ? Can we define some priorities/owners about them ? | |
| 16:26:21 | sean-k-mooney | there are a number of the ci/testing changes that woudl be nice to also land | |
| 16:26:25 | gibi | dansmith: I see multiple image cache unit test passing in https://4d426b9917a9f7962308-d3c6167007a6d8477c4cec901604a1c7.ssl.cf1.rackcdn.com/openstack/4beb6e364cb74aaeb2b8ff0b475d966f/testr_results.html with threading. So if we move the refactor under the unit test series we can verify the change | |
| 16:26:37 | sean-k-mooney | but i wonder if we woudl be oke with some of those happening post FF | |
| 16:27:01 | dansmith | gibi: ack | |
| 16:27:17 | gibi | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/953475/20 adds the job, but later patches enables more and more tests | |
| 16:27:21 | sean-k-mooney | the 6 patches including and up to https://review.opendev.org/c/openstack/nova/+/955791/9 are strech goals for me | |
| 16:27:42 | gibi | yeah the unit test enablement was planned post FF as streach for Flamingo | |
| 16:28:13 | gibi | but if we block the nova-conductor refactor on it then I will at least make an arrangement to have test results | |
| 16:28:31 | gibi | I mean unit test results with image cache tests running in threading mode | |
| 16:28:42 | gibi | do you want me to do that? | |
| 16:29:47 | bauzas | I agree with the statement that I feel that the patches above n-api seem more important for now | |
| 16:29:58 | opendevreview | Max proposed openstack/nova master: db: add indexes for source/dest migrations https://review.opendev.org/c/openstack/nova/+/958556 | |
| 16:30:36 | gibi | bauzas: so you say lets prioritize the unit test enablement over the nova-conductor work. | |
| 16:30:39 | gibi | ? | |
| 16:30:58 | gibi | (I have no problem with it if that is what the team wants) | |
| 16:31:00 | bauzas | for me, at least, I'd prefer reviewing those patches | |
| 16:31:47 | sean-k-mooney | i was going to suggest rebasign the 6 packages on top of the conductor one | |
| 16:31:55 | bauzas | and I think those would be more beneficial for the rest of the work we have to do | |
| 16:32:09 | bauzas | sean-k-mooney: you mean, on the bottom ? | |
| 16:32:13 | sean-k-mooney | no | |
| 16:32:18 | gibi | sean-k-mooney: yeah that what I meant by arranging to get test results | |
| 16:32:36 | sean-k-mooney | because i disagree that havign the ablity to run the condcutor in treading mdoe is less valuable | |
| 16:33:03 | gibi | sambork: Are you around? ^^ :) | |
| 16:33:04 | sean-k-mooney | if we have the conductor patch we can test it and ask operator to test it and report bugs | |
| 16:33:08 | bauzas | if wre're not able to run it in CI, what's the benefit ? | |
| 16:33:17 | sean-k-mooney | we can trivially | |
| 16:33:26 | sean-k-mooney | its liktrally a 1 line change to the zuul job | |
| 16:33:37 | sean-k-mooney | to set the var in devstack to enabel threading mode | |
| 16:33:44 | bauzas | but we won't execute that code, right? | |
| 16:33:50 | gibi | sean-k-mooney: I think what bauzas imply is that we don't have tempest coverage for the changed code path | |
| 16:34:07 | sean-k-mooney | ok that should nto block the code change | |
| 16:34:07 | bauzas | I mean, the zuul config change will be meaningless | |
| 16:34:23 | bauzas | if we don't have zuul jobs that call image caching | |
| 16:34:25 | sean-k-mooney | its wont be it makes sure everythign else in the conductor works | |
| 16:34:39 | sean-k-mooney | i.e. that we are not implcitly relying on eventlet indrectly | |
| 16:34:53 | bauzas | that is a good point | |
| 16:35:12 | bauzas | we could have missed other eventlet dependencies | |
| 16:35:25 | bauzas | I then agree, | |
| 16:35:27 | sean-k-mooney | by the way i think we could land both if we are willing to merge the unit test change up to RC1 | |
| 16:35:51 | sean-k-mooney | my suggesting is for gibi to rebase them so we get more results today/tomorrow | |
| 16:36:01 | bauzas | agreed, I like that approach | |
| 16:36:03 | sean-k-mooney | and then we can decied if we are comfrotabel with merging the conductor changes | |
| 16:36:05 | gibi | I will do the rebase today to get unit test results with threadin on image cache | |
| 16:36:11 | bauzas | thanks gibi | |
| 16:36:14 | gibi | sambork: ^^ FYI | |
| 16:36:29 | sean-k-mooney | gibi: can we add the devstack job change too if you or sambork have time | |
| 16:36:32 | gibi | I will do local testing of the lock explicitly | |
| 16:36:37 | gibi | sean-k-mooney: I will do so | |
| 16:36:41 | sean-k-mooney | +1 | |
| 16:37:41 | gibi | I have nothing further | |
| 16:38:05 | Uggla | asking again, assuming gibi is working on the eventlet refactor any chance to land something in the above list ? Can we define some priorities/owners about them ? | |
| 16:38:37 | bauzas | about the other things to review, I want to speak about SEV-ES | |
| 16:38:46 | bauzas | but tkajinam is not here I guess (and hope) | |
| 16:38:56 | tkajinam | I am | |
| 16:38:59 | bauzas | wow | |
| 16:39:01 | bauzas | okayu | |