| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-11-19 | |||
| 20:35:52 | gmann | melwitt dansmith gibi can you check this easy one https://review.opendev.org/c/openstack/nova/+/934578 | |
| 20:37:51 | melwitt | gmann: +2 | |
| 20:38:12 | gmann | thanks | |
| #openstack-nova - 2024-11-20 | |||
| 01:37:48 | opendevreview | Takashi Kajinami proposed openstack/nova master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/nova/+/935733 | |
| 01:40:14 | opendevreview | Takashi Kajinami proposed openstack/placement master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/placement/+/935734 | |
| 02:45:44 | opendevreview | Takashi Kajinami proposed openstack/nova master: Detect AMD SEV-ES support https://review.opendev.org/c/openstack/nova/+/925685 | |
| 02:45:44 | opendevreview | Takashi Kajinami proposed openstack/nova master: Migrate MEM_ENCRYPTION_CONTEXT from root provider https://review.opendev.org/c/openstack/nova/+/921814 | |
| 02:45:45 | opendevreview | Takashi Kajinami proposed openstack/nova master: Add hw_mem_encryption_model image property https://review.opendev.org/c/openstack/nova/+/927706 | |
| 02:47:11 | opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Launch instances with SEV-ES memory encryption https://review.opendev.org/c/openstack/nova/+/926106 | |
| 03:17:27 | opendevreview | Takashi Kajinami proposed openstack/placement master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/placement/+/935734 | |
| 03:29:39 | opendevreview | melanie witt proposed openstack/nova master: Add [quota]unified_limits_resource_(strategy|list) https://review.opendev.org/c/openstack/nova/+/924025 | |
| 03:31:30 | melwitt | dansmith, sean-k-mooney: I also updated the config options patch to reflect the latest PS of the spec ^ | |
| 04:10:31 | opendevreview | Takashi Kajinami proposed openstack/nova master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/nova/+/935733 | |
| 04:33:15 | opendevreview | melanie witt proposed openstack/nova master: Add [quota]unified_limits_resource_(strategy|list) https://review.opendev.org/c/openstack/nova/+/924025 | |
| 05:46:14 | opendevreview | Rajesh Tailor proposed openstack/nova master: Add support for showing finish_time https://review.opendev.org/c/openstack/nova/+/928933 | |
| 06:01:59 | opendevreview | Merged openstack/nova master: Remove workaround for eventlet < 0.27.0 https://review.opendev.org/c/openstack/nova/+/930726 | |
| 09:54:52 | frickler | bauzas: (or other cores) seems we still do need sean-k-mooney's fix for os-vif https://review.opendev.org/c/openstack/os-vif/+/935653 in order to be able to get https://review.opendev.org/c/openstack/os-vif/+/932436 in in order to unblock oslo.utils in reqs, please have a look | |
| 10:08:58 | opendevreview | Takashi Kajinami proposed openstack/os-vif master: DNM: Testing 932436 https://review.opendev.org/c/openstack/os-vif/+/935755 | |
| 10:24:07 | opendevreview | Merged openstack/nova-specs master: Re-propose config option for behavior of unset unified limits https://review.opendev.org/c/openstack/nova-specs/+/934391 | |
| 12:12:42 | gibi | elodilles: Hi! Can I get review on the next set of patches in the backort series of https://review.opendev.org/q/topic:%22bug/2085975%22 | |
| 12:15:03 | gibi | frickler: I fast approved the os-vif change to get things unblocked | |
| 12:23:26 | elodilles | gibi: looking | |
| 12:29:36 | elodilles | +2'd them | |
| 12:50:57 | gibi | elodilles: thanks a lot | |
| 12:55:11 | elodilles | np | |
| 14:20:05 | tkajinam | sean-k-mooney, hi o/ Could you check my reply in https://review.opendev.org/c/openstack/nova/+/931582 when you have time ? | |
| 14:21:55 | sean-k-mooney | do you plan to add that funcitonaltiy in oslo in the future? | |
| 14:22:27 | sean-k-mooney | im ok with having it in nova in the interim but i think it woudl be better to have it in oslo eventurally | |
| 14:22:52 | tkajinam | yeah I can move it to oslo.utils | |
| 14:23:11 | sean-k-mooney | ok i wont block on that then we can just do that when it makes sense to do so | |
| 14:23:46 | tkajinam | I also replaced netifaces used in oslo.utils by psutil so the proposed implementation is consistent with the ones we maintain in oslo.utils | |
| 14:24:11 | tkajinam | so there is not strict blocker for future migration | |
| 14:24:15 | sean-k-mooney | im not sure if nova is already usign psutils but i was hoping to aovid a direct dep if we were not | |
| 14:24:30 | tkajinam | it does. | |
| 14:24:51 | tkajinam | https://github.com/openstack/nova/blob/master/requirements.txt#L51 | |
| 14:25:00 | sean-k-mooney | https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/virt/libvirt/volume/quobyte.py#L22 | |
| 14:25:04 | sean-k-mooney | for that | |
| 14:25:15 | sean-k-mooney | https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/virt/libvirt/volume/quobyte.py#L55-L62 | |
| 14:25:26 | tkajinam | it's for quobyte, yeah | |
| 14:25:48 | sean-k-mooney | that code looks suspect as we really shoudl not be trying to look at systemd and if its running | |
| 14:25:55 | sean-k-mooney | but ok | |
| 14:26:03 | sean-k-mooney | since its already a dep it does not impact packaging | |
| 14:26:16 | tkajinam | we deprecated quobyte support so that code may go away next cycle | |
| 14:26:27 | sean-k-mooney | well it moving form optional to requried but its also requried because fo oslo so i guess its fine | |
| 14:26:41 | tkajinam | yes | |
| 14:27:39 | sean-k-mooney | ok so i have only one other comment, there is no new testcoverage for get_machine_ips | |
| 14:28:01 | sean-k-mooney | you removed https://review.opendev.org/c/openstack/nova/+/931582/6/nova/tests/unit/compute/test_utils.py | |
| 14:28:29 | sean-k-mooney | the error case but didnt provide any repalcement coverage. and with the poison that impoiles this is not called in other tests or is mocked | |
| 14:28:51 | sean-k-mooney | so i dont think we have any coverage of that code. | |
| 14:30:00 | sean-k-mooney | can you add a small testcase that mocks psutil.net_if_addrs() and jsut cover the body of the for | |
| 14:30:32 | sean-k-mooney | i.e. i.e. the removal of the ipv6 adresses? im not sure how hard that would be | |
| 14:31:04 | sean-k-mooney | sory its not remivng the ipv6 adresses | |
| 14:31:21 | sean-k-mooney | btu it is stripign the content after % | |
| 14:32:14 | tkajinam | that removes scope from ipv6 address | |
| 14:32:17 | sean-k-mooney | i guess we didnt have coverate for that before... | |
| 14:32:24 | tkajinam | let me update the change to include unit tests. That's a very good point | |
| 14:32:26 | sean-k-mooney | so its technially not a regression | |
| 14:32:51 | sean-k-mooney | im kind of +1.5 | |
| 14:33:18 | sean-k-mooney | i dont have a sense of how worried i shoudl be about a regression in that code | |
| 14:33:49 | sean-k-mooney | you didnt actully chagne andy of the logic that is processing the adresses | |
| 14:34:24 | sean-k-mooney | you just removed the try excpet | |
| 14:35:03 | sean-k-mooney | presumably because `psutil.net_if_addrs().items()` alwasy returns a dict even if its an empty one | |
| 14:35:45 | sean-k-mooney | or well an iterator over a mapping/dict view | |
| 14:36:42 | tkajinam | yeah | |
| 14:36:43 | sean-k-mooney | tkajinam: im kind of feelign that the missing unit test is a prexisiting bug and i should not ask you to fix it while your are fixing the netifaces issue | |
| 14:37:45 | tkajinam | sean-k-mooney, that's also ok. I can add some test coverage in an independent follow-up | |
| 14:38:29 | tkajinam | yeah it reminds me that I only removed the test case because the exception may no be raised in a new implementation and there are no actual test coverage for normal cases | |
| 14:40:00 | sean-k-mooney | i left a coment to that effect and a +2 | |
| 14:40:38 | sean-k-mooney | if you have time to add coverage in a followup then that great if not its ok | |
| 14:41:10 | sean-k-mooney | ill leave it to others to disagree on the reivew if they feel differently | |
| 14:42:31 | tkajinam | sean-k-mooney, thank you ! | |
| 14:44:15 | sean-k-mooney | frickler: looks liek even with the exteneed timeout it can still sometiems fail | |
| 14:45:00 | sean-k-mooney | i think for now it would be best to skip that test when using the native driver | |
| 14:45:32 | sean-k-mooney | it always passes for the ovs-vsctl one and we can revisti this once the current set of release are done | |
| 14:45:45 | sean-k-mooney | *requirements bumps | |
| 14:49:01 | sean-k-mooney | unfortunetly we do not capture the ovs logs in this job but i suspect there might be an error of some kidn that we can see | |
| 14:49:35 | sean-k-mooney | s/can see/can't see/ | |
| 15:10:05 | ildikov | bauzas: sean-k-mooney: Hi, I'm pinging you about the Bridging the Gap effort that we last discussed at the recent PTG. | |
| 15:10:08 | ildikov | We talked about some simple improvements for Nova, like documenting your process to set priorities for the work that's happening during release cycles. | |
| 15:11:48 | ildikov | I was looking into the Nova docs a little bit, and was doing a Google search on this topic as well to see what a newcomer might find when looking for information about how to work with the Nova team. | |
| 15:12:07 | ildikov | I found a page in the Nova docs that talks about specs, blueprints, etc: https://docs.openstack.org/nova/latest/contributor/blueprints.html | |
| 15:12:42 | ildikov | It looks like a good place to add some additional information about processes that the team is using throughout release cycles to organize your work. | |
| 15:12:46 | ildikov | What do you think? | |
| 15:13:56 | sean-k-mooney | that is one place but not the only one we have | |
| 15:14:19 | sean-k-mooney | this is the starting point https://docs.openstack.org/nova/latest/contributor/contributing.html | |
| 15:14:38 | sean-k-mooney | the doc you linked to is fpr mew featire [;ammomg | |
| 15:15:11 | sean-k-mooney | here you will see the more detailed docs https://docs.openstack.org/nova/latest/contributor/index.html | |
| 15:16:41 | sean-k-mooney | ildikov: we maintian an entire subseciton of our docs "the contibutor guide/docs" | |
| 15:16:52 | sean-k-mooney | so we can add more contet to this as required | |
| 15:17:12 | sean-k-mooney | a high level overview of our process is here https://docs.openstack.org/nova/latest/contributor/process.html | |
| 15:17:51 | sean-k-mooney | ildikov: https://docs.openstack.org/nova/latest/contributor/process.html#why-do-we-have-priorities is where we discuss priorities but we have nto enforced them for some time | |
| 15:18:21 | sean-k-mooney | this was the section we added most recently https://docs.openstack.org/nova/latest/contributor/process.html#what-the-review-priority-label-in-gerrit-are-use-for | |
| 15:19:03 | sean-k-mooney | to document how to sue review priority label but bauzas proposed a diffent way to do it via etherpads last cycle which has more or less replaced it | |
| 15:19:28 | sean-k-mooney | we also have old documetion of the runwasy system we used before that https://docs.openstack.org/nova/latest/contributor/process.html#runways | |
| 15:19:59 | sean-k-mooney | ildikov: so i can see why it might be confusitn since we still have to docs for the previous ways of working | |
| 15:20:34 | sean-k-mooney | ildikov: this is somthing that is continuiously evloving as we try thigns for a release or two and then try somethign new | |
| 15:21:29 | sean-k-mooney | so nova has a lot of docs on this topic already but most have not been updated for a few releases | |
| 15:22:21 | sean-k-mooney | ildikov: that partly why i tough the fedback toward nova was rather unfare in the ptg discussions | |
| 15:23:25 | sean-k-mooney | ildikov: it seamed to ignroe all of the existing doc we had and the efforts we had made to make it transparent | |
| 16:06:56 | dansmith | melwitt: okay I see, it's being echoed once because of the manage_output=$() and once for the [[ $manage_output =~ SUCCESS ]] | |