| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-03 | |||
| 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: add command to configure trusted mode for VFs https://review.openstack.org/458513 | |
| 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: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 | jaypipes | edleafe: as I wrote in the spec response. | |
| 13:53:24 | edleafe | <shudder> | |
| 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 | |
| 13:59:54 | jaypipes | edleafe: ? sure there is... you just list a set of allocations for a consumer. | |
| 13:59:55 | cdent | I wonder if maybe I'm not understanding jay's suggestion properly | |
| 14:00:16 | efried | FWIW, I agree that we should add generation to PUT, even if not doing so would allow us to sidestep the question of what generation should be specified for creation, which it doesn't. | |
| 14:00:21 | cdent | how is post to /allocations different from put to /allocations/consumer_uuid (with a generation)? | |
| 14:00:52 | efried | jaypipes: POST still needs to be *able* to be used to *create*. Because migration scenario. So we still have to address the question of "what generation do we send down for a consumer that doesn't yet exist?" | |
| 14:00:56 | edleafe | jaypipes: changing the POST code to make it additive would break migrations | |
| 14:01:11 | cdent | efried: yeah, as I said on the spec the none thing is meh-ish, but seems kinda gotta with the model being considered | |
| 14:01:29 | efried | IMO there's no meh about it. It's ick-but-necessary. | |
| 14:01:41 | cdent | isn't that what meh means? | |
| 14:01:50 | cdent | resignation in the face of crushing reality | |
| 14:01:56 | efried | To me "meh" means "could do with or without it" | |