| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-13 | |||
| 20:05:43 | dansmith | that's the confusion though: | |
| 20:05:45 | efried | It has the whole picture as Placement knows about it right now. | |
| 20:05:58 | efried | ...at least the picture that's rooted at the compute host RP. | |
| 20:06:20 | dansmith | does ProviderTree.update(<not including things that were in the child>) blow those away? or are you saying the virt driver needs to look and include those things in its update? | |
| 20:07:24 | efried | ProviderTree.update_[thingy](rp_uuid) for thingy in (inventory, traits, aggregates) will replace thingy for just the RP indicated by rp_uuid. | |
| 20:07:50 | efried | E.g. if you replace inventory via that method, but not traits, then traits stay whatever they were before. | |
| 20:08:02 | efried | Also, those update_* methods don't muck with children. | |
| 20:08:06 | dansmith | yeah but that's not the question | |
| 20:08:12 | dansmith | okay, _that_ is the quesation | |
| 20:08:19 | dansmith | whether update() touches the children | |
| 20:08:22 | dansmith | um | |
| 20:08:26 | dansmith | "affects the children" | |
| 20:08:28 | efried | To muck with hierarchy, you have to ProviderTree.remove(), .new_root(), .new_child()... | |
| 20:08:33 | efried | There is no update(). | |
| 20:08:39 | dansmith | I know, I'm saving letters | |
| 20:08:45 | dansmith | I think that's his question | |
| 20:08:54 | dansmith | so I had assumed it would only affect the level you're on, and was replying, | |
| 20:09:04 | dansmith | but then re-read the bit of your text he's taking exception to and wasn't sure | |
| 20:09:10 | dansmith | so I think you just need to tighten that up a smidge | |
| 20:09:42 | efried | Okay. That's a true statement ("...only affects the level you're on"). *except* that if you .remove(rp), you also remove all of rp's descendants. That's what the sentence is saying. | |
| 20:09:48 | dansmith | sure | |
| 20:09:50 | dansmith | and, | |
| 20:10:15 | dansmith | I was going to say that if the compute node thinks it needs to remove its provider, then it _should_ affect the external thing that still thinks it is reporting resources for that provider | |
| 20:10:23 | dansmith | i.e. compute is authoritative for the existence of itself | |
| 20:10:29 | efried | right. | |
| 20:11:07 | efried | It'll be a possibly-not-enforced-but-highly-recommended rule that these entities that share control of a tree never share control of a single RP within that tree. | |
| 20:11:22 | efried | that way lies madness. | |
| 20:11:29 | dansmith | not sure how you would enforce tat, but agree, that should be the assumption | |
| 20:11:33 | dansmith | *that | |
| 20:11:35 | mriedem | stephenfin: still around? | |
| 20:12:19 | efried | Okay, I'll post an edit. Thanks for the discussion dansmith, mriedem | |
| 20:21:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup the manage-volumes admin doc https://review.openstack.org/544066 | |
| 20:25:13 | jaypipes | efried, dansmith, mriedem: just finished reading back... yes, agree that the spec could use some clarity/language around the update_provider_tree() method implementations not calling ProviderTree.remove() for nodes in the tree that it doesn't manage. | |
| 20:25:24 | dansmith | aye | |
| 20:25:30 | efried | Roger wilco. | |
| 20:26:19 | jaypipes | efried: I still don't understand this from tetsuro though: "Note that at least libvirt driver today removes CUSTOM_RCs which users set manually via placement API for a compute host when the periodic updating task happens." | |
| 20:26:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup the manage-volumes admin doc https://review.openstack.org/544066 | |
| 20:26:58 | efried | jaypipes: I think he's saying libvirt removes *inventory* in custom RCs it didn't create itself. | |
| 20:27:13 | efried | I don't think we're talking about removing the actual resource classes from placement. | |
| 20:27:26 | jaypipes | efried: is he just saying that if an operator manually calls POST /resource_providers/{cn_uuid}/inventories and adds a CUSTOM_XXX resource class, that it will be removed by the libvirt virt driver's get_inventory() method? | |
| 20:27:33 | efried | I think so, yes. | |
| 20:27:44 | jaypipes | efried: gotcha. ok. and yes, that is definitely by design... | |
| 20:27:47 | efried | yup. | |
| 20:28:23 | mriedem | that is eventually due to this check? https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L831 | |
| 20:28:31 | jaypipes | efried: since clearly the virt driver owns the resources on the compute node. but it's worth saying that update_provider_tree() implementations will *not* remove a *node* from the tree of providers if it doesn't know about the node. | |
| 20:28:37 | jaypipes | mriedem: yea | |
| 20:28:57 | jaypipes | mriedem: oh, no... | |
| 20:29:26 | efried | That's just an optimization that'll short-circuit if nothing has changed. | |
| 20:29:29 | jaypipes | mriedem: that check is seeing if what we got from get_inventory() is the same as what is cached in the reportclient's provider tree. | |
| 20:29:36 | jaypipes | what efried said. | |
| 20:29:57 | mriedem | but if the driver reports some inventory change, for VCPU, MEMORY_MB and DISK_GB, | |
| 20:30:07 | mriedem | and some external service had set some CUSTOM_FOO inventory, | |
| 20:30:12 | mriedem | that's the code that's going to blast it away right? | |
| 20:30:34 | jaypipes | mriedem: that conditional will be true if the operator set some CUSTOM_XXX resource class inventory and libvirt doesn't report that resource class. | |
| 20:30:52 | jaypipes | mriedem: it's just that inv_data is always exactly and only what the virt driver sets it to. | |
| 20:30:53 | mriedem | yeah ok | |
| 20:31:04 | jaypipes | mriedem: and that will overwrite anything the operator manually set. | |
| 20:31:07 | mriedem | yup | |
| 20:32:57 | jaypipes | mriedem: right, but again, that's get_inventory(). the update_provider_tree() method will blow away inventory for the specific (root) compute node provider and set it to whatever it thinks it should be. but it won't delete child nodes it doesn't manage. so if the operator (or external agent) set inventory for, say, NET_EFRESS_BYTES_PER_SECOND on a child provider node, the update_provider_tree() method won't delete that node and its inventory. | |
| 20:33:22 | jaypipes | EGRESS... | |
| 20:33:50 | mriedem | EFRIED_BYTES | |
| 20:33:54 | mriedem | jaypipes: got it | |
| 20:34:07 | jaypipes | mriedem: but if an operator manually set the VCPU inventory on the compute node resource provider to something, the virt driver absolutely will overwrite that inventory to what it thinks is correct. | |
| 20:34:13 | jaypipes | mriedem: ok, sorry for babbling | |
| 20:34:19 | mriedem | yup makes sense | |
| 20:35:39 | efried | jaypipes: tbc, we have no way (other than code reviews) to enforce that update_provider_tree doesn't muck with child nodes it doesn't manage. | |
| 20:36:09 | mriedem | functional tests? | |
| 20:36:33 | jaypipes | efried: correct. | |
| 20:36:48 | efried | mriedem: Functional tests that would have to know something about the expected structure of the tree for the individual virt driver. | |
| 20:36:59 | efried | structure & management | |
| 20:38:08 | jaypipes | efried: we can't programmatically determine resource ownership and policy for the various virt drivers and potential external agents ahead of time. all we can do is docstring the heck out of update_provider_tree() and explain the ramifications of calling ProviderTree.remove(). | |
| 20:38:12 | efried | But yeah. That's my point. Whoever writes & reviews that test, and the code it operates on, has to know which bits are managed by virt and which by external agents. | |
| 20:38:38 | efried | jaypipes: ++ I need to add words to the docstring for this. | |
| 20:39:00 | efried | I think there are words there, but they don't really talk about the implications, just about the effects. | |
| 20:39:50 | jaypipes | efried: for instance, the PowerVM virt driver may just as well WANT to call ProviderTree.remove() on child nodes if the PowerVM virt driver is fully managing things like NICs and disk subsystems, etc.. | |
| 20:39:56 | jaypipes | just no way to know ahead of time. | |
| 20:40:02 | efried | exactly so. | |
| 20:41:15 | jaypipes | efried: and, furthermore, the virt driver's update_provider_tree() method is pretty much exactly what I'd like us to propose to external agents and systems like Cinder/Neutron to use as their "interface" into the resource provider modeling of Placement's API. | |
| 20:41:38 | efried | ++ | |
| 20:42:07 | openstackgerrit | Jeremy Stanley proposed openstack/nova master: Include PCRE headers in bindep manifest https://review.openstack.org/544071 | |
| 20:42:58 | fungi | just ran across that ^ due to https://ask.openstack.org/question/112467 | |
| 20:43:12 | jaypipes | edleafe: unless they're being blocked as spam, I don't think the email notifications on your blog comments are working :( | |
| 20:50:09 | edleafe | jaypipes: arrrrggghh | |
| 20:50:20 | edleafe | recent update must have changed the settings | |
| 20:50:34 | edleafe | lemme investigate | |
| 20:53:56 | jaypipes | sean-k-mooney: so... | |
| 20:54:14 | jaypipes | sean-k-mooney: are you or are you not in support of this spec? https://review.openstack.org/#/c/533077/3/specs/rocky/approved/libvirt-qemu-numa-support.rst | |
| 20:56:47 | openstackgerrit | melissaml proposed openstack/nova-specs master: Replace Chinese quotes to English quotes https://review.openstack.org/544076 | |
| 20:57:48 | openstackgerrit | Merged openstack/python-novaclient stable/ocata: Zuul: Remove project name https://review.openstack.org/544018 | |
| 20:58:35 | mriedem | fungi: https://review.openstack.org/#/c/542340/ | |
| 21:00:39 | fungi | mriedem: excellent! i'll abandon mine. thanks | |
| 21:02:10 | openstackgerrit | Nicolas Bock proposed openstack/nova master: install-guide: Make formatting of console consistent https://review.openstack.org/544081 | |
| 21:03:58 | dansmith | efried: maybe you can answer this: https://review.openstack.org/#/c/540447/3 | |
| 21:04:00 | dansmith | or jaypipes ^ | |
| 21:04:28 | efried | on it | |
| 21:05:38 | edleafe | jaypipes: should be working now. LMK if you don't get emails for future comments | |
| 21:06:04 | efried | dansmith: Done. | |
| 21:08:18 | dansmith | efried: replied, thanks | |
| 21:09:07 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Update Queens NRP spec to reflect reality https://review.openstack.org/544086 | |
| 21:09:20 | efried | mriedem, jaypipes, dansmith: There's that edit we talked about yesterday ^ | |