| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-02 | |||
| 20:02:52 | fried_bunny | And, this is a weird behavior, so it needs the shite documented out of it. | |
| 20:02:53 | fried_bunny | okay. | |
| 20:03:03 | fried_bunny | me or you? | |
| 20:03:09 | edleafe | both agents get the current state for the allocations, which is empty | |
| 20:03:12 | edleafe | me | |
| 20:03:31 | edleafe | Both try to PUT their allocations with con_gen=None | |
| 20:03:46 | edleafe | One succeeds, and the generation is then set at 0 | |
| 20:03:55 | edleafe | The second fails with a 409 response. | |
| 20:04:22 | edleafe | The second then refreshes its view, and gets allocations this time, with con_gen=0 | |
| 20:04:43 | edleafe | The second adds its allocations to that, and PUTs with a con_gen of 0 | |
| 20:04:45 | fried_bunny | yeah, the behavior you describe is what needs to happen. | |
| 20:04:52 | fried_bunny | But I don't think that's the way it's coded up. | |
| 20:04:54 | fried_bunny | could be wrong. | |
| 20:04:57 | edleafe | The second succeeds this time, and now con_gen=1 | |
| 20:05:15 | fried_bunny | I think the way it's coded up, you're assuming None means "old microversion, so just increment" | |
| 20:05:32 | edleafe | What I worry about is the second agent posting to an old microversion | |
| 20:05:40 | fried_bunny | That's his fault. | |
| 20:05:45 | edleafe | Which means "preserve the existing behavior" | |
| 20:05:50 | edleafe | which means overwriting | |
| 20:06:50 | fried_bunny | As far as that guy is concerned, the generation field doesn't exist. For the sanity of the first guy, we have to increment it. But if you're trying to use allocations from multiple clients and you're not using the generation-supporting microversion, you're toast anyway. | |
| 20:06:58 | edleafe | It's (very) possible that I missed a case in the code. But the approach is what was designed | |
| 20:07:45 | edleafe | agree that even if there is a post with an old microversion, the generation should be incremented | |
| 20:08:36 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053 | |
| 20:08:52 | fried_bunny | edleafe: I'm going through the spec again, but I don't recall this case being covered. | |
| 20:09:09 | fried_bunny | It needs to be described explicitly, because it's weird. | |
| 20:10:09 | fried_bunny | "Here's a new microversion where you have to send generation to ensure consistency. Oh, except the first time, when you have to send None (or leave it empty??). But if you do that when the consumer already exists, you'll fail. | |
| 20:10:11 | fried_bunny | " | |
| 20:11:46 | edleafe | it might not be in the spec | |
| 20:12:23 | edleafe | But it's not that weird. Standard practice is to get existing allocations before adding any new ones | |
| 20:12:49 | edleafe | I know it was discussed | |
| 20:12:52 | fried_bunny | okay, so what will you return? | |
| 20:12:55 | fried_bunny | in that first one? | |
| 20:13:00 | fried_bunny | Right now you get {} | |
| 20:13:04 | edleafe | but those discussions happened in several places at various times | |
| 20:13:12 | fried_bunny | Will you return { "consumer_generation": None } ? | |
| 20:13:43 | fried_bunny | or am I responsible for doing result.get('consumer_generation', None) ? | |
| 20:13:44 | edleafe | If there are no allocations for a consumer, then there is no consumer | |
| 20:14:19 | fried_bunny | Cool. So when I do my PUT, do I say "consumer_generation": None, or do I exclude the "consumer_generation" field entirely? | |
| 20:14:50 | edleafe | If you're PUTting to the new microversion, you would have to pass the generation, so the former | |
| 20:15:10 | fried_bunny | In which case the schema needs to tolerate [int or null] | |
| 20:15:29 | edleafe | guess it will have to :) | |
| 20:16:04 | edleafe | I'm just starting the patch that adds the microversion. I think these cases will make for excellent functional tests | |
| 20:16:24 | fried_bunny | I'm gonna mark up the spec review. | |
| 20:16:58 | edleafe | good idea | |
| 20:18:59 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 20:21:12 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM: Cold Migrate & Resize https://review.openstack.org/553583 | |
| 20:21:12 | melwitt | fried_bunny: replied to your comments. this is my remaining open question https://review.openstack.org/#/c/533821/25/nova/tests/functional/api/openstack/placement/test_report_client.py@812 | |
| 20:21:27 | fried_bunny | melwitt: Roger that, looking... | |
| 20:21:35 | fried_bunny | (thought you were bunnying today, melwitt) | |
| 20:22:02 | melwitt | unfortunately no | |
| 20:26:09 | fried_bunny | edleafe: https://review.openstack.org/#/c/556971/ done been marked up. | |
| 20:31:24 | fried_bunny | melwitt: Responded. | |
| 20:37:41 | fried_bunny | edleafe: tbc, you're not intending to do the microversion bump in https://review.openstack.org/#/c/557959/ right? | |
| 20:38:26 | edleafe | fried_bunny: yeah, it will be the next one in that series | |
| 20:38:41 | fried_bunny | rgr | |
| 20:39:54 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 20:41:29 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM: Cold Migrate & Resize https://review.openstack.org/553583 | |
| 20:45:54 | fried_bunny | edleafe: Hum, we need to change the POST /allocations and PUT /allocations/{c} responses to 200 with payload. | |
| 20:46:03 | fried_bunny | they're 204 right now. | |
| 20:49:00 | fried_bunny | starting to look like a mriedem commentary - three separate comment-leavings in a row on the same patch. | |
| 20:56:13 | edleafe | fried_bunny: not sure I get the reasoning behind changing the 204 from PUT/POST | |
| 20:57:08 | fried_bunny | edleafe: It's the same reason we did http://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/generation-from-create-provider.html | |
| 20:58:21 | fried_bunny | edleafe: So clients don't have to do GET, PUT, GET, PUT -- can just do PUT, PUT instead. | |
| 20:58:37 | edleafe | But for a consumer? RPs are long-lived and frequently changed. Consumers rarely get changed, and I can't think of a use case where you would create allocs, and then create some more right away | |
| 21:01:28 | fried_bunny | eh? | |
| 21:01:42 | fried_bunny | Hold on, processing that... | |
| 21:01:43 | melwitt | dansmith: is this your comment on L45? https://etherpad.openstack.org/p/nova-runways-rocky if all non-WIP changes are merged, I think we can bump the blueprint out of the runway and put the next in line in the runway. does that sound cool? | |
| 21:02:08 | dansmith | melwitt: bope not mine | |
| 21:02:10 | dansmith | er, nope | |
| 21:02:32 | fried_bunny | edleafe: But instinctive reaction is: We need to avoid focusing on Nova as the only consumer (again/still). | |
| 21:02:33 | dansmith | melwitt: but yes, the WIP bits are waiting on pieces of placement that don't exist yet, so it's "done" | |
| 21:02:56 | melwitt | oh, guh. I'm really bad at remembering what author color people are | |
| 21:03:08 | fried_bunny | it changes, too :( | |
| 21:03:11 | dansmith | melwitt: maybe want to, at least initially, copy things out of a runway to a log area and put some comments about how it went? | |
| 21:03:19 | edleafe | fried_bunny: I'm not thinking of Nova | |
| 21:03:35 | melwitt | dansmith: sounds like a good idea | |
| 21:03:36 | dansmith | melwitt: for that set, very little review was had for about a month and then it got review, iteration, and then merged quickly whilst in that slot | |
| 21:03:41 | fried_bunny | dansmith, melwitt: ++ to that | |
| 21:03:52 | fried_bunny | (the keeping-logs-of-runways) | |
| 21:04:10 | edleafe | for any placement user: if you are making allocations, you would GET the current state, modify those allocations to add/remove what is needed, and then PUT that back | |
| 21:04:36 | fried_bunny | edleafe: And cache that, so that next time, you don't have to reGET. | |
| 21:04:46 | fried_bunny | exactly like we're doing for RPs. | |
| 21:05:01 | edleafe | fried_bunny: you'll have to come up with a scenario where that is at all plausible | |
| 21:05:06 | fried_bunny | resize | |
| 21:05:10 | fried_bunny | and | |
| 21:05:14 | fried_bunny | symmetry with the RP API | |
| 21:05:16 | fried_bunny | :P | |
| 21:05:45 | melwitt | jackie-truong: hi, your blueprint https://blueprints.launchpad.net/nova/+spec/nova-validate-certificates is next in line for a review runway that we're looking to fill. is now a good time to iterate quickly on code review for the next two weeks in a runway? | |
| 21:05:47 | fried_bunny | Is it possible to GET before PUT every time? Sure. But so easy to avoid, so why? | |
| 21:05:59 | edleafe | with resize you'd only change the consumer once | |
| 21:06:16 | edleafe | Symmetry is only important if the things are similar enough | |
| 21:06:20 | fried_bunny | edleafe: per resize | |
| 21:06:45 | edleafe | ?? | |
| 21:06:50 | fried_bunny | edleafe: or maybe not. Maybe we're passing the allocation around to cinder to change storage allocs, cyborg to change dev allocs, neutron to change net allocs.... | |
| 21:07:13 | fried_bunny | So it's half a dozen times per spawn, per resize, per whatever. | |
| 21:07:38 | edleafe | fried_bunny: that's the case for adding a generation. These will be presumably done in parallel, so it is racy by desing | |
| 21:07:42 | edleafe | design, even | |
| 21:07:46 | fried_bunny | But what's the resistance? Just that it's more code? | |
| 21:08:08 | edleafe | no, because it doesn't fit how allocations should work | |
| 21:08:19 | fried_bunny | well, that I disagree with. | |