| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-08 | |||
| 20:12:10 | fried_rice | And then pass that ProviderTree to virt, via ComputeDriver.update_provider_tree. | |
| 20:12:35 | fried_rice | So it'll have just the compute RP as a root (no children), no inventory, no traits, no aggs, no sharing RPs. | |
| 20:13:26 | cdent | ✔ | |
| 20:14:06 | fried_rice | virt does whatever it wants, as long as it's possible via the public ProviderTree interfaces. This includes creating new root RPs (which will probably typically be sharing RPs, though we're not enforcing that) or child RPs (usually descendants of the compute RP, though we're not enforcing that). | |
| 20:14:19 | fried_rice | It also includes setting traits and inventories on any of those guys. | |
| 20:14:30 | fried_rice | And assigning aggregate UUIDs. | |
| 20:15:16 | fried_rice | virt is responsible for returning True or False from that method to let us know whether it changed anything. (TBH, I'm not sure that's necessary, cause it's pretty easy for us to figure it out, and that might be better than trusting virt to get it right.) | |
| 20:16:27 | fried_rice | Anyway, resource tracker gets that provider tree back and diffs it with what it passed in originally (which, I should have mentioned, is a copy of what it had built up); and then makes report client calls to push those changes back to the placement db. | |
| 20:16:33 | cdent | can I pause you for a moment? | |
| 20:16:36 | fried_rice | sho | |
| 20:18:26 | cdent | this is making sense, mostly, so far, except for one thing. Can you give me a scenario or two in which the provider tree will have more than one root? Is this for something like ironic, of clustering hypervisor or something else or something in addition to that? | |
| 20:19:57 | fried_rice | I can't say what'll happen for ironic. It's possible they'll have their root be their (I'm going to get the terminology wrong, but) compute "host" with children being the compute "nodes". Or maybe the compute "nodes" will all be roots. I dunno. | |
| 20:20:02 | fried_rice | A real scenario for multiple roots is sharing RPs. | |
| 20:20:32 | fried_rice | So if I have a shared disk provider that my virt driver knows about, the compute manager won't know about it yet. | |
| 20:20:59 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add instance action record for backup instances https://review.openstack.org/523676 | |
| 20:21:12 | fried_rice | virt will populate the provider tree with that shared disk provider as a new root with the MISC_SHARES_VIA_AGGREGATE trait and tag both it and the compute RP with the aggregate UUID. | |
| 20:21:33 | cdent | let me know when you are EOF, so I can ask my followup, but I don’t want to do it if you are stil typing away | |
| 20:21:44 | fried_rice | And the rt will have to handle the case where we go to create that guy in placement and find it already exists, because some other compute node that *also* shares with it already created it. | |
| 20:22:29 | fried_rice | By the same token, the various virts sharing a given sharing RP will have to make sure they use the same name and/or UUID (and we should probably document some rules for them doing that) so that we can in fact do that coordination. | |
| 20:22:58 | fried_rice | We've been saying we won't support sharing in Q, but we at least need to consider this stuff so we don't paint ourselves into a corner. | |
| 20:22:59 | fried_rice | EOF | |
| 20:25:03 | cdent | Hmm, okay. I see there are benefits to that model for sharing, especially for virtdrivers from our respective employers, but it surprises me a bit. I was imagining that a compute manager would only ever be responsble for managing stuff that it can be said to own, and ownership in this case is weird because of the -shared- aspect of things. over | |
| 20:26:29 | fried_rice | I think the flexibility of this model is good here, because it allows for some third party to manage the sharing RPs, but if such a third party doesn't exist for a given driver, it also allows for it all to be coordinated by the virt driver. | |
| 20:27:36 | fried_rice | I can speak for PowerVM, when it comes to our shared storage pool impl, that there's no third party process that's going to be able to do that stuff (unless it's at the behest of the virt driver itself under the covers, which would pretty much violate the "virt driver not talking to placement" thing). | |
| 20:27:38 | fried_rice | over | |
| 20:28:30 | fried_rice | or I can proceed with brain dump if we don't need to discuss this aspect further at the moment. | |
| 20:30:04 | cdent | I’ll just make one more comment, as I agree that the flexibility is appropriate given constraints, and then we can move on: I’d rather see the virt drivers have license to talk to placement, especially since we can’t (and shouldn’t want) to control that anyway. All you need is the right keystone token. carry on my wayward son | |
| 20:30:41 | fried_rice | You'd have to take that up with not-me, for sure. | |
| 20:31:43 | fried_rice | To the point of whether virt can create roots, we could document (and even enforce) what virt drivers are and are not allowed to do to the ProviderTree, if we wanted to. But note that the code that lets you create a new root in a ProviderTree is already merged :) | |
| 20:32:26 | fried_rice | So anyway, I pretty much started with that, wrote that patch first, and then worked my way backwards to all the plumbing we need underneath it to make it workable. | |
| 20:32:59 | fried_rice | It's in the neighborhood of 16 patches at the moment. I think we only need one more :) | |
| 20:33:14 | fried_rice | Oh, two. | |
| 20:33:24 | fried_rice | So here's where I go into my "state of the series" address. | |
| 20:34:28 | fried_rice | The bottom-most WIP was only still WIP because I wanted to add some more meat to the functional test: https://review.openstack.org/#/c/521098/10/nova/tests/functional/api/openstack/placement/test_report_client.py@332 | |
| 20:35:42 | fried_rice | That's a nice big functional test that creates a bunch of interconnected providers such as a (fairly complex) virt driver might, including inventory. See ascii diagram at the top. | |
| 20:36:14 | fried_rice | The crux is then where we call get_provider_tree, and make sure what we get back has all the stuff in it. | |
| 20:37:14 | fried_rice | BTW, the above story only talked about the first time we call into this stuff; the general case whenever we do update compute node (or whatever it's called in the periodic task) will be the opportunity for virt to tell us what, if anything, has changed in the provider world. It can shuffle inventories, aggregates, etc. | |
| 20:37:34 | fried_rice | One would hope such changes to be rare to the point of almost-never, because virt doesn't manage allocations. | |
| 20:37:53 | fried_rice | So we're talking about if someone hotplugs storage or CoDs a bank of CPUs or something. | |
| 20:37:58 | fried_rice | anyway... | |
| 20:38:23 | cdent | (if virt could manage allocations, that would be cooler, for hybrid environments) | |
| 20:38:32 | fried_rice | As I was adding the code for that TODO at the bottom, I found that the traits were in fact not showing up for certain providers. | |
| 20:38:45 | fried_rice | Which turns out to be because the traits PUT is 400ing. | |
| 20:39:01 | cdent | but nobody can tell because of None? | |
| 20:39:05 | fried_rice | No | |
| 20:39:12 | fried_rice | Because we don't have a report client method for creating traits. | |
| 20:39:27 | fried_rice | or aggregates | |
| 20:39:36 | fried_rice | so I'm doing that directly in the test, and not checking the result. | |
| 20:39:58 | fried_rice | So that's one (or maybe two) still-needed patch(es). | |
| 20:40:47 | fried_rice | While we're talking about None, there's another patch (possibly more than one) needed, which could actually be done outside of this series, to make sure we're raising where we should. | |
| 20:41:13 | fried_rice | Per discussion yesterday, we only need to do it for methods where we don't always check the return | |
| 20:41:30 | fried_rice | I identified that at least the one that gets inventories qualifies. | |
| 20:41:34 | fried_rice | There may be others. | |
| 20:43:19 | fried_rice | Now, I think I identified a change that's needed to what's already in gerrit for the get_provider_tree method to make sure we're pulling the latest traits & aggs for all "associated" providers to populate the ProviderTree. But I won't know until I figure out & fix whatever that PUT /traits deal is. | |
| 20:43:42 | fried_rice | (not PUT /traits -- PUT /resource_provider/%u/traits or whatever - you get the idea) | |
| 20:45:46 | fried_rice | Okay, next to-do: The report client method that diffs what virt driver returns and pushes those changes back to placement. It'll go between the top two patches in the series, I think. Working title for that method is update_from_provider_tree, per https://review.openstack.org/#/c/520246/12/nova/compute/resource_tracker.py@839 | |
| 20:46:54 | fried_rice | The last major thing that affects the series is how we handle generations, which we started talking about some this morning. | |
| 20:48:35 | fried_rice | Honestly, I would need to relook at pretty much the whole report client with that in mind to figure out what we're gonna need to do about it. | |
| 20:49:49 | fried_rice | Absent the proposed get-everything-about-a-provider API (https://review.openstack.org/#/c/521875/) we would have to do stuff like: | |
| 20:49:51 | cdent | do you have a summary statement of the the “generations problem”? | |
| 20:50:44 | fried_rice | when we GET a provider's traits, that response payload contains the RP generation as known to the server. If that doesn't match what we have in the local ProviderTree, we should force-refresh everything we know about that provider. | |
| 20:51:36 | cdent | or more generally: any time we get a generation in a response, if it is not a match to us, refresh? | |
| 20:52:09 | fried_rice | yeah, I think that sums it up at least to the 80/20 point :) | |
| 20:52:53 | fried_rice | At the moment, my code that retrieves traits & aggs is ignoring the rp generation field in the response. | |
| 20:54:06 | fried_rice | In particular, what it's *not* doing is calling the set_*() method with the generation from that response, which would actually be the *wrong* thing to do I think. | |
| 20:54:44 | fried_rice | Because that would be updating the generation based on thing A that changed without taking into account that thing B (whose content we still have at the old generation level) might have also changed. | |
| 20:55:38 | cdent | quite | |
| 20:56:56 | fried_rice | Beyond all that, while restacking, there are some nice review comments to address from finucannot, giblet, figleaf, takashin... | |
| 20:57:44 | cdent | If you had to identify the scariest-here-be-dragons aspect, what would it be? | |
| 20:57:44 | fried_rice | And writing up an executive summary, as you suggested in your email today, which could largely comprise text from the foregoing. | |
| 20:58:23 | fried_rice | Scariest as in potential for largest amount of rework of stuff that's not already marked WIP? The generation thing. | |
| 20:59:43 | cdent | k | |
| 21:00:26 | cdent | Let me see if I can sort of translate some things to do: | |
| 21:01:06 | cdent | a) decode the last hour of the log to extract salient links, main points, action items | |
| 21:01:22 | cdent | b) attend to comments from reviews whilst restacking | |
| 21:01:57 | cdent | c) make the linked functional test, notably it’s ascii diagram “work” and make the code do that tree and allow that tree to change, and deal with generation changes | |
| 21:03:38 | cdent | eof | |
| 21:03:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: mask InjectionInfo.admin_pass https://review.openstack.org/526772 | |
| 21:03:58 | cdent | (since c) ended up being sort of d) and e) as well) | |
| 21:04:03 | fried_rice | Pretty much. WRT (c), the only part of the diagram that the code doesn't "do" right is the traits on *some* providers. | |
| 21:04:25 | fried_rice | Which may be solved by resolving that 400, but I actually think there's more to it. I'm leaving some code comments with more info. | |
| 21:08:09 | fried_rice | Done: https://review.openstack.org/#/c/521098/10 | |
| 21:08:53 | cdent | cool | |
| 21:09:32 | fried_rice | Okay, I gotta run get the kids. Realistically, I'll be working a few scattered hours over the course of the next week, so drop me an email if you want to set up some more discussion time. | |
| 21:09:41 | cdent | I will endeavor to keep things moving. I suspect once I start breaking it it will make considerably more sense than now and happiness will abound and unicorns will leap and frolic etc | |
| 21:09:42 | figleaf | Any word from HR? | |
| 21:09:51 | figleaf | oops, wrong channel | |
| 21:09:56 | fried_rice | Yeah, don't tell HR I'm working. | |
| 21:10:26 | fried_rice | They're like, "you have HOW much vacation time left? Yeah, you gotta go." | |
| 21:10:29 | figleaf | Don't tell them I'm not :) | |
| 21:10:33 | leakypipes | figleaf: I've reported you to HR. | |
| 21:11:03 | fried_rice | leakypipes Dunno if you followed any/all of that, but would be interested to know whether you concur on the high points. | |
| 21:11:10 | figleaf | leakypipes: aw, c'mon - it was just a little hug | |
| 21:11:16 | leakypipes | figleaf: :) | |
| 21:11:23 | leakypipes | fried_rice: I was not paying attention, no... | |
| 21:11:53 | fried_rice | leakypipes And likewise, shoot me an email if we need to get together for an hour or whatever. | |
| 21:12:01 | cdent | leakypipes: the plan is to replace the scheduler, placement, the compute managers with etcd watches | |
| 21:12:16 | leakypipes | cdent: ah, cool. so k8s then. | |
| 21:12:18 | figleaf | leakypipes: running on Cassandra, of course | |
| 21:12:19 | cdent | before christmas | |