| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-13 | |||
| 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. | |
| 21:14:12 | mriedem | i would recommend not reading the backscroll... | |
| 21:14:47 | efried | Basically, the Q one lays the NRP groundwork, and the stuff coming off of that spiders in multiple directions - update_provider_tree, NRP-in-alloc-cands, wtf we're going to do about NUMA, etc. - each of which ought to have its own spec. | |
| 21:14:54 | mriedem | but if you do, your reward is some nice simpsons clip | |
| 21:16:33 | jaypipes | efried: ack | |
| 21:17:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add admin guide doc on volume multiattach support https://review.openstack.org/544090 | |
| 21:17:30 | openstackgerrit | Merged openstack/nova-specs master: Add generation support in aggregate association https://review.openstack.org/540447 | |
| 21:19:48 | edleafe | efried: will review shortly | |
| 21:20:24 | efried | d'oh, I forgot to tag the commit message with the bp. Editing... | |
| 21:20:51 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Update Queens NRP spec to reflect reality https://review.openstack.org/544086 | |
| 21:20:53 | efried | done ^ | |
| 21:21:57 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Clarify old microversion behavior https://review.openstack.org/544092 | |
| 21:22:02 | efried | dansmith: There's that followup ^ | |
| 21:24:06 | dansmith | efried: I sent it to its maker, thanks | |
| 21:29:50 | openstackgerrit | Nicolas Bock proposed openstack/nova master: install-guide: Wrap long console command https://review.openstack.org/544093 | |
| 21:30:39 | openstackgerrit | Merged openstack/nova master: doc: merge numa.rst to cpu-topologies.rst https://review.openstack.org/540585 | |
| 21:38:19 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Update Queens NRP spec to reflect reality https://review.openstack.org/544086 | |
| 21:39:52 | mriedem | efried: lgtm, thanks | |
| 21:42:43 | openstackgerrit | Merged openstack/nova master: install-guide: Make formatting of console consistent https://review.openstack.org/544081 | |
| 21:43:08 | openstackgerrit | Merged openstack/nova-specs master: Clarify old microversion behavior https://review.openstack.org/544092 | |
| 21:45:46 | mriedem | dansmith: easy peasy https://review.openstack.org/#/c/543571/ | |
| 21:45:50 | mriedem | likely RC2 fodder | |
| 21:47:26 | dansmith | whoopsie | |
| 21:54:00 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Bindep does not catch missing libpcre3-dev on Ubuntu https://review.openstack.org/544108 | |
| 21:54:05 | mriedem | esberglu: want to post a backport to queens for https://review.openstack.org/#/c/543571/ ? | |
| 21:54:37 | openstackgerrit | Eric Berglund proposed openstack/nova stable/queens: Use correct arguments in task inits https://review.openstack.org/544109 | |
| 21:54:54 | mriedem | with the -x option on git cherry-pick please | |
| 22:03:22 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Update Provider Tree https://review.openstack.org/540111 | |
| 22:03:37 | efried | dansmith, mriedem, jaypipes, edmondsw, edleafe: ^ | |