| *** mtreinish_ is now known as mtreinish | 05:13 | |
| *** hemna2 is now known as hemna | 07:37 | |
| *** priteau_ is now known as priteau | 11:16 | |
| *** bhagyashris__ is now known as bhagyashris | 11:49 | |
| gibi | anyone with placement insights do you agree my conclusion in the bug https://storyboard.openstack.org/#!/story/2009795 ? | 11:53 |
|---|---|---|
| sean-k-mooney | gibi: ill read over it now | 11:58 |
| gibi | sean-k-mooney: thanks | 11:59 |
| gibi | I've just added one more link to the end of the ticket supporting that it is doc bug | 12:01 |
| sean-k-mooney | oh this is the resouce less traits issue | 12:01 |
| sean-k-mooney | we talked about this before with regrads to neted resource providres and numa i think | 12:01 |
| gibi | yes, the nested magic spec class these traits as provider traits compared to resource traits that are tight to a specific resource class | 12:01 |
| gibi | yes the nested magic spec captures some of that discussion I believe | 12:02 |
| sean-k-mooney | we only have one type of traits in plamcent | 12:03 |
| sean-k-mooney | we dont make a distinciton in the api today | 12:03 |
| gibi | yes | 12:03 |
| sean-k-mooney | af far as i know | 12:03 |
| gibi | the distinction is just logical not implemented | 12:03 |
| sean-k-mooney | right im assserting there is not distinciton and this is a bug not a docs bug | 12:04 |
| sean-k-mooney | but ill re read teh nested magic spec now | 12:04 |
| sean-k-mooney | im really not sure i agree with https://docs.openstack.org/placement/latest/specs/train/implemented/2005575-nested-magic-1.html#resource-versus-provider-traits | 12:07 |
| sean-k-mooney | i do not belive we can assume all custom traits on a resouceless rp are provider traits | 12:07 |
| sean-k-mooney | gibi: so https://docs.openstack.org/placement/latest/specs/train/implemented/2005575-nested-magic-1.html#why-enforce-resourceless-same-subtree does not apply to https://storyboard.openstack.org/#!/story/2009795 since its not a request for a resoucless request group | 12:15 |
| sean-k-mooney | the unnamed group is speical and unlike all other groups it does not require the resouce in it to come form the same RP | 12:16 |
| sean-k-mooney | every named request group does | 12:16 |
| gibi | sean-k-mooney: so you are on the side that an required trait in an unnamed group can come from any RP of the tree? | 12:21 |
| sean-k-mooney | yes stongly so | 12:21 |
| gibi | the code today only considers RPs that are providing resources to the request | 12:21 |
| sean-k-mooney | we do not use root_requires today | 12:21 |
| gibi | I'm reading https://docs.openstack.org/placement/latest/user/provider-tree.html | 12:22 |
| gibi | it seems to be relevant too | 12:22 |
| sean-k-mooney | so if we moved all the resouces form the root rp to numa nodes but did not move the capablity traits they woudl stop working | 12:22 |
| gibi | either we need root_required or we need a resource on the root rp | 12:24 |
| gibi | * resource on the root rp that is requested | 12:24 |
| sean-k-mooney | or the unnamed group is allowed to match it | 12:24 |
| sean-k-mooney | but ya we would have to mvoe to root_reqiured which is an upgrade issue | 12:25 |
| sean-k-mooney | as we would have to make 2 queires and combine them | 12:25 |
| sean-k-mooney | to support rooling upgrades | 12:25 |
| gibi | "Traits can be requested explicitly in the GET /allocation_candidates operation with the required query parameter, but traits on resource providers never span other resource providers. If a trait is requested, one of the resource providers that appears in the allocation candidate should have the trait regardless of sharing or nested providers." | 12:26 |
| gibi | https://docs.openstack.org/placement/latest/user/provider-tree.html#filtering-by-traits | 12:26 |
| gibi | this shows me that the intention is the one that I see in the code | 12:26 |
| sean-k-mooney | well the rp would appear in the allocagtion candiate in the provider tree summary even if it did not provide a resouce correct? | 12:27 |
| gibi | "A dictionary keyed by resource provider UUID included in the allocation_requests, of dictionaries of inventory/capacity information. " | 12:28 |
| gibi | so no, only the RPs that provides resources appear in the summary | 12:28 |
| sean-k-mooney | the current approch impleie si can have a sharing resouceless resouce provider with traits and have them match the unnamed group | 12:28 |
| gibi | the "unused" child RPs are not | 12:28 |
| sean-k-mooney | which woudl be problematic since that is how we do isolated aggreates right | 12:29 |
| sean-k-mooney | https://docs.openstack.org/nova/latest/reference/isolate-aggregates.html | 12:29 |
| gibi | hm, that feature adds the trait on the root RP | 12:30 |
| sean-k-mooney | actully no this i swhy we have to set the trait on every host in the aggreated which is a major ux issue | 12:30 |
| sean-k-mooney | yes today it does | 12:30 |
| sean-k-mooney | but you shoudl be able to create 1 rp | 12:30 |
| sean-k-mooney | add the trait to that | 12:31 |
| sean-k-mooney | with misc_share_via aggreate | 12:31 |
| gibi | OK, I think except the API doc, the rest of the doc is self consistent (even if we don't like how they work) | 12:31 |
| sean-k-mooney | then add other hosts ot it | 12:31 |
| sean-k-mooney | well i think we can agree on not likeign the current behavior | 12:33 |
| gibi | :) | 12:33 |
| sean-k-mooney | i dont think the behaivor shoudl change based on if the rp has a trait or not | 12:33 |
| sean-k-mooney | what happens is you use an older micorverion | 12:35 |
| gibi | I can be convinced to change the logic later / separately. I just hit this while implemented the any-trait logic and first I thought it is a code bug based on the API doc. But now with the other docs in the picture and the actual usage of this in the isolate-aggregate I think this is just a doc bug | 12:35 |
| gibi | sean-k-mooney: do you have a particular microversion to check? I have test env so I can try | 12:35 |
| sean-k-mooney | i think its more subtle then that | 12:35 |
| sean-k-mooney | i think the doc captured the intent | 12:36 |
| gibi | which doc? the API or the provider-tree doc? | 12:36 |
| sean-k-mooney | api doc captures how i expected this to work the provider-tree doc captures what the code is doing | 12:37 |
| sean-k-mooney | i find the current behavior hard to reasonable usefully | 12:37 |
| sean-k-mooney | to me the idea that a trait in the unnamed group can match agaisnt any RP in the tree or in a sharing rp is a fundemental part fo the api regardelss of if resouces are requested form it | 12:38 |
| sean-k-mooney | gibi: with out it i dont know how to reason about trait request from images | 12:38 |
| sean-k-mooney | or in the flavor to some degree but images are the most restict way a user can express traits | 12:39 |
| sean-k-mooney | they can only form part of the unnamed group | 12:39 |
| gibi | from the nested magic spec I feel that the current implemented logic supports resource traits, qualities related to certain resource classes (happen to be on the same RP) for the provider trait case we _shoudl_ use root_quired | 12:39 |
| gibi | probably the traits from the image should be translated to root_required | 12:40 |
| sean-k-mooney | that the thing i really dont like the intordcution fo provdier traits | 12:40 |
| gibi | but I guess we put them in the unnamed group | 12:40 |
| sean-k-mooney | how can we know if a CUSTOM_trait is a resouce trait or a provider tarit | 12:40 |
| sean-k-mooney | it should not be based on if there is an inventoy on the RP | 12:40 |
| gibi | with the current model we don't | 12:40 |
| sean-k-mooney | that inventory may not be related to the trait | 12:40 |
| gibi | simply the current trait model cannot nicely describe both type of traits | 12:41 |
| gibi | and it seems the impl opt to consider a trait a resource trait by default | 12:41 |
| gibi | and provider triats are harder to model properl;y | 12:41 |
| gibi | with the current model | 12:42 |
| sean-k-mooney | i would put it more stongly right now there are only one type fo traiats and provider/resouce traits shoudl not be discussed | 12:42 |
| gibi | but they exists logically | 12:42 |
| gibi | just not modelled | 12:42 |
| sean-k-mooney | gibi: to me the grouping in that spec was lexical | 12:42 |
| sean-k-mooney | and not part of the data model | 12:42 |
| sean-k-mooney | so they dont exist | 12:42 |
| sean-k-mooney | they were only grouping to think about idfferent type of trait but should have no baring on impelemention | 12:43 |
| gibi | we use both types from nova | 12:43 |
| gibi | "types" | 12:43 |
| sean-k-mooney | not without a way to catgories CUSTOM_traits | 12:43 |
| gibi | we consider compute feature traits as provider traits | 12:43 |
| sean-k-mooney | we do but i would not expect them to have diffeerent behaviors today | 12:43 |
| sean-k-mooney | i do not | 12:44 |
| gibi | today no different behavior, we carefully putting them to the root RP where we know that we will always consume resources from today | 12:44 |
| gibi | so our nova model works | 12:44 |
| sean-k-mooney | yes because they are beign trated as resouce_traits today | 12:44 |
| sean-k-mooney | by placement | 12:44 |
| gibi | yes | 12:45 |
| sean-k-mooney | so i woudl reassert we are not using provider_traits or root_required today in nova | 12:45 |
| gibi | nova (as the client of placement) needs to simulate the provider_trait logic, by putting provider traits on the root and alway allocation from the root | 12:46 |
| sean-k-mooney | and i dont think we can until we have a way to model provider_traits and resouce_traits in a way that is indepent form the existnace of inventories | 12:46 |
| sean-k-mooney | i dissagree | 12:46 |
| sean-k-mooney | with out a way to model it in the data model we shoudl not try to use a half implemented feature | 12:46 |
| gibi | then which resource CUSTOM_SUPPORT_MULTIATTACH is connected? | 12:46 |
| gibi | today there is no such resource | 12:47 |
| gibi | so it has to be a provider trait :) | 12:47 |
| sean-k-mooney | no since that is not part of the api | 12:47 |
| gibi | it is part of the resource model of the nova | 12:47 |
| gibi | stored in placement | 12:48 |
| sean-k-mooney | no since we dont need it | 12:48 |
| sean-k-mooney | all our logic in nova works without the concpet of a provider trait | 12:48 |
| sean-k-mooney | gibi: if we want to have resouce traits and provider traits there is a simple solution | 12:49 |
| sean-k-mooney | we need to move resouce tratis into the inventory | 12:49 |
| gibi | "simple" | 12:49 |
| gibi | :) | 12:49 |
| gibi | not even to inventory but connecting each trait to an instance of a resource class | 12:49 |
| sean-k-mooney | yes if we want to have resouce traits and provider traits we need to store taits both on the resouce and proviers | 12:49 |
| sean-k-mooney | i think the inventory is simpler to reason about | 12:50 |
| sean-k-mooney | since it jsut works for CUSTOM_ traits | 12:50 |
| sean-k-mooney | if they are applied to the inveotry then they apply to the resouce clas of that invetory | 12:50 |
| sean-k-mooney | so i woudl propose inventory_traits and provider_traits as the speration instead | 12:51 |
| gibi | a cpu flag trait should be connected to the VCPU or PCPU inventory but not a MEMORY_MB inventory | 12:51 |
| sean-k-mooney | traits are opaque stings to placment | 12:51 |
| sean-k-mooney | it does not know the releation ship | 12:51 |
| sean-k-mooney | i dont think we as a client shoudl require placment too | 12:51 |
| sean-k-mooney | unless we change what tratis are | 12:52 |
| sean-k-mooney | and add a mapping tabel where we register every trait with the RC it applies too | 12:52 |
| sean-k-mooney | and an api do that | 12:52 |
| gibi | yes, but if a trait only connects to the inventory but not a resource class instance then a VCPU providing a different set of flags than a PCPU cannot be modelled by the same set of cpu flag traits | 12:52 |
| sean-k-mooney | it can | 12:52 |
| sean-k-mooney | since they are two different inventoies | 12:53 |
| gibi | on the same root RP providing both VCPU and PCU | 12:53 |
| gibi | then we are talking about "inventory" differently | 12:53 |
| sean-k-mooney | correct | 12:53 |
| gibi | I define inventory of an RP as a set of resource class instance | 12:53 |
| gibi | you define an inventory of an RC instead :) | 12:53 |
| sean-k-mooney | im say that the inventory need to change form Resouce class + count | 12:53 |
| sean-k-mooney | to resouce_class + count+ traits | 12:53 |
| gibi | I agree | 12:54 |
| gibi | I think we are in agreement on this | 12:54 |
| gibi | just using different meeting behind "inventory" | 12:54 |
| sean-k-mooney | so we woudl be extending the invotry concpet with a new triats field | 12:54 |
| gibi | we would extend the inventory of an RC concpet :D | 12:54 |
| gibi | anyhow we agree here and we are pretty far from my original problem | 12:55 |
| sean-k-mooney | well an invtory is a top level object in placment | 12:55 |
| sean-k-mooney | https://github.com/openstack/placement/blob/master/placement/objects/inventory.py | 12:55 |
| sean-k-mooney | so im saying we shoudl add a traits filed that is a list of traits | 12:55 |
| gibi | I will not change the current behavior of the trait filtering logic when I implement any-traits | 12:55 |
| sean-k-mooney | ack | 12:56 |
| sean-k-mooney | i still think its a bug as currently defined | 12:56 |
| sean-k-mooney | but we can change it fi we add inventory level traits | 12:56 |
| gibi | yeah | 12:56 |
| gibi | I think it is not a bug based on the original intent (the magic spec) it might be a bug from the current intent we have | 12:57 |
| sean-k-mooney | i dont think the magic spec ful filles its intended uscases but it may be the intent of the autours | 12:57 |
| gibi | still as there is a disconnect between the API doc and the behavior I would change the doc to avoid a loss of a day of others like me reading only the aPI doc | 12:58 |
| sean-k-mooney | we likely shoudl adress teh api doc issue yes to make it aling with how it work | 12:58 |
| sean-k-mooney | but i also think we shoudl avoid using any of the feature intoduced in the magic spec | 12:58 |
| sean-k-mooney | until we reconsider the data model | 12:59 |
| gibi | you mean we should not use root_required from nova? | 12:59 |
| sean-k-mooney | correct | 12:59 |
| gibi | I'm fine with that limit | 12:59 |
| gibi | we already use same_subtree tough | 12:59 |
| sean-k-mooney | well | 12:59 |
| gibi | which was also defined by the magic spec | 12:59 |
| sean-k-mooney | rather then limit i shoudl say be very very carful of depening on this if we want to change it going forward | 13:00 |
| sean-k-mooney | im not sure the curent magic spec is compatblie with how i tought about numa in placment | 13:00 |
| gibi | that is a good question | 13:00 |
| gibi | the numa spec could be an input to rething the placement model | 13:01 |
| sean-k-mooney | right now i dont think we are useing resouceless rps correct | 13:01 |
| sean-k-mooney | i think that is the point we have to be careful | 13:01 |
| sean-k-mooney | root_required is proably ok | 13:01 |
| sean-k-mooney | untill we have resourceless rps the distinction does not matter | 13:02 |
| sean-k-mooney | so that is the bit i would avoid for now | 13:02 |
| gibi | I agree | 13:02 |
| gibi | resourceless rp feels like an edge case | 13:02 |
| sean-k-mooney | do we have any features that require it currently that are in flight | 13:03 |
| sean-k-mooney | i do not think so but just wondering | 13:03 |
| sean-k-mooney | nic affinity/anti affintiy i think was on eof the usecase form the spec but we dont have that today | 13:04 |
| sean-k-mooney | and my draft pci in placment spec did not use it | 13:04 |
| gibi | I dont know about any use case that needs it now | 13:04 |
| sean-k-mooney | i was only going to model providres of PF ro VF but not the nic | 13:05 |
| sean-k-mooney | ok | 13:05 |
| sean-k-mooney | so we can likely update the api doc without "breaking" exisitng usecases | 13:05 |
| gibi | yeah | 13:05 |
| sean-k-mooney | i think i need to breing up the ponit that we shoudl be spending more time workin on plamcnet internally | 13:06 |
| sean-k-mooney | again | 13:06 |
| gibi | yeah | 13:07 |
| gibi | I feel the pain touching placement internals now as I lost most of the context and honestly the core placement devs are not with us any more | 13:07 |
| sean-k-mooney | placement is decptive. it mostly just works and is well tested, but there are sharp edges in places where we dont use a feature yet | 13:08 |
| sean-k-mooney | the bits we use work great | 13:08 |
| gibi | ... and the sql in the implementation is way more complex than what I can easily grok | 13:09 |
| *** dasm|off is now known as dasm | 13:09 | |
| opendevreview | Imran Hussain proposed openstack/nova master: [nova/libvirt] Support for checking and enabling SMM when needed https://review.opendev.org/c/openstack/nova/+/825496 | 13:17 |
| opendevreview | Balazs Gibizer proposed openstack/placement master: Clarify trait filtering in the API doc https://review.opendev.org/c/openstack/placement/+/825501 | 13:28 |
| gibi | sean-k-mooney: ^^ this is the API doc update | 13:28 |
| gibi | and thanks for talking it through with me | 13:32 |
| sean-k-mooney | ack no worries | 13:38 |
| sean-k-mooney | i did not expect the behavior in the story so its good to know to not get bitten by it | 13:38 |
| sean-k-mooney | i have an downstream ironic edgecase that might be related to this by the way although i think they just forgot to zero out cpu ram and disk requests in the flavor | 13:39 |
| sean-k-mooney | they do not have a resouce class associated with the ironic node | 13:40 |
| sean-k-mooney | so they likely have no inventoies on teh RP | 13:40 |
| sean-k-mooney | they are seing the request to palcment elimiate teh placment rps for the ironic nodes | 13:41 |
| sean-k-mooney | im waiting for the db dumps and flavor details to confirm but thats also likely a docs bug in our downstream docs | 13:42 |
| sean-k-mooney | we dont have the changes required for moderen ironic where it does not report cpu ram and disk inveories in our downstream docs | 13:42 |
| stephenfin | gibi: bauzas: Think you folks could look at two large-ish outstanding doc reviews I have? They're good stuff, IMO https://review.opendev.org/c/openstack/nova/+/814562 https://review.opendev.org/c/openstack/nova/+/814563 | 14:00 |
| bauzas | stephenfin: ack, on a meeting | 14:03 |
| sean-k-mooney | stephenfin: ill add them to my list | 14:09 |
| sean-k-mooney | stephenfin: i was also looking at your unittests mock patch | 14:09 |
| sean-k-mooney | it woudl be nice to land that this cycle | 14:09 |
| gibi | stephenfin: added to my list | 14:09 |
| *** bhagyashris is now known as bhagyashris|ruck | 15:17 | |
| * bauzas disappears for 30 mins | 15:19 | |
| opendevreview | Merged openstack/nova master: [doc] propose Review-Priority label for contribs https://review.opendev.org/c/openstack/nova/+/816861 | 17:05 |
| opendevreview | melanie witt proposed openstack/nova stable/ussuri: Ensure MAC addresses characters are in the same case https://review.opendev.org/c/openstack/nova/+/817689 | 17:08 |
| melwitt | lyarwood: astupnik updated https://review.opendev.org/c/openstack/nova/+/776250 awhile back in response to your comments, if you could take another look. if you won't be able to get to it, lmk | 17:19 |
| sean-k-mooney | just an fyi. im more or less going to be afk until tommrrow. im going to minimis my irc and email and try and get some development envs (ooo and devstack) deployed so i likely wont see any pings until tomorrow | 18:08 |
| *** ministry is now known as __ministry | 20:20 | |
| *** artom__ is now known as artom | 20:47 | |
| opendevreview | melanie witt proposed openstack/nova master: Add stub unified limits driver https://review.opendev.org/c/openstack/nova/+/712137 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Assert quota related API behavior when noop https://review.opendev.org/c/openstack/nova/+/712140 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Make unified limits APIs return reserved of 0 https://review.opendev.org/c/openstack/nova/+/712141 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: DNM Run against unmerged oslo.limit changes https://review.opendev.org/c/openstack/nova/+/812236 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Add logic to enforce local api and db limits https://review.opendev.org/c/openstack/nova/+/712139 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Enforce api and db limits https://review.opendev.org/c/openstack/nova/+/712142 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Update quota_class APIs for db and api limits https://review.opendev.org/c/openstack/nova/+/712143 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Update limit APIs https://review.opendev.org/c/openstack/nova/+/712707 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Update quota sets APIs https://review.opendev.org/c/openstack/nova/+/712749 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Tell oslo.limit how to count nova resources https://review.opendev.org/c/openstack/nova/+/713301 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Enforce resource limits using oslo.limit https://review.opendev.org/c/openstack/nova/+/615180 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Add legacy limits and usage to placement unified limits https://review.opendev.org/c/openstack/nova/+/713498 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Update quota apis with keystone limits and usage https://review.opendev.org/c/openstack/nova/+/713499 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Add reno for unified limits https://review.opendev.org/c/openstack/nova/+/715271 | 21:58 |
| opendevreview | melanie witt proposed openstack/nova master: Enable unified limits in the nova-next job https://review.opendev.org/c/openstack/nova/+/789963 | 21:58 |
| *** dasm is now known as dasm|off | 23:13 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!