| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-03 | |||
| 13:03:57 | johnthetubaguy | kashyap: sorry this is dragging on, tricky to get the right balance and make it backportable, it turns out | |
| 13:04:28 | kashyap | johnthetubaguy: I first actually raised an exception, and then Matt and Dan came along saying: maybe a warning is less "heavy-handed" | |
| 13:04:31 | stephenfin | johnthetubaguy: Ack on https://review.openstack.org/#/c/552242/ | |
| 13:04:43 | stephenfin | sean-k-mooney[m]: Looking | |
| 13:04:44 | johnthetubaguy | kashyap: yeah, for the backport its a good idea | |
| 13:04:47 | kashyap | johnthetubaguy: Yeah, would like to get this rid of my plate; getting a bit too much of it. But yes, it's a tricky change with right balance | |
| 13:05:03 | kashyap | s/with right balance/that needs the right balance/ | |
| 13:05:20 | johnthetubaguy | stephenfin: thanks, wanted to make sure I didn't just misread it an wait for mikal to wake up after I sign off for the day. | |
| 13:06:50 | kashyap | johnthetubaguy: On your other question about 'host-passthrough': Yes, it is invalid. As libvirt wouldn't mangle anything, and if the CPU has it, libvirt will pass it right through to the guest. | |
| 13:06:51 | jaypipes | efried: I'm happy to do a hangout about the consumer generation today if you need to. | |
| 13:07:08 | efried | jaypipes: Is there email I need to catch up on first? | |
| 13:07:09 | kashyap | johnthetubaguy: To summarize, I'll quickly add: the 'extra_flags = None', before the warning. | |
| 13:07:37 | jaypipes | efried: no, just saying I left things yesterday afternoon in a bit of a "too tired to talk about this right now" state. | |
| 13:08:09 | johnthetubaguy | kashyap: yeah, +1 | |
| 13:08:21 | efried | jaypipes: Okay. The open issue is whether we should change the responses to PUT /alloc/{c} and POST /alloc from 204 No Content to 200 with payload. | |
| 13:08:26 | kashyap | johnthetubaguy: Thanks for the eagle eyes. | |
| 13:10:41 | jaypipes | efried: ok, I will answer that on the spec if that's what you prefer. | |
| 13:11:02 | efried | jaypipes: wfm, unless it needs back-and-forth. | |
| 13:11:12 | efried | cdent: Wouldn't mind an opinion from you as well. | |
| 13:11:35 | efried | cdent: oh, looks like you rendered one. Reading... | |
| 13:12:01 | kashyap | johnthetubaguy: Hmm, isn't it a bit "dirty" that we're quietly setting it to 'extra_flags' to None? Or is the onus is on the Operator to read the docs to see exactly what is valid? | |
| 13:12:33 | johnthetubaguy | kashyap: its not great, but I think we just change this to a raise in the follow on patch that isn't backported | |
| 13:13:25 | johnthetubaguy | kashyap: so it comes from our usual backport rules that mean no new config options or features, etc | |
| 13:17:20 | kashyap | johnthetubaguy: Raise an "error" you mean? | |
| 13:17:26 | kashyap | johnthetubaguy: (In a follow-up.) | |
| 13:17:43 | kashyap | Okay, noted. | |
| 13:30:13 | kashyap | stephenfin: Hmm, what is the alternative then, just don't indent it?: https://review.openstack.org/#/c/534384/22/nova/conf/libvirt.py@539 | |
| 13:30:31 | stephenfin | kashyap: Correct | |
| 13:31:07 | stephenfin | You indent subsequent lines of the same bullet point, but the bullet points themselves should not be indented | |
| 13:32:59 | kashyap | stephenfin: Okido; like it is for the disk cache modes. | |
| 13:33:03 | kashyap | Thanks | |
| 13:36:08 | kashyap | stephenfin: Just a quick note: I'm using the back ticks for the lower-cased ``pcid``, and for the rest, with upper-cased 'PCID' (leaving it as is) | |
| 13:36:15 | kashyap | That fine with you? | |
| 13:36:25 | stephenfin | kashyap: a-ok by me, yup :) | |
| 13:36:59 | kashyap | :-) One upper-case ``PCID`` (because that is talking about valid config value) | |
| 13:37:07 | kashyap | Thanks! | |
| 13:40:54 | kashyap | stephenfin: johnthetubaguy: Before I upload yet-another version, mind having a quick gander: http://paste.openstack.org/show/718287/ | |
| 13:41:12 | kashyap | Or is it hard to read, without colours? (Can understand if so.) | |
| 13:41:38 | openstackgerrit | Mark Goddard proposed openstack/nova master: Include only required fields in ironic node cache https://review.openstack.org/539508 | |
| 13:42:30 | kashyap | Also add the "Related options" section | |
| 13:42:56 | johnthetubaguy | kashyap: probably easier in context, but that looks about right | |
| 13:43:52 | kashyap | Yeah; will post a patch in a min | |
| 13:44:31 | openstackgerrit | Mark Goddard proposed openstack/nova master: Request only instance_uuid in ironic node list https://review.openstack.org/539509 | |
| 13:46:00 | jaypipes | cdent, efried, edleafe: k, commented on the consumer generation spec. | |
| 13:46:07 | efried | jaypipes: ack, thx | |
| 13:46:16 | jaypipes | cdent, efried, edleafe: in short, I don't believe much change is needed from the spec at all. | |
| 13:47:15 | edleafe | jaypipes: thx | |
| 13:47:52 | edleafe | jaypipes: why do you think we shouldn't require sending the consumer gen on a PUT? | |
| 13:47:55 | openstackgerrit | sahid proposed openstack/nova master: network: update pci request spec to handle trusted tags https://review.openstack.org/458820 | |
| 13:47:55 | openstackgerrit | sahid proposed openstack/nova master: network: add command to configure trusted mode for VFs https://review.openstack.org/458513 | |
| 13:47:56 | openstackgerrit | sahid proposed openstack/nova master: libvirt: configure trust mode for vfs https://review.openstack.org/458514 | |
| 13:48:03 | edleafe | jaypipes: How else could it detect a conflict? | |
| 13:48:44 | efried | edleafe: He's suggesting PUT should *always* 409 if the consumer exists. | |
| 13:48:46 | jaypipes | edleafe: see my response. I believe that for PUT /allocations/{consumer_uuid}, if *any* allocations exist for a consumer with {consumer_uuid}, we should simply return 409 Conflict | |
| 13:49:06 | efried | I.e. PUT is only for first-time create; POST is for modify. | |
| 13:49:09 | cdent | that's a huge change jaypipes | |
| 13:49:29 | cdent | POST exists explicitly for multiple consumers | |
| 13:49:39 | efried | jaypipes: Also, we would still need to support create-via-POST. | |
| 13:49:40 | edleafe | yeah, not for a single consumer | |
| 13:49:42 | cdent | for single consumer PUT should remain the locus | |
| 13:50:09 | efried | ...which still needs an answer to "what should the generation be for a heretofore-nonexistent consumer | |
| 13:50:11 | efried | ?" | |
| 13:50:15 | edleafe | The canonical case we are handling is nova and neutron both allocating resources for an instance at roughly the same time | |
| 13:50:41 | edleafe | they both get the current allocations, if any, and make their changes, and PUT them back | |
| 13:50:52 | jaypipes | edleafe: no, they aren't doing it "at the same time". | |
| 13:51:11 | edleafe | One will get back a 409, and it will have to re-GET the allocations, add its allocs, and then PUT that with the new gen | |
| 13:51:31 | jaypipes | edleafe: yes. that is precisely the point. | |
| 13:52:01 | edleafe | jaypipes: that exact example is in the Problem Description of the spec | |
| 13:52:31 | jaypipes | edleafe: but in real life, what will happen is neutron will allocate first for the consumer some networky thing. then the claim_resources() will run in the scheduler for the other compute resources, get a 409 Conflict about consumer already existing, and then it will merge the allocations and do a POST /allocations with the updated/merged allocations list. | |
| 13:52:47 | edleafe | jaypipes: so without a consumer gen in the PUT, how would the second actor ever modify allocations? | |
| 13:53:16 | jaypipes | edleafe: the second actor does GET /allocations/{consumer_uuid}, then POST /allocations | |
| 13:53:24 | edleafe | <shudder> | |
| 13:53:24 | jaypipes | edleafe: as I wrote in the spec response. | |
| 13:53:58 | edleafe | jaypipes: that would be a PATCH, which I proposed but was argued against | |
| 13:54:04 | cdent | jaypipes: why POST instead of PUT? It is in fact (restully speaking) a PUT | |
| 13:54:18 | edleafe | the POST, as cdent mentioned, is for multiple consumers | |
| 13:54:36 | edleafe | The POST actually does a series of PUTs, one for each consumer | |
| 13:55:06 | jaypipes | cdent: because that ship has already sailed? I argued for PATCH instead of POST /allocations to handle multiple consumers, but we went with POST /allocations to handle the "change multiple allocations at once" scenario for migrations/resizes | |
| 13:55:58 | edleafe | jaypipes: sure, but we aren't changing *multiple* | |
| 13:56:08 | cdent | jaypipes: what edleafe just said | |
| 13:56:18 | jaypipes | edleafe: nothing about POST /allocations implies or denotes changing multiple things. | |
| 13:56:29 | cdent | there's only one consumer uuid. and yes it does | |
| 13:56:35 | cdent | post allocations is for multiple consumers | |
| 13:56:49 | jaypipes | cdent: where? | |
| 13:57:02 | efried | POST is allowed to be used for just one, sure. | |
| 13:57:03 | cdent | that's why we created it and how I wrote it | |
| 13:57:09 | edleafe | jaypipes: the handler for POST loops on consumer_uuid. It's pretty clear that it is designed to handle multiple consumers | |
| 13:57:11 | cdent | it _can_ do one, but that's not it's point | |
| 13:57:37 | jaypipes | so tell me folks, what about my proposal *won't work*? | |
| 13:57:55 | edleafe | jaypipes: it also doesn't *add* them to any existing allocations for a consumer; it overwrites them | |
| 13:58:02 | cdent | there's no reason not to have generation on PUT because what we want to do, on PUT, is to write allocations for the consumer uuid in the URI. That's what it is for. | |
| 13:58:06 | cdent | What's wrong with that | |
| 13:58:08 | jaypipes | edleafe: which is what we want here. | |
| 13:58:19 | cdent | that's what PUT means jaypipes | |
| 13:58:25 | cdent | overwrite allocations for consumer X | |
| 13:58:35 | cdent | the generation is acting a the etag | |
| 13:58:44 | jaypipes | cdent: are you talking about HTTP/REST semantics? | |
| 13:58:57 | cdent | I'm talking about what we already have in put, minus the generation | |
| 13:59:22 | cdent | adding a generation to PUT makes more sense than overloading POST when we already have an existing semantic that is correct | |
| 13:59:30 | edleafe | jaypipes: using the POST code, there is still no way to add allocations to a consumer | |