Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-02-12
14:56:47 bauzas yeah
14:56:54 bauzas or something else I'm thinking
14:57:06 dansmith yeah I see, sorry.. that's old, totally forgot it was there, but fair enough
14:57:12 bauzas do you think I can just add the mixin directly in the ImageMetaProperties object ?
14:57:29 bauzas I think we don't need to bump a version for this, right?
14:57:48 dansmith you don't but IIRC lots of work was done to remove it :)
14:58:28 bauzas okay, then lemme use this function directly by the weigher
14:58:40 bauzas thanks
14:59:18 dansmith I would just move it to the weigher, yeh
15:17:05 sean-k-mooney you know i provided an implentation of the method in my comment
15:17:20 sean-k-mooney that grabbed only the populated field form the obejct
15:17:24 sean-k-mooney and idd not need to primative
15:18:13 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/940642/comment/489497dc_ec3feaf3/
15:18:34 sean-k-mooney is there a reson we are not doing that
15:18:42 sean-k-mooney perhaps with a better name
15:18:46 sean-k-mooney if needed
15:20:00 sean-k-mooney i speicifcly suggested puting it in ImageMetaProps https://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L158
15:20:20 sean-k-mooney so we coudl give it a good name and keep it targeted to the usecase we needed for the weigher
15:22:06 sean-k-mooney the implemenmtaion i descibed would work for any obejct too for what its worth so it could be in base which is why i used the generic name set_fields
16:00:51 bauzas sean-k-mooney: sorry was on a meeting
16:01:52 bauzas sean-k-mooney: there are many possibilities, what would be the one you would prefer ? either I use that method only in the weigher or I provide it in the ImageMetaProps object, as you want
16:03:39 sean-k-mooney so i wanted to put it in ImageMetaProps because i tought that was cleaner btu i dont want you to call obj_to_primitae at all
16:04:03 sean-k-mooney so if you want to do it in the wegiehr we could but i tough it was more useful on the object
16:04:30 ratailor__ sean-k-mooney, gibi could you please provide your feedback on https://review.opendev.org/q/topic:"bp/image-props-in-server-show" patches.
16:04:31 sean-k-mooney to me its better encapsulation of code
16:19:30 bauzas sean-k-mooney: https://github.com/openstack/nova/blob/master/nova/objects/base.py#L331 does all the magic that we need and I don't want to copy it, so I'd call that helper
16:23:38 sean-k-mooney it does more then you need and is ineefect
16:23:46 sean-k-mooney so i dont think you shoudl use that
16:24:37 sean-k-mooney wehn you do obj_to_primitive it produces a dict which asl has all the ovo version info
16:25:38 sean-k-mooney unless that is diffent then obj.obj_to_primitive()
16:26:06 sean-k-mooney oh this is form nova object base so maybe it is diffent then the ovo version fo the funtion
16:26:50 sean-k-mooney ok so its going to take this branch https://github.com/openstack/nova/blob/master/nova/objects/base.py#L340-L344
16:27:21 sean-k-mooney bauzas: ok ya the nova obj_to_primitive does what you need
16:28:01 bauzas yup, I just need to call it and adapt the response in order to get a set of tuples instead of a dict
16:28:08 sean-k-mooney lightly more then you need in that it provides the full dict
16:28:20 sean-k-mooney so you shoudl call .keys() on teh result
16:28:41 bauzas set(mydict.items()) would give me what I need
16:28:59 sean-k-mooney you do not want items
16:29:01 bauzas provided mydict is returned by base.obj_to_prim
16:29:26 bauzas I want the values of the properties, that's important
16:29:44 bauzas I'll compare the tuples (key, value), not only the keys
16:30:01 sean-k-mooney i dont think the values shoudl be considered
16:30:10 sean-k-mooney well
16:30:20 bauzas os_type values need to be considered :)
16:30:26 sean-k-mooney ok so you want to mage on the key and value for think like os_type
16:30:28 sean-k-mooney ok
16:30:33 sean-k-mooney i see why you want both
16:30:48 bauzas gibi's point was to optionally provide a conf option that would tell which keys to look at
16:30:53 sean-k-mooney ok then ya itmes
16:31:00 sean-k-mooney ya i really dont like that
16:31:11 bauzas I leave that comment open
16:31:22 dansmith I was really hoping we would not need to do that too
16:31:38 sean-k-mooney to me this is getting overly complex if we need per image prop wights
16:31:40 dansmith even though I gave a concrete example of how, I also gave a concrete example of how it would break down
16:31:50 bauzas but I'll repropose a new patch for fixing my items() method
16:32:49 bauzas dansmith: sean-k-mooney: well, the weigher would be defaulting to check for all props, but we could optionally change its behaviour by looking at only a few of them
16:33:05 bauzas that's the proposal I made on my last comment but I leave the comment open
16:33:11 bauzas feel free to chime in
16:33:11 sean-k-mooney filterign properties is one thing
16:33:22 sean-k-mooney per property multiplers is another
16:33:54 sean-k-mooney that the bit that im not really fond of althogh im not super happy with either
16:34:09 dansmith well my point is that I feel like the minute you start making some properties worth more than others (filtered properties count as $weight, unfiltered properties count as zero) then,
16:34:24 dansmith the ask will be to make something important (os type) more important than something less (numa, or disk or something)
16:34:36 sean-k-mooney ya
16:35:01 sean-k-mooney so ignoring the compelxity this was ment to be a very simple weigher with littel overhead
16:35:23 sean-k-mooney i.e. you could enable it with very littel conisdiertation to the schduilign performace impact
16:35:25 dansmith I think the question is not about just simplicity, but also how well it works
16:36:03 dansmith the originally-proposed mechanism was very simple and also very weak, and I assumed that was the goal... to help clump some things, but not be very strong (which needs more complexity to be effective I think)
16:36:24 dansmith honestly, this might be the kind of thing where "fix it in post" via watcher is more effective
16:36:35 sean-k-mooney right os i have wanted to do soemthign similar but idffernt for a number of years, so i propose that loose affintiy apprch"
16:37:25 sean-k-mooney what i have wanted ot add is a diffent weigher that i call the boreign host wiether that perfer hosts with less traitsand less complicate RP trees
16:38:13 dansmith correct me if I'm wrong,
16:38:44 dansmith but if you want to clump your windows instances on windows machines you can set a trait on those hosts, and a requested-but-not-required trait on the image, right?
16:38:45 sean-k-mooney baiscly avoiding host with complciated things unless you ask for it. in this uescase they wanted to group similar isntances together based on the image properies
16:38:59 sean-k-mooney dansmith: no
16:39:02 dansmith it won't dynamically scale the pool, but otherwise that will be a strong but uncomplicated magnet right?
16:39:04 sean-k-mooney we dont have prefered traits
16:39:10 dansmith no?
16:39:29 sean-k-mooney if we did have a prefered trait wiegher for that it woudl be ideal
16:39:54 sean-k-mooney but we would also have to dynmical export the widnows trait once an instnace lands on the host in that case
16:40:15 dansmith we have trait=required but no trait=anythignelse?
16:40:26 dansmith no we wouldn't
16:40:29 dansmith we could
16:40:37 dansmith but not strictly required, IMHO
16:41:03 sean-k-mooney i guess you could pre export it with provider yaml
16:41:10 dansmith right
16:41:13 sean-k-mooney i.e. tag the nodes up front
16:41:21 dansmith I think I've made this mistake before, I guess because I assume if there's an =required there must be an =somethingelse :)
16:41:46 dansmith right.. that doesn't make it fully automatic, but also makes it less complicated, predictable, and strong
16:42:28 sean-k-mooney so we coudl do both. the simple automatic clumping
16:42:36 sean-k-mooney and in the future add prefered traits supprot
16:42:42 sean-k-mooney with another weither
16:42:44 sean-k-mooney https://github.com/openstack/glance/blob/5cfecc79995c6a9524f0dbc4553b4a88761b382f/doc/source/admin/useful-image-properties.rst?plain=1#L277-L306
16:43:03 sean-k-mooney ^ we only supprot required but not forbiden or prefered in the image
16:43:09 dansmith preferred traits needs no placement support, right? just a weigher
16:43:16 dansmith yep
16:43:21 sean-k-mooney just a weigher and much of the work is done
16:43:42 sean-k-mooney we jsut need to put the provider summeries in the hoststate object
16:43:45 dansmith is that not basically the same amount of work as this? why not just do that?
16:44:09 dansmith oh, do we not have the ability to see traits on hosts from the weighers now

Earlier   Later