| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-06 | |||
| 15:46:08 | dansmith | sean-k-mooney: oh that's not the problem, it's the swap | |
| 15:46:31 | sean-k-mooney | well right now i think we do the swap an creation of the dest allocation in one call | |
| 15:46:33 | gibi | if a flag in the current API is to anti REST then lets have a separate resource explicitly for swapping | |
| 15:46:57 | dansmith | either way, for the purposes of one-time-use devices, can we say this is already a problem that needs to be solved and it'll block cold migrate for OTU but once we fix it, we're good? | |
| 15:47:15 | gibi | yes that is fair to say | |
| 15:47:22 | dansmith | I will commit to fixing both, FWIW | |
| 15:47:31 | sean-k-mooney | i.e. dont we update the allcoation with the instance uuid to the test host RPs and move the exiting allcotions to the migraiton uuid in one call that just "modifies multiple allcotions" | |
| 15:48:16 | gibi | sean-k-mooney: we remove the allocation from the instance uuid and add it to the migration uuid in a POST call then we do a scheduling | |
| 15:48:16 | dansmith | gibi: I think a flag would be okay if the semantics are "allow equal or less oversubscription to exist after you do this" | |
| 15:48:45 | dansmith | right now it is "oversubscription means fail" even if it's the same amount as before | |
| 15:48:57 | gibi | dansmith: if the client explicity call a swap between two consumers placement should just do a swap without extra checks | |
| 15:49:15 | gibi | i.e. keep the API generic | |
| 15:49:20 | sean-k-mooney | if we move the current allction to the migration uuid before we schedule | |
| 15:49:20 | dansmith | gibi: well, that's not very RESTful I think | |
| 15:49:29 | sean-k-mooney | then im a little confused | |
| 15:49:34 | dansmith | gibi: what about a PATCH to just change the consumer uuid? | |
| 15:49:45 | gibi | dansmith: sounds good to me | |
| 15:49:47 | dansmith | (not sure if we can be that fine-grained about it, I'd have to look) | |
| 15:49:59 | sean-k-mooney | because at that point we jus tneed to convert the allcoation candiate abck to an allction with the instance uuid | |
| 15:50:04 | sean-k-mooney | and then elete the miggration uuid | |
| 15:50:11 | sean-k-mooney | so it does not need ot be in one api post | |
| 15:50:34 | gibi | it is really should be a simple operation to update the owner of the allocation (of a full blown swap between two consumers but we don't need that right now) | |
| 15:50:46 | sean-k-mooney | well that you can do | |
| 15:50:49 | gibi | s/of/or/ | |
| 15:50:55 | sean-k-mooney | you can modify the consumer uuid | |
| 15:51:39 | sean-k-mooney | well the consumer but iint the allcotion uuid the instance uuid or migration uuid depending | |
| 15:51:52 | sean-k-mooney | we added consumer_type to the api at some point to track that | |
| 15:52:34 | gibi | I don't think we have a single placement API operation where you can update the consumer uuid without a reallocation (like the POST call we do currently) | |
| 15:53:43 | gibi | dansmith: so for me I don't need a full blown swap operation between any two consumer. I'm OK to have just a PATCH on an existing allocation to update the consumer uuid in it and do nothing else. | |
| 15:54:13 | sean-k-mooney | this is the existing api we are talking about yes https://docs.openstack.org/api-ref/placement/#update-allocations | |
| 15:54:18 | dansmith | gibi: yeah if we can do that cleanly that might be good.. I'll have to start digging in to determine, but yeah | |
| 15:54:49 | dansmith | are you thinking PATCH /allocations/uuid1 with {consume_uuid: uuid2} ? | |
| 15:55:05 | dansmith | that will work, not sure how the REST police feel, but I'd think it's better than a POST /swap_action | |
| 15:55:16 | gibi | sean-k-mooney: we are talking about https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L2059 that is what we do today | |
| 15:55:50 | gibi | dansmith: yeah lets ask the REST gods about it but the PATCH feels simple to me | |
| 15:55:57 | dansmith | ack | |
| 15:56:09 | sean-k-mooney | patch is an update to an exiting resouce | |
| 15:56:22 | sean-k-mooney | the resouce is keyd by the consumer_uuid /allocations/{consumer_uuid} | |
| 15:56:41 | sean-k-mooney | we could supprot patch to chagne that | |
| 15:56:50 | sean-k-mooney | wihtout conflicting with the existing put | |
| 15:57:25 | sean-k-mooney | but it feesl a littel od ot do /allocations/{instance_uuid} bode: consumer_uuid=<migration uuuid> | |
| 15:57:27 | gibi | yeah we need to change the "key" that is why it feels problematic from REST perspective | |
| 15:57:32 | bauzas | I started to pay attention to this channel but I failed getting the problem to solve | |
| 15:57:40 | sean-k-mooney | i dont hate it | |
| 15:57:51 | bauzas | can someone summarize me the thread ? | |
| 15:58:12 | sean-k-mooney | the ohter option is somthign liek /allocations/{old uuid}/swap/{new uuid} | |
| 15:58:20 | dansmith | bauzas: https://bugs.launchpad.net/nova/+bug/1943191 | |
| 15:58:23 | gibi | bauzas: move operations does not work if the source RP is over allocated or allocated and reserved | |
| 15:58:24 | sean-k-mooney | that would be a post i guess | |
| 15:58:37 | dansmith | bauzas: that and several other bugs, plus my OTU spec | |
| 15:59:06 | bauzas | I see | |
| 16:00:14 | sean-k-mooney | gibi: i think if we document things properly iether could work i think POST /allocations/{consumer_uuid}/swap/{new_consumer_id} is clear honestly | |
| 16:01:25 | sean-k-mooney | tha twould not have any body | |
| 16:01:46 | sean-k-mooney | so there is no posiblity of changing the allcotin or capsity useage and there for its ok to not do checks | |
| 16:02:21 | sean-k-mooney | if we go with patch we would have to ensure the resouces are not changed as well | |
| 16:07:46 | gibi | yeah I'm not against either the PATCH or the POST //swap/ way | |
| 16:10:07 | sean-k-mooney | i think we can all agree that if it alllow us to finally resovle thos bugs in nova either approch is better then what we have today. so if that the direction we head in im fine with say lets define that in the placement api spec | |
| 16:13:11 | gibi | I agree | |
| 16:14:01 | dansmith | yup | |
| 17:06:15 | melwitt | sean-k-mooney: not sure what you'll think of this but I uploaded a patch in regard to the oslo.limit endpoint discovery https://review.opendev.org/c/openstack/nova/+/943507 fyi | |
| 17:17:52 | sean-k-mooney | so we remove the end point check since oslo limists will do that for use internally | |
| 17:18:44 | sean-k-mooney | and we repalce the end point fuction with one that returns the limits for this service/region | |
| 17:19:26 | sean-k-mooney | these are the global limits right not the project specific ones | |
| 17:21:25 | sean-k-mooney | melwitt: without properly loadign context and looking at oslo limits its seams reasonable at first glance | |
| 17:23:50 | melwitt | sean-k-mooney: yeah the thing I think is controversial is the private attributes access | |
| 17:24:08 | sean-k-mooney | that not required with the latest version of oslo.limits right | |
| 17:24:09 | melwitt | maybe in parallel propose some public accesses in oslo.limit and update it when available? dunno | |
| 17:25:19 | sean-k-mooney | https://review.opendev.org/c/openstack/oslo.limit/+/914783/3/oslo_limit/limit.py | |
| 17:26:46 | melwitt | what is there currently will work for both old and new versions | |
| 17:27:09 | melwitt | and underneath oslo.limit will do what it needs to | |
| 17:27:10 | sean-k-mooney | right in the new version we can just call get_registered_limits | |
| 17:27:13 | sean-k-mooney | on the enforcer | |
| 17:27:48 | sean-k-mooney | https://github.com/openstack/oslo.limit/blob/8ada297e6a82f7e50bf746453b8dd21716743798/oslo_limit/limit.py#L183 | |
| 17:29:43 | sean-k-mooney | melwitt: do you need to use the connection today | |
| 17:30:50 | sean-k-mooney | oh that funciton takes a set of resouces to check | |
| 17:33:25 | melwitt | sean-k-mooney: yeah I do that because I want to pull all the resourcs limits in one API call. oslo.limiit does them one API call per resource | |
| 17:34:07 | sean-k-mooney | it sound like we shoudl add a funciton to oslo limits to do that | |
| 17:34:17 | sean-k-mooney | but i think we can work around it in nova for now | |
| 17:34:33 | melwitt | maybe. there's already one so not sure if we change the internal impl or make another alongside | |
| 17:35:45 | sean-k-mooney | you could make None a seential value for all or use the sentenial object pattern | |
| 17:36:18 | melwitt | yeah | |
| 17:38:14 | sean-k-mooney | so gibi's main concern seams to be those are private filed and there for could break which is valid. if oslo were ok providing public access in some way i would be ok with this as a backportable approch as long as we update to using the public appropch in the future | |
| 17:39:31 | melwitt | sean-k-mooney: right. it's mine too, which is why I mentioned it in the commit message. yeah I think that sounds reasonable | |
| 17:39:33 | sean-k-mooney | i dont hate it but if we could just do ENFORCE.get_registered_limists(None) | |
| 17:39:39 | sean-k-mooney | i would be happier | |
| 17:39:56 | melwitt | sure | |
| 17:39:57 | sean-k-mooney | the other option for now | |
| 17:40:12 | melwitt | will that work currently? /me looks | |
| 17:40:15 | sean-k-mooney | is kep doing it the old way and instead fo raising try and do it this way | |
| 17:40:28 | sean-k-mooney | melwitt: i was trying ot figure that out | |
| 17:40:36 | sean-k-mooney | you can pass it i just dont know what happens if you do | |
| 17:41:09 | sean-k-mooney | its this right https://docs.openstack.org/api-ref/identity/v3/#list-registered-limits | |
| 17:41:13 | melwitt | it looks like it would do nothing | |
| 17:41:20 | melwitt | https://github.com/openstack/oslo.limit/blob/master/oslo_limit/limit.py#L355 | |
| 17:41:30 | sean-k-mooney | it say resouce name is optional in the api | |
| 17:41:53 | sean-k-mooney | you could try [""] | |
| 17:42:16 | melwitt | yes that's why I'm using the connection directly | |
| 17:42:24 | sean-k-mooney | i.e. ENFORCE.get_registered_limists([""]) | |
| 17:42:44 | sean-k-mooney | ya well modle.connection is public it seams | |
| 17:42:58 | sean-k-mooney | its just the endpoint id and service id that not right | |