Earlier  
Posted Nick Remark
#openstack-nova - 2018-04-03
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
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"
14:01:57 jaypipes edleafe: sorry, I'm not suggesting modifying POST /allocations to be additive. I am suggesting to keep it as "overwrite the allcations for all consumers listed in the resquest payload"
14:02:40 efried jaypipes: Cool, so how do you handle the generation for a newly-created consumer in POST?
14:02:57 edleafe jaypipes: so in the example case of two actors creating their subset of allocations to placement, how will the second one ever be able to get theirs in?
14:03:18 jaypipes efried: overwrite.
14:03:31 efried jaypipes: There's nothing to overwrite. The consumer doesn't exist.
14:03:40 jaypipes efried: so it is created?
14:03:50 jaypipes efried: I'm not proposing anything different for that.
14:03:54 efried yes, it is created by the POST (in e.g. the migration scenario).
14:04:24 efried jaypipes: Right, I'm asking, now that we have to specify the generation per consumer chunk in the POST payload: what generatio do we send down for the new consumer?
14:04:39 jaypipes edleafe: the second one gets a 409 Conflict when trying to do the PUT /allocations/{consumer}. It then does a GET /allocations/{consumer} and merges its needed resources into a call to POST /allocations
14:04:41 edleafe for allocations, POST == a set of PUTs
14:04:51 jaypipes efried: we don't.

Earlier   Later