| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-18 | |||
| 16:37:28 | sean-k-mooney | so they dont nessiarly link to the correct code any more | |
| 16:37:28 | dansmith | ishanwar[m]: that bug was filed by a neutron person so I suspect we won't be able to answer your question authoritatively without really digging in | |
| 16:38:08 | sean-k-mooney | ishanwar[m]: you are correct that validate_networks does use the quota api already https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L2714 | |
| 16:38:50 | sean-k-mooney | i guess the real quetion is do we use that in all places we do quota checks | |
| 16:39:05 | sean-k-mooney | this was orginaly reported downstream for trian | |
| 16:39:13 | sean-k-mooney | so its very possibel that htis was fixed already in master | |
| 16:39:25 | ishanwar[m] | Yes correct, but it's still listing all ports to count them, rather than using show_quota_details, right ? | |
| 16:40:07 | sean-k-mooney | we do a port list in get_requested_resource_for_instance | |
| 16:40:43 | sean-k-mooney | but that not listing all ports for the porject | |
| 16:40:49 | ishanwar[m] | sean-k-mooney: not sure if this is fixed elsewhere, I was only able to find one usage of len(ports) | |
| 16:40:52 | sean-k-mooney | that is geting port for the current instnace wihch is fine | |
| 16:41:03 | ishanwar[m] | sean-k-mooney: yes | |
| 16:41:16 | dansmith | we fetch the quota, but we do our own usage calculation it looks like | |
| 16:41:31 | sean-k-mooney | oh its this second call https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L2721 | |
| 16:41:42 | dansmith | we don't ask if the quota is exceeded, we ask what the quota is and then sum the current+new and see if it's over | |
| 16:41:45 | sean-k-mooney | that ^ is the line that shoudl be updated | |
| 16:41:49 | ishanwar[m] | dansmith: yes in https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L2714 | |
| 16:41:49 | ishanwar[m] | we are still counting all the ports | |
| 16:42:06 | sean-k-mooney | ishanwar[m]: so the bug is this | |
| 16:42:23 | sean-k-mooney | there is another extention that gives use both the quota and the uses cont in one call | |
| 16:42:34 | ishanwar[m] | yes correct, i think we can use show_quota_details here right ? | |
| 16:42:42 | ishanwar[m] | sean-k-mooney: which one ? | |
| 16:42:48 | sean-k-mooney | so instead of getting all the ports for the curent project just to count them we can use show_quota_details | |
| 16:43:02 | sean-k-mooney | ishanwar[m]: so the performace bug is still there | |
| 16:43:04 | dansmith | ...if available | |
| 16:43:07 | sean-k-mooney | on this line https://github.com/openstack/nova/blob/6042300453411133652dd8d3b789f5057a084075/nova/network/neutron.py#L2721 | |
| 16:43:24 | sean-k-mooney | but ya as dan impleid we have to fall back if the extension is not there | |
| 16:43:45 | dansmith | it's also racy either way, which is unfortunate | |
| 16:43:55 | ishanwar[m] | so we can use a try catch mech to check if the extension is available or not | |
| 16:43:55 | ishanwar[m] | sean-k-mooney: correct I agree | |
| 16:44:06 | ishanwar[m] | dansmith: oh, how is that ? | |
| 16:44:27 | dansmith | ishanwar[m]: if we calculate the quota, allow to proceed and other things in parallel use more quota, we'll be wrong and run out of quota while actually creating the ports | |
| 16:45:06 | dansmith | since we already need to handle that, why check quota at all? Just try to create the ports and roll back if you run out.. I guess maybe to provide a quick answer via API, but it will never be guaranteed | |
| 16:45:25 | sean-k-mooney | well we do prot creation on the comptues | |
| 16:45:36 | sean-k-mooney | so we do the api check to fail fast if we are already at quota | |
| 16:45:40 | dansmith | this can say "definitely not enough quota" but it can't say "definitely enough" | |
| 16:45:50 | dansmith | right | |
| 16:46:30 | ishanwar[m] | Not sure I understood 😓 | |
| 16:46:45 | dansmith | there's a long window between checking quota and consuming it | |
| 16:46:45 | sean-k-mooney | ishanwar[m]: so to answer your question, the bug is still there adn we can still optimize this call to neturon | |
| 16:46:59 | sean-k-mooney | but we need to fallback if the extion is not there to the current logic. | |
| 16:47:58 | ishanwar[m] | I have another question, i am running the openstack nova dev env on devstack. is there a way how I can test my changes for this bug, sorry I am new to this ? | |
| 16:48:53 | dansmith | with devstack yes, make your changes and test manually, but this will also need unit/functional tests in the patch | |
| 16:49:37 | sean-k-mooney | ishanwar[m]: to do ^ you just modify the code and do "systemctl restart devstack@n-*" | |
| 16:49:50 | sean-k-mooney | i.e. modify the code in /opt/stack/nova | |
| 16:50:53 | Uggla | ishanwar[m], also finding something similar to the bug in gerrit can be a good example to know what the patch should have in terms of unit/fn tests. | |
| 16:50:53 | sean-k-mooney | i generally do it slightly idffently by have a copy in another location and pip install that into the devstack virutal env but you can basiclly just modify the code and restart the affected services. in thsi case devtack@n-api is the nova api | |
| 16:51:33 | ishanwar[m] | this restarts all nova services right ? Shouldn't we check just one, in general what process do you follow ? Do you add breakpoints etc for testing things out ? | |
| 16:51:40 | sean-k-mooney | you can look at the function above https://github.com/openstack/nova/blob/6042300453411133652dd8d3b789f5057a084075/nova/network/neutron.py#L2672-L2693 | |
| 16:51:48 | sean-k-mooney | it does an if/else on if the extenion is supproted | |
| 16:52:07 | sean-k-mooney | you can look at howe has_extended_resource_request_extension is implemnted to test for the quta details one | |
| 16:52:10 | ishanwar[m] | sean-k-mooney: yes understood, makes sense . | |
| 16:52:41 | sean-k-mooney | ishanwar[m]: so you cant really run nova under a debugger | |
| 16:52:46 | sean-k-mooney | its posssible but hard | |
| 16:52:52 | sean-k-mooney | so generally we add debug logs | |
| 16:52:53 | ishanwar[m] | sean-k-mooney: yeah I can implement ifelse for this | |
| 16:53:13 | sean-k-mooney | that useful for production as well as in production we will generally only have logs aviable | |
| 16:53:43 | ishanwar[m] | sean-k-mooney: okay I see. | |
| 16:54:44 | ishanwar[m] | sean-k-mooney: hmm i see. So from what I understood, add logs, for testing, restart the n-api service. Use openstack command line for testing, correct ? | |
| 16:55:06 | sean-k-mooney | initally but then you will need to add unit test for each branch | |
| 16:55:20 | sean-k-mooney | so mocking the has_extenion_* function for true and false | |
| 16:55:37 | ishanwar[m] | Can you share an example ? | |
| 16:55:40 | sean-k-mooney | and assert list_port or quota_detials is called approreatly | |
| 16:56:00 | sean-k-mooney | yep we can proably do that perhaps after the meeting | |
| 16:56:05 | sean-k-mooney | incase there are any other topics | |
| 16:56:35 | ishanwar[m] | sure, thanks sean-k-mooney dansmith Uggla | |
| 16:56:35 | ishanwar[m] | i think we can close this discussion | |
| 16:56:40 | ishanwar[m] | back to you Uggla | |
| 16:56:49 | Uggla | thx @ishanwar[m] | |
| 16:57:15 | Uggla | any other topic ? | |
| 16:58:25 | Uggla | looks we can close the meeting | |
| 16:58:34 | Uggla | 3 | |
| 16:58:51 | bauzas | shoot :) | |
| 16:58:53 | Uggla | 2 | |
| 16:58:55 | dansmith | blastoff | |
| 16:59:05 | Uggla | thanks all | |
| 16:59:10 | opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2025/nova.2025-03-18-16.01.log.html | |
| 16:59:10 | opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2025/nova.2025-03-18-16.01.txt | |
| 16:59:10 | opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2025/nova.2025-03-18-16.01.html | |
| 16:59:10 | opendevmeet | Meeting ended Tue Mar 18 16:59:10 2025 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | |
| 16:59:10 | Uggla | #endmeeting | |
| 16:59:17 | bauzas | heh, that worked well :) | |
| 16:59:18 | gibi | bauzas, Uggla thanks | |
| 16:59:20 | bauzas | thanks Uggla | |
| 16:59:27 | elodilles | thanks Uggla bauzas o/ | |
| 16:59:49 | masahito | thank you, Uggla and bauzas | |
| 16:59:53 | sean-k-mooney | ishanwar[m]: so if you look at https://github.com/openstack/nova/commit/191bdf2069086493be2a2e0351afa7f3efad7099#diff-de8fba9a58a07803b3d46cfb1f58d1b89e7cc63684f02c30297e6bdabffe870eR4373 you will see some exampel fo what the unit test should look like | |
| 16:59:58 | Uggla | bauzas, thanks | |
| 17:01:03 | ishanwar[m] | Hmm I can see mock objects sean-k-mooney , I'll have a detailed look | |
| 17:01:07 | ishanwar[m] | thanks | |
| 17:01:53 | sean-k-mooney | yep we mock the calls to neutron and assert they are called or not called depending on what should happen | |
| 17:02:18 | sean-k-mooney | ishanwar[m]: you should be able to find an exeitin tst for the validate_networks funciton | |
| 17:02:58 | sean-k-mooney | make a copy and test that it calls or does not call list_prots by mocking teh function that will check for the extension | |
| 17:07:02 | ishanwar[m] | aah i see, right, I think can use that for checking whether the function is available. | |
| 18:02:17 | cardoe | Will I be able to sneak https://review.opendev.org/c/openstack/nova/+/942019 into 2025.1 or will that have to be a backport? | |
| 18:03:28 | sean-k-mooney | cardoe: unless its a regression introduced in 2025.1 it will be a a post release backport | |
| 18:03:44 | cardoe | it's a regression introduced in 2024.1 | |
| 18:03:46 | sean-k-mooney | cardoe: ah the loggign fix | |
| 18:03:55 | sean-k-mooney | ya so that wont qualify for rc2 | |
| 18:04:08 | sean-k-mooney | and we just? curt the stable branch form rc1 | |