Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-05
20:24:50 jaypipes cdent: the placement client embedded in the scheduler that called GET /allocation_candidates is one client. The scheduler client embdedded in the cell conductor that needs to call PUT /allocations/ is a different client.
20:24:59 cdent yes, I know
20:25:07 dansmith cdent: really not much about rpc apis going on here
20:25:33 cdent I was attempting to clarify dan’s statement about “maximum version they supprot” < that’s not true for the report client, it asks for a specific version, not a max
20:25:46 dansmith cdent: we're sending a blob to the rpc consumer, which has an inbuilt version that it should use to communicate with an external thing.. it's opaque to _that_ service
20:26:15 dansmith cdent: it's an rpc client, it's tightly controlled by us, versioned interface, and we have more strict versions that are "compatible"
20:26:54 dansmith cdent: it's the maximum semantically, the server doesn't know anything other than what the client said is it's version (i.e. the max for this request)
20:27:14 cdent yes, per request
20:29:56 mriedem i'll say i remember saying we should include the microversion that we used to build the allocation request in the scheduler and pass that down so the conductor makes the same request at the same microversion later
20:30:01 mriedem rather than 'latest'
20:30:17 dansmith mriedem: it's actually always making 1.10 right now, not even latest
20:30:24 dansmith which makes it even worse, IMHO
20:30:39 mriedem but i'd have to lookup where i said this, which might just have been irc...
20:33:10 jaypipes this is the conversation that edleafe and I were having at a whiteboard in the corner.
20:33:29 dansmith yeah, I wasn't over there, sorry about that
20:33:49 dansmith so, let me just summarize where I think we stand
20:34:00 jaypipes and I said it would be ok to have the placement API service handle seamlessly understanding old formats of allocation request body.
20:34:00 dansmith call me out if I'm being biased
20:34:36 mriedem i don't think the placement api should be trying to retrofit the request
20:34:41 mriedem that's weird
20:34:42 dansmith me either
20:34:49 jaypipes I had previously asked edleafe to include the microversion the scheduler created the a-r for in the returned object from select_destinations()
20:35:08 cdent mikal: I assume you’re not with us at the moment, but when you join does this make sense to you: https://review.openstack.org/#/c/509417/
20:36:02 dansmith so I was going to summarize...
20:36:34 dansmith edleafe is of the opinion that this should definitely not be versioned
20:36:40 dansmith I feel it should be
20:37:00 dansmith cdent leans towards versioning
20:37:15 dansmith I think jaypipes is saying he does too (is that right?)
20:37:34 cdent I lean toward not doing the microversino fakery failover server side
20:37:43 cdent I’m agnostic about what happens on the client side
20:37:57 dansmith and mriedem seems to think strict microversion style versioning as well
20:38:01 jaypipes dansmith: I lean towards placing a version field in the object returned from select_destinations(), yes.
20:38:29 dansmith cdent: okay not sure how to avoid the fakery without the client being complicit.. your point is no fakery, whatever that means for the client behavior right?
20:38:37 cdent yes
20:42:43 cfriesen if we don't version it, doesn't that heavily restrict what changes can be made in the future?
20:43:38 dansmith cfriesen: if we don't version it, then anything in the future becomes a archaeology expedition to decide what we can and can't do, yeah
20:43:50 cfriesen like you could add/remove fields but not change the meaning of an existing field
20:43:56 dansmith a version doesn't fix that completely, but it certainly helps draw a box around what we can do and how
20:44:16 dansmith cfriesen: not necessarily even that
20:44:42 dansmith I feel like we're at this point,
20:45:00 dansmith that we said we'd call out, recognize, and avoid rat-holing
20:45:19 jaypipes edleafe: are you still with us?
20:46:03 edleafe I had to walk away for a while. I was getting way too frustrated that this thing that we have said all along was an opaque blob now has versioned information
20:47:07 edleafe No one seems to consider that an a-r is a placement artifact, not a scheduler/conductor thing
20:47:36 edleafe it is created by placement, and is used only by placment, and is never persisted
20:47:42 edleafe it is always "the latest"
20:48:04 edleafe But we want to treat it like it is a versioned data structure that scheduler "can know about"
20:49:21 cdent (what ed just said about “always latest” is part of why I’m agnostic on the client side. It would be fundamentally correct for the report client to send the header as ‘latest’ because the placement service is always its own latest)
20:50:03 dansmith cdent: but the client has to construct the actual request
20:50:20 dansmith right now it puts the user/project in there, and may have to do other things later
20:50:33 dansmith so it can't say latest for the request, it has to say a version
20:50:42 cdent yes, that’s where the notion of opaque blob falls apart
20:50:53 edleafe dansmith: right now it just passes back the a-r to claim. It doesn't build anything
20:50:55 dansmith the other thing is,
20:51:03 edleafe the user/project is in the a-r
20:51:07 dansmith the scheduler cannot communicate with placement at version latest
20:51:14 dansmith and it does have to look at the results of those calls
20:51:14 jaypipes edleafe: not currently it isn't, no.
20:51:40 jaypipes edleafe: the scheduler client's claim_resources() method adds user id and project id to the HTTP request payload.
20:51:41 dansmith so the compute or conductor saying latest _cannot_ be right forever
20:51:46 edleafe jaypipes: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/allocation.py#L77
20:52:25 openstackgerrit Dan Smith proposed openstack/nova master: Revert allocations by migration uuid https://review.openstack.org/498949
20:52:25 openstackgerrit Dan Smith proposed openstack/nova master: Pre-create migration object https://review.openstack.org/498950
20:52:26 openstackgerrit Dan Smith proposed openstack/nova master: Make migration uuid hold allocations for migrating instances https://review.openstack.org/506420
20:52:26 openstackgerrit Dan Smith proposed openstack/nova master: Refactor resource tracker to account for migration allocations https://review.openstack.org/506419
20:52:27 openstackgerrit Dan Smith proposed openstack/nova master: Make live migration hold resources with a migration allocation https://review.openstack.org/507638
20:52:34 jaypipes edleafe: that's the request payload for PUT /allocations, not the format of the allocation_request object that is returned in the GET /allocation_candidates HTTP response.
20:53:24 jaypipes edleafe: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/allocation_candidate.py#L57-L66
20:53:53 jaypipes edleafe: we don't currently add the user and project ID into the allocation_request object in the return from GET /allocation_candidates
20:54:04 jaypipes edleafe: unfortunately. was an oversight on my part.
20:54:18 jaypipes edleafe: I'm sure dansmith at some point told me to put it in there and I just forgot.
20:54:36 dansmith it doesn't matter, because 'latest' is not the version used by the scheduler (in the future when we're doing things correctly and placement is external)
20:56:24 cdent so if scheduler and placement are out of sync, grind
20:56:56 dansmith when placement is external, we must tolerate them being out of sync
20:59:32 edleafe oh, geez, I give up. I missed this: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L154
20:59:48 edleafe Forget everything I said about a-rs being opaque
21:00:00 mriedem yes i remember pointing out late in pike that we needed to update nova-status' check for the required minimum placement microversion to be 1.10 because that's what the scheduler was requesting during claim_resources
21:00:01 edleafe that ship has sailed.
21:00:14 mriedem we really only needed 1.8 for the user_id/project_id thing (i think?)
21:00:56 mriedem and we had to put something in the release notes saying you have to make sure to upgrade placement before scheduler since scheduler requires this new higher microversoin in placement that wasn't available in ocata
21:01:08 edleafe I'm going to finish the stuff I've been trying to work on and then I'll rethink how to change the series to add a versioned allocation_request to the Selection object
21:01:29 mriedem if the pike scheduler was requesting 'latest' to an ocata placement, the request might pass at whatever 'latest' is for placement in ocata, but not what the pike scheduler client actually needs
21:02:02 dansmith edleafe: okay and you caught the bit I said about the selectionlist object potentially being okay if we're going to use it for holding a version right?
21:03:04 edleafe dansmith: yeah, but that's minor
21:03:48 dansmith edleafe: yep, just saying, if you wan to go back to doing it that way, I'm cool with it
21:03:50 cdent edleafe’s link raises another wart doesn’t it? If _move_operation_alloc_request is working in the guts of alloc request, it has to know the version
21:04:21 cdent is that called from only the scheduler, or also in the cells?
21:04:28 cdent (and presumably there are others like it?)
21:04:38 jaypipes cdent: we're trying to get rid of that entirely.
21:04:45 jaypipes cdent: and do the migration owns allocation thing.
21:04:51 edleafe cdent: it will be called from within the cells too
21:04:52 mriedem cdent: it's called from the scheduler and, for the time being, superconductor
21:04:55 cdent yes, but will still inspect don’t we?
21:04:58 mriedem during force live migrate and force evacuate
21:05:01 mriedem where the scheduler is skipped
21:05:06 mriedem edleafe: not within the cells
21:05:18 cdent and in any case that code is pike
21:05:21 mriedem edleafe: oh you mean with alternate hosts yeah
21:05:22 edleafe mriedem: the cell conductor will have to claim
21:05:26 mriedem right right

Earlier   Later