| opendevreview | Takashi Kajinami proposed openstack/nova master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/nova/+/935733 | 01:37 |
|---|---|---|
| opendevreview | Takashi Kajinami proposed openstack/placement master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/placement/+/935734 | 01:40 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Migrate MEM_ENCRYPTION_CONTEXT from root provider https://review.opendev.org/c/openstack/nova/+/921814 | 02:45 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Detect AMD SEV-ES support https://review.opendev.org/c/openstack/nova/+/925685 | 02:45 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Add hw_mem_encryption_model image property https://review.opendev.org/c/openstack/nova/+/927706 | 02:45 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Launch instances with SEV-ES memory encryption https://review.opendev.org/c/openstack/nova/+/926106 | 02:47 |
| opendevreview | Takashi Kajinami proposed openstack/placement master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/placement/+/935734 | 03:17 |
| opendevreview | melanie witt proposed openstack/nova master: Add [quota]unified_limits_resource_(strategy|list) https://review.opendev.org/c/openstack/nova/+/924025 | 03:29 |
| melwitt | dansmith, sean-k-mooney: I also updated the config options patch to reflect the latest PS of the spec ^ | 03:31 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/nova/+/935733 | 04:10 |
| *** __ministry is now known as Guest243 | 04:26 | |
| opendevreview | melanie witt proposed openstack/nova master: Add [quota]unified_limits_resource_(strategy|list) https://review.opendev.org/c/openstack/nova/+/924025 | 04:33 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Add support for showing finish_time https://review.opendev.org/c/openstack/nova/+/928933 | 05:46 |
| opendevreview | Merged openstack/nova master: Remove workaround for eventlet < 0.27.0 https://review.opendev.org/c/openstack/nova/+/930726 | 06:01 |
| *** __ministry is now known as Guest253 | 06:54 | |
| 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 | 09:54 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: DNM: Testing 932436 https://review.opendev.org/c/openstack/os-vif/+/935755 | 10:08 |
| 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 | 10:24 |
| 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:12 |
| gibi | frickler: I fast approved the os-vif change to get things unblocked | 12:15 |
| elodilles | gibi: looking | 12:23 |
| elodilles | +2'd them | 12:29 |
| gibi | elodilles: thanks a lot | 12:50 |
| elodilles | np | 12:55 |
| 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:20 |
| sean-k-mooney | do you plan to add that funcitonaltiy in oslo in the future? | 14:21 |
| 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 |
| tkajinam | yeah I can move it to oslo.utils | 14:22 |
| sean-k-mooney | ok i wont block on that then we can just do that when it makes sense to do so | 14:23 |
| 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:23 |
| tkajinam | so there is not strict blocker for future migration | 14:24 |
| 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 |
| tkajinam | it does. | 14:24 |
| tkajinam | https://github.com/openstack/nova/blob/master/requirements.txt#L51 | 14:24 |
| sean-k-mooney | https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/virt/libvirt/volume/quobyte.py#L22 | 14:25 |
| sean-k-mooney | for that | 14:25 |
| sean-k-mooney | https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/virt/libvirt/volume/quobyte.py#L55-L62 | 14:25 |
| tkajinam | it's for quobyte, yeah | 14:25 |
| sean-k-mooney | that code looks suspect as we really shoudl not be trying to look at systemd and if its running | 14:25 |
| sean-k-mooney | but ok | 14:25 |
| sean-k-mooney | since its already a dep it does not impact packaging | 14:26 |
| tkajinam | we deprecated quobyte support so that code may go away next cycle | 14:26 |
| sean-k-mooney | well it moving form optional to requried but its also requried because fo oslo so i guess its fine | 14:26 |
| tkajinam | yes | 14:26 |
| sean-k-mooney | ok so i have only one other comment, there is no new testcoverage for get_machine_ips | 14:27 |
| sean-k-mooney | you removed https://review.opendev.org/c/openstack/nova/+/931582/6/nova/tests/unit/compute/test_utils.py | 14:28 |
| 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 |
| sean-k-mooney | so i dont think we have any coverage of that code. | 14:28 |
| 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 |
| sean-k-mooney | i.e. i.e. the removal of the ipv6 adresses? im not sure how hard that would be | 14:30 |
| sean-k-mooney | sory its not remivng the ipv6 adresses | 14:31 |
| sean-k-mooney | btu it is stripign the content after % | 14:31 |
| tkajinam | that removes scope from ipv6 address | 14:32 |
| sean-k-mooney | i guess we didnt have coverate for that before... | 14:32 |
| tkajinam | let me update the change to include unit tests. That's a very good point | 14:32 |
| sean-k-mooney | so its technially not a regression | 14:32 |
| sean-k-mooney | im kind of +1.5 | 14:32 |
| sean-k-mooney | i dont have a sense of how worried i shoudl be about a regression in that code | 14:33 |
| sean-k-mooney | you didnt actully chagne andy of the logic that is processing the adresses | 14:33 |
| sean-k-mooney | you just removed the try excpet | 14:34 |
| sean-k-mooney | presumably because `psutil.net_if_addrs().items()` alwasy returns a dict even if its an empty one | 14:35 |
| sean-k-mooney | or well an iterator over a mapping/dict view | 14:35 |
| tkajinam | yeah | 14:36 |
| 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:36 |
| tkajinam | sean-k-mooney, that's also ok. I can add some test coverage in an independent follow-up | 14:37 |
| 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:38 |
| sean-k-mooney | i left a coment to that effect and a +2 | 14:40 |
| sean-k-mooney | if you have time to add coverage in a followup then that great if not its ok | 14:40 |
| sean-k-mooney | ill leave it to others to disagree on the reivew if they feel differently | 14:41 |
| tkajinam | sean-k-mooney, thank you ! | 14:42 |
| sean-k-mooney | frickler: looks liek even with the exteneed timeout it can still sometiems fail | 14:44 |
| sean-k-mooney | i think for now it would be best to skip that test when using the native driver | 14:45 |
| 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 |
| sean-k-mooney | *requirements bumps | 14:45 |
| 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 |
| sean-k-mooney | s/can see/can't see/ | 14:49 |
| 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 |
| 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:10 |
| 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:11 |
| 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 |
| 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 |
| ildikov | What do you think? | 15:12 |
| sean-k-mooney | that is one place but not the only one we have | 15:13 |
| sean-k-mooney | this is the starting point https://docs.openstack.org/nova/latest/contributor/contributing.html | 15:14 |
| sean-k-mooney | the doc you linked to is fpr mew featire [;ammomg | 15:14 |
| sean-k-mooney | here you will see the more detailed docs https://docs.openstack.org/nova/latest/contributor/index.html | 15:15 |
| sean-k-mooney | ildikov: we maintian an entire subseciton of our docs "the contibutor guide/docs" | 15:16 |
| sean-k-mooney | so we can add more contet to this as required | 15:16 |
| sean-k-mooney | a high level overview of our process is here https://docs.openstack.org/nova/latest/contributor/process.html | 15:17 |
| 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:17 |
| 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:18 |
| 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 |
| 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 |
| 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:19 |
| 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:20 |
| 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:21 |
| sean-k-mooney | ildikov: that partly why i tough the fedback toward nova was rather unfare in the ptg discussions | 15:22 |
| 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 | 15:23 |
| dansmith | melwitt: okay I see, it's being echoed once because of the manage_output=$() and once for the [[ $manage_output =~ SUCCESS ]] | 16:06 |
| dansmith | melwitt: so it might be better to do this: | 16:08 |
| dansmith | nova_manage blah | tee /tmp/output | 16:08 |
| dansmith | if grep SUCCESS /tmp/output; | 16:08 |
| dansmith | so we get one copy | 16:09 |
| melwitt | the job output is so hard to parse for me :( ok, I'll try that | 16:10 |
| dansmith | also I just realized you're not checking the return code and just looking for the SUCCESS, but a script will be using the former, so we should probably as well yeah? | 16:10 |
| dansmith | yeah I want the job output to be easier to read for sure and not stuffing large multi-line outputs into vars will help I think | 16:10 |
| melwitt | yeah that would work, originally I didn't have it returning non zero for the warning case and I didn't think to change it | 16:12 |
| melwitt | I'll do that also | 16:12 |
| ildikov | sean-k-mooney: thank you for sharing all the links, and some details on how the documentation has been evolving | 18:52 |
| ildikov | sean-k-mooney: I'm sorry the feedback towards the documentation felt unfair. It seems like there might be a challenge for newcomers and casual contributors to find their ways around the currently existing docs, where the disconnect might be. | 18:57 |
| ildikov | sean-k-mooney: You mentioned that some of the docs haven't been updated in a while now. Which parts of the docs should be updated or removed, if any, to accurately reflect how the Nova team works today? | 18:59 |
| opendevreview | melanie witt proposed openstack/nova master: nova-manage: Add flavor scanning to migrate_to_unified_limits https://review.opendev.org/c/openstack/nova/+/924110 | 19:13 |
| melwitt | sean-k-mooney: you are fast :) | 19:23 |
| sean-k-mooney | i happend ot be doing my "finsih the day be going through my emails" routine when the notificaoin came in | 19:25 |
| melwitt | woohoo | 19:26 |
| sean-k-mooney | ill try and review the second patch again tomorrow | 19:28 |
| melwitt | cool, thank you | 19:29 |
| sean-k-mooney | i have an older veriosn checked out in one... of my devstack vms | 19:30 |
| sean-k-mooney | i might see if i can try enabeling it and test the basics | 19:30 |
| melwitt | the more testing the better :) | 19:30 |
| melwitt | I realized it wouldn't be the right thing to validate resource classes (for quotas) in init_host() bc that's a nova-compute thing and the quota checking happens in nova-api and nova-conductor. so I'm looking for a similar spot I can add the validation for startup | 19:33 |
| melwitt | oh derp, init_host() can be added for any service I see now | 19:39 |
| melwitt | nova-api needs to be different though bc it won't go through nova.service I don't think | 19:41 |
| melwitt | ok the wsgi app does use a nova Service but calls only Service.create() | 19:44 |
| sean-k-mooney | it does not enven need to be init_host | 19:45 |
| sean-k-mooney | we just need a early point to hook after the config is parsrced | 19:45 |
| sean-k-mooney | so the post start hook woudl work too | 19:45 |
| sean-k-mooney | actully pre_start_hook would work too | 19:47 |
| sean-k-mooney | but ya init_host is my go too | 19:47 |
| sean-k-mooney | melwitt: if your feellign laze it technially could go in __init__ https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/manager.py#L92 but ya init_host or pre_start_hook are proably better | 19:50 |
| melwitt | sean-k-mooney: I think the nova-api wsgi app doesn't go through init_host or pre_start_hook bc it doesn't call Service.start(). so that has to either be done differently or go into __init__ or Service.create | 19:52 |
| sean-k-mooney | is that true... it might be i tough it was common but you might be right | 19:53 |
| sean-k-mooney | i didnt think the manager part changed between the too | 19:53 |
| dansmith | I'm quite sure melwitt is right here | 19:54 |
| dansmith | there's really no service lifecycle for it under wsgi | 19:54 |
| sean-k-mooney | sure but this is not part of the service | 19:54 |
| sean-k-mooney | its part of the manager | 19:54 |
| melwitt | sean-k-mooney: I was looking at this https://github.com/openstack/nova/blob/master/nova/api/openstack/wsgi_app.py#L52 | 19:54 |
| sean-k-mooney | the service calls it but init_host is a methond on the manager | 19:54 |
| dansmith | called by the service stuff though I think | 19:55 |
| melwitt | nova-api has no manager, no? | 19:55 |
| sean-k-mooney | sshhh with you logic and correctness | 19:56 |
| sean-k-mooney | your right it does not | 19:56 |
| melwitt | 😂 | 19:56 |
| melwitt | I'm wrong most of the time, so there's that | 19:56 |
| sean-k-mooney | that has not been my expnce but if you say so | 19:57 |
| melwitt | heh | 19:57 |
| sean-k-mooney | taking a step back | 19:58 |
| sean-k-mooney | we are just talking about validating the config right | 19:58 |
| melwitt | right | 19:58 |
| sean-k-mooney | does oslo allow ues to register a callback for that on the config opt | 19:58 |
| melwitt | hm, dunno but I'll look | 19:58 |
| melwitt | not seeing something like that in https://github.com/openstack/oslo.config/blob/master/oslo_config/cfg.py | 20:01 |
| sean-k-mooney | so i know we can define our own otp types | 20:01 |
| sean-k-mooney | and for string we have the ablity to provide enum vlaues | 20:01 |
| melwitt | we need dynamic enum (from os-resource-classes standard classes) and a regex for CUSTOM_stuff I think | 20:02 |
| sean-k-mooney | well i was thinking of inheriting form listOpt | 20:03 |
| sean-k-mooney | and just adding the validation | 20:04 |
| sean-k-mooney | basically providing a limitListOPT class | 20:04 |
| melwitt | ah ok | 20:05 |
| melwitt | that sounds interesting. I'll check it out | 20:05 |
| sean-k-mooney | looking at how its implemted fro string | 20:06 |
| sean-k-mooney | https://github.com/openstack/oslo.config/blob/68cefad313bd03522e99b3de95f1786ebea45d4b/oslo_config/types.py#L68 | 20:06 |
| sean-k-mooney | when you create and isntance of the opt you can provide choices (for a enum) or a regex | 20:07 |
| sean-k-mooney | and then the __call__ method actully does the validation | 20:07 |
| melwitt | that's cool | 20:07 |
| sean-k-mooney | and raise a valueError if it invalid | 20:07 |
| sean-k-mooney | https://github.com/openstack/oslo.config/blob/68cefad313bd03522e99b3de95f1786ebea45d4b/oslo_config/types.py#L141-L170 | 20:07 |
| sean-k-mooney | so if you inherited form List your call could call super which would provide a list and you could just loop over all the values and check if they were valid | 20:09 |
| sean-k-mooney | if yes just return the result form the call to super else rais | 20:09 |
| sean-k-mooney | melwitt: im not sure if that is overkill but its doable | 20:09 |
| sean-k-mooney | and it keeps the valdiation in teh config code so thats nice cause it will work for the api the same as everything else | 20:10 |
| sean-k-mooney | anyway o/ | 20:11 |
| melwitt | we could show off pro level oslo.config usage | 20:11 |
| melwitt | thanks for the ideas. seeya tomorrow o/ | 20:11 |
| frickler | sean-k-mooney: sorry if I missed that in scrollback, but https://review.opendev.org/c/openstack/os-vif/+/935653 failed in gate with the error it is supposed to fix, iiuc you wanted to skip that check completely, then? | 20:56 |
| sean-k-mooney[m] | ya i didnt get around to pushing a patch but i was going to make that test skip if the native driver was used | 20:57 |
| sean-k-mooney[m] | i just finished dinner but i can update the patch to do that in a little bit | 20:58 |
| sean-k-mooney[m] | we have 2 drivers ovs-vsctl and the nantive python on. its only the native one that sometimes times out | 20:59 |
| opendevreview | sean mooney proposed openstack/os-vif master: address test stablity under load https://review.opendev.org/c/openstack/os-vif/+/935653 | 21:22 |
| sean-k-mooney | frickler: ^ updated to skip | 21:22 |
| sean-k-mooney | the rest of the change is technially not needed but it does not hurt so i kept it | 21:22 |
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Clean up Windows support https://review.opendev.org/c/openstack/os-vif/+/932436 | 21:23 |
| sean-k-mooney | i rebsased and reappoved ^ on top so hopefuly that will pass and we can merge both soon | 21:24 |
| frickler | thx, will check for results tomorrow | 21:27 |
| opendevreview | Cyril Roelandt proposed openstack/python-novaclient master: Replace glanceclient with openstacksdk https://review.opendev.org/c/openstack/python-novaclient/+/931276 | 21:44 |
| opendevreview | melanie witt proposed openstack/nova master: nova-manage: Add flavor scanning to migrate_to_unified_limits https://review.opendev.org/c/openstack/nova/+/924110 | 21:54 |
| dansmith | melwitt: why the most recent change? | 21:56 |
| dansmith | did checking the rc not work? | 21:56 |
| dansmith | because if not, that needs fixing I think | 21:56 |
| dansmith | oh I see, you kept the output check without the ... output | 21:56 |
| melwitt | dansmith: yeah :( dumb | 22:11 |
| sean-k-mooney | dansmith: melwitt: https://review.opendev.org/c/openstack/os-vif/+/935653 passed with the test skip woudl one of ye mind reappoving that to unblock the follow up patch which is holding up the oslo requirement bump | 23:54 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!