Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-05
20:13:50 edleafe no, it over-engineers things, so it does hurt
20:13:54 dansmith edleafe: okay, so I know I don't have lots of karma to burn with you, but we could just put it in there, call it the dan_is_dumb field, and move forward without costing much else, right?
20:14:32 dansmith a single field over-engineers?
20:14:41 edleafe dansmith: you do realize that it's a lot more than just adding a field, right?
20:15:14 edleafe it's now having to do version checking in placement for a thing that will always be the current version
20:15:30 dansmith we damn sure better be doing that version checking in placement anyway
20:15:40 edleafe it's changing the placement API to add the version
20:15:51 dansmith eh?
20:16:08 dansmith this is protocol stuff. the envelope.. it's built into every call we make to placement if we pass version=something in report client, no?
20:16:15 edleafe where is that a-r version coming from?
20:17:17 jaypipes edleafe: it's currently hard-coded in the report client's claim_resources() method. That would need to be updated to pass an optional kwarg for the microversion override if received in the select_destinations() returned objects.
20:17:44 jaypipes i.e. claim_resources(..., version=$what_i_got_from_scheduler)
20:18:05 dansmith r = self.put(url, payload, version='1.10')
20:18:06 dansmith exactly
20:18:13 jaypipes we would want to have the scheduler package up the user and project ID into the allocation request blob it sends to.
20:18:24 jaypipes to the caller of the select_destinations() method
20:19:18 edleafe jaypipes: So we have an old scheduler, old conductor, and new placement. Are you saying that placement should modify the structure of the a-r part of the response based on the old scheduler/conductor?
20:19:23 jaypipes also, instead of adding a separate version attribute to the Selection object, we could return a SelectionList object that had two fields, version and selections so we don't have to repeat the version field over and over again.
20:19:39 dansmith edleafe: that's what versioning an API is all about
20:20:03 dansmith jaypipes: I said no list object because there was no need. this would be a need, so that's fine with me, if that's desired
20:20:14 dansmith when I argued against it, there was no such extra property
20:21:34 edleafe dansmith: I know what versioning an API is about.
20:22:04 edleafe dansmith: the issue is whether the a-r should be dependent on placement, or on the things that call placement
20:22:29 dansmith anything returned to the client is done so at the maximum version they both support
20:23:57 cdent are we talking rpc api or http api here, because http api, it is the version the client asked for
20:24:10 cdent which we have been generally controlling per request for nova->placement
20:24:14 jaypipes cdent: two different clients here.
20:24:30 edleafe the clients support receiving a-rs. That's all the versioning we need. If they can get an a-r blob, that's sufficient
20:24:39 edleafe http
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.

Earlier   Later