Wednesday, 2023-11-08

opendevreviewMerged openstack/nova stable/victoria: reenable greendns in nova.  https://review.opendev.org/c/openstack/nova/+/83343602:16
zigostephenfin: Hi there! How are you?08:25
zigoWhere is tracked the SQLAlchemy 2.x patches, so I can attempt backporting them to Bobcat in Debian?08:25
bauzaszigo: I can help, all changes for support SQLA2 'language' in SQL1.4 are in this gerrit topic https://review.opendev.org/q/topic:sqlalchemy-2008:41
zigoAh, thanks a lot, will try.08:42
zigoGosh... too many... how do I know what's missing from Bobcat?08:42
bauzaswhat do you exactly want to backport ?08:46
bauzassqla changes that were merged after Bobcat RC1 ?08:46
zigoYeah.08:46
bauzasall projects or just nova ?.08:47
zigoAll projects.08:47
zigoMaybe I can just use the date of last change ...08:47
bauzaswell, the RC1 date is different for every project, that's the point08:47
bauzasbut I can filter out the gerrit query08:48
bauzassec08:48
zigoOh, there's a way to know with a simple Gerrit query? :)08:48
bauzasfor sure08:50
bauzasI was looking at some query field that would be 'includedin' but I can't find it so I'll just use mergedafter08:51
bauzasand you'll just compare with the list of merged patches to see whether we missed any patch during the RC period08:51
zigoI can just compare the code, yeah ...08:52
zigohttps://review.opendev.org/q/topic:sqlalchemy-20+project:openstack/heat+mergedafter:2023-09-11 <--- This looks like doing the trick for heat...08:53
bauzashttps://review.opendev.org/q/topic:sqlalchemy-20+is:merged+mergedafter:%25222023-09-15%2522,2508:53
bauzasyeah, just be aware some projects may have branched RC1 late08:54
bauzas(like nova, whoops)08:54
zigoThanks a lot, that's very helpful. So far, I backported 5 heat patches, continuing ... :)09:21
opendevreviewsean mooney proposed openstack/nova master: add pyproject.toml to support pip 23.1  https://review.opendev.org/c/openstack/nova/+/89975309:29
zigosean-k-mooney: Oh my god... I'm having a headake reading your commit header!!! :/09:48
bauzaspyproject.toml--09:49
sean-k-mooneybauzas: thats not the problem and you know it09:50
bauzas#xkcd87209:50
sean-k-mooneyits we have not adapted to the pip change that was deprecated 2 years ago09:50
bauzaserr, #xkcd972 - standards09:50
zigoLast time I checked, there was 4 ways to list the modules in a Python package, depending on what python packaging tooling is in use.09:50
bauzassean-k-mooney: sure I'm not debating your own changes, I'm rather arguing about pyproject.toml instead09:50
sean-k-mooneyzigo: this will eventually affect all distos not just centos so im trying to get ahead of it by enausering we have basic supprot for pep 517/518/66009:51
bauzassean-k-mooney: I'm basically hating what the python community did for packaging, ie. creating yet another standard09:51
sean-k-mooneyzigo: we are goign to continue to use reqiurements.txt09:51
zigosean-k-mooney: Guess which distro will be affected first ... :/09:51
sean-k-mooneyzigo: its actully centos not debian09:52
sean-k-mooneywell depending on how you look at it. im usign debian sid right now and its works fine without this09:52
sean-k-mooneyfor now09:52
zigoWell, it's not like if I was maintaining 400+ python packages for OpenStack that will all need patching ... :/09:52
bauzaszigo: yeah, thanks the python community for that brilliant idea09:53
sean-k-mooneybauzas: you know it annoys me every time you do that right09:53
sean-k-mooneybauzas: your being dilberatly dismissive of there efforts to solve an issue that many of there users had09:54
zigoOn the bright side, I very much agree that the setup.py thingy was a disaster, ie: having to execute code to be able to find out things instead of reading a configuration file was kind of a very broken idea.09:54
sean-k-mooneyzigo: ya i really dont like impertive systems for packaging09:55
sean-k-mooneynot that debina or rpm format get it correct either09:55
zigoThough there a better ways than breaking everyone at once...09:55
sean-k-mooneyi dont know 2 years of deprecation after several years of supprot is kind of reasonsble09:56
sean-k-mooneyand to be clear the only thing that broke for us09:56
bauzas2 years is a very short timeframe IMHO09:56
sean-k-mooneyis an extention to setuptools we have in pbr09:56
zigoI agree with Sylvain.09:56
sean-k-mooneyif we used plain setuptools with no extentiosn we would not ahve been broken at all09:56
bauzassean-k-mooney: fwiw, I +2'd the change itself09:56
bauzasthat's not because I hate that direction that I'll hold it09:56
zigoThere are thousands of upstream of python modules that will *NOT* be reactive enough.09:57
bauzasthe ship sailed and I need to move on09:57
sean-k-mooneybauzas: ack, we needed to wait for pbr 6.0.0 for the py 39 job to pass, that was released 7 hours ago so we will see what the ci says when it completes09:57
bauzasbut I'm just throwing my hate by explaining why I think 2 years is not a respectful timeframe 09:57
sean-k-mooneyzigo: right but thats why they implemetned a fallback that will work for vaniall setuptools out of the box09:57
bauzasbut that's not the first time the python community is trampling us09:58
sean-k-mooneyi mean we dont supprot or release for 2 years09:58
bauzasanyway, I should stop arguing here as it's useless, I should rather invest time in the python community itself 09:58
sean-k-mooneywe only supprot them for 18 months09:58
sean-k-mooneyoffically anyway09:58
sean-k-mooneyso your holding them ot a higher standard then we hold ourselves?09:59
zigoBTW, Python 3.12 thingy will start in Unstable next November.10:00
zigoExpect bug reports and patches from me ...10:00
bauzasI have the opinion that the lower you're a dependency, the longer you need to keep support10:00
zigoYet another thing: how come every single new version breaks my world ...10:00
sean-k-mooneynext november as in 2025 or as in that then next thing to start in novemerb on your backlog10:00
bauzasand a language sounds to me like probably one of the lowest dependencies we have and the most commonly shared10:01
zigoOh sorry, as in this month !10:01
sean-k-mooneyok :)10:01
sean-k-mooneyi was like that seams far off10:01
zigo3.14 is planned for Trixie (ie: debian 13).10:01
bauzasforcing people to upgrade by 2 years if you're a very common library is kinda harsh10:01
sean-k-mooneybauzas: your only forced to upgrade if you dont pin your deps10:02
sean-k-mooneyyou can always you an older pip to get the old behavior10:02
sean-k-mooneybauzas: the reason that is not viabel for us10:02
sean-k-mooneyis openvswitch now requried pep 517 to build in debian and newer releases then we ship in centos/ubuntu10:03
bauzasthat's my point10:03
sean-k-mooneyso pining pip breaks once you have a new enought ovs10:03
bauzasif this wasn't ovs, this would be something else10:03
bauzassince lots of projects are moving on their own timeframe, the other ones also need to adjusty10:04
sean-k-mooneywell the ovs issue is they did not keep supprot for both like i am10:04
sean-k-mooneyim just adding pyproject.toml supprot without removing setup.py support10:04
sean-k-mooneyzigo: ^ important point for you10:04
bauzassean-k-mooney: yeah, hence my +210:04
bauzaswe're continuing to do the old way but we allow doing it the new way10:05
sean-k-mooneywe can drop setup.py support in the future but its not breaking anything so i didnt see a point10:05
zigosean-k-mooney: Yeah, I'll look at what you did!10:05
bauzasagain, my rant wasn't directed to your change10:05
sean-k-mooneybauzas: i know10:05
sean-k-mooneyi just feel like i have to play devels avocate and defend the python core team10:06
* sean-k-mooney wonders if the joys of workign with a gpu company starting with n has darkend bauzas world view in general :P10:06
jrossertalking of gpu company starting with n, A100/A30 removal from the vGPU driver v16 is causing us a massive headache right now10:13
sean-k-mooneyjrosser: yep...10:14
jrosseranyone with ideas what to do would be interesting10:14
sean-k-mooneyother then keepign the old driver and pinnign your kernel till you retire the hardware10:14
sean-k-mooneythere isnt much you can do10:14
sean-k-mooneyjrosser: what os are you on10:14
jrosserubuntu10:14
sean-k-mooneycool well i think you can continue to use the 15.x driver for the lifetime of 22.04 right10:15
sean-k-mooneyalthough obviorulsy wihtout any future security/preformance improvements10:15
jrosseri don't believe we are going to be able to get any more vCS licences once ours expire10:15
sean-k-mooneyoh right the lisciense have an experation date... 10:16
jrosseryup10:16
sean-k-mooneythey have a diffent driver that the created for some of there skus10:16
sean-k-mooneydo you know fi the a100 moved to the new dirver10:16
sean-k-mooneythey are spliting there product skus between enterpirse and ai/ml usecases i belive10:17
sean-k-mooneyi havent really looked at the detail but i know the grid driver is being split in two for two diffent classes of hardware/workloads but i dont knwo where the a100 lands in that regard10:18
jrossersean-k-mooney: we have been attempting to PCI passthrough the MIG sriov VF (this might be pointless and never work, but it is at the advice of someone from nv) and get this https://paste.opendev.org/show/bTDDSu1EaNMhflc12ROB/ - do you see anything like that before with PCI passthrough?10:30
sean-k-mooneyjrosser: my understanding is while that could work the nvida driver in the guest does not curretnly work on the vf10:32
jrosseri could totally believe that10:33
sean-k-mooneyheader type 127 does seam famialr for some reason 10:33
sean-k-mooneyi belive its one fo the extended pci capablity headers10:34
jrosserand that certainly the case for vGPU/GRID driver on the host and free DataCenter driver in the guest which would be the most obvious licencing bypass you could do, so I expect that not to work10:34
jrosserthe advice we got was to try DataCenter driver on the host and in the guest10:34
sean-k-mooneyhttps://forum.level1techs.com/t/unknown-pci-header-type-127-for-device-000000-0/180125/410:35
sean-k-mooneyappreently its somethign to do with the graphsics card sleep modes10:35
opendevreviewMerged openstack/nova-specs master: Re-propose spec for ephemeral storage encryption  https://review.opendev.org/c/openstack/nova-specs/+/89750210:35
opendevreviewMerged openstack/nova-specs master: Re-propose spec for ephemeral encryption for libvirt  https://review.opendev.org/c/openstack/nova-specs/+/89750310:35
sean-k-mooneyjrosser: so its apprently because the card is not reseting properly when attached/detached form the vm10:37
sean-k-mooneyjrosser: thats a pretty common thing on consumer graphics cards that are pci passhtough to a vm10:37
jrosserhmm10:37
sean-k-mooneyim suprised thats a problem for a100 but since they never intended the vfs to be passed to the guest its not that surprising10:38
sean-k-mooneyhttps://forum.level1techs.com/t/solved-gpu-passthrough-suddently-stopped-working-on-windows-10-vm/171273/510:41
sean-k-mooneyi have not tired this but apprently you can disbable the d3 idel state for that device10:41
sean-k-mooneyoptions vfio-pci ids=10de:2486,10de:228b disable_vga=1 disable_idle_d3=110:41
opendevreviewMerged openstack/nova-specs master: Re-propose "Allow Manila shares to be directly attached to an instance when using libvirt" for Caracal  https://review.opendev.org/c/openstack/nova-specs/+/89831910:42
sean-k-mooneyjrosser: if it is just a sleep issue i woudl give that a try for the mig vfs10:42
jrosserah thats interesting, thanks10:43
bauzasjrosser: fwiw, please know that you could be hit with https://bugs.launchpad.net/nova/+bug/2041519 if you use A10010:43
bauzasjrosser: and yeah, I send hugs to you about vGPU GRID16 EOL support for AI 10:44
jrosserbauzas: yeah i saw that discussion the other day - the whole thing is a total disaster /o\ 10:44
bauzasyou now need to buy a fancy expensive AIE license : https://docs.nvidia.com/ai-enterprise/4.0/release-notes/index.html10:44
jrosserindeed, for 50 to 100% of the cost of the hardware10:45
sean-k-mooneyi recently bought an intel a750 apprently you can flash that to run there datacenter firmware10:45
sean-k-mooneyand get sriov supprot on the consumer card10:45
bauzasjrosser: IMO, now working with some n guys, I can say that they're excellent hardware vendors but terrible software engineers10:45
sean-k-mooneyi have been debating about doing that10:46
bauzassean-k-mooney: btw. when you and dansmith have time, I'd like to discuss on what direction we should do with https://bugs.launchpad.net/nova/+bug/204151910:46
bauzasthe alternative series I wrote is not helping as it doesn't delete existing RPs https://review.opendev.org/q/topic:bug%252F2041519-new10:47
bauzas(the above patch is just a nice try to pass a PF in device_addr instead of VFs, but that's not mandatory)10:47
sean-k-mooneyok ill need to load context on this again  so let either set up a meeting ro  choose a time to discuss it but it woudl be good to sumemrise what you have tried and what did/didnt work10:49
bauzassean-k-mooney: and you know, I haven't darken my view of the world, I'm just mattman as matthew perry said10:49
sean-k-mooneybauzas: im currently reviewing dans spec10:49
sean-k-mooneybut i can take a look at the patches after10:49
bauzassean-k-mooney: sure, let's do a gmeet with dan when he's up10:49
zigojrosser: Am I reading well that the A100 removal is from the Linux kernel ?!?10:49
bauzassean-k-mooney: no urgency10:49
sean-k-mooneyzigo: i dont think soo i think its just being droped form the nvidia binary driver they ship via there portal10:50
bauzaszigo: no, A100 are no longer supported in GRID, that's the point10:50
bauzaszigo: starting with GRID1610:50
sean-k-mooneyit would work as a normal graphics card with teh opensouce neuvous driver10:50
bauzashttps://docs.nvidia.com/grid/16.0/whats-new-vgpu/index.html10:50
dvo-plvHello, All. I would like to discuss results of yesterday debate regarding packed ring. Currently we have tow way traits and prefilter based on the release version. How we properly can solve this problem, Should be ask gibi for his ?10:50
sean-k-mooneybut none of there vGPU stuff is supproted in the the opensouce dirver10:51
bauzasdvo-plv: I said yesterday that eventually, I'll let the spec sail10:51
bauzasso ask gibi for a review 10:51
bauzasdvo-plv: sean-k-mooney had a point on the fact we shouldn't conditionnally provide the trait, so update your spec and add testing and I'll let sean-k-mooney +2 it10:52
dvo-plvokay, I see your point, thank you10:52
bauzasfor me, I'll pass as I don't want to give an approval for some direction I have arguments against, but on the other way, I don't wanna hold the series, so I defer to other cores10:52
bauzasI'll just not vote on the spec10:53
sean-k-mooneydvo-plv: tl;dr  we bumped libvirt min to 7.0.0 last cycle so no need to do a check on the version anymore10:53
sean-k-mooneyeither for the trait or in the driver10:53
sean-k-mooneyyou can just assume its new enough10:53
dvo-plvyes, i remember your comment under the nova patch, that i have to update it, because this https://review.opendev.org/c/openstack/nova/+/887255 was merge first10:54
bauzascorrect10:54
sean-k-mooneyyep10:54
sean-k-mooneyit should simplfy your patch although you had already added the checks10:54
dvo-plvsure, i will update spec first and then nova. Thank you for your time10:55
bauzasdvo-plv: fwiw, even if I want to abstain on the spec, I can review the implementation10:55
bauzasnow that I have the whole context, I surely can review it quickly once the design is approved by other cores but me10:56
dvo-plvsure10:56
dvo-plvit sounds critical:)10:56
zigostephenfin: There are many more services that fail with SQLA 2.x, that haven't been addressed: https://qa.debian.org/excuses.php?experimental=1&package=sqlalchemy10:57
zigoNamely, cloudkitty, trove, ironic-inspector, masakari, sahara, senlin, vitrage, zaqar ...10:57
bauzasyour spec will allow operators to not care about mixing hypervisors, which is good, it just doesn't solve our general problem on libvirt knobs we wanna handle10:57
bauzassean-k-mooney: see, I'm not that dark10:57
sean-k-mooneybauzas: by the way i think we can do a better job on the adress size spec. i -1 with a request to supprot it in image properties adn enchanment to automatically use triats and enchnce the image propperties filter10:58
sean-k-mooneyits a minimal expantionin the scope but will basically make it work out of the box for most peopel10:59
bauzasack, I'll review that one10:59
bauzastbh, it was a straight reproposal without much additions, I didn't put my brain on it like I did for some other spec :)11:00
sean-k-mooneyya im not sure why i didnt raise these point last cycle11:00
bauzassean-k-mooney: I also reconsidered the debt we put on ourselves by adding traits11:00
sean-k-mooneywhich i said in the spec review11:00
bauzassince traits aren't allocations, we can remove compute traits whenever we want, provided we support N-1 or 2 computes11:01
bauzaswithout need of a reshape11:01
sean-k-mooneyyou mean requesting and or reporting them?11:01
bauzasboth11:01
sean-k-mooneyin general we shoudl never remove standard traits11:01
sean-k-mooneyas it can break flavors or iamges11:01
sean-k-mooneywith requried traits requests11:02
bauzasnew computes can stop reporting a trait, but the prefilter needs to speak old language until next SLURP11:02
sean-k-mooneyno that woudl still break explcit traits request in teh flavor/image11:02
bauzassean-k-mooney: right, but dvo-plv's spec is not exposing the trait itself, which is less tech debt11:02
sean-k-mooneybauzas: it wont require it to be set manually11:03
sean-k-mooneyif that is what you mean11:03
bauzassean-k-mooney: that's why I think we said to avoid as much as possible to expose the traits in flavors11:03
sean-k-mooneypartly11:03
bauzassean-k-mooney: what I mean is that I would strongly disapprove the qemu spec if I hear people using the trait directly in a flavor11:03
sean-k-mooneyi dont like askign for things twice (once with a traint and again with an extra spec)11:03
bauzasbut I meh it, if we have a prefilter that hides that trait11:03
bauzasoperators using that trait directly for scheduling would then do something unsupported11:04
sean-k-mooneyright so in general a prefilter or and extention to ResouceRequest.from_request_sepc11:04
sean-k-mooneyis the preferd way  to use reqirued traits for standard traits11:04
sean-k-mooneyResouceRequest.from_request_spec is what you shoudl extend if its going to be in both the flavor and image11:05
bauzascorrect, standard traits should never be used directly in flavors, and I think we said that while ago11:05
sean-k-mooneycool with me11:05
bauzasonly custom traits can be used in flavors11:05
sean-k-mooneywell11:05
bauzasbut I think we never documented that11:05
sean-k-mooneyno there are some standard taits that can be in the flavors11:05
sean-k-mooneyspecificaly the one for cpu fetature flags11:06
bauzasI know, and I'd want us to take a stand11:06
sean-k-mooneyi.e. ones that are not related to any flavor extra spec11:06
sean-k-mooneyso the nounce is standard traits shodl be avoided if they can be requested via an extra spec instead11:07
bauzasthat's sad, PTG is just behind us but I realized we never went to a conclusion on whether we should expose our traits in flavors and the consequences it has11:07
sean-k-mooneywell you knwo i generally dont like exposing resouces: or traits in the flavor/image when we  can have an indriction via something else11:08
bauzasditto here, but again, we haven't documented that tenet11:08
sean-k-mooneyright but we cant say (you shoudl never request a standard triat in the flvor/image)11:09
bauzasand operators could be trying to test fancy things by exposing those traits in a flavor11:09
stephenfinzigo: nothing I can do about that, I'm afraid. I've pushed patches for Masakari and repeatedly trumpted the fact that this needs to be addressed in other projects11:09
bauzasI'd rather do an explicit list of traits we allow to specify11:09
sean-k-mooneyspecific these ones are genrally valid to request https://github.com/openstack/os-traits/tree/master/os_traits/hw/cpu11:09
bauzassean-k-mooneylike a public API11:09
bauzaswe have a stand on the contractual API we support11:09
sean-k-mooneybauzas: we could always enchnae the flavor validtor11:09
stephenfinzigo: Those will break when we uncap SQLAlchemy. They'll have to fix it then 🤷‍♂️11:10
opendevreviewSteven Relf proposed openstack/nova master: Adding basic auth to dynamic vendordata api calls  https://review.opendev.org/c/openstack/nova/+/90025211:10
bauzassean-k-mooney: I don't know what to do, that's the point11:10
bauzaswhat's easier is when you have a prefilter or the requestspec translator 11:11
bauzasthen we can say 'sorry, no, not intended to be used this way'11:11
sean-k-mooneyyep so when adding new extraspecs/image proeprts and a trait11:11
zigostephenfin: Thanks for letting me know, I'll see if we can invest time in at least cloudkitty that we use in production.11:11
sean-k-mooneyi think we shoudl alwasy prvied the prefilter/translator11:11
bauzassean-k-mooney: sounds a direction11:11
sean-k-mooneyand we could enhance the flavor validation api to also reject the use of the "resrved traits" in the future11:12
sean-k-mooneybauzas: https://github.com/openstack/nova/blob/master/nova/api/validation/extra_specs/traits.py11:12
bauzasI mean, for the short future, we need to agree between us as cores11:12
sean-k-mooneyyep 11:13
sean-k-mooneywe shoudl likely update the contibutors guide or add a new refence doc11:13
bauzasI'll capture that in a doc patch11:13
sean-k-mooneyi.e. how to design with triats and resouce classes11:13
bauzasthat's a start11:13
sean-k-mooneyi woudl add a new doc in https://github.com/openstack/nova/tree/master/doc/source/reference11:14
sean-k-mooneyor here https://github.com/openstack/nova/tree/master/doc/source/contributor11:14
sean-k-mooneybauzas: on a related note i really dont like usign resouces: in genreal in the flavor either11:14
bauzasyeah the latter was the one I thought11:15
sean-k-mooneyi kind of whish we had vgpu: instead11:15
bauzassean-k-mooney: I don't disagree11:15
sean-k-mooneythe main reason i say this is i dont like the way grouping works11:15
bauzasparticularly  if a VGPU resource could be a mdev or a VF in the next future11:15
sean-k-mooneyi think that is leaking too much about how we modle things in placment11:15
bauzasyeah11:15
bauzasand it creates us some binding11:16
sean-k-mooneyya so i woudl prefer vgpu: to work kind fo liek the pci alias11:16
bauzasdoes it sound another action item for vgpus ?11:16
sean-k-mooneyso we have a layer of indirection11:16
bauzasman, I really should invent the cloning machine first11:16
sean-k-mooneyam im not sure based on the lifespan of vgpus11:16
sean-k-mooneybut doign this as mdev: i could get behind11:16
sean-k-mooneyim expecgint that we will just use the pci_alias goign forward for sriov vgpus implemenations11:17
bauzasactually, having mdevs: flavor resources sounds better for operators if we start support vfio-pci for GPUs11:17
sean-k-mooneyhaving this indrection for mdevs in general i thikn would be nice but not required11:17
bauzasyeah11:18
sean-k-mooneyso effectivly we woudl extend the devices section or one of the dynmic section with an alias option11:18
bauzaswhen I read my code last week, I thought I spoiled too many vgpu acronyms in the code while this can be mdev agnostic11:18
stephenfinzigo: nw. The topic bauzas mentioned will bring up most patches, and I suspect there are solutions to most potential upgrade issues there11:18
stephenfintopic:sqlalchemy-2011:19
dvo-plvsean-k-mooney, bauzas I a little bit confused after this conversation. you talked regarding prefilter, as far as I understand about that https://review.opendev.org/c/openstack/nova/+/876075/25/nova/scheduler/request_filter.py#27511:19
dvo-plvBut i did not get if i need to change approach 11:19
zigostephenfin: Yeah, I managed to upload Heat 21.0.0 with all the 12 patches already ! :)11:19
zigoCurrently working on Glance.11:19
bauzasdvo-plv: no, you don't need to change the approach if sean-k-mooney and some other core agree on your updated spec11:20
sean-k-mooneydvo-plv: that is fine in my view, no change required11:20
sean-k-mooneydvo-plv: we just have 2 places where you can do this11:20
sean-k-mooneydvo-plv: either as a trival prefilter there11:20
bauzasbased on the discussion I just had with sean at the last min, I could even approve the spec now, provided we draw the line11:20
sean-k-mooneyor in the schduler utils11:21
sean-k-mooneydvo-plv: like this https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L288-L30411:21
sean-k-mooneydvo-plv: it has the same effect11:22
sean-k-mooneydvo-plv: the only reason to prefer a prefiler over a translate function like that is if you want ot make it configurable11:22
bauzasright11:23
sean-k-mooneydvo-plv: in your case we dont need or want it to be configurable (its not in your patch) 11:23
bauzasyou can opt-out 11:23
sean-k-mooneybut we have mandaotry pre-filters too11:23
bauzas(or opt-in, depending on the default value :) )11:23
sean-k-mooneyso either approch works11:23
bauzasactually11:23
bauzasthis sounds terrible for interop if we make those image properties be config-driven, right?11:24
sean-k-mooneydvo-plv: i think we get slightly better logging in general with the prefilter but its basically the same11:24
sean-k-mooneybauzas: it woudl be but its not11:24
sean-k-mooneytriat help with interup11:24
sean-k-mooneybut only if you have access to placement11:24
bauzaswell, if you disable the prefilter, then your property is useless, right?11:25
sean-k-mooneybauzas: the prefilter cant be disabeld11:25
bauzasin the current code, yeah11:25
bauzasbut in general, we have prefilters that are config-driven11:25
sean-k-mooneyyes but those are genrelaly not related to extra specs11:26
bauzassure, but that's again errorprone11:26
dvo-plvokay, great that you are on the same page now and all left the friends :) So, I will resolve comments asap, than kyou again11:26
sean-k-mooneybauzas: over time i would expect use to make some prefilters that you can currently opt out of mandatory11:26
bauzassome day we may want to have some configurable prefilter that would do property translation11:26
sean-k-mooneyi recently did that for the az prefilter when i remvoed teh az filter11:27
sean-k-mooneyi could see use makign the require_image_type_support or transform_image_metadata required at some point11:27
sean-k-mooneybauzas: actully looking at the code more are required then optional today11:28
bauzasyeah fortunately11:28
sean-k-mooneybauzas: we have for image properties already11:29
sean-k-mooneyi wrote a genric one to work with all image properties that have corresponding triats11:29
sean-k-mooneywe just need to extend that as it make sense11:29
sean-k-mooneyi didnt do that for flavors mainly because there are less extra spec that map to triat11:29
sean-k-mooneybut reflectign on that it might make sense too revisit that11:30
sean-k-mooneythere is likely some consoliation i coudl to with low amoutn of work11:30
sean-k-mooneydo you think that woudl be valuable. i just havent felt the effort was worth it previously11:31
bauzasthat's a very good question11:31
sean-k-mooneyprefilters/translaotr tend to be a single short funciton that is easy to test in isolation11:31
sean-k-mooneycombining them loose some of that simplicty depending on how you approch it11:32
bauzasI'm still afraid by the imagination of our lovely operators and I want us to draw a solid yellow line in terms of support, since I don't wanna buy some trait forever11:32
bauzasso, any forgery hiding traits sounds preferable11:33
sean-k-mooneyi think its fair to say that if you have very complex resouce and traits request in flavor that opens you up to some upgrade impacts that you woudl otehrwise not be exposed too11:33
bauzaswhether we should factorize the forgeries into one piece is debatable11:33
sean-k-mooneyperhasp something to consier in the future11:33
sean-k-mooneybut i dint think its required right now11:34
bauzasyeah11:34
bauzasalso, have we ever tried to sanitize os-traits before ?11:34
bauzassounds now nearly impossible to me11:35
sean-k-mooneywe are not allowed to remove traits form os-traits11:36
sean-k-mooneyever11:36
sean-k-mooneyplacement does not support that today11:36
bauzasI reached to that conclusion11:36
sean-k-mooneyit could be done with some mettadata but basically we need to resver the triat ids in the db11:36
* bauzas shivers11:39
bauzasanyway, I need to deserve time for lunch and writing my own spec11:40
opendevreviewMerged openstack/nova-specs master: Add libvirt-dev-alias spec  https://review.opendev.org/c/openstack/nova-specs/+/89280611:56
*** Continuity__ is now known as Continuity12:19
opendevreviewsean mooney proposed openstack/nova master: add pyproject.toml to support pip 23.1  https://review.opendev.org/c/openstack/nova/+/89975312:19
Continuityhey all, I have submitted a PR (https://review.opendev.org/c/openstack/nova/+/900252) and I'm struggling to understand and fix the build errors. Mostly around openstack-tox-cover, openstack-tox-py38 and nova-tox-py310-with-sqlalchemy-2x. It seems to be a case of, works fine on my local machine :D12:22
Continuityanyone have any pointers12:22
sean-k-mooneyContinuity: the pep8 failures look valid https://zuul.opendev.org/t/openstack/build/914ce175822a4ada8271b3d5ae9d6877/log/job-output.txt#4269-428212:28
sean-k-mooneyContinuity: we will also need a bug or specless blueprint to proceed with this feature12:30
sean-k-mooneythe unit test failrue also look valid https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_3b1/900252/2/check/openstack-tox-py38/3b1dc22/testr_results.html12:31
sean-k-mooneyrequests.exceptions.MissingSchema: Invalid URL 'POST': No scheme supplied. Perhaps you meant https://POST?12:31
Continuitysean-k-mooney: yeah I agree they look correct, thats the issue, when I run the tox commands locally they pass :S12:33
sean-k-mooneyContinuity: the fact you are gettign fewer error in 3.10 vs 3.8 is concerning12:33
Continuityand yes i need to drop a BP in as well12:33
sean-k-mooneyas it imply your relaying on a behaivor in requests that is diffent between the two versions12:33
Continuitynot intentionally, but its possible. I will dig a litter deeper12:34
sean-k-mooneyin general in not sure how i feel about adding http basic auth to this feature12:35
sean-k-mooneyescially since we have not tempest testing of this at all12:35
sean-k-mooneyContinuity: we will have to see how others feel about that and if a spec is required and ci coverave beyond unit test. i woudl like to see at least some functionl coverate for this i think12:36
sean-k-mooneybut maybe even devstack/tempest testing12:36
Continuityfair enough, basic auth felt like a good step between having to shoehorn keystoneauth into and endpoint that may already exist and nothing.12:37
sean-k-mooneybasically devstack woudl jsut have to stand up a fake webservice with http_auth configured that returns some static data to show the auth actully works12:37
Continuityyeah i can deffo take a look at that12:37
Continuitylet me get a BP created at least then we can invite conversation..12:38
sean-k-mooneysure. im not agaisnt basic auth in general but i dont want to see us supproting more auth protocl in the future12:38
sean-k-mooneyi.e. oath or saml tokens12:38
sean-k-mooneyif we needed anything more advanced then you shoudl use keystone12:38
Continuityyeah 12:39
Continuityi agree12:39
Continuityit would get massively messy otherwise. 12:39
sean-k-mooneyare you missign POST here https://review.opendev.org/c/openstack/nova/+/900252/3/nova/api/metadata/vendordata_dynamic.py#11612:40
sean-k-mooneyoh no12:41
sean-k-mooneyyour using session.post12:41
sean-k-mooneyis there a reason your not using res = self.session.request(url, 'POST',12:42
sean-k-mooneyah sessionis a difent object12:42
sean-k-mooneydependin on if you use _load_ks_session or _load_request_session12:42
sean-k-mooneyone is using a keystone auth client vs a requests client directly12:43
Continuityyeah12:48
Continuitymight be the wrong approach ...12:48
Continuityi know that that ks_sessions is built ontop of requests, but had issues picking it apart12:48
sean-k-mooneyit is yes12:48
Continuityand was trying to not muddy the two12:48
sean-k-mooneyusing request directly is fine12:49
sean-k-mooneybut can you try and use the same method assumign it exits12:49
sean-k-mooneyi commented on the patch 12:49
sean-k-mooneyyou are not mockign the funciton that you are calling in the unit test correctly12:49
sean-k-mooneyyou are using session.post but mocking session.request12:49
sean-k-mooneyif we can just use session.request in both cases that woudl be preferable12:50
sean-k-mooneyor session.post in both cases12:50
Continuityright, thanks12:55
Continuitysean-k-mooney: appreciate the pointers. I will take another look12:55
dansmithbauzas: I'm around now14:36
bauzassean-k-mooney: cool, let's see if sean-k-mooney can also be around14:39
* bauzas writing the mdev live-migration spec atm14:39
bauzasheh, sorry, I meant dansmith14:39
dansmithbauzas: if you just want sean-k-mooney twice and not me, that's fine14:40
bauzasdansmith: nah, was saying let's see if sean-k-mooney and you have time for discussing about the bug :)14:41
dansmithI know :)14:41
bauzas'sup, sean ?14:41
bauzasdansmith: also fwiw, I eventually accepted to use a trait for the virtio-packedring thing if *to be clear* we use a prefilter that we can delete 14:42
bauzasbecause I want to be clear that standard traits shouldn't be directly used in flavors14:42
bauzasthis way, we could do something else for hypervisory-only verifications if we want without holding the spec itself that we already accepted before14:43
dansmithstandard traits shouldn't be used in flavors? what does that mean?14:44
bauzasthey *could* be used if we don't have prefilters or request spec transformation but,14:47
bauzasfor the other ones that have a prefilter, nova shouldn't support the fact to call directly Placement with a resource request using those traits14:47
bauzasby saying "shouldn't support", I think people could do it, but in case computes no longer provide the traits, it wouldn't be a bug14:48
bauzaslike we do with, say, virt driver API which is an internal API14:48
dansmithokay I guess I'm confused - one of the primary reasons for this stuff in the first place was so you could have a flavor that guaranteed you some CPU flag like sse3, which is a standard trait14:48
bauzasthat's the exception I agree14:48
dansmiththat's the _rule_ IMHO :)14:49
bauzashypervisor specific traits like sse3 are okay to me to be directly used in flavors14:49
bauzaswe wouldn't change the computes to no longer pass those traits14:49
dansmithokay I don't see that we can, should, or need to draw the line around things like this new one.. if it's a trait it's a trait14:49
bauzasbut for other traits that we create, like the one we discussed yesterday, I'm torn to say 'sure, the compute will always provide it"14:50
sean-k-mooneybauzas: i generally agree with dan the reason traits exist is to request hardware capabliteis like sse314:50
sean-k-mooneybut we realised that not all capabliteis are hardware capabliets14:50
dansmithright, this new capability is "virtual hardware capability" not unlike a cpu flag, IMHO14:51
sean-k-mooneywhich is why the compute namespace exists to model the capablity of a hyperviors to emulate/provide a functionaltiy14:51
sean-k-mooneyright so https://github.com/openstack/os-traits/tree/master/os_traits/compute14:51
sean-k-mooneyis for software cpablies14:51
sean-k-mooneyand https://github.com/openstack/os-traits/tree/master/os_traits/hw is for host capablities14:52
dansmithexactly14:52
bauzasyeah, so my point is that some of the traits can directly be used, for sure, like the hardware ones14:53
sean-k-mooneywhere i kind of sit in this debate is if we have an extra spec to enable a functionlatiy that should automatically result in requeest the capablity14:53
bauzasbut for some traits we only use internally for scheduling, those shouldn't be directly used14:53
sean-k-mooneyi.e. you should not need to request hw:viommu=true and traits:viommu=required14:53
sean-k-mooneythe former shoudl result in the latter being added automatically14:54
bauzasif one day, we think that COMPUTE_NET_VIRTIO_PACKED is not the right trait to provide for supporting the virtio_packed thing, we could be able to no longer provide it14:54
opendevreviewDanylo Vodopianov proposed openstack/nova-specs master: Re-propose using VirtIO PackedRing Configuration support for 2024.1  https://review.opendev.org/c/openstack/nova-specs/+/89592414:54
sean-k-mooneyyep and since we dont expect anywone to directly request it (just use the flavor extra spec) that shoudl be fine14:55
bauzasanyway, I don't want us to discuss that much now14:55
sean-k-mooneywe can just document "hay dont set this  nova will do it for you"14:55
bauzasagreed14:55
dansmithwell, it's that reason why I think it's not *worth* a trait really, and a service version is enough to get through this current hoop we have14:56
bauzasdansmith: that's exactly why I meh'd to the spec14:56
dansmithbut I'm not wed to either alternative as they both have pros and cons14:57
bauzasif we only do this in a prefilter, we can modify it later14:57
bauzaslike, say, by one time, we create some other trait that says "heh, look, I'm a libvirt host"  (I know, I know, this is something some people disagree but that's just an example), then we could remove that trait, provided we support both traits during a SLURP time14:58
bauzasagain, don't nitpick that example, we could be smarter14:58
dvo-plvsean-k-mooney, bauzas Under words "user should not use trait, only nova". Should I add this here https://review.opendev.org/c/openstack/nova/+/876075/25/releasenotes/notes/packed-virtqueue-filter-43a376674cb5b345.yaml ?14:59
bauzasdvo-plv: I don't think we need14:59
sean-k-mooneydvo-plv: not in the release notes15:00
sean-k-mooneybut it could be a note in the feature doc15:00
dansmithI don't think you should say it at all15:00
sean-k-mooneythat also ok15:00
dansmithif it's a trait there's no point in us trying to get people to not use it15:00
dansmiththat's just confusing15:00
sean-k-mooneyprovided we dont refence it in our doc15:00
dansmith(and won't work)15:00
sean-k-mooneyi.e. just mention the extra spec not the trait in the doc15:00
*** blarnath is now known as d34dh0r5315:00
dansmithI'm confused15:00
dansmithwhy are we not mentioning it?15:00
sean-k-mooneyto use this feature you do not need to set the trait manually in the flavor ro the image15:01
sean-k-mooneyyou just need to set the flavor extra spec15:01
dansmithbut why?15:01
sean-k-mooneythe trait is an internal detail and does not need to be in the doc15:01
dansmiththen we should _not_ be using a trait, IMHO15:01
sean-k-mooneybecuase the prefilte adds it based on teh extra spec15:02
dansmiththe only way I think a trait makes sense is if you want to be able to say "I asked for packed queue in hw_ and I want you to FAIL if you can't find me a host that supports that"15:02
sean-k-mooneythat what it will do15:03
sean-k-mooneythe prefilter will check if the flavor has the packed virt queue extra spec and add a required trait15:03
sean-k-mooneyso it will fail if we dont find a host15:03
dansmithbut you're talking about a prefilter to always do that15:03
sean-k-mooneyyes15:03
dansmithwhat other hw_ things do we enforce at prefilter time that way?15:04
sean-k-mooneyunless we just dont do the trait at all15:04
sean-k-mooneysev15:04
sean-k-mooneyvtpm15:04
sean-k-mooneywe technialy do thoese in teh ResouceRequest.from_request_spec function15:04
dansmith"sev" and "tpm" are not in request_filter15:04
sean-k-mooneyrahter then a prefilter15:04
sean-k-mooneyif you want a prefilter example cyborg15:05
sean-k-mooneythey both run at more or less the same time15:05
dvo-plvdansmith, it will set trait if user required packed here https://review.opendev.org/c/openstack/nova/+/876075/25/nova/scheduler/request_filter.py#27515:05
dansmiththis seems like mixing scopes to me15:06
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L187-L19515:06
sean-k-mooneydansmith: that the other place we do this ^ 15:06
dansmithack, well, I don't really see why this needs to be this way, but whatever15:08
bauzasanyway, I don't have time this cycle to do anything with traits15:09
bauzasI just wanted to make sure we don't agree to use that trait I dislike in flavors15:09
opendevreviewDanylo Vodopianov proposed openstack/nova-specs master: Re-propose using VirtIO PackedRing Configuration support for 2024.1  https://review.opendev.org/c/openstack/nova-specs/+/89592415:10
bauzasand I can write some contrib doc saying "please avoid passing hw traits directly in flavors" or some paragraph we could bikeshed15:10
dansmithbauzas: I'm really opposed to that15:10
bauzaswhat exactly ? hw traits ?15:11
sean-k-mooneybauzas: i would also be opposed to saying dont pas hw traits15:11
dansmithI think acting like there are some 3rd rail traits that you shouldn't use is ridiculous and makes no sense.. if you're going to have an un-disable-able request filter then there's no _point_ in putting one in a flavor,15:11
dansmithbut sayin that you shouldn't for some unexplainable reason makes no sense to me15:11
bauzasokay, then the yellow line I would draw would be "don't use standard traits that are automatically queried by either a scheduler prefilter or a requestspec utils method"15:12
dansmithno.15:12
sean-k-mooneyi mean you proably shouldnt but i dont think we need to docuemnt that15:12
sean-k-mooneythere may be valid reasons to do it15:12
dansmith"there is no need to specify traits like hw_packed_virt_queue because nova does it for you"15:12
dansmithis the strongest I'd say, but I wouldn't even say that15:12
sean-k-mooneyi think all we have to do is document the minimal extra spec requried to enabel the feature15:13
dansmithyes15:13
sean-k-mooneyand jsut not mention the triats outside the spec15:13
bauzasokay, I won't argue15:13
sean-k-mooneywether there are triats, compute service bump ectra are detail we need to agree on but in general should not impact end users or operators15:14
bauzasI personnally think there are 'consumable traits' and 'non-consumable ones' but OK to not explain to operators the names of the 'non-consumable' ones :)15:14
bauzasthat said, they'll get them with placement15:15
dansmithbauzas: I reject your reality and substitute my own15:15
sean-k-mooney:)15:15
bauzasokay, nevermind, I'm done with this15:16
sean-k-mooneybauzas: all traits shoudl be "consumabel traits" they may not exist in yoru cloud but thats a seperate probelm15:16
bauzaslike I said, there are other cores but me for the spec15:16
dansmithall traits *are* consumable, so that's just the end of it :)15:16
bauzasthis just implies that we won't never remove a trait from a compute once we provide it15:17
bauzasbut OK15:17
sean-k-mooneybauzas: we have never done it before15:17
sean-k-mooneyand i assuemd that was going to be the case yes15:17
sean-k-mooneyi mean unless15:17
sean-k-mooneywe remove a feature15:17
sean-k-mooneyif we remove the ablit to do somethign removing the trait is reflecting reality15:18
bauzassure, I wasn't talking about removing a feature15:18
dansmithI too don't think there's any way to remove a trait other than not supporting a thing it represents15:19
bauzasbut rather rebuilding the feature by doing other scheduler way15:19
dansmithwhich is also why traits are massively more expensive than bumping an internal integer15:19
bauzas^ that is my concern15:19
dansmithwell, that was my argument in the beginning15:19
sean-k-mooneyif we decied because we have done the min libvirt version we dont need the triat thats fine15:20
bauzasso, again, if we are not able to draw a line (which I can understand), then I won't accept the spec15:20
sean-k-mooneybut i dont really like bumping a min compute version for a driver only change15:20
bauzasand I'll just wait for another core to accept it15:20
sean-k-mooneybut if that is preferabel fine15:20
sean-k-mooneywe are just scoping out supproting this until the cloud is fully upgraded15:20
bauzaswe already have a min compute version that works15:20
dansmithsean-k-mooney: https://github.com/openstack/nova/blob/master/nova/objects/service.py#L13615:21
sean-k-mooneyright but normally i would not expect a compute service bump for a change that only affect a virt driver15:21
bauzashttps://github.com/openstack/nova/blob/master/nova/objects/service.py#L26415:21
sean-k-mooney dansmith  file backed memroy required toher chagnes15:21
dansmiththis feels kinda similar: https://github.com/openstack/nova/blob/master/nova/objects/service.py#L22015:22
sean-k-mooneydansmith: im more ok with documenting "do a compute service bump for dirver only changes" then docuementing anything about not suing traits15:24
dansmithI definitely understand the "this is purely a driver thing so not service version eligible" but I think that this is (a) the primary virt driver, (b) in tree, (c) has hard version requirements built into the driver15:25
bauzasguys, I originally wanted to discuss about https://bugs.launchpad.net/nova/+bug/2041519 status and we sidetracked on the trait thing where I'm done and not much interested. Fancy moving to that ?15:25
dansmithand also maybe (d) most of the other live migration and neutron things we bump the service version for are really just for libvirt features/changes15:25
dansmithso I dunno15:25
dansmithI think the relative cost makes it worth it15:25
dansmithbauzas: yes please ;P15:26
dansmithbauzas: gmeet or here/15:26
bauzasgmeet, will be faster if you don't mind getting yet another meeting15:26
dansmithI'd rather do less typing15:26
dansmithhere it's 7:26 and I'm already sick of typing15:26
bauzasalready had 2 hours of meeting already15:26
bauzasokay, so15:26
bauzashttps://meet.google.com/mdo-owzo-zka15:27
bauzaspeople wanting to discuss on https://bugs.launchpad.net/nova/+bug/2041519 are free to come here ^15:27
dvo-plvFYI, I've updated packed ring spec file https://review.opendev.org/c/openstack/nova-specs/+/89592415:28
dansmithsean-k-mooney: ?15:31
dansmithah nevermind, you're busy sorry15:32
sean-k-mooneyya sorry on two calls currently15:37
bauzassean-k-mooney: nevermind, dansmith found me a solution that should work :)15:41
bauzasI just need to make sure we can verify the resource providers by init_host :)15:41
*** artom_ is now known as artom16:01
*** artom_ is now known as artom16:14
bauzasdang, since we call check_on_dest before check_on_source in the live migration worflow, I don't have any way to hard guess the mdev type the instance was set against17:43
*** JayF is now known as JasonF18:53
*** JasonF is now known as JayF18:53
opendevreviewErik Olof Gunnar Andersson proposed openstack/nova master: Fix coverage issues with eventlet  https://review.opendev.org/c/openstack/nova/+/90011620:17

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!