| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-06 | |||
| 15:25:26 | sean-k-mooney | we can update/create mutliple allocation at once | |
| 15:25:43 | dansmith | right, but they will still check the reserved amount | |
| 15:25:59 | sean-k-mooney | possibly | |
| 15:26:05 | dansmith | they do | |
| 15:26:15 | sean-k-mooney | ok | |
| 15:26:15 | dansmith | https://github.com/openstack/placement/blob/master/placement/objects/allocation.py#L73 | |
| 15:26:41 | dansmith | see my logic above, this ^ will run after the swap and fail because we're over capacity due to the reserved amount | |
| 15:26:58 | sean-k-mooney | right i think that what happend for evacuate too | |
| 15:27:35 | dansmith | yeah, so I think from the outside, it's reasonable for an api consumer to assume that "allocations.replace_all()" is just a swap but it's not | |
| 15:28:11 | sean-k-mooney | well maybe | |
| 15:28:29 | sean-k-mooney | its was not orgianlly for swappign it was for doign muitple updates i think | |
| 15:28:32 | gibi | from API semantic perspective it is not well defined what it means when such a POST call has multiple consumers | |
| 15:28:41 | dansmith | it's also reasonable to assume that we check the capacity again, I'm just saying I think it's a bit ambiguous | |
| 15:28:47 | dansmith | gibi: exactly | |
| 15:29:12 | gibi | it might be a move it might be two independent allocation in the POST | |
| 15:29:23 | sean-k-mooney | yep | |
| 15:29:28 | dansmith | so one way to resolve that would be to say "if we were over capacity before, we allow you to be over capacity by the same exact amount after, otherwise fail" | |
| 15:29:42 | sean-k-mooney | that or a flag to contole the behavior | |
| 15:29:50 | sean-k-mooney | either way i think it need a new microversion | |
| 15:30:07 | dansmith | yeah I was also thinking a flag would be a thing | |
| 15:30:25 | dansmith | sean-k-mooney: can you dig up a reference on the evacuate bug? | |
| 15:31:09 | sean-k-mooney | gibi found it not long ago i can check my email/irc/slack logs and see | |
| 15:31:45 | sean-k-mooney | i know the last time it came up as a downstream it was because the oeprator had adjusted the allcoation ratio | |
| 15:31:57 | sean-k-mooney | putting the host into an oversubsiped state and then later could not evacuate | |
| 15:32:10 | sean-k-mooney | but there was a second edge case too | |
| 15:32:56 | dansmith | right, so that's what I was going to say, | |
| 15:33:13 | dansmith | I think that an alloc ratio change or reserved change will make our existing code fail to swap an allocation of equal size, when it really shouldn't | |
| 15:33:18 | dansmith | for all the move ops | |
| 15:34:37 | sean-k-mooney | in the evacuate case i think we only have one allcoation however it just happens ot have resouce form 2 hosts or somethign slightly odd | |
| 15:35:15 | dansmith | any feeling on allowing the reserved check to be skipped if the total size of the allocations against a provider are not changing? or do you think we need a flag to ask for that specifically | |
| 15:35:28 | dansmith | I tend to think a new microversion after which we say that's the behavior is okay, | |
| 15:35:31 | sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1943191 https://bugs.launchpad.net/nova/+bug/1924123 and https://bugs.launchpad.net/nova/+bug/1941892 | |
| 15:35:37 | dansmith | but if we want to only do that if we specifically ask for it then... | |
| 15:36:40 | dansmith | heh the subject of that first bug is exactly what I'm saying yeah | |
| 15:37:39 | sean-k-mooney | non of those actully say evacuate bu ti think its a releated issue | |
| 15:38:02 | dansmith | but blocking cold migrate is worse | |
| 15:38:26 | dansmith | brb | |
| 15:38:34 | sean-k-mooney | as is sid i think, i could be wrong about this, that for evacuate we do not have a migration uuid becuase its actully not a "move op" in that sence it just rebuidl to a new host | |
| 15:38:57 | sean-k-mooney | so for evacuate i think we extend the single allcoaiton with rp for the dest | |
| 15:39:18 | sean-k-mooney | i could be wrong about that because i know we change that at some point | |
| 15:39:26 | sean-k-mooney | i would have to look at the detail | |
| 15:40:49 | dansmith | yeah idk, I'm more familiar with the cold migrate case, which is clearly broken already | |
| 15:41:05 | gibi | sean-k-mooney: yeah evac is implement the way you describe | |
| 15:42:33 | sean-k-mooney | we talked about chaging htat at some point but there was a reaons why it was hard | |
| 15:42:37 | gibi | I'm wondering if it is enough to check if the allocated value does not change to use the swap mode instead of using the current independent allocation mode. I mean from placement API perspective there just not enough information about the intention of the user. | |
| 15:42:55 | gibi | what if at some point we want a swap behavior that also decreases the size | |
| 15:43:12 | dansmith | gibi: I don't think there are multiple different operations are there? | |
| 15:43:17 | dansmith | we're using POST /allocations for both | |
| 15:43:40 | dansmith | and placement is using the same logic regardless of if we're deleting and re-allocating the same provider vs. different ones, AFAIK | |
| 15:44:22 | gibi | yeah, my point is that POST call can be understood both ways (swap or create/delete) even if the total allocated resources does not change in the process | |
| 15:44:23 | dansmith | but yeah that was my point.. if the operation is not further over-subscribing the provider (or maybe not changing the amount at all) it seems reasonable to let it through | |
| 15:44:26 | gibi | so placemetn does not know | |
| 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 | 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 | |