| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-19 | |||
| 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 | |
| 20:59:47 | mriedem | it was an admin-only extension, | |
| 20:59:49 | mriedem | we said, fuck it | |
| 21:00:17 | dansmith | melwitt: heh, right, sorry. I meant we dropped support for this stuff I'm removing :D | |
| 21:00:28 | dansmith | melwitt: if you think it's important I can rev it | |
| 21:00:44 | melwitt | when I first read it I was like O.o | |
| 21:01:24 | melwitt | I'll just put a note to self on there | |
| 21:01:27 | mriedem | dropped the migration compat code | |
| 21:01:30 | mriedem | like for flavors | |
| 21:01:54 | melwitt | yeah. I see it in the patch, was just trying to connect the commit message in case there was something really major I was under a rock about | |
| 21:23:21 | mriedem | dansmith: after reading the bug for https://review.openstack.org/#/c/543970/ can you make sure my comment is correct before I +W? | |
| 21:30:33 | dansmith | mriedem: replied for posterity, but in short: yep. | |
| 21:31:50 | hrw | sean-k-mooney: will be | |
| 21:31:55 | mriedem | cool; i'll start the backport party | |
| 21:35:58 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Lazy-load instance attributes with read_deleted=yes https://review.openstack.org/545987 | |
| 21:37:38 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Lazy-load instance attributes with read_deleted=yes https://review.openstack.org/545988 | |
| 21:38:25 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Lazy-load instance attributes with read_deleted=yes https://review.openstack.org/545989 | |
| 21:43:51 | hrw | sean-k-mooney: added myself to nova etherpad | |
| 21:45:52 | mriedem | not sure why we'd even be lazy loading instance.system_metadata in that evacuate cleanup path, the db api should manually join it https://github.com/openstack/nova/blob/stable/pike/nova/db/sqlalchemy/api.py#L2141-L2143 | |
| 21:50:28 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: libvirt: Don't VIR_MIGRATE_NON_SHARED_INC without migrate_disks https://review.openstack.org/519636 | |
| 21:51:14 | openstackgerrit | Merged openstack/nova master: Drop extra loop which modifies Cinder volume status https://review.openstack.org/539758 | |
| 21:51:45 | openstackgerrit | Merged openstack/nova master: Store block device mappings in cell0 https://review.openstack.org/544748 | |
| 21:52:13 | openstackgerrit | Merged openstack/nova master: Add functional tests to ensure BDM removal on delete https://review.openstack.org/544747 | |