| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-12 | |||
| 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 | |
| 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. :) | |
| 16:43:20 | dansmith | efried: I assert we don't have to worry about cases where the operator adds a trait to an x86 compute node of TRAIT_CPU_POWER7_THINGY | |
| 16:43:21 | ygl | i want to launch a vm with a dummy nic interface without an IP assigned to it . can someone help me how to do it please | |
| 16:43:45 | jaypipes | dansmith: but clearly I don't segregate between what the compute and the virt think of as asserted/removed | |
| 16:43:46 | efried | ygl: Try #openstack | |
| 16:43:46 | dansmith | jaypipes: this is not always and ignore, this is always and remove, or something | |
| 16:43:54 | jaypipes | dansmith: understood. | |
| 16:44:11 | dansmith | jaypipes: and this is exposed from the virt driver, not encoded in a separate editable file | |
| 16:44:44 | jaypipes | dansmith: would this be exposed as a config option (or 2 options)? | |
| 16:44:50 | dansmith | jaypipes: no | |
| 16:45:02 | efried | dansmith: I'm not talking about the admin adding traits - we already decided she gets to do that. I'm talking about a trait the virt driver needs to remove because e.g. someone hot-unplugged a disk. | |
| 16:45:16 | dansmith | jaypipes: the virt driver would return things like this, which may vary based on virt driver config and talking to the hypevisor | |
| 16:45:24 | efried | The virt driver operates by discovering what it has and asserting inventory/traits accordingly. | |
| 16:45:24 | jaypipes | k | |
| 16:45:51 | efried | If it discovers at time T and has a disk, it will assert some DISK_GB inventory and the STORAGE_DISK_SSD trait. | |
| 16:46:18 | jaypipes | dansmith: a perfectly satisfactory solution. | |
| 16:46:37 | dansmith | jaypipes: this is what I was describing from my corner on friday while we were discussing this | |
| 16:46:43 | efried | Then it discovers at time T+1 and it doesn't have a disk; it will not assert DISK_GB inventory (which works, because it's the only source of inventory info) and it will not assert the STORAGE_DISK_SSD trait. | |
| 16:46:56 | efried | But it *doesn't* know that it needs to *de*assert the STORAGE_DISK_SSD trait. | |
| 16:47:11 | efried | Because it has no memory that it asserted it at time T. It just knows it isn't there now. | |
| 16:47:13 | dansmith | efried: I completely don't understand that | |
| 16:47:13 | jaypipes | dansmith: yes, I recognize the +/- syntax now. just didn't have in my mind how the virt driver would expose this (automatically vs. config-based) | |
| 16:47:22 | dansmith | jaypipes: ack | |
| 16:47:52 | dansmith | efried: if you as a virt driver ever expose trait FOO, then you need to de-assert FOO if you determine there is no reason to assert it, right? | |
| 16:49:04 | efried | That goes exactly back to my earlier statement about how the virt driver would have to know the full set of traits it *might* be responsible for, and then what subset of those it thinks should be set, so that it can explicitly deassert the remainder. | |
| 16:49:19 | dansmith | why is that unreasonable? | |
| 16:49:32 | dansmith | it doesn't have to be the full set of traits that any virt driver might expose, | |