| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-19 | |||
| 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. | |
| 18:15:53 | jroll | awesome, thanks efried, this was helpful for my understanding too :) | |
| 18:16:31 | efried | jroll: Your help is much appreciated too. Let's have a Guinness next week. | |
| 18:16:44 | jroll | ++ | |
| 18:24:37 | openstackgerrit | sean mooney proposed openstack/nova master: Add Neutron port capabilities to devspec in request https://review.openstack.org/451777 | |
| 18:24:38 | openstackgerrit | sean mooney proposed openstack/nova master: Format NIC features using os-traits definitions https://review.openstack.org/466051 | |
| 18:24:38 | openstackgerrit | sean mooney proposed openstack/nova master: Read Neutron port 'binding_profile' during boot https://review.openstack.org/507481 | |
| 18:27:00 | mriedem | this pretty simple bug fix has been hanging out with a +2 for a few months now https://review.openstack.org/#/c/519464/ - simply unquiesce an instance if we quiesced it but creating volume snapshots fails | |
| 18:30:18 | openstackgerrit | sean mooney proposed openstack/nova-specs master: Reintroduced nic feature based scheduling for rocky https://review.openstack.org/545951 | |
| 18:39:46 | dansmith | mriedem: got that and the two above it | |
| 18:40:00 | dansmith | which seem like obvious should-be-doing-that items | |
| 18:41:28 | openstackgerrit | Merged openstack/nova master: Imported Translations from Zanata https://review.openstack.org/541561 | |
| 19:14:52 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Update Provider Tree https://review.openstack.org/540111 | |
| 19:15:14 | efried | mriedem, jaypipes, edleafe, cdent: I'm no stephenfin, but ^ | |
| 19:15:17 | efried | jroll: ^ | |
| 19:18:57 | edleafe | efried: if only you included an ASCII diagram of CN2... | |
| 19:19:17 | efried | edleafe: Eh? Did I not? | |
| 19:19:25 | mriedem | dansmith: thanks | |
| 19:20:00 | edleafe | efried: I meant for the PT when UPT is invoked for CN2 | |
| 19:20:23 | efried | oh. It's symmetrical. Are you feing bunny? | |
| 19:20:31 | edleafe | efried: just busting your chops, of course :) | |
| 19:20:35 | efried | phew | |
| 19:20:49 | efried | consider my chops busted | |
| 19:24:04 | openstackgerrit | Merged openstack/nova stable/ocata: libvirt: bandwidth param should be set in guest migrate https://review.openstack.org/519635 | |
| 19:24:20 | openstackgerrit | Merged openstack/python-novaclient master: Fix the docstring for the update method https://review.openstack.org/545819 | |
| 19:29:26 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Combine error handling blocks in _do_build_and_run_instance https://review.openstack.org/545960 | |
| 19:31:54 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: unquiesce instance on volume snapshot failure https://review.openstack.org/545961 | |
| 20:03:26 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: unquiesce instance on volume snapshot failure https://review.openstack.org/545966 | |
| 20:06:17 | openstackgerrit | Merged openstack/nova stable/ocata: Use proper user and tenant in the owner section of libvirt.xml. https://review.openstack.org/525997 | |
| 20:08:14 | mriedem | dansmith: want to get this simple fixture cleanup patch? https://review.openstack.org/#/c/539758/ | |
| 20:08:21 | mriedem | that will unblock a few other approved changes | |
| 20:08:50 | mriedem | this is the series where bfv failing during scheduling always orphans your volumes | |
| 20:08:52 | mriedem | which sucks | |
| 20:10:40 | mriedem | FYI in case any cores want to shed 1300+ LOC https://review.openstack.org/#/c/544698/ | |
| 20:16:40 | dansmith | mriedem: ack | |
| 20:22:40 | openstackgerrit | Merged openstack/os-vif master: zuul: Enable functional tests in gate https://review.openstack.org/530961 | |
| 20:22:46 | openstackgerrit | Merged openstack/nova master: unquiesce instance on volume snapshot failure https://review.openstack.org/519464 | |
| 20:32:25 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: unquiesce instance on volume snapshot failure https://review.openstack.org/545973 | |
| 20:59:25 | melwitt | dansmith, mriedem: on https://review.openstack.org/#/c/544698, "We dropped support for aggregates in newton" just means dropped support for the old "main db" aggregates, not dropped support for aggregates altogether, right? | |
| 20:59:42 | mriedem | melwitt: we just deleted the API | |