Posted	Nick	Remark
#openstack-nova - 2017-10-05
19:52:44	dansmith	edleafe: that has nothing to do with anything
19:52:48	dansmith	edleafe: services are long-lived
19:53:01	dansmith	edleafe: I upgrade placement to rocky a month before I upgrade my nova
19:53:05	edleafe	They are obtained from placement, and returned unchanged
19:53:15	dansmith	edleafe: the scheduler has to tell placement what it understands
19:53:52	edleafe	but a change to the format of an allocation_request does not affect the scheduler
19:53:57	edleafe	it's an opaque blob
19:53:59	dansmith	edleafe: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L270
19:54:06	dansmith	that stops working when placement is kicked out
19:54:12	dansmith	it's not even really right currently, but we have't done it
19:54:24	dansmith	if placement were not lockstep with scheduler,
19:54:32	dansmith	then we'd break if we don't tell placement what we understand
19:54:36	dansmith	this is the whole point of microversions
19:54:46	edleafe	dansmith: for everything else between the scheduler and placement I agree
19:55:25	edleafe	but an a-r is an opaque blob, and will *always* be in the format that placement wants
19:59:15	sdague	mriedem: you want to land this backport - https://review.openstack.org/#/c/509774/ ?
20:00:57	dansmith	edleafe: a-r cannot be an opaque blob to the scheduler because it has to interpret the results to weigh things
20:01:15	dansmith	edleafe: that means that scheduler and placement have to agree on a format between them in order for the scheduler to reliably do its thing
20:01:42	dansmith	if the scheduler is behind placement and received an older formatted thing,
20:01:43	cdent	it uses the providers half of the tuple to weigh, not the a-r?
20:02:02	edleafe	cdent: correct
20:02:03	dansmith	but compute throws that at placement either at its version or "no version assume latest" it may be wrong
20:02:03	cdent	nm, I guess it has to scan the a-r
20:02:29	edleafe	why?
20:02:29	dansmith	cdent: doesn't matter if it did.. surely we're not suggesting having a versioned document where one part of it is "may be newer, don't look behind this curtain"
20:02:59	cdent	we do have a section that we’re claiming is “don’t look behind this curtain”
20:03:11	dansmith	but that's crazy
20:03:15	mriedem	penick: have you seen this thread? http://lists.openstack.org/pipermail/openstack-dev/2017-September/122904.html
20:03:17	dansmith	that's not how people work with APIs
20:03:25	mriedem	penick: rybridges: aren't you guys doing something similar?
20:03:41	cdent	edleafe: I may be wrong. I was thinking that during the process of choosing which of the a-rs to use, you have to know the rp ids
20:03:43	openstackgerrit	Eric Fried proposed openstack/nova master: Use ksa adapter for cinder client https://review.openstack.org/509892
20:03:57	cdent	dansmith: I agree with you, for the most part, I’m just reporting on “things we say"
20:04:05	penick	mriedem indeed we are, I think he asked about it in channel last week. I've been meaning to reply to the thread
20:04:07	cdent	“opaque"
20:04:07	dansmith	cdent: yep, understand
20:04:17	mriedem	penick: ah cool, on a call with him now
20:04:23	mriedem	this is over my head
20:04:40	dansmith	cdent: the only opaqueness I think we need is just between scheduler and the things downstream of it which need to throw it back at placement
20:04:56	cdent	(under it all I find the allocation_candidates thing way overly-specific and not very api-like, but it is is what we’ve reached as a workable solution when many other things would not, so… hard to keep my guns)
20:05:03	dansmith	having a big chunk of data that looks useful being exposed to the client and told that there be dragons within is not a good plan, IMHO
20:05:06	openstackgerrit	Eric Fried proposed openstack/nova master: Use ksa adapter for neutron client https://review.openstack.org/509892
20:05:44	edleafe	dansmith: having a big chunk of data being passed around is not a good plan IMO either, but this is what we are working with
20:06:33	edleafe	cdent: we do key on rp_uuid from the a-r, which was another design compromise
20:06:35	dansmith	how does the jsonschema validation work if you have a change in the a-r? does it just ignore a subtree of something and validate it separately or something?
20:06:40	penick	mriedem I'll reply to the ML today or tonight.. he notes that vendordata doesn't allow you to pass parameters, but I think that's something that can be addressed. Or he can write a vendordata driver
20:06:52	cdent	edleafe: so it isn’t opaque
20:07:02	edleafe	cdent: which is why for a given host there may be several a-rs, and we just take the first one for claiming
20:07:06	mriedem	penick: nova passes some stuff to the vendordata service
20:07:35	cdent	“passes some stuff” is the new api guideline
20:07:43	cdent	what should my api do? “pass some stuff”
20:07:52	edleafe	cdent: {$rp_uuid: <opaque>}
20:08:08	mriedem	penick: https://github.com/openstack/nova/blob/master/nova/api/metadata/vendordata_dynamic.py#L77
20:09:06	cdent	edleafe: that’s not what an a-r looks like now: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/allocation_candidate.py#L55-L68
20:09:10	jaypipes	cdent: the whole "I told you guys that this all sucks but don't have a better idea about how to solve this problem" attitude gets really old.
20:09:15	cdent	and we’re planning to change it with the new format
20:09:15	penick	Is he right about plaintext http only? That seems an easy fix
20:09:25	cdent	jaypipes: it gets especially old when you think that’s what we are doing and we aren’t actually
20:09:53	jaypipes	cdent: sure seems like it.
20:09:56	penick	er, mriedem: ahah, thanks. Is he right about plaintext only? that'd be an easy fix. I'll reply to him on the ML
20:10:26	cdent	jaypipes: my apologies then, but I think you’ll find that this started because I pointed dan at some clarifying info about questions he reaised on a review
20:10:36	cdent	since then we’ve been talking, that’s all
20:10:37	penick	mriedem nm I see the ssl bit in the code
20:10:48	mriedem	penick: yeah we ust send some shit over json in the request and we hope the vendordata service finds it useful
20:11:05	edleafe	jaypipes: having to have this conversation about opaqueness gets pretty old, too
20:11:05	mriedem	and yeah we do the normal ksa stuff
20:11:08	mriedem	for the service user
20:11:12	jaypipes	cdent: ""microversion thing way too fluid” was my concern too, but, like I said, I decided to capitulate" <-- attitude.
20:11:39	cdent	jaypipes: because at the ptg you declared, with dan, that we should argue less, so I did: I capitulated, as requested
20:12:08	cdent	you perceive so much, without confirm it, and place me in this position of being a bad guy. It. Is. Not. Me.
20:12:08	jaypipes	cdent: like I said... attitude.
20:12:40	cdent	you read into that statement some kind of smug bullshit that is not there
20:12:47	cdent	it’s just me saying "okay"
20:12:48	dansmith	okay, let's pause the personal stuff for a minute
20:12:58	cdent	I’m so sick of this.
20:12:58	jaypipes	fine with me.
20:13:01	dansmith	edleafe: your argument against a version is just that it's not needed, right?
20:13:25	dansmith	edleafe: doesn't hurt anything, just isn't strictly required, right?
20:13:25	edleafe	for allocation_requests, yes, it's not needed
20:13:50	edleafe	no, it over-engineers things, so it does hurt
20:13:54	dansmith	edleafe: okay, so I know I don't have lots of karma to burn with you, but we could just put it in there, call it the dan_is_dumb field, and move forward without costing much else, right?
20:14:32	dansmith	a single field over-engineers?
20:14:41	edleafe	dansmith: you do realize that it's a lot more than just adding a field, right?
20:15:14	edleafe	it's now having to do version checking in placement for a thing that will always be the current version
20:15:30	dansmith	we damn sure better be doing that version checking in placement anyway
20:15:40	edleafe	it's changing the placement API to add the version
20:15:51	dansmith	eh?
20:16:08	dansmith	this is protocol stuff. the envelope.. it's built into every call we make to placement if we pass version=something in report client, no?
20:16:15	edleafe	where is that a-r version coming from?
20:17:17	jaypipes	edleafe: it's currently hard-coded in the report client's claim_resources() method. That would need to be updated to pass an optional kwarg for the microversion override if received in the select_destinations() returned objects.
20:17:44	jaypipes	i.e. claim_resources(..., version=$what_i_got_from_scheduler)
20:18:05	dansmith	r = self.put(url, payload, version='1.10')
20:18:06	dansmith	exactly
20:18:13	jaypipes	we would want to have the scheduler package up the user and project ID into the allocation request blob it sends to.
20:18:24	jaypipes	to the caller of the select_destinations() method
20:19:18	edleafe	jaypipes: So we have an old scheduler, old conductor, and new placement. Are you saying that placement should modify the structure of the a-r part of the response based on the old scheduler/conductor?
20:19:23	jaypipes	also, instead of adding a separate version attribute to the Selection object, we could return a SelectionList object that had two fields, version and selections so we don't have to repeat the version field over and over again.
20:19:39	dansmith	edleafe: that's what versioning an API is all about
20:20:03	dansmith	jaypipes: I said no list object because there was no need. this would be a need, so that's fine with me, if that's desired
20:20:14	dansmith	when I argued against it, there was no such extra property

Earlier   Later