| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-02-12 | |||
| 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 | sean-k-mooney | filterign properties is one thing | |
| 16:33:11 | bauzas | feel free to chime in | |
| 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 | |
| 16:44:28 | sean-k-mooney | no but we already have the info in the host manger | |
| 16:44:33 | sean-k-mooney | we only pass the allcoations | |
| 16:44:45 | sean-k-mooney | so we can see what you asked for but not what is on the host | |
| 16:44:52 | sean-k-mooney | so we can just pass both | |
| 16:45:10 | sean-k-mooney | because we have both in the schduler alrady so no change to the placement request | |
| 16:45:20 | dansmith | ack, seems like that would be a non-trivial increase in the hoststate though | |
| 16:46:06 | sean-k-mooney | its more like a exnetion in the time before its garbage collected maybe | |
| 16:46:24 | sean-k-mooney | i think we end up keepign them in memory anyway because of the function call path but i might eb wrong | |
| 16:46:39 | dansmith | so we already report we just don't expose it to the weighers?/ | |
| 16:46:59 | sean-k-mooney | yes | |