Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-12
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
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.

Earlier   Later