Earlier  
Posted Nick Remark
#openstack-nova - 2025-03-06
15:44:28 dansmith yeah
15:44:36 sean-k-mooney if the set of RPs in the set of allction dont change and the total resouces consuemd dont hcange then we might be able to skip the capastiy check
15:45:05 gibi if nova tells placement it is a swap, then do a swap regardless of the allocation size. but otherwise keep the current behavior of create/delete
15:45:06 dansmith on the one hand, if you have an allocation for X, right now you can't even *decrease* the allocation size to get closer to happy because "closer to happy is still oversubscribed so reject"
15:45:26 sean-k-mooney honestly it would be better to do this in 2 calls
15:45:43 dansmith sean-k-mooney: I don't see how.. two calls seems worse to me
15:45:49 sean-k-mooney 1 call to create the dest allocaotn and a dedicated call to swap the 2 allocations
15:45:50 dansmith gibi: a flag to just skip the consistency check also feels wrong to me
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 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: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: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 dansmith gibi: well, that's not very RESTful I think
15:49:20 sean-k-mooney if we move the current allction to the migration uuid before we schedule
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

Earlier   Later