| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-05 | |||
| 20:11:39 | cdent | jaypipes: because at the ptg you declared, with dan, that we should argue less, so I did: I capitulated, as requested | |
| 20:12:08 | cdent | you perceive so much, without confirm it, and place me in this position of being a bad guy. It. Is. Not. Me. | |
| 20:12:08 | jaypipes | cdent: like I said... attitude. | |
| 20:12:40 | cdent | you read into that statement some kind of smug bullshit that is not there | |
| 20:12:47 | cdent | it’s just me saying "okay" | |
| 20:12:48 | dansmith | okay, let's pause the personal stuff for a minute | |
| 20:12:58 | cdent | I’m so sick of this. | |
| 20:12:58 | jaypipes | fine with me. | |
| 20:13:01 | dansmith | edleafe: your argument against a version is just that it's not needed, right? | |
| 20:13:25 | dansmith | edleafe: doesn't hurt anything, just isn't strictly required, right? | |
| 20:13:25 | edleafe | for allocation_requests, yes, it's not needed | |
| 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 | dansmith | call me out if I'm being biased | |
| 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: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 | jaypipes | edleafe: not currently it isn't, no. | |
| 20:51:14 | dansmith | and it does have to look at the results of those calls | |
| 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 | |