| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-01 | |||
| 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 | |
| 18:52:42 | sean-k-mooney | but not modify any resouce provider it did not created | |
| 18:57:33 | sean-k-mooney | efried: today are we doing the provider tree update by put or patch. if put what would it take to make it a patch so nova can do a partial update and merge it on the placement side | |
| 18:58:27 | efried | mriedem: quick fix pls | |
| 18:59:29 | efried | sean-k-mooney: patch is only applicable if you're talking about modifying part of a single provider. Which I think we're not considering. | |
| 18:59:56 | efried | sean-k-mooney: IIUC, you're suggesting modifying some providers in the tree, but not others. That's still PUT - one per provider to be modified. | |
| 19:00:05 | efried | And it's what we do today, see update_from_provider_tree | |
| 19:00:17 | sean-k-mooney | efried: ok cool | |
| 19:00:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove TODO from get_provider_tree_and_ensure_root https://review.openstack.org/614835 | |
| 19:00:51 | efried | +2 ^ | |
| 19:03:31 | sean-k-mooney | what i was actully suggesting was make it a single patch call to placement to update all resouce providers in a tree owned by service x but thats a different conversation | |
| 19:03:46 | efried | totally | |
| 19:04:48 | sean-k-mooney | i am still not aware of an usecase that woudl require nova to be aware of a resouce provider created by another service by the way | |
| 19:05:10 | sean-k-mooney | when i say nova i sepcfically mean the compute agent | |
| 19:07:59 | dansmith | efried: so was there some outcome? | |
| 19:08:21 | efried | dansmith: Remember that thing they did where they disabled the refresh_associations? | |
| 19:08:51 | dansmith | oh they reverted that? | |
| 19:08:54 | dansmith | accidentally | |
| 19:08:59 | efried | dansmith: That's why all those calls were zeroes in queens and nonzero once they upgraded (because that hack was no longer there). Yeah. | |
| 19:09:04 | dansmith | ah cool. | |
| 19:09:17 | efried | So belmiro is going to reinstate that and come back at us. | |
| 19:09:23 | dansmith | right on | |
| 19:09:32 | efried | but it's still a shit ton of calls | |
| 19:10:06 | efried | Matt and Sean and I brainstormed briefly on whether we could just get rid of the cache completely (and whether that would actually help). | |
| 19:10:22 | efried | And what we actually ended up doing was getting rid of a comment: https://review.openstack.org/614835 :( | |
| 19:13:04 | sean-k-mooney | efried: well i think we could maybe get rid of the cache but i think it needs a spec not irc ideas | |
| 19:13:24 | efried | sean-k-mooney: I would rather see a PoC in code for that one than a spec. | |