| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-12 | |||
| 17:00:54 | efried | Assuming we go forward with this, all the merge logic needs to be handled by the individual virt drivers. Unless we're going to invent some interface outside of u_p_t. | |
| 17:00:55 | efried | except for get_traits, which I freakin knew was going to bite us in the ass. | |
| 17:01:20 | dansmith | I guess we could also provide the current set of traits to the virt driver so it gets to decide if it thinks any of the ones set are important to ack or nak | |
| 17:01:29 | efried | dansmith: That's what we do. | |
| 17:01:34 | efried | in u_p_t | |
| 17:02:11 | dansmith | efried: I don't think the virt drivers should be doing merging in the general case | |
| 17:02:27 | dansmith | efried: just being write-only for the majority of things, this ironic thing notwithstanding | |
| 17:02:28 | efried | dansmith: Then we need nontrivial redesign of u_p_t. | |
| 17:02:49 | dansmith | well I dunno what to say dude.. this is clearly a thing we missed in the design :) | |
| 17:03:20 | efried | The way it's currently designed: The resource tracker builds the ProviderTree, which includes all the providers' traits as known by placement, and passes that to u_p_t. The virt driver mungs the ProviderTree as it sees fit and then returns. Then resource tracker flushes those changes back to placement. | |
| 17:03:54 | dansmith | yeah, well, we can provide merging routines to the virt drivers to use on the tree, | |
| 17:04:00 | dansmith | and expect that most just use those | |
| 17:04:12 | efried | Sure we can. But the merging still has to be the responsibility of the virt driver. | |
| 17:05:12 | sean-k-mooney | efried: perhaps but it would be nicer if it was the respociblity of the compute manager or some other componet just above the virt driver so it could be shared acrosss all virtdirvers | |
| 17:05:44 | dansmith | sean-k-mooney: yeah, that's what I'd like, to expose some augmented topology from the virt driver so the compute can update the tree, | |
| 17:05:46 | efried | sean-k-mooney: And I'm saying that would require some semantic that's not compatible with u_p_t as designed. | |
| 17:05:51 | efried | The ProviderTree business was so we didn't have to have separate virt driver methods for get_inventory, get_traits, get_aggregates, and (this is the main one) somehow_structure_the_provider_tree_hierarchy | |
| 17:05:52 | dansmith | but it sounds like we've already exposed this all the way down | |
| 17:06:22 | bauzas | efried: dansmith's proposal patch is simple | |
| 17:06:24 | dansmith | so, whatever, we can argue about cleaning that up later if we want, unrelated to fixing this merging thing | |
| 17:06:26 | efried | dansmith: The code isn't merged. We could still rip it all out and change our minds. But I believe we'd be getting a lot more complicated. | |
| 17:06:32 | admin__ | mridem: http://logs.openstack.org/24/550324/2/gate/legacy-tempest-dsvm-neutron-full/211271c/job-output.txt.gz#_2018-03-12_16_02_09_694703 failed | |
| 17:06:33 | bauzas | it doesn't require to modify what we currently have | |
| 17:06:42 | bauzas | it's just a convention | |
| 17:06:43 | dansmith | efried: oh, I thought you meant they're already getting the tree | |
| 17:07:26 | dansmith | I dunno, exposing that all the way down isn't my preference, but if that's what we've agreed on so far, I wouldn't hold it up because I don't like it | |
| 17:07:27 | bauzas | like, in a nested RP world, I'd just see the tree with the root RP having a set of traits, each of those be ether prefixed by plus or minus | |
| 17:07:38 | bauzas | or, say a NUMA node | |
| 17:07:39 | dansmith | we just need to integrate this wrinkle at least | |
| 17:07:51 | efried | dansmith: The code was written in Q, but the series didn't merge in time. So we wrote and approved the spec for R (it didn't have a bp/spec in Q - was just kinda folded into the NRP work). The series starts here: https://review.openstack.org/#/c/537648/ | |
| 17:07:51 | dansmith | bauzas: it's more complicated than that | |
| 17:08:06 | dansmith | we need a full topology back from the virt driver with traits at every level It hink | |
| 17:08:12 | dansmith | ack | |
| 17:08:22 | bauzas | mmm | |
| 17:08:45 | bauzas | IIRC, the proposal is the virt driver that passes a tree that has RPS with traits attached, right? | |
| 17:08:49 | efried | dansmith: Yes, the full topology. Does that topology include +/- prefixes? In which case it's not a ProviderTree but something different (ProviderTreeDelta?) | |
| 17:09:23 | bauzas | a trait is attached to a RP, right? | |
| 17:09:30 | dansmith | efried: in the model you have proposed, | |
| 17:09:32 | sean-k-mooney | dansmith: you may not have traits at all levels but there is noting preventing that either. it just depends on the resouces but in all likely hood most RPs will have traits | |
| 17:09:38 | efried | dansmith: Otherwise, like I say, we have to have granular methods that return deltas for traits, inventories, aggregates, and (somehow) the tree structure itself. | |
| 17:09:40 | bauzas | the fact that the RP is a child or a root RP, is just a detail | |
| 17:09:50 | dansmith | efried: we just need a provider.merge_traits(my_assertions) method that takes the +/- things right? | |
| 17:09:56 | bauzas | yeah | |
| 17:10:06 | bauzas | that's a compute method | |
| 17:10:09 | jaypipes | efried: we would want to change the ProviderTree.set_traits() stuff to use the merge traits +/- approach | |
| 17:10:13 | bauzas | no need to change the virt driver interface | |
| 17:10:14 | efried | dansmith: Yes. And virt drivers have to use it. Which is okay with me. | |
| 17:10:19 | dansmith | efried: where provider would be any element in the tree | |
| 17:10:20 | jaypipes | doh, jinx | |
| 17:10:24 | dansmith | efried: yeah, I think that's fine | |
| 17:10:42 | bauzas | the merge_traits() thing is a compute manager method, right? | |
| 17:10:45 | efried | dansmith: But to clarify, it still means the virt driver is responsible for merging traits. | |
| 17:10:48 | dansmith | bauzas: no | |
| 17:10:50 | jaypipes | bauzas: no. ProviderTree | |
| 17:10:54 | efried | ^ | |
| 17:11:00 | bauzas | meh to that | |
| 17:11:04 | dansmith | efried: understood, and I don't like that as much, but I'm willing to defer that argument until later :) | |
| 17:11:18 | bauzas | I mean, the caller would be the compute manager ? | |
| 17:11:19 | dansmith | efried: so I still don't think this is a major disruption, just a change in protocol a little bit | |
| 17:11:25 | dansmith | bauzas: in some cases | |
| 17:11:30 | dansmith | bauzas: but not the ones we're talking about here | |
| 17:11:39 | bauzas | I'm missing some cases then | |
| 17:11:42 | efried | dansmith: Not really even that. The only thing we would need to change about what's proposed is the u_p_t docstring. | |
| 17:11:48 | bauzas | but I don't want to rathole | |
| 17:12:05 | efried | Adding the ProviderTree.merge_traits method is a nice-to-have, but until then, u_p_t could still do that manually. | |
| 17:12:17 | bauzas | oh, unrelated, I'll take a couple of days off | |
| 17:12:19 | dansmith | efried: um, not sure about that | |
| 17:12:22 | jaypipes | bauzas: technically, the resource tracker is the caller of update_provider_tree() on a virt driver, but the goal is to have update_provider_tree() API be what other agents (including Neutron) use for managing their hierarchy of providers, inventories, and traits. | |
| 17:12:36 | dansmith | efried: but get some code up so we can argue over that, which will be more clear right? | |
| 17:12:39 | bauzas | jaypipes: dansmith: cdent: mriedem: others: I'll be on PTO thursday to monday included | |
| 17:12:41 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: Fix N358 hacking check https://review.openstack.org/547670 | |
| 17:12:47 | jaypipes | bauzas: ack | |
| 17:12:56 | bauzas | disneyland... | |
| 17:13:11 | bauzas | not for the dog this time | |
| 17:13:23 | efried | dansmith: I'm rebasing the u_p_t series now. I'll throw a comment in the appropriate spot. And then at some point I guess I can propose the ProviderTree.merge_traits method. | |
| 17:13:33 | dansmith | okay | |
| 17:13:49 | bauzas | jaypipes: ack, yeah, makes sense | |
| 17:14:17 | bauzas | it's 6:13pm here, my stomatch starts to tell me it's time to hang off | |
| 17:14:30 | bauzas | but I'll be back tonight | |
| 17:14:31 | bauzas | \o | |
| 17:14:33 | jaypipes | ciao | |
| 17:23:58 | openstackgerrit | Eric Fried proposed openstack/nova master: New-style _set_inventory_for_provider https://review.openstack.org/537648 | |
| 17:23:59 | openstackgerrit | Eric Fried proposed openstack/nova master: Use update_provider_tree from resource tracker https://review.openstack.org/520246 | |
| 17:23:59 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient.update_from_provider_tree https://review.openstack.org/533821 | |
| 17:24:00 | openstackgerrit | Eric Fried proposed openstack/nova master: Move refresh time from report client to prov tree https://review.openstack.org/535517 | |
| 17:24:00 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix nits in update_provider_tree series https://review.openstack.org/531260 | |
| 17:24:01 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Add nested resources to server moving tests https://review.openstack.org/527728 | |
| 17:24:01 | openstackgerrit | Eric Fried proposed openstack/nova master: Make generation optional in ProviderTree https://review.openstack.org/539324 | |
| 17:26:52 | efried | ^ rebase. The first patch should still be good to go regardless. The next one still needs some work. | |
| 17:28:13 | mriedem | is someone going to summarize all of the 'merge traits' discussion/decisions/etc from today into the ML per the retrospective at the ptg? | |
| 17:29:29 | openstackgerrit | Chris Dent proposed openstack/nova master: Move resource class fields https://review.openstack.org/540049 | |
| 17:29:30 | openstackgerrit | Chris Dent proposed openstack/nova master: Reparent placement objects to oslo_versionedobjects https://review.openstack.org/551529 | |
| 17:29:30 | openstackgerrit | Chris Dent proposed openstack/nova master: Move resource provider objects into placement hierarchy https://review.openstack.org/551528 | |
| 17:29:31 | openstackgerrit | Chris Dent proposed openstack/nova master: Isolate placement database config https://review.openstack.org/541435 | |
| 17:29:31 | openstackgerrit | Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766 | |
| 17:29:32 | openstackgerrit | Chris Dent proposed openstack/nova master: Move placement exceptions into the placement package https://review.openstack.org/549862 | |
| 17:30:53 | efried | mriedem: Well, I was planning to propose an update to the update-provider-tree spec. | |
| 17:31:14 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add 'member_of' param to GET /allocation_candidates https://review.openstack.org/552098 | |
| 17:31:20 | edleafe | dansmith: ^^ | |
| 17:31:42 | edleafe | dansmith: Still have to do tests, but I wanted jaypipes to review the SQL join code | |