Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-13
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 ^
21:10:29 efried edleafe: ^
21:12:26 jaypipes efried: cool, thanks
21:13:20 efried jaypipes: In case you didn't read that backscroll (link in the commit message of ^) the plan is to propose a more or less new Rocky spec to talk about NRP-in-GET-alloc-cands stuff, rather than "reproposing" the NRP spec.

Earlier   Later