| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-05-08 | |||
| 11:58:23 | sean-k-mooney | i know its possibel to use isloate i fyou know exactly how the flavor will be used | |
| 11:59:07 | sean-k-mooney | i.e. if you have a flavor for a specific aplication that that will only be used with prots that either dont hav eresouce request or wont be fullfield form the same rp | |
| 11:59:45 | sean-k-mooney | but i think adding it to the flavor in its current form was a mistake so if we can avoid admin ever settign it by defaulting to None | |
| 11:59:53 | sean-k-mooney | it hink that will server better longterm | |
| 12:00:31 | sean-k-mooney | gibi: there is a similar but diffent issue with it too in that it breakes our filter because it not namespaced | |
| 12:00:49 | sean-k-mooney | stephen and i fixed that in train for the only other extra spec that did not have a namespace | |
| 12:01:23 | sean-k-mooney | we created a namespaced version as an alias and maded the non name "deprecated" | |
| 12:01:58 | sean-k-mooney | i think we shoudl do the same here and add group:policy=isolate|none | |
| 12:02:32 | sean-k-mooney | but again that for a slightly diffent reasons | |
| 12:03:23 | sean-k-mooney | until we can supprot expressign the policy betwen sepcific groups to me isolate is not practiclly usable unless you a nova/placment core or have spent a very very long time lookign at all the details | |
| 12:04:00 | sean-k-mooney | so im wondering if we can take some fo the sharp edges off in a backwards compatible way | |
| 12:04:34 | sean-k-mooney | by 1 defaulting to none in all cases if not set, and 2 providing the namespaced version to fix the filters | |
| 12:53:26 | gibi | I won't block the defaulting patch. | |
| 12:53:39 | gibi | I'm OK to fix the namespaceing | |
| 12:54:27 | gibi | I have no capacity to dig deeper in the discussion, but I'm happy to review the namespacing fix | |
| 13:17:54 | sean-k-mooney | gibi: thanks. i just want to get some early feedback on direction. i will try and find time to file the relevent bug tracker and work on some kind of poc of this for wider dicussion | |
| 14:27:38 | gibi | thanks | |
| 14:50:38 | gibi | dansmith: sean-k-mooney: FYI: here are the pre-fork executor cleanup change https://review.opendev.org/c/openstack/nova/+/947966/9/nova/cmd/scheduler.py it is cleaner in my eyes. It works well in my local non testing and on the gate (CI result is in the nova-next job of the last patch in the series) | |
| 14:51:10 | dansmith | nice | |
| 14:52:15 | gibi | oslo also did some changes based on our feedback about the fork issue but I haven't been able to check it out yet. | |
| 15:00:57 | gmaan | melwitt: thanks. ++ | |
| 15:09:13 | opendevreview | Merged openstack/nova-specs master: Add reminder about Tempest API response schemas https://review.opendev.org/c/openstack/nova-specs/+/949113 | |
| 15:20:19 | sean-k-mooney | gibi: how woudl you feall about getign rid of the glbal and doign this instead https://review.opendev.org/c/openstack/nova/+/947966/comment/dd7fc7e3_f4e926fb/ | |
| 15:20:49 | sean-k-mooney | gibi: and yes teh reset looks cleaner then i was expecting too so +1 on that | |
| 15:23:46 | sean-k-mooney | gibi: by the way we likely should modify our base testcase to ensure we shutdown teh threadpool and reset it between test runs | |
| 15:24:28 | sean-k-mooney | i belive i had that in my verison ot prevent leakign thread betwen tests | |
| 15:24:39 | sean-k-mooney | if we ever do leak a thread in the pool it should fail the test | |
| 15:24:55 | sean-k-mooney | the same way our greenthread leak proctection works | |
| 15:26:14 | sean-k-mooney | in the test we can kind of get awar with it by using the synconousthreadpool via a fixture too | |
| 15:27:34 | sean-k-mooney | but we do have fixture to mange this currently https://github.com/openstack/nova/blob/master/nova/test.py#L197 https://github.com/openstack/nova/blob/master/nova/test.py#L324-L329 | |
| 15:27:55 | sean-k-mooney | so we likely need to extend this to the new scather gather pool | |
| 15:29:19 | sean-k-mooney | i made the SynchronousThreadPoolExecutorFixture also apply to the futurist.ThreadPoolExecutor https://review.opendev.org/c/openstack/nova/+/922497/5/nova/tests/fixtures/nova.py#1254 | |
| 15:33:10 | sean-k-mooney | gibi: bug i think you shoudl also update https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L1191-L1247 and https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L2056 or add new ones for this thread pool | |
| 16:59:55 | gmaan | sean-k-mooney: RE on group_policy, I think namespace is good idea, not sure why it was left in initial effort. | |
| 17:01:05 | gmaan | sean-k-mooney: about changing default to None. is not it None as default in ResourceRequest object and as you mentioned it is set 'None' explicitly if two group from different than flavor source | |
| 17:01:47 | gmaan | I mean just checking what is value if it is not passed and groups from cmg from flavor only | |
| 17:02:50 | sean-k-mooney | gmaan: right i know we set it if we get groups form 2 souces | |
| 17:03:06 | sean-k-mooney | im jsut suggesting updating that logic ot set it if we ever have 2 groups and its not set | |
| 17:03:23 | sean-k-mooney | so if you set it in the flavor that woudl alays overwrite it | |
| 17:04:06 | gmaan | sean-k-mooney: yeah, I agree on that. I am just saying we might be doing the same currently (not in flavor but somewhere in request spec/ResourceRequest )? | |
| 17:04:56 | sean-k-mooney | gmaan: not based on gorkas testing of tryign to use 2 groups to ask for two vgpus | |
| 17:05:23 | sean-k-mooney | but maybe there are some code paths where we will | |
| 17:05:47 | gmaan | humm, and is it set 'isolated' in that case? | |
| 17:05:56 | sean-k-mooney | no its not pass to placement at all | |
| 17:06:03 | gmaan | ohk | |
| 17:06:03 | sean-k-mooney | and then placement raises an error i belive | |
| 17:06:23 | sean-k-mooney | i can ask for some logs or access to there enve where they hit this | |
| 17:06:35 | sean-k-mooney | we likely can create a repoducer | |
| 17:07:46 | gmaan | I think I agree with your proposal. As 'none' means request can be satisfied by different or same providers. if not passed, setting it default to None in flavor make sense. I do not think it change behavior from user perspective. | |
| 17:08:24 | gmaan | we are just adding default here and not actually *changing* the default | |
| 17:08:44 | sean-k-mooney | correct that is what it means | |
| 17:09:16 | sean-k-mooney | gmaan: at least based on what we have documetned here https://docs.openstack.org/nova/latest/user/flavors.html | |
| 17:09:52 | gmaan | yeah, I do not think we need microversion in that case, we can fix it as bug fix | |
| 17:10:47 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/949235 | |
| 17:10:57 | sean-k-mooney | thansk for confirming. as i said above i didnt like the idea of a server create failign with a given flavor based on the microversion used | |
| 17:10:59 | sean-k-mooney | or resize | |
| 17:11:15 | sean-k-mooney | that just seam very confussing for the end user to reason about | |
| 17:12:43 | gmaan | yeah | |
| 17:13:20 | sean-k-mooney | gmaan: there si already a bug for the filter part https://bugs.launchpad.net/nova/+bug/2048874 so ill likely resue that | |
| 17:14:27 | sean-k-mooney | ill proably just take over the orgianl authors patch and try an dalign it to what we have dicussed https://review.opendev.org/c/openstack/nova/+/905210 | |
| 17:14:57 | sean-k-mooney | they were orginall just oging to skip that key which may have worked but its nto really fixign the problem | |
| 17:15:45 | sean-k-mooney | although i might just reuse the bug and not the patch since the logic of the fix will end up beign differnt | |
| 17:17:25 | sean-k-mooney | while i have this open https://review.opendev.org/c/openstack/nova/+/947541 is a nice small fix form melwitt that might be nice ot land if folks agree | |
| 17:19:26 | melwitt | sean-k-mooney: thanks for reviewing that :) I am happy to respin with code comment + release note btw | |
| 17:20:00 | sean-k-mooney | frickler: fungi: do either of ye op access on the channel it might be nice to update the "development-planning: https://etherpad.opendev.org/p/nova-2025.1-status" to "development-planning: https://etherpad.opendev.org/p/nova-2025.2-status" in the topic if Uggla agrees. i think the rest of the topic looks correct | |
| 17:20:25 | melwitt | the above uploaded patch in the backscroll (https://review.opendev.org/c/openstack/nova/+/949235) looks like one of the garbage AI ones | |
| 17:20:27 | sean-k-mooney | melwitt: i got asked about that issue downstream, found the bug and was like oh melwitt has a fix up | |
| 17:20:50 | melwitt | sean-k-mooney: haha fun | |
| 17:21:01 | sean-k-mooney | melwitt: i have been thating to the reporter a bit | |
| 17:21:17 | sean-k-mooney | not the reportor | |
| 17:21:31 | sean-k-mooney | the sumbitter of https://review.opendev.org/c/openstack/nova/+/949235 | |
| 17:21:50 | sean-k-mooney | so they are not really familary with workign in openstack and i think git | |
| 17:22:06 | sean-k-mooney | but ya the commit message is not forllwoing or normal convetion | |
| 17:22:25 | sean-k-mooney | is more like what you do on mailing list where for each revision to expalin what you changed form the last one | |
| 17:22:25 | melwitt | yeah. it's such a tiny thing to fix I thought might as well + backport it. I agree with you that the long term thing will be to stop formatting eph disks | |
| 17:22:34 | melwitt | sean-k-mooney: the commit message claims stuff that isn't in the patch though, like what is that | |
| 17:22:46 | melwitt | huh ok | |
| 17:22:57 | sean-k-mooney | melwitt: so it explainging what has change over time | |
| 17:23:06 | sean-k-mooney | whic is fine in a mailing list btu weired in gerrit | |
| 17:23:24 | sean-k-mooney | also the lost the change id so https://review.opendev.org/c/openstack/nova/+/945204/6 | |
| 17:23:31 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204 | |
| 17:23:31 | sean-k-mooney | is actully the review they shoudl of updated | |
| 17:23:49 | melwitt | I see | |
| 17:24:31 | sean-k-mooney | ishanwar[m]: so you now have two reviews for the same thing because you lost the change id at one point | |
| 17:25:34 | sean-k-mooney | ishanwar[m]: i think you ment to locally squash the two pathces before doing git review | |
| 17:25:47 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204 | |
| 17:27:03 | sean-k-mooney | ishanwar[m]: ok ^ is better but as i was dissusing with melwitt above you shoudl update the commit message to expalin what the current revion of the patch is changing not how the patch changed over time | |
| 17:27:32 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204 | |
| 17:27:55 | sean-k-mooney | ishanwar[m]: https://wiki.openstack.org/wiki/GitCommitMessages document the convetions we try to follow | |
| 17:27:59 | fungi | sean-k-mooney: i can update it, though separately i recommend the nova team consider adding some channel operators at https://opendev.org/openstack/project-config/src/branch/master/accessbot/channels.yaml#L239 | |
| 17:28:18 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204 | |
| 17:28:49 | sean-k-mooney | fungi: oh i didnt know we could do that. i assuem we cant just update the topic via a git reivew? | |
| 17:29:10 | sean-k-mooney | fungi: i htink we used to be able to do this in the chanel before we moved to oftc | |
| 17:29:19 | opendevreview | Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204 | |
| 17:29:26 | sean-k-mooney | but its been so long i kind of forget the detials | |
| 17:30:18 | ishanwar[m] | sean-k-mooney: i have fixes the pep8 issue kindly have a look whenver you can | |
| 17:30:28 | fungi | sean-k-mooney: you'd add people to an ops list for the channel in that file, then any of them can ask chanserv to set the channel topic like the example in https://docs.opendev.org/opendev/system-config/latest/irc.html#basic-channel-operator-commands | |
| 17:31:03 | sean-k-mooney | fungi: ah cool ill add that to the irc metting adgenda for next week so | |
| 17:31:19 | sean-k-mooney | and we can see what folks think | |
| 17:31:26 | ishanwar[m] | sean-k-mooney: yep changing that | |