Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-19
17:27:47 efried jaypipes, edleafe, cdent ^
17:32:50 edleafe efried: stop grabbing me!
17:32:59 efried mriedem: Updated that diagram a little bit too https://awwapp.com/b/u8ghqr9rr/
17:33:09 efried edleafe: Dangit, there goes my career 40 years from now.
17:34:11 mriedem efried: then i think you definitely need a diagram in the spec to give an example
17:34:14 jroll efried: I see nodename everywhere in this spec, that signals to me that one tree per ironic node is expected
17:34:20 mriedem of what's in vs what's out when the driver gets the provider tree object
17:34:55 efried jroll: That's probably just a lack of precision on my part more than anything.
17:35:14 efried jroll: I still don't understand the difference between nodename, hypervisor_hostname, and host.name.
17:35:54 efried jroll: But I wrote the code first, and 'nodename' was the var that made sense to pass around in there, so that's probably how it ended up in the spec.
17:36:02 jroll efried: nodename ~= hypervisor_hostname, I'm not sure what host.name refers to
17:36:46 efried nodename == hypervisor_hostname, except for ironic where they're never equal; but hypervisor_hostname == host.name always ??? Or something ???
17:37:28 jroll in the ironic world, nodename == hypervisor_hostname, for sure. compute_node.host is the different one (and the one that actually looks like a hostname)
17:37:37 efried jroll: When you say ~= is that "regular expression match" à la Perl, or "not equal" à la other things?
17:37:47 efried oh, okay.
17:38:19 efried So that makes zero sense to me, that ironic node name should be the same as the hypervisor host's name. So the latter is actually a total misnomer.
17:38:22 jroll efried: equal, but I'm not 100% sure on weird drivers, so I'm going with approximately equal :)
17:38:28 efried gotcha
17:39:24 jroll sure, it's a poorly named thing. originally it was for things like xen, where compute.host (how you reach n-cpu) might be separate from the actual hypervisor (how you reach xenapi)
17:39:40 jroll s/hypervisor/hypervisor_hostname/
17:40:31 mordred mriedem: :) ... and yah - the lack of info about default in config in the az list is, I think, the thing
17:41:04 mordred mriedem: came up in some discussions around nodepool and azs - thanks for confirming
17:41:21 mriedem mordred: from what i remember, there are actually >1 nova config options related to a 'default' az
17:41:49 mordred mriedem: awesome. and of course there are :)
17:41:55 openstackgerrit sean mooney proposed openstack/nova master: Change 'InstancePCIRequest' spec field https://review.openstack.org/449257
17:41:56 openstackgerrit sean mooney proposed openstack/nova master: Add Neutron port capabilities to devspec in request https://review.openstack.org/451777
17:41:57 openstackgerrit sean mooney proposed openstack/nova master: Format NIC features using os-traits definitions https://review.openstack.org/466051
17:41:57 openstackgerrit sean mooney proposed openstack/nova master: Read Neutron port 'binding_profile' during boot https://review.openstack.org/507481
17:42:05 mriedem default_availability_zone and default_schedule_zone
17:42:38 jroll efried: as far as the one vs many trees for ironic... _update_available_resource is called once per ironic node, not once per nova-compute host. (and thus the same for get_inventory and such). in old-nova terms, it's called once per compute_node record, of which many may exist per compute service. does that make sense?
17:42:46 mordred mriedem: at root is that currently nodepool gets a list of azs and the balances across them by default - which has led to unexpected behavior before (citycloud had an az specifically for a type of flavor and were confused why were launching nodes there)
17:42:56 jroll efried: I'll do a full review of the spec and drop some comments there, if that works better for you
17:43:32 mordred mriedem: but on clouds with more than one az where they're all the same, if we DON'T balance across them we wind up with usage only in the default zone
17:43:46 jroll efried: I guess I'm mostly curious if the "one tree" constraint you're thinking of is "one root per compute service" or "one root per resource provider"?
17:43:51 efried jroll: That would be great, though I don't know that we're going to get much satisfaction out of that.
17:44:23 mriedem mordred: ok, and in the citycloud case, as a user, you don't have access to see the flavor linked to the AZ via host aggregate
17:44:28 mriedem because users can't see host aggregates
17:44:31 efried jroll: I explicitly state in the spec that update_provider_tree is being called once per ironic node, BUT that doesn't really affect how the result is used.
17:44:53 mriedem mordred: in other words, if using this flavor, don't specify AZ
17:45:18 mordred mriedem: yup
17:45:47 mriedem in queens they can at least now put a description on the flavor to say that, but your tooling likely doesn't care about that until you've already tried debugging what's going wrong
17:46:01 mordred mriedem: so there's a few potentially missing pieces of metadata .. however, clarkb has been advocating that we stop trying to balance across azs by default and instead require someone to configure az use explicitly
17:46:04 mriedem and then you have to put in special logic for that flavor on that cloud
17:46:33 mordred mriedem: exactly. when if there was some metadata, I could even validate the flavor/az combo in shade/openstacksdk
17:46:49 mordred before we even bother making an API call
17:46:54 mordred "dude, this combo won't work"
17:46:57 jroll efried: hm, having trouble finding that note, but also seeing now that this code is mostly done, so maybe I need to back up and look at the bigger picture
17:47:20 mriedem mordred: so do you end up getting a novalidhost in the citycloud case with that flavor and some other AZ?
17:47:23 efried jroll: I was not trying to (actually "trying not to") predict how ironic is going to model. What I'm trying to say is, if ironic wants to model with a separate tree/root per ironic node, this spec is insufficient to handle it.
17:48:14 mordred mriedem: I don't think so - no - I think what happened was we got scheduled on hardware that was intended for something else (they weren't expecting people to request anything in that az unless someone told them to) so we got nodes that had messed up networking or something else
17:48:28 jroll efried: sure, I'm trying to figure out what about this spec precludes doing such a thing
17:48:29 efried jroll: https://review.openstack.org/#/c/540111/5/specs/rocky/approved/update-provider-tree.rst L57-61
17:48:48 efried jroll: It's not anything about the spec. It's the way the rest of Nova is shaped currently.
17:50:00 jroll efried: ah, that block. I guess that reads to me as "one root per compute node", nothing that compute service != compute node. I guess I need to read the code.
17:50:23 efried jroll: Actually, the freshly-updated text in the note on L69-76 *does* preclude the multiple-trees-that-aren't-sharing thing.
17:50:52 jroll hmm
17:51:17 jroll ok, I will dig, I need to learn more about NRP before I can say much else. thanks, efried
17:52:04 efried jroll: Let me know if you need pointers. Much of the code for this is not yet merged.
17:52:24 efried jroll: Pending code is in series starting at https://review.openstack.org/#/c/537648/
17:52:30 jroll efried: yep, I'm there
17:53:25 openstackgerrit Merged openstack/nova stable/ocata: Add 'delete_host' command in 'nova-manage cell_v2' https://review.openstack.org/513721
17:53:53 openstackgerrit Merged openstack/nova stable/ocata: Fix test_instance_get_all_by_host https://review.openstack.org/516486
17:54:45 lyarwood mriedem: back online, yeah I'm around this week, I'll take a look this evening if there's anything left.
17:59:09 efried jaypipes: Do you have a couple minutes to help me understand the RT flow for ironic?
18:00:39 jroll efried: looking through some of this, I'm too far behind on how some of this works to fully discuss why this is or isn't fundamentally broken for ironic today, but I hope to be able to later this week, or worst case in person next week
18:00:54 jroll I can try to help you understand the current flow, if you have questions in mind
18:01:30 efried jroll: Yeah, if you don't mind.
18:01:48 efried I'm trying to understand how the current get_inventory is called.
18:02:11 efried In the ironic case, there's actually multiple ComputeNode s ?
18:02:29 jroll correct - there is a ComputeNode per ironic node
18:02:36 efried And there's a loop over those, and RT does the update_compute_node stuff for each?
18:03:01 jroll actually, there's an instance of the RT class created for each ComputeNode, IIRC
18:03:13 efried ah, that would explain why I wasn't finding said loop.
18:03:35 efried But ultimately what it means is that we are indeed creating a provider in placement for each ironic node, separately.
18:03:35 jroll that may have changed somewhere
18:03:40 jroll correct
18:04:34 efried Okay, that helps a lot. I need to go through this again and be more precise about using "host" vs "node", and probably reword the stuff in the aforementioned block about nodename.
18:05:11 openstackgerrit Merged openstack/os-vif master: Configure privsep binary https://review.openstack.org/531358
18:05:16 efried Though TBH, I feel like we've reached the Pareto point with this spec...
18:05:24 jroll efried: ah, it's still a singleton RT. but we call update_available_resource() for each node. this method is called per node: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L7241
18:06:23 efried jroll: Got it - here's the loop https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L7282 -- thanks.
18:06:28 jroll efried: well, if each *node* may be a root, rather than each *host*, I think we'd be good to go (it would match what we're doing before nested RPs)
18:06:31 jroll yep
18:07:15 efried jroll: Yes, I didn't realize we were already handling separate providers per node. So what I was saying earlier about things we don't handle was a lie (sean-k-mooney mriedem).
18:07:52 jroll gotcha, cool. this seems workable then :)
18:08:02 efried And in fact without further hacking, ironic *gets* a separate root/tree per node - doesn't actually have a choice in the matter :)
18:08:39 jroll yep!
18:08:53 efried Bootstrap-wise, update_provider_tree will get just the root for the node on the initial call; it'll be up to the virt's impl of update_provider_tree if it wants to make child providers under that root or whatever.
18:09:26 jroll I think the code is good to go anyway - line 1405 here concerns me a bit https://review.openstack.org/#/c/533821/22/nova/scheduler/client/report.py
18:10:03 efried YES
18:10:25 efried Because I *think* that guy is actually going to contain *all* the trees for *all* the nodes.
18:10:42 jroll yeah, either that or just the last tree operated on
18:10:49 jroll old_tree = self._provider_trees[nodename] :)
18:12:39 jroll because, we don't have an RP representing the compute host to be the root for all of those node RPs, they're all independent. if that makes esense.
18:12:43 jroll s/esense/sense/
18:13:06 efried jroll: yeah. I think that needs to be fixed in two or three places.
18:14:19 jroll possibly
18:14:27 efried I think it's a bigger problem
18:14:56 efried or maybe, as you say, I do that filtering only here.
18:15:24 efried I'm going to have to look at this with fresh eyes, now that I have a better understanding of the flow.

Earlier   Later