| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-12 | |||
| 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 | bauzas | defined by the operator | |
| 16:22:32 | jaypipes | cdent: I don't think so, no. | |
| 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 | |
| 16:31:26 | efried | We still have the case where u_p_t is called, and the virt driver decides that it wants to assert a set of traits, but something has changed since the last time, and that set of traits is a *subset* of its previous. Then it has no way to know that it should remove the extraneous traits. | |
| 16:31:58 | bauzas | for me, the compute service should provide all the traits that are reported from various ways, and shouldn't care whether it's not existing in Placement or not | |
| 16:31:59 | jaypipes | efried: what we're saying is that it should never "remove the extraneous traits". | |
| 16:32:04 | bauzas | ++ | |
| 16:32:06 | efried | jaypipes: And that's a problem. | |
| 16:32:09 | dansmith | efried: wait for my example code | |
| 16:32:23 | cdent | jaypipes: that's difficult in the cluster virt drivers | |
| 16:32:34 | efried | Because something may have changed that legitimately warrants the removal of such a trait. | |
| 16:32:45 | cdent | where hardware is added on the fly, or hot pluggable hardware | |
| 16:32:47 | efried | Otherwise why would we be bothering to do this in a periodic? | |
| 16:32:52 | efried | cdent: Yes, or *removed* | |
| 16:32:58 | cdent | yes | |
| 16:33:35 | efried | So I go back to asserting this doesn't work for the general case. | |
| 16:34:02 | jaypipes | efried: "the general case" <-- what is that? | |
| 16:34:14 | efried | jaypipes: I just mean "for all cases". | |
| 16:34:20 | edleafe | If an admin wants to "remove" a trait FOO set by the virt driver, she could add a CUSTOM_FOO_BAD trait, and then make that a forbidden trait | |
| 16:35:07 | efried | edleafe: I think we're past that one. It doesn't have a realistic use case, most likely. | |
| 16:35:40 | edleafe | efried: good | |
| 16:37:28 | efried | dansmith, jaypipes: Specifically the case I'm talking about is, for example, hot-pluggable storage. Let's say you start with some disk. Virt driver presents inventory of DISK_GB and trait STORAGE_DISK_SSD. Then you pull out the disk. The next u_p_t needs to be able to remove the DISK_GB inventory *and* remove the STORAGE_DISK_SSD trait. With this design, it can't do the latter. The admin would have to do it manually. | |
| 16:37:31 | jaypipes | efried: it's not that I don't see your use case (for example, dynamically marking some child providers representing a PF with a CUSTOM_TRUSTED trait or something). But I also don't necessarily think that what dansmith is proposing would *preclude* us from moving in a more complicated direction if and when such a direction was determined to be required. | |
| 16:39:55 | dansmith | this is what I think we need: https://pastebin.com/mM6NMzQ4 | |
| 16:40:18 | dansmith | note the operator's traits are untouched, yet the virt and compute-owned traits are asserted or de-asserted as things change | |
| 16:41:28 | cdent | oh that's interesting | |
| 16:41:58 | efried | dansmith: What I'm asserting is that the virt driver has no way of knowing it wants to -SOMETHING | |
| 16:42:02 | cdent | but can/does that interop with how u_p_t is planned to behave | |
| 16:42:27 | ygl | hi all | |
| 16:42:35 | dansmith | efried: it totally does | |
| 16:42:43 | efried | cdent: What we decided on DublinFriday is that we're going to merge u_p_t as currently written and then bolt this on after. | |
| 16:42:55 | jaypipes | dansmith: that's essentially what my traits:always and traits:ignore in the provider config file format were doing. :) | |