| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-31 | |||
| 17:43:41 | mriedem | dansmith: so i +2ed the bottom change but i think there is a thing in the top change (or two) that we want to do in the bottom | |
| 17:44:28 | dansmith | oh yeah sorry | |
| 17:44:37 | dansmith | was trying to fight through the call brain | |
| 17:44:56 | mriedem | np, i was pushing infra patches during a call this morning, so once those patches land and everything breaks we'll know why | |
| 17:45:17 | mriedem | those patches = my infra patches | |
| 17:45:59 | dansmith | noted | |
| 17:46:03 | openstackgerrit | Dan Smith proposed openstack/nova master: Make put_allocations() retry on concurrent update https://review.openstack.org/516708 | |
| 17:46:04 | openstackgerrit | Dan Smith proposed openstack/nova master: Refactor claim_resources() to use retries decorator https://review.openstack.org/516709 | |
| 17:48:29 | mriedem | jaypipes: easy peasy ^ | |
| 17:48:52 | jaypipes | mriedem: cool. just wrapping up something locally and will look | |
| 18:19:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: do not remove inst_base when volume-backed during resize https://review.openstack.org/516395 | |
| 18:19:37 | mriedem | oomichi: ^ and https://review.openstack.org/516396 in tempest are linked | |
| 18:19:58 | mriedem | verified the failure via the tempest test in the ceph and nfs jobs, now the tempest change depends on the nova fix which will have to go back to pike and ocata | |
| 18:27:41 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add Flavor.description attribute https://review.openstack.org/514371 | |
| 18:27:41 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add FlavorPayload.description for versioned notifications https://review.openstack.org/514782 | |
| 18:27:42 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add microversion to allow setting flavor description https://review.openstack.org/515213 | |
| 18:35:54 | dansmith | man the gate is some kinda jammed up | |
| 18:37:33 | jaypipes | yeah.. | |
| 18:46:30 | mriedem | https://www.youtube.com/watch?v=FcArnepkhv0 ? | |
| 18:57:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement power_off/power_on for the FakeDriver https://review.openstack.org/509935 | |
| 19:03:24 | efried | mriedem Bringing FakeDriver to parity with the PowerVM in-tree driver, eh? | |
| 19:03:30 | efried | Bout time you caught up. | |
| 19:04:13 | mriedem | :) | |
| 19:05:37 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404 | |
| 19:08:39 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP(5): PowerVM driver: ovs vif https://review.openstack.org/422512 | |
| 19:09:49 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404 | |
| 19:10:41 | mriedem | edleafe: cdent: i think i've found a chink in this armor https://review.openstack.org/#/c/508164/ | |
| 19:10:58 | mriedem | and precisely why we wanted to include the microversion in the Selection object that the scheduler used to get allocation_candidates | |
| 19:11:27 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP(5): PowerVM driver: ovs vif https://review.openstack.org/422512 | |
| 19:12:36 | cdent | mriedem: so the assumption I was making there is that that spec (and the related POST one) makes no demands on what the nova-side is doing, and nova can continue doing its thing at whatever microversion it likes, and we can migration the report client to the newer microversion across the board as required | |
| 19:12:57 | cdent | so while we can change the output of allocation_candidates we don’t have to | |
| 19:13:05 | mriedem | i think they should be consistent | |
| 19:13:11 | cdent | but if we do, we should do it soon | |
| 19:13:17 | cdent | s/should/could/ | |
| 19:13:41 | mriedem | the point of GET /allocation_candidates was to return a thing that the caller can re-use for PUT /allocations/{consumer_id}, | |
| 19:13:52 | mriedem | so to make those inconsistent with the new PUT /allocations/{consumer_id} would be confusing | |
| 19:13:57 | cdent | right, which it still can, if you use the right microversion | |
| 19:14:02 | mriedem | especially since nova (on the client side) doesn't do any microversoin negotiation | |
| 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 | edleafe | the whole point of allocation_candidates is defeated | |
| 19:19:46 | cdent | both of your are a bit wrong | |
| 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 | cdent | one missing point | |
| 19:22:17 | mriedem | cool | |
| 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 | cdent | we probably do | |
| 19:23:24 | mriedem | that can wait for now as it's opt-in | |
| 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 | |