| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-31 | |||
| 19:14:14 | cdent | where “right” is the same microversion it is currently using | |
| 19:14:34 | mriedem | we tend to hard-code to the latest in the client side nova code | |
| 19:14:37 | mriedem | which is bad, i realize | |
| 19:14:39 | cdent | it’s _good_ that it doesn’t do negotiation, instead its explicit, right? | |
| 19:14:44 | cdent | no, it’s good | |
| 19:14:46 | mriedem | well, in this case, | |
| 19:14:52 | cdent | it’s always good | |
| 19:15:00 | cdent | it’s a thing we’ve realized during the many microversion discussions | |
| 19:15:02 | mriedem | the client (scheduler/cell conductor) would need to know, do i PUT at version 1.12 or 1.8? | |
| 19:15:13 | cdent | s/discussions/therepy sessions/ | |
| 19:15:40 | edleafe | mriedem: how would the client ever get anything other than "latest"? | |
| 19:15:45 | cdent | it would carry on doing whatever it is doing now, until we change it | |
| 19:15:54 | cdent | edleafe: because it always asks for a specific version | |
| 19:16:33 | cdent | mriedem: note, I’m not saying we should fix allocation_candidates (we probably should), just saying we’re not forced | |
| 19:16:45 | mriedem | ok so scheduler does a GET /allocation_candidates at 1.10 and stores the allocation_request in the Selection object which goes to the cell conductor, | |
| 19:16:59 | mriedem | the cell conductor makes a PUT /allocations/{consumer_id} request at 1.10, things should be OK | |
| 19:17:01 | mriedem | because it's old format | |
| 19:17:03 | cdent | right | |
| 19:17:27 | mriedem | now let's say in the future, we add something to GET /allocation_candidates that the scheduler needs, so it starts requesting 1.14 | |
| 19:17:33 | mriedem | we shove 1.14 in the Selection object, | |
| 19:17:49 | mriedem | the cell conductor blindly does a PUT /allocations/{consumer_id} at 1.14 which is now the wrong format | |
| 19:17:53 | mriedem | and you get a 400 back | |
| 19:18:05 | cdent | except it would potentially die before then anyway | |
| 19:18:08 | edleafe | wait - why is 1.14 the wrong format for the PUT? | |
| 19:18:21 | mriedem | edleafe: because PUT /allocations/{consumer_id} format is going to change | |
| 19:18:26 | mriedem | per https://review.openstack.org/#/c/508164/5 | |
| 19:18:39 | cdent | as it currently has code which modifies the allocation data structures | |
| 19:19:11 | edleafe | So requesting microversion 1.14 will return an alloc_cand that can't be PUT with 1.14? | |
| 19:19:12 | cdent | until we change the report client code that cooks allocations client side, we can’t up the microversion at which allocation_candidates is requested | |
| 19:19:16 | mriedem | cdent: die because the scheduler is assuming a certain format you mean right? | |
| 19:19:21 | cdent | yes | |
| 19:19:22 | mriedem | edleafe: correct | |
| 19:19:34 | edleafe | that's completely messed up | |
| 19:19:46 | cdent | both of your are a bit wrong | |
| 19:19:46 | edleafe | the whole point of allocation_candidates is defeated | |
| 19:19:54 | mriedem | cdent: you have a fair point | |
| 19:19:59 | mriedem | so i can concede that on the scheduler side, | |
| 19:20:08 | cdent | if we bump the microverson on allocation_candidates we _have_ to change the format of the allocations | |
| 19:20:14 | mriedem | but in general i think we need allocation_requests in GET /allocation_candidates to match PUT /allocations at the same microversion | |
| 19:20:25 | cdent | by which I mean “at which allocation_candidates is requested" | |
| 19:20:41 | cdent | yes, I agree, but it wasn’t specificied in that spec because I was trying to be limited | |
| 19:20:56 | mriedem | yeah i understand, i think it will save us some headache later though | |
| 19:20:59 | mriedem | to just do it now | |
| 19:21:04 | edleafe | the only reason allocation_candidates exist is to be immediately PUT back in a claim | |
| 19:21:12 | mriedem | edleafe: yes, that's my point in why we should be consistent | |
| 19:21:20 | edleafe | those processses should always be in sync | |
| 19:22:09 | mriedem | ok so i think we're all basically agreeing | |
| 19:22:14 | cdent | yes | |
| 19:22:17 | mriedem | cool | |
| 19:22:17 | cdent | one missing point | |
| 19:23:01 | cdent | do you think we need to update the client side too, or can that wait (that is can we pin report client to not go past 1.10 when get /allocation_cand for now)? | |
| 19:23:24 | mriedem | that can wait for now as it's opt-in | |
| 19:23:24 | cdent | we probably do | |
| 19:23:33 | cdent | since we want to get rid of the migration uuid races | |
| 19:23:42 | cdent | although, again, we don’t _have_ to | |
| 19:23:53 | mriedem | well, | |
| 19:24:02 | mriedem | conductor is going to be calling POST /allocations for the swap, | |
| 19:24:14 | mriedem | scheduler will still call GET /allocation_candidates at 1.10 for now | |
| 19:24:45 | cdent | right, but when calling POST /allocations for the swap, we are using allocations that are dict-ish | |
| 19:24:55 | cdent | and the current cooking code is list ish | |
| 19:25:01 | jaypipes | efried: ok, almost ready to push my "refactor AllocationCandidates._get_by_filters() mega-method" series. I think you're gonna dig it. | |
| 19:25:02 | cdent | many cascades | |
| 19:25:45 | mriedem | when the conductor code that starts calling POST /allocations does that thing, it will have to know the microversoin and request format to use for that thing, | |
| 19:25:56 | mriedem | but i don't think that's tied to GET /allocation_candidates, | |
| 19:25:58 | cdent | yes | |
| 19:26:01 | mriedem | it's maybe for GET /allocations | |
| 19:26:11 | mriedem | but still, gonna have to know what we're requesting when we do POST /allocations | |
| 19:26:20 | efried | jaypipes Looking forward to it. For my part, I'm about to push a new rev of the placement side of the granular parsing. Much improved, with y'all's suggestions. | |
| 19:26:28 | mriedem | i think the conductor code will be building that request itself, not taking it from GET /allocation_candidates | |
| 19:26:48 | cdent | if that’s the case then cool | |
| 19:27:10 | mriedem | cdent: yeah i think we'll be ok with the migration allocation swap stuff | |
| 19:27:20 | mriedem | it's the scheduler -> cell via reschedule stuff that is hairy | |
| 19:27:56 | mriedem | cdent: ok so you're going to tweak the spec and such? i've got to run for the elementary school halloween parade of joyfulness | |
| 19:28:52 | cdent | mriedem: not tonight, but should be able to squeeze something out before I catch a plane tomorrow. if I get lost and can’t decide what to do, I’ll respond to your comments with a “huh?” | |
| 19:30:38 | mriedem | cdent: sure, wfm | |
| 19:30:39 | mriedem | thanks | |
| 19:30:57 | cdent | u r welcome | |
| 19:36:50 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: Parse granular resources & traits https://review.openstack.org/514091 | |
| 19:36:51 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Numbered groupings to GET /allocation_candidates https://review.openstack.org/514092 | |
| 19:36:56 | efried | jaypipes cdent alex_xu ^ | |
| 19:37:31 | efried | and gibi too ^ | |
| 19:37:43 | edleafe | mriedem: bouncing between meeting and IRC. What did you mean by "i think the conductor code will be building that request itself, not taking it from GET /allocation_candidates" | |
| 19:43:05 | cdent | edleafe: that was when managing the doubling of allocations for migrations etc | |
| 19:45:17 | edleafe | cdent: ah, that bit | |
| 20:03:29 | efried | What be wrong with the zuul? | |
| 20:07:03 | openstackgerrit | Jay Pipes proposed openstack/nova master: begin refactor AllocCandidates._get_by_filters() https://review.openstack.org/516778 | |
| 20:07:04 | openstackgerrit | Jay Pipes proposed openstack/nova master: build alloc request resources for shared resources https://review.openstack.org/516780 | |
| 20:07:04 | openstackgerrit | Jay Pipes proposed openstack/nova master: build ProviderSummary objects in sep function https://review.openstack.org/516779 | |
| 20:07:05 | openstackgerrit | Jay Pipes proposed openstack/nova master: finish refactor AllocCandidates._get_by_filters() https://review.openstack.org/516782 | |
| 20:07:05 | openstackgerrit | Jay Pipes proposed openstack/nova master: create allocation request for single provider https://review.openstack.org/516781 | |
| 20:07:14 | jaypipes | efried, alex_xu: ^^ | |
| 20:07:32 | efried | jaypipes Sweet. Trade ya. | |
| 20:07:53 | jaypipes | efried: on it. | |
| 20:13:00 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: Contributor doc microversion checklist https://review.openstack.org/516783 | |
| 20:13:11 | efried | mriedem Updates we talked about the other day ^ | |
| 20:15:56 | efried | mriedem Is a similar update needed at https://docs.openstack.org/nova/pike/contributor/microversions.html#other-necessary-changes ? | |
| 20:34:39 | efried | jaypipes Is this series going to have any additional UT? | |
| 20:34:51 | jaypipes | efried: no sir. | |
| 20:34:58 | efried | or functional, more to the point. | |
| 20:35:02 | jaypipes | efried: none of this is unit-tested. | |