Posted	Nick	Remark
#openstack-nova - 2018-03-12
16:13:00	bauzas	that's where I see the difference
16:13:06	dansmith	bauzas: compute does not own cpu flag traits
16:13:10	bauzas	meaning that some operator could want to hide a trait if they'd like
16:13:11	jaypipes	dansmith: right, and aggregate stuff complicates that a bit, too.
16:13:27	dansmith	bauzas: it does the setting based on what virt says, but it doesn't own the decision about which ones to set
16:14:08	efried	++
16:14:14	bauzas	dansmith: the problem I see with that approach is that we have different traits
16:14:14	dansmith	jaypipes: yup, but the ratio thing is pretty straightforward I think.. we just decide what we're going to do at first startup, and what to do on subsequent ones
16:14:31	bauzas	dansmith: some that we don't want the operator to modify, some we accept that
16:14:36	jaypipes	dansmith: ack. and I'm cool with your default_xxx_allocation_ratio suggestion.
16:14:47	bauzas	anyway, it's a set, right
16:14:47	bauzas	?
16:14:51	dansmith	bauzas: I don't understand what you're saying
16:15:09	bauzas	so, it's either an additive or a removal operation for a trait
16:15:26	jaypipes	dansmith: do you think the default_xxx_allocation_ratio solution warrants a spec?
16:15:35	dansmith	jaypipes: I bet mriedem does
16:15:45	jaypipes	yeah, makes sense
16:15:51	jaypipes	I'll hack one together quick.
16:16:01	jaypipes	scoped only to this narrow use case.
16:16:09	dansmith	maybe it would help if I wrote some example code for how I think we should handle the trait thing?
16:16:14	bauzas	dansmith: the point I'm saying is that if we don't accept operators to remove a trait for a specific usecase (here a CPU feature), we shouldn't accept that for *any* trait too
16:16:16	dansmith	between virt traits, compute traits, and operator-set traits?
16:16:27	dansmith	bauzas: that makes no sense to me
16:16:33	jaypipes	dansmith: sure, that would help
16:16:35	efried	It seems to be a requirement for the admin to be able to set/unset at least *certain* traits. And I personally don't want to get into a situation where we treat some traits differently from others. Having to figure out and enforce which is which == nightmare. So we need the admin-override mechanism to be generic. And if they want to remove something that clearly should be owned by the virt driver (like a CPU trait), th
16:16:39	jaypipes	(the sample code)
16:16:47	dansmith	bauzas: virt can be authoritative for TRAIT_VMX and not be involved in TRAIT_SOMETHINGELSE
16:17:18	bauzas	dansmith: so the compute would know which resource is authoritative for a trait, then ?
16:17:29	jaypipes	efried: did you cut off that last sentence?
16:17:33	efried	dansmith: IMO that's crazypants, to try to make lists of traits that can or can't be controlled by the admin, or are "owned" by virt vs. not, etc.
16:17:35	dansmith	bauzas: does't need to be by any set of complex rules, IMHO
16:17:47	dansmith	efried: I don't think we need a set of rules
16:18:05	dansmith	efried: I think the virt driver and compute services can just be authoritative for the ones they need to care about, and any others will be left alone
16:18:09	edleafe	efried: if the virt driver sets traits A,B,C, and the admin sets traits X,Y,Z, there is no problem, right? It's only when the admin wants to set/clear one of A, B, or C
16:18:15	bauzas	dansmith: I mean, how the compute can know whether it can accept an operator's defined list of traits to remove if one of those traits is TRAIT_VMX for example ?
16:18:16	jaypipes	efried: dansmith is saying "whatever the virt driver returns as traits should be authoritative for *those* traits"
16:18:25	dansmith	jaypipes: exactly
16:18:27	efried	I don't see how that can work in practice.
16:18:30	edleafe	jaypipes: yes
16:18:37	efried	Thinking about how the virt driver is going to discover traits...
16:19:07	efried	In the general case, it gets its info from the platform and translates to trait strings.
16:19:17	dansmith	efried: the virt driver is write-only for traits, right? it doesn't care what traits are currently set, it only generates a list of traits that *should* be set based on talking to the hypervisor
16:19:45	efried	Yes, but we're talking about the virt driver having to *know* about things it simply doesn't get from the platform as still being under its control.
16:20:20	dansmith	I'm not sure I understand what you mean, but.. I don't think I agree
16:20:23	bauzas	dansmith: so, if we accept some mechanism for the operator to remove some trait, it would just be a conditional in the compute service that would say 'don't touch what the virt driver returns", right?
16:20:24	jaypipes	efried: but dan isn't suggesting that the virt driver overwrite the complete set of traits, only that it will ensure that some traits are added to the provider
16:20:32	efried	Especially if some new CPU trait (or whatever) comes into existence that the platform didn't know about, but is still expected to be the authority on.
16:20:48	bauzas	can I backup for a second ?
16:20:56	jaypipes	efried: and if the admin added such a trait, the virt driver wouldn't remove it.
16:21:13	bauzas	after all, do we need to provide some mechanism for an operator to *remove* a trait from what the compute service will report ?
16:21:24	dansmith	efried: are you concerned about the operator adding that new cpu trait to the compute node thinking it will magically make it work?
16:21:33	dansmith	because that's not a problem we need to solve, IMHO
16:21:35	bauzas	if we only allow the operator to give a list of human-defined traits, I don't see the problem
16:21:36	efried	dansmith: No
16:21:38	jaypipes	bauzas: I believe what dansmith is saying is "no, we don't need such functionality".
16:21:48	bauzas	jaypipes: then, I agree
16:21:50	cdent	just to repeat my concern, so it's on the table, before I forget: The thing that I wonder being a problem is if the virtdriver reports too many traits. Is that ever going to be an issue?
16:22:01	bauzas	adding a possibility to remove a trait opens a lot of problems
16:22:04	efried	dansmith: But traits don't always equate to CPU features.
16:22:20	bauzas	(17:11:22) bauzas: I was only thinking about additive traits
16:22:32	jaypipes	cdent: I don't think so, no.
16:22:32	bauzas	defined by the operator
16:23:13	jaypipes	efried: yes, traits aren't always CPU features. and nobody is saying that the only thing the virt driver would return is CPU features.
16:23:22	efried	It sounds like dansmith is simply advocating that, whatever mechanism we provide for admins to add or remove traits, they can't remove traits that the virt driver sets. (Or rather, they'll be restored the next time u_p_t runs.) And I'm on board with that.
16:23:35	dansmith	efried: yes, that's what I'm saying
16:23:49	dansmith	but I think that's all we need
16:23:54	jaypipes	efried: all that is being stipulated is that the virt driver will return a set of traits that it feels should always be set on the provider. it will not remove any traits manually added by an admin.
16:23:55	dansmith	the only other interface we need for admins is placement
16:24:02	bauzas	+1 to the idea of keeping things somple
16:24:04	bauzas	simple
16:24:20	efried	dansmith: Via osc, yah?
16:24:24	efried	that's sufficient?
16:24:28	dansmith	efried: yeah
16:24:32	dansmith	totally sufficient, IMHO
16:25:06	openstackgerrit	Matt Riedemann proposed openstack/nova-specs master: Spec for volume multiattach enhancements https://review.openstack.org/552078
16:25:13	jaypipes	OK, I will attempt to summarize here.
16:25:14	bauzas	dansmith: but tbc, we agree on if the admin stupidely deletes a trait in the placement API, then on the next virt driver call, the compute service will add again that trait
16:25:26	bauzas	I now understand what you're saying
16:25:29	efried	Okay, I'm on board now. I think this works. The stipulation that traits owned by the virt driver will magically reappear in some indeterminate amount of time - that's going to be a little weird to describe. But if that's the only wrinkle, I can accept it.
16:25:34	jaypipes	bauzas: if it's a trait the virt driver reports, yep.
16:25:58	bauzas	jaypipes: that slight detail makes me a little wondering
16:26:16	efried	Yeah
16:26:29	efried	It's going to make the admin ask the question, "Okay, how do I find out which traits I'm not allowed to remove?"
16:26:36	bauzas	I don't see a reason why we shouldn't consider the compute service as the single source of truth for any compute-defined trait
16:26:44	dansmith	efried: I'm less worried about that,
16:27:04	dansmith	efried: because they're going to be adding some trait because a doc told them to, like "add a trait to this if it is a trusted PF"
16:27:10	jaypipes	efried: it's a valid concern but I'm not particularly worried about that either.
16:27:35	efried	but sure.
16:27:37	dansmith	that is easy to document,
16:27:38	bauzas	dansmith: adding a trait is totally fine by me
16:27:51	efried	dansmith: I agree it should be documented. I don't agree it's easy to document.
16:27:55	bauzas	dansmith: that's the removal case that makes me wondering why we should special case the virt ones
16:27:56	dansmith	the "what new stuff can I add" is a little harder, but really not a huge deal I think
16:27:57	efried	But we'll burn that bridge when we cross it.
16:28:25	dansmith	I'm writing a really simple example bit of code to explain what we just discussed I think
16:28:57	jaypipes	bauzas: if "special casing" == "the virt driver understands these traits should be applied to this provider", then I don't agree it's an issue.
16:29:53	efried	but wait
16:30:11	bauzas	jaypipes: no
16:30:45	bauzas	jaypipes: the special case I mentioned would be "the compute manager would see a removed trait, and only add it again to Placement API if that's coming from the virt driver"
16:31:04	bauzas	that's the "if..." I wonder

Earlier   Later