| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-08 | |||
| 19:13:41 | dansmith | not really well named | |
| 19:13:48 | melwitt | so it's called just before select_destinations in conductor, for example. but it's also called for conductor tasks like migrate | |
| 19:14:01 | melwitt | yeah | |
| 19:14:03 | jaypipes | mriedem: done | |
| 19:14:15 | mriedem | nova.scheduler.utils.shove_group_crap_in_reqspec | |
| 19:14:22 | openstackgerrit | Ameed Ashour proposed openstack/osc-placement master: Change documentation theme https://review.openstack.org/542378 | |
| 19:15:21 | dansmith | melwitt: well, I think I would have done it differently so we could scatter/gather and use the cell cache in there | |
| 19:15:36 | dansmith | but this should work I guess if it's in front of all the places where we need it | |
| 19:15:51 | melwitt | dansmith: do you mean like duplicate the get_hosts code there? I considered that but wasn't sure what you would prefer | |
| 19:16:00 | melwitt | I do wish to use scatter-gather ideally | |
| 19:16:26 | dansmith | I mean like just pull the whole group with hosts from the cells and then use the .hosts from those things | |
| 19:16:48 | dansmith | however, | |
| 19:16:49 | dansmith | can we really not pull a group out with the hosts pre-populated? | |
| 19:16:55 | dansmith | I don't see any that let you specify it | |
| 19:16:57 | melwitt | because scatter-gather expects to call a method that takes a context as the first arg and get_hosts uses the object._context | |
| 19:17:11 | melwitt | oh, I see | |
| 19:17:25 | dansmith | right, so just do a query for the group and get those back | |
| 19:17:56 | dansmith | you can always adapt s/g by just using a closure | |
| 19:18:14 | dansmith | oh | |
| 19:18:16 | dansmith | yeah, | |
| 19:18:18 | dansmith | this would be better | |
| 19:18:19 | dansmith | hang on | |
| 19:20:33 | dansmith | https://pastebin.com/Gv26a83b | |
| 19:20:48 | dansmith | that will just do the load with the right context in each cell in parallel | |
| 19:20:57 | dansmith | then you can stitch the hosts together after | |
| 19:20:57 | dansmith | right? | |
| 19:21:56 | melwitt | seems like it ... let me try it out | |
| 19:23:03 | melwitt | I wanted to do something like that, be able to pass something for scatter-gather. so if this works that would be ideal | |
| 19:28:28 | openstackgerrit | Merged openstack/nova master: TrivialFix: Add a blankline https://review.openstack.org/542094 | |
| 19:29:22 | melwitt | I noticed something else weird, another query for get_hosts. trying to figure out what to do with that https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L687 | |
| 19:30:26 | melwitt | it seems like it's redundant. we already got hosts for the group, then we query for an instance group again based on instance uuid, then get the hosts for that one? | |
| 19:32:11 | melwitt | I guess it's that, first get the hosts for the request_spec.instance_group. but if request_spec.instance_uuid is populated, override the old request_spec.instance_group with the instance group associated with the instance_uuid | |
| 19:32:13 | cfriesen | So we recently tripped over an interesting live migration bug....if you have an instance with an rbd-backed root disk and a config drive, libvirt will try to block-migrate the rbd drive and fail. The libvirt folks (dpb) seem to be saying that nova shouldn't set VIR_MIGRATE_NON_SHARED_INC if there aren't any disks to migrate. | |
| 19:32:19 | mriedem | dansmith: just realized we have some compat code doing up-calls in the compute manager, so that's fun https://review.openstack.org/#/c/541005/6 | |
| 19:33:23 | dansmith | mriedem: yep but super old, before multi-cell could be a thing anyway | |
| 19:33:38 | melwitt | cfriesen: that sounds like something artom might know about ^ | |
| 19:33:40 | mriedem | right. comment in https://review.openstack.org/#/c/541035/ btw | |
| 19:34:09 | mriedem | cfriesen: i think mdbooth already patched that | |
| 19:34:50 | dansmith | mriedem: ack, thanks | |
| 19:34:52 | mriedem | https://review.openstack.org/#/q/I9b545ca8aa6dd7b41ddea2d333190c9fbed19bc1 | |
| 19:34:55 | mriedem | cfriesen: ^ | |
| 19:36:27 | cfriesen | mriedem: yeah, I'm pretty sure we're seeing this in pike, which should have that change in it. Will try and bottom it out. | |
| 19:36:45 | mriedem | pike 16.0.4+? | |
| 19:38:08 | cfriesen | I can see the change in our version of the code, I just need to confirm we can reproduce the bug with the current code. | |
| 19:41:16 | dansmith | mriedem: I'm going to fix that self.reservations thing in a follow-on patch because that bubbles up pretty high | |
| 19:41:32 | mriedem | that's fine | |
| 19:54:02 | mriedem | melwitt: given you're going to want to backport this to pike and ocata, https://review.openstack.org/#/c/541442/ - i'm not sure if you want to address nits now or not | |
| 19:54:22 | mriedem | but i'll be around for 2 more hours if you do so i can +W | |
| 19:55:59 | melwitt | mriedem: I'm cool with fixing nits. thanks for the heads up. I'll update it right after I update this instance group thing. adding test coverage | |
| 19:58:00 | dansmith | melwitt: es worky? | |
| 19:58:12 | melwitt | dansmith: yis. thank you | |
| 19:58:16 | dansmith | \o/ | |
| 19:58:24 | melwitt | o/ high five! | |
| 19:59:28 | dansmith | let it be known I had a good idea once | |
| 20:00:28 | melwitt | heh | |
| 20:01:11 | artom | We have no way to detach an interface when the compute is down, right? | |
| 20:01:35 | artom | I know it's an RPC cast and everything, so compute needs to be running to receive it | |
| 20:02:12 | mriedem | yes | |
| 20:02:15 | mriedem | correct i mean | |
| 20:02:24 | melwitt | yeah. you could at best tell neutron to do things with the port | |
| 20:02:24 | artom | But just sanity-checking, you can't mark an interface as detached and then later unplug the vif when compute comes back, right? | |
| 20:02:40 | dansmith | artom: not currently, but also, | |
| 20:02:46 | dansmith | consider if compute manager is all that is down, | |
| 20:02:54 | dansmith | but the interface and address are still being used on the data plane | |
| 20:02:59 | dansmith | that would be like bad and stuff to re-assign it | |
| 20:03:17 | artom | dansmith, ah, indeed. | |
| 20:03:29 | artom | So not only we don't do it, but we don't even want to do it | |
| 20:03:54 | artom | Thanks dudes :) | |
| 20:03:58 | artom | (And Mel) | |
| 20:03:59 | artom | ;) | |
| 20:04:22 | melwitt | :) | |
| 20:07:12 | openstackgerrit | Ameed Ashour proposed openstack/osc-placement master: Change documentation theme https://review.openstack.org/542378 | |
| 20:10:27 | openstackgerrit | Dan Smith proposed openstack/nova master: Compute RPC client bump to 5.0 https://review.openstack.org/541035 | |
| 20:10:28 | openstackgerrit | Dan Smith proposed openstack/nova master: Clean up reservations in migrate_task call path https://review.openstack.org/542409 | |
| 20:10:49 | dansmith | artom: not without a lot more infrastructure around it I would say | |
| 20:10:50 | dansmith | mriedem: ^ as promised with cleanup | |
| 20:16:14 | mriedem | dansmith: https://review.openstack.org/#/c/541035/5/nova/compute/rpcapi.py@752 | |
| 20:16:21 | mriedem | my point there was the same as the reservations things you removed | |
| 20:17:22 | dansmith | mriedem: replied | |
| 20:17:47 | mriedem | sure, my point is, pass migration_id=None | |
| 20:17:53 | dansmith | but that wasn't valid | |
| 20:17:58 | dansmith | reservations=None was | |
| 20:18:08 | dansmith | something else will try to use migration_id as an integer and fail | |
| 20:18:47 | dansmith | oh, it was defaulted to none | |
| 20:19:21 | dansmith | FFS, fine.. we have migration still passed here and don't for reservations, so it seems like the most compatible to just do the right thing but whatever | |
| 20:19:38 | mriedem | long ago it didn't have a default of None https://review.openstack.org/#/c/287997/19/nova/compute/manager.py@5167 | |
| 20:20:10 | dansmith | ah, that was tdurakov's mistake yeah, | |
| 20:20:13 | mriedem | we could also just remove that later in cleanup | |
| 20:20:20 | dansmith | it's not compatible that way | |
| 20:20:46 | dansmith | so passing None there would break that older code if we were to interact with ti | |
| 20:20:53 | dansmith | mriedem: all the 4.x stuff goes away in rocky anyway | |
| 20:21:04 | dansmith | so yes, the same cleanup that removes all this would remove that anyway | |
| 20:21:13 | openstackgerrit | melanie witt proposed openstack/nova master: Make scheduler.utils.setup_instance_group query all cells https://review.openstack.org/540258 | |
| 20:21:19 | mriedem | +2 | |
| 20:21:22 | mriedem | onto the cleanup patch | |
| 20:22:16 | dansmith | I havent' written the "drop 4.x" patch yet, but can get on that once we start to slow down | |
| 20:22:52 | mriedem | +2s all the way | |
| 20:22:56 | mriedem | time to find another core | |
| 20:23:00 | mriedem | they tend to hide | |
| 20:26:08 | openstackgerrit | Merged openstack/nova master: Workaround glanceclient bug when CONF.glance.api_servers not set https://review.openstack.org/541008 | |