| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-21 | |||
| 18:48:47 | mriedem | when you're force live migrating to a specific destination host, conductor checks that there is enough ram https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/conductor/tasks/live_migrate.py#L103 | |
| 18:48:54 | mriedem | via the compute node info, not placement | |
| 18:49:17 | dansmith | ah interesting | |
| 18:49:18 | mriedem | not an immediate problem, | |
| 18:49:32 | mriedem | but will be once we don't track that anymore in the RT/claim | |
| 18:49:35 | dansmith | yeah | |
| 18:49:53 | mriedem | i'll report a bug to track that | |
| 18:49:55 | sean-k-mooney | is there an equivelent to nova reset-state in the openstack client? | |
| 18:54:43 | mriedem | dansmith: i think this existing bug will suffice to track that https://bugs.launchpad.net/nova/+bug/1427772 | |
| 18:54:44 | openstack | Launchpad bug 1427772 in OpenStack Compute (nova) "Instance that uses force-host still needs to run some filters" [Low,Confirmed] | |
| 18:55:00 | mriedem | i just left a new comment for the new state of the world wrt scheduler and compute and RT | |
| 18:55:13 | mriedem | and tagged with placement so it shows up in cdent's weekly email | |
| 18:56:11 | dansmith | I guess.. the link between the two is a little vague | |
| 18:56:56 | mriedem | it could be a separate bug | |
| 18:59:43 | mriedem | dansmith: so the bug is that when forcing the live migration to a specific host, we don't call scheduler_client.select_destinations | |
| 18:59:55 | mriedem | which is eventually the thing that does the double claim on source and dest | |
| 19:00:16 | mriedem | and we don't 'heal' the allocations since everything, at least in this scenario, is pike, so the RT isn't healing | |
| 19:00:48 | mriedem | because of https://review.openstack.org/#/c/491012/ | |
| 19:01:13 | mriedem | in both cases, conductor calls def check_can_live_migrate_destination on the dest host | |
| 19:01:33 | mriedem | so we could create the allocation for the instance and that compute there, or just do it in conductor if we're bypassing the scheduler | |
| 19:02:07 | mriedem | probably simpler to just do it in from conductor, plus then we don't have to worry about check_can_live_migrate_destination trampling over allocations that the scheduler already created | |
| 19:06:41 | dansmith | mriedem: right | |
| 19:07:31 | dansmith | mriedem: I guess I was thinking we should do select_destinations with a force_host of where we're expecting to go so that it creates the allocations properly | |
| 19:08:10 | dansmith | the whole point of the mess we made in pike was to stop managing allocations from separate places (i.e. the compute nodes) so adding another one is not ideal | |
| 19:08:41 | mriedem | yeah it wouldn't be fun doing a new thing from conductor just to mimic what the scheduler is doing | |
| 19:08:54 | mriedem | also because takashi wants to do this same thing for cold migrations | |
| 19:09:06 | mriedem | well, takashi and others... | |
| 19:09:34 | dansmith | aye | |
| 19:10:13 | mriedem | so now i'm looking at what conductor does when there isn't a forced destination host https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/conductor/tasks/live_migrate.py#L153 | |
| 19:10:38 | mriedem | the request spec stuff gets pretty confusing in here | |
| 19:10:44 | mriedem | for compat things | |
| 19:11:24 | mriedem | we wouldn't call _find_destination i realize, | |
| 19:11:43 | mriedem | i'm just not sure how much we need to pass in the existing request spec with some "use this host, seriously" field set | |
| 19:13:42 | mriedem | would the scheduler also run the filters on the forced host in this case? | |
| 19:14:49 | mriedem | that might take the 'force-ness' out of it a bit if you fail to force because of a scheduler filter - but would allow us to remove this code https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/conductor/tasks/live_migrate.py#L103 | |
| 19:14:59 | dansmith | mriedem: the scheduler won't know why you're running it | |
| 19:15:16 | dansmith | the point is, | |
| 19:15:32 | dansmith | the select_destinations() call should eliminate the need to manually check for memory space on the destination, | |
| 19:15:49 | mriedem | from conductor | |
| 19:15:51 | mriedem | i agree | |
| 19:15:53 | dansmith | because if it doesn't return the host you asked for then you fail with "sorry this peg doesn't fit in that hole" | |
| 19:15:55 | dansmith | yeah | |
| 19:16:03 | mriedem | yeah which is nice | |
| 19:16:07 | mriedem | removes my issue with that code not using placement | |
| 19:17:21 | mriedem | so it seems all we need to do then is have conductor set force_hosts/nodes on the request spec and call select_destinations - if that raises NoValidHost, we know what to do | |
| 19:17:47 | mriedem | i seriously feel like if i try to monkey with the request spec there is some landmine that will blow off at least 2 fingers | |
| 19:18:39 | dansmith | hah | |
| 19:18:54 | mriedem | this could also go away assuming you have the ComputeFilter enabled https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/conductor/tasks/live_migrate.py#L85 | |
| 19:22:07 | mriedem | i think it's basically this that we need https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/compute/api.py#L3880-L3884 | |
| 19:26:52 | mriedem | god, why doesn't this check if self.requested_destination is set when converting down to a legacy filter_properties dict? https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/objects/request_spec.py#L347 | |
| 19:27:04 | mriedem | requested_destination essentially supersedes force_hosts/nodes doesn't it? | |
| 19:30:32 | dansmith | mriedem: afaik yeah | |
| 19:31:04 | mriedem | weird, ok, because it's not consulted at all, from what i can tell, when converting a request spec to an older version | |
| 19:32:10 | mriedem | oh you know what else, this bypasses our same-cell check | |
| 19:32:21 | mriedem | so you could force a host for live migration in another cell | |
| 19:33:39 | mriedem | this force live migration thing is all sorts of f'ed | |
| 19:35:02 | dansmith | yeah, so all the more reason to let it do the cell and memory check in one go of select_destinations right? | |
| 19:35:23 | mriedem | yeah, although i don't see where in the scheduler we check that the requested destination is in the same cell... | |
| 19:35:44 | mriedem | get_host_states_by_uuids ? | |
| 19:36:06 | mriedem | https://github.com/openstack/nova/blob/16.0.0.0rc1/nova/scheduler/host_manager.py#L635 | |
| 19:36:33 | dansmith | yeah | |
| 19:36:52 | mriedem | cool, ok, so i think this narrows things a bit | |
| 19:37:09 | mriedem | i think we are missing some stuff in the request spec backport to legacy format stuff, but i'm not messing with that | |
| 19:37:13 | mriedem | those are all bauzas questions | |
| 19:37:25 | mriedem | since it seems there are 4 ways to construct a request spec for the scheduler | |
| 19:39:03 | dansmith | yeah that whole mess is still...a mess | |
| 19:39:26 | mriedem | well it's 2:40pm on my first day back and i'm already knee deep in a mess that has to be fixed for rc2 | |
| 19:39:27 | mriedem | yay! | |
| 19:40:23 | dansmith | well, | |
| 19:40:42 | dansmith | presumably we _could_ just document and fix post release if needed | |
| 19:40:48 | dansmith | if it helps | |
| 19:40:53 | mriedem | yeah i thought about that | |
| 19:40:57 | mriedem | known issue and all | |
| 19:41:00 | mriedem | "known regression" | |
| 19:41:16 | mriedem | "at least we let you know about it beforehand, you're welcome" | |
| 19:41:21 | dansmith | we wouldn't want to wait long, of course, but.. | |
| 19:41:29 | dansmith | things could be worse | |
| 19:41:33 | mriedem | yeah | |
| 19:41:38 | mriedem | i.e. optional api | |
| 19:42:26 | mriedem | ok i'll wip something together and put it on top of https://review.openstack.org/#/c/495170/ and see what happens | |
| 19:43:04 | dansmith | okay | |
| 19:43:56 | openstackgerrit | Merged openstack/nova master: Correct statement in api-ref https://review.openstack.org/495724 | |
| 19:54:54 | mriedem | this was the other bug, I haven't triaged it yet https://bugs.launchpad.net/nova/+bug/1712045 | |
| 19:54:55 | openstack | Launchpad bug 1712045 in OpenStack Compute (nova) "nova doesn't clean up the resources after live migrate" [Undecided,New] | |
| 19:55:43 | mriedem | looks similar to what was needed with evacuate though https://review.openstack.org/#/c/494625/1/nova/compute/manager.py | |
| 19:55:51 | mriedem | remove the allocations for the instance and the source node | |
| 19:56:46 | dansmith | yeah | |
| 19:58:17 | dansmith | mriedem: that test for the bug you're working on has the post assertion for the destination's allocations commented out, | |
| 19:58:30 | dansmith | so fixing 1712045 will be required for that one too I think | |
| 20:06:02 | openstackgerrit | Merged openstack/nova master: doc: Address review comments for contributor index https://review.openstack.org/491517 | |
| 20:07:10 | mriedem | wtf, is local docs build blowing up a known issue? | |
| 20:07:25 | mriedem | https://gist.github.com/mriedem/4a6dcb52ed867af14f16989d7b83739a | |
| 20:18:40 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: Address review comments for main index https://review.openstack.org/492645 | |
| 20:19:31 | mriedem | dansmith: i think we should probably get ^ to rc2 to fix the "OpenSack" thing in the first section you read in nova's docs | |
| 20:19:53 | dansmith | lol | |
| 20:26:45 | openstackgerrit | Ilya Popov proposed openstack/nova master: Tests: Add cleanup of 'instances' directory https://review.openstack.org/491589 | |
| 20:32:50 | mriedem | as for that live migration test, yeah it's a mix of both bugs, and the comments are wrong for some of the existing test | |
| 20:32:55 | mriedem | i'll update that test also | |
| 20:32:58 | dansmith | mriedem: well, I have the change made for the post-migration update I think, but I can't really use it until I have your patch to create the doubled allocation in the scheduler | |
| 20:33:11 | mriedem | cleaning up the test atm | |