| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-05-07 | |||
| 19:58:24 | opendevreview | Balazs Gibizer proposed openstack/nova master: Rename DEFAULT_GREEN_POOL to DEFAULT_EXECUTOR https://review.opendev.org/c/openstack/nova/+/948086 | |
| 19:58:24 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make the default executor configurable https://review.opendev.org/c/openstack/nova/+/948087 | |
| 19:58:25 | opendevreview | Balazs Gibizer proposed openstack/nova master: Print ThreadPool statistics https://review.opendev.org/c/openstack/nova/+/948340 | |
| 19:58:25 | opendevreview | Balazs Gibizer proposed openstack/nova master: WIP: allow service to start with threading https://review.opendev.org/c/openstack/nova/+/948311 | |
| 19:58:26 | opendevreview | Balazs Gibizer proposed openstack/nova master: DNM:Run nova-next with n-sch in threading mode https://review.opendev.org/c/openstack/nova/+/948450 | |
| #openstack-nova - 2025-05-08 | |||
| 02:28:56 | opendevreview | melanie witt proposed openstack/nova-specs master: Add reminder about Tempest API response schemas https://review.opendev.org/c/openstack/nova-specs/+/949113 | |
| 02:30:24 | melwitt | Uggla, gmaan: this is a spec template update to add a note about API response schemas in Tempest ^ | |
| 02:30:43 | melwitt | (fyi) | |
| 10:02:04 | 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 https://review.opendev.org/c/openstack/nova/+/945204 | |
| 10:04:01 | ishanwar[m] | opendevreview: sean-k-mooney: made a few changes as your requests UTs seems to be working fine now kindly have a look | |
| 11:20:13 | sean-k-mooney | gmaan: question for you https://docs.openstack.org/nova/latest/user/flavors.html | |
| 11:20:39 | sean-k-mooney | we currently default group_policy to non if request groups come form neutron ports | |
| 11:21:06 | sean-k-mooney | but we require group_policy to be passed if you pass multiple groups in the flavor | |
| 11:21:36 | sean-k-mooney | it almost never makes sesne to use isolate because it will break port based request if you ever want more then one port form the same nic | |
| 11:21:43 | sean-k-mooney | i.e. 2 ovs prot or 2 sriov ports | |
| 11:21:52 | sean-k-mooney | so we never recommend using that in general | |
| 11:22:14 | sean-k-mooney | so my question is if we jsut want to extend the defaulting to None to the falvor | |
| 11:22:50 | sean-k-mooney | can tha tbe done without a microverison. i think the answer is know althoguh im hopeing i can argure that we coudl because we are relaxing the constraint | |
| 11:29:00 | sean-k-mooney | its a trivial spec to write if we decied we need a microversion for tha that but honestly i thinik havign server create fail if a user uses the old microvtion with a flavor and pass with the new versiuon and exactly the same flavor is terrible ux | |
| 11:29:46 | sean-k-mooney | we have previouysly said that new extra specs dont need a microvetion and that changing the default shoudl not either | |
| 11:30:16 | sean-k-mooney | so i think we can do this withou one simiarl to the fix for neutron prots https://github.com/openstack/nova/commit/ad4f79836264ec79068f087008367e2d2ae8cbea | |
| 11:31:12 | sean-k-mooney | gibi: ^ you wrote that for the neutron port qos policy i belive do you see any reaons not to extend the defauling to none ot flavor based groups too. | |
| 11:34:13 | ishanwar[m] | <ishanwar[m]> "sean-k-mooney: made a few..." <- Build has passed as well. I think it looks good now | |
| 11:37:08 | sean-k-mooney | ishanwar[m]: i ment to push my comment a while ago but its going to fail our code style checks | |
| 11:37:53 | sean-k-mooney | ishanwar[m]: i got pinged for soemthign else so i didnt finish the review but i pushed my comment about the whitespace sissue | |
| 11:37:58 | ishanwar[m] | sean-k-mooney: pep checks seem to have passed though | |
| 11:37:58 | ishanwar[m] | sorry new to this | |
| 11:38:01 | sean-k-mooney | i think i agree that this is good to go | |
| 11:38:31 | sean-k-mooney | you shoudl stil fix https://review.opendev.org/c/openstack/nova/+/945204/6/nova/tests/unit/network/test_neutron.py#4101 | |
| 11:38:49 | sean-k-mooney | but i think the logic is correct | |
| 11:39:31 | ishanwar[m] | sean-k-mooney: yeah I have double checked | |
| 11:39:31 | ishanwar[m] | let me fix the style | |
| 11:39:31 | ishanwar[m] | i'll ping back | |
| 11:40:53 | opendevreview | Stephen Finucane proposed openstack/nova master: wsgi: Don't create, use lock in same line https://review.opendev.org/c/openstack/nova/+/949161 | |
| 11:45:00 | sean-k-mooney | ishanwar[m]: so running ti locally it fails both the trim trailign spaces check adn the lighleght check when i run tox -e pep8 | |
| 11:45:33 | sean-k-mooney | ishanwar[m]: but it also fixes if for you | |
| 11:46:14 | sean-k-mooney | well it fixes the white space you will need to manually fix the linelenght issue | |
| 11:53:29 | gibi | sean-k-mooney: I think if the groups are coming from the flavor then we can simply ask the flavor creator to add policy according to the needs. I.e. if they want two vgpus from two different pgpus and they won't use qos ports then it is a case when isolate policy is needed. For the neutorn port case we need a defaulting as the person creating the port might not be the same as the one creating the | |
| 11:53:35 | gibi | flavor and we dont support adding policy to the port | |
| 11:57:23 | sean-k-mooney | gibi: yes im aware | |
| 11:57:30 | sean-k-mooney | we currently require you to set it | |
| 11:57:43 | sean-k-mooney | but i think it woudl be a better ux if we jsut defautl to none | |
| 11:58:01 | sean-k-mooney | partly because i honestly think we shoudl delete the suprpot for group_polcy in the flavor in its currnt form | |
| 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 | sean-k-mooney | and then placement raises an error i belive | |
| 17:06:03 | gmaan | ohk | |
| 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 | |