| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-01 | |||
| 18:20:03 | efried | are you saying "as long as nothing is changing, we don't need to call update_provider_tree" ? | |
| 18:20:25 | mriedem | update_provider_tree is what returns the inventory from the driver to the RT to push off to placement every 60 seocnds | |
| 18:20:27 | mriedem | *seconds | |
| 18:20:28 | mriedem | right? | |
| 18:21:04 | efried | Yes | |
| 18:21:05 | mriedem | and assuming that disk/ram/cpu on a host doesn't change all that often, at least without a restart of the host, it seems odd we need to cache that information | |
| 18:21:18 | efried | But how else would we know whether to push the info back to placement? | |
| 18:21:51 | mriedem | in the before upt times, didn't the RT/report client just pull inventory, compare to what was reported by the driver, and the PUT it back if there were changes? | |
| 18:22:08 | efried | What does "pull inventory" mean, though? | |
| 18:22:22 | efried | pull from placement | |
| 18:22:28 | mriedem | GET /resource_providers/{rp_uuid}/inventories | |
| 18:22:29 | efried | i.e. GET /rps/UUID/inventory | |
| 18:22:32 | efried | yeah | |
| 18:22:42 | sean-k-mooney | efried: well the driver could have a perodic check but rememebr the last value it sent and only send a value if it detactes there was a chage | |
| 18:22:50 | efried | sean-k-mooney: ^ cache | |
| 18:22:58 | sean-k-mooney | that not the same as a cache | |
| 18:23:00 | efried | and that's what we do | |
| 18:23:39 | mriedem | get_provider_tree_and_ensure_root is what gets the provider tree from the report client and pulls the current inventory from placement, yes? | |
| 18:23:48 | mriedem | and also checks to see that the provider exists on every periodic | |
| 18:23:48 | efried | yes | |
| 18:23:51 | mriedem | which we should actually know | |
| 18:24:37 | efried | yeah, we could conceivably expect the compute RP not to disappear once we've created it. | |
| 18:24:45 | efried | I mean, I don't know how resilient we're trying to be in the face of OOB changes. | |
| 18:24:59 | efried | we do offer a placement CLI, not just for GETs but for writes as well | |
| 18:25:47 | mriedem | the compute service record, compute node record, and rp can all be deleted if the compute service record is deleted | |
| 18:25:56 | mriedem | but to get the compute service record back, you have to restart the compute service | |
| 18:26:02 | mriedem | to recreate the record which would also re-create the compute node | |
| 18:26:04 | mriedem | and then the RP | |
| 18:26:13 | mriedem | since we know https://github.com/openstack/nova/blob/1e823f21997018bcd197057ebd4d6207a5c54403/nova/compute/resource_tracker.py#L780 | |
| 18:26:14 | sean-k-mooney | efried: true but in that case could we do what we do with neutron and have placment send a notificaiton to nova that it was changed instead of polling | |
| 18:26:16 | mriedem | we can pass that down | |
| 18:26:22 | mriedem | i'll hack something up quick | |
| 18:26:24 | efried | IOW, only have the create code path on start=True | |
| 18:26:36 | efried | and don't bother with the existence check otherwise | |
| 18:26:45 | mriedem | not even start true | |
| 18:26:59 | mriedem | since https://github.com/openstack/nova/commit/418fc93a10fe18de27c75b522a6afdc15e1c49f2 we have a flag to pass through when we create the compute node | |
| 18:27:05 | mriedem | we just don't plumb it far enough | |
| 18:27:10 | mriedem | i can push up a change that does | |
| 18:27:25 | efried | mriedem: That's what pike looked like, though. The stuff that's causing the spike is necessary for *enablement* of nrp, which we haven't started using yet | |
| 18:27:29 | mriedem | that might save precious ms for belmoreira :) | |
| 18:27:31 | efried | so it seems useless atm | |
| 18:27:52 | efried | but as soon as we get e.g. neutron or cyborg adding shit to the tree, we're going to need to do that ?in_tree call every periodic. | |
| 18:28:14 | efried | unless there's some kind of async notification hook to trigger a refresh | |
| 18:28:24 | efried | yeah, what sean-k-mooney said. | |
| 18:28:25 | mriedem | i'm saying we can resolve this todo i think https://github.com/openstack/nova/blob/1e823f21997018bcd197057ebd4d6207a5c54403/nova/scheduler/client/report.py#L1011 | |
| 18:28:28 | mriedem | can we agree on that? | |
| 18:29:04 | mriedem | neutron sending an event is possible, but it's also per instance... | |
| 18:29:06 | mriedem | not per host | |
| 18:29:22 | efried | mriedem: unfortunately not anymore, because we plan to allow other-than-nova to edit the tree. | |
| 18:29:39 | mriedem | including delete the compute node root provider? | |
| 18:29:44 | mriedem | that nova creates? | |
| 18:29:50 | efried | no, not that. | |
| 18:29:58 | mriedem | well isn't that what that todo is all about? | |
| 18:30:06 | mriedem | create the resource provider for the compute node if it doesn't exist | |
| 18:30:13 | efried | no | |
| 18:30:28 | sean-k-mooney | mriedem: we are going to allow them to manage there onw subtrees only so the wont be allowed to modify any nodes created by nova | |
| 18:31:09 | efried | mriedem: You could probably factor out *just* the root provider part of that; but you can't get rid of the whole method. | |
| 18:31:42 | mriedem | i'm not saying remove the method | |
| 18:31:47 | efried | and the GET that _ensure_resource_provider is doing is the ?in_tree one that we can't get rid of anyway. | |
| 18:35:13 | mriedem | because of something external adding/removing things from the root | |
| 18:35:24 | mriedem | right? | |
| 18:36:56 | sean-k-mooney | well external entitiy can only leagally add nested resouce providers to the root node they cant add invetories or traits | |
| 18:37:27 | sean-k-mooney | technicall the api does not enforce that as we dont have owner of resouce proivers in the api however | |
| 18:38:28 | mriedem | what i'm hearing is we can't remove this todo https://github.com/openstack/nova/blob/1e823f21997018bcd197057ebd4d6207a5c54403/nova/scheduler/client/report.py#L1011 even if we know we didn't just create the root compute node, because we need to call it anyway to determine if there are new nested providers under that pre-existing compute node | |
| 18:38:40 | mriedem | s/remove/resolve/ | |
| 18:38:50 | mriedem | iow, the todo should be removed b/c we can't do anything about it | |
| 18:38:56 | mriedem | even if we *know* the compute node record was just created | |
| 18:39:23 | sean-k-mooney | am i dont know if we need to know if there are new nested resouce providers | |
| 18:39:38 | mriedem | that's the whole in_tree thing i thought | |
| 18:39:41 | sean-k-mooney | in fact i would asser as the compute node we dont need to know that | |
| 18:40:48 | mriedem | ok well what i'm saying is we (the RT) know when we created a new compute node record, and thus need to create its resource provider, i guess i'll wait for someone to tell me if that's worth doing so we can avoid the GET /resource_providers?in_tree=<uuid of the thing we just created and thus doesn't exist yet> case | |
| 18:42:00 | sean-k-mooney | mriedem: in that case i think you are right wew dont need the /resource_providers?in_tree=<thing i just created> call | |
| 18:43:11 | sean-k-mooney | the placement api will not allow me to create a resouce provider with a parrent uuid that does not exist | |
| 18:44:18 | efried | sean-k-mooney: We *do* need to know if there are new nested providers. | |
| 18:44:52 | sean-k-mooney | efried: there cant be nested resouce providers of a compute node if we have not created the compute node yet right | |
| 18:45:08 | efried | That's what ?in_tree is about, though. | |
| 18:45:26 | efried | ?in_tree=$compute_rp gives me the compute RP and any descendants. | |
| 18:45:35 | sean-k-mooney | yes | |
| 18:46:06 | sean-k-mooney | but if the compute RP does not exist yet then cyborge cant create nested resources under it | |
| 18:46:14 | efried | So at T0, it gives me nothing, so I create the compute RP. At T1 it gives me the compute RP. At T2, cyborg creates a child provider for a device. At T3 ?in_tree=$compute_rp gives me both providers. | |
| 18:46:28 | efried | If I didn't call ?in_tree I would never know about that device RP | |
| 18:46:44 | efried | and I need to know about that device RP e.g. from my virt driver so I can white/blacklist it and/or deploy it. | |
| 18:46:48 | sean-k-mooney | efried: sure you dont own that and as nova you cant directly modify it | |
| 18:47:17 | efried | Unclear whether blacklisting happens at cyborg or at nova | |
| 18:47:41 | efried | but what you say makes sense (single ownership) so it would have to be at cyborg. | |
| 18:48:14 | efried | Which means virt driver-esque code would need to be invoked by cyborg to do discovery in the first place. | |
| 18:48:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove TODO from get_provider_tree_and_ensure_root https://review.openstack.org/614835 | |
| 18:48:41 | sean-k-mooney | efried: well not nessisarily | |
| 18:49:30 | sean-k-mooney | we did suggest that in update_provider_tree we could call os-acc to do that | |
| 18:49:40 | efried | yes | |
| 18:49:45 | efried | wait | |
| 18:50:06 | sean-k-mooney | but what cyborg need to do is 1 lookup the root provider for the compute node | |
| 18:50:15 | efried | update_provider_tree would call os_acc with a list of discovered-and-already-whitelist-scrubbed devices so that cyborg can create the providers? | |
| 18:50:31 | sean-k-mooney | efried: no | |
| 18:50:55 | sean-k-mooney | if nova is doing the deicovery we are rebuiling cyborg in nova | |
| 18:51:33 | sean-k-mooney | the idea was that we woudl pass in the current tree to cyborg and it woudl do the discovery itslf and append to that tree | |
| 18:51:38 | sean-k-mooney | but the other approch | |
| 18:51:46 | sean-k-mooney | which is what we were going to do | |
| 18:52:03 | sean-k-mooney | was cybroge poll placement for compute node to be created. | |
| 18:52:24 | sean-k-mooney | then it would add child resouce providers to the tree created by nova | |