Resolve TODO in _remove_host_allocations
The Selection object returned by select_destinations has a compute node UUID in it so we don't have to look up the compute node object by host/nodename when reverting allocations during live migration. Change-Id: I0156cda8543f847d50e16683e1eb29fbdd556d27
This commit is contained in:
parent
17b5a1ab85
commit
29f22b3b51
@ -489,38 +489,23 @@ class LiveMigrationTask(base.TaskBase):
|
||||
# The scheduler would have created allocations against the
|
||||
# selected destination host in Placement, so we need to remove
|
||||
# those before moving on.
|
||||
self._remove_host_allocations(host, selection.nodename)
|
||||
self._remove_host_allocations(selection.compute_node_uuid)
|
||||
host = None
|
||||
# TODO(artom) We should probably just return the whole selection object
|
||||
# at this point.
|
||||
return (selection.service_host, selection.nodename, selection.limits)
|
||||
|
||||
def _remove_host_allocations(self, host, node):
|
||||
"""Removes instance allocations against the given host from Placement
|
||||
def _remove_host_allocations(self, compute_node_uuid):
|
||||
"""Removes instance allocations against the given node from Placement
|
||||
|
||||
:param host: The name of the host.
|
||||
:param node: The name of the node.
|
||||
:param compute_node_uuid: UUID of ComputeNode resource provider
|
||||
"""
|
||||
# Get the compute node object since we need the UUID.
|
||||
# TODO(mriedem): If the result of select_destinations eventually
|
||||
# returns the compute node uuid, we wouldn't need to look it
|
||||
# up via host/node and we can save some time.
|
||||
try:
|
||||
compute_node = objects.ComputeNode.get_by_host_and_nodename(
|
||||
self.context, host, node)
|
||||
except exception.ComputeHostNotFound:
|
||||
# This shouldn't happen, but we're being careful.
|
||||
LOG.info('Unable to remove instance allocations from host %s '
|
||||
'and node %s since it was not found.', host, node,
|
||||
instance=self.instance)
|
||||
return
|
||||
|
||||
# Now remove the allocations for our instance against that node.
|
||||
# Note that this does not remove allocations against any other node
|
||||
# or shared resource provider, it's just undoing what the scheduler
|
||||
# allocated for the given (destination) node.
|
||||
self.report_client.remove_provider_tree_from_instance_allocation(
|
||||
self.context, self.instance.uuid, compute_node.uuid)
|
||||
self.context, self.instance.uuid, compute_node_uuid)
|
||||
|
||||
def _check_not_over_max_retries(self, attempted_hosts):
|
||||
if CONF.migrate_max_retries == -1:
|
||||
|
@ -33,10 +33,12 @@ from nova.tests.unit import fake_instance
|
||||
|
||||
fake_limits1 = objects.SchedulerLimits()
|
||||
fake_selection1 = objects.Selection(service_host="host1", nodename="node1",
|
||||
cell_uuid=uuids.cell, limits=fake_limits1)
|
||||
cell_uuid=uuids.cell, limits=fake_limits1,
|
||||
compute_node_uuid=uuids.compute_node1)
|
||||
fake_limits2 = objects.SchedulerLimits()
|
||||
fake_selection2 = objects.Selection(service_host="host2", nodename="node2",
|
||||
cell_uuid=uuids.cell, limits=fake_limits2)
|
||||
cell_uuid=uuids.cell, limits=fake_limits2,
|
||||
compute_node_uuid=uuids.compute_node2)
|
||||
|
||||
|
||||
class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||
@ -476,7 +478,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||
self.assertEqual(("host2", "node2", fake_limits2),
|
||||
self.task._find_destination())
|
||||
# Should have removed allocations for the first host.
|
||||
mock_remove.assert_called_once_with('host1', 'node1')
|
||||
mock_remove.assert_called_once_with(fake_selection1.compute_node_uuid)
|
||||
mock_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||
mock_select.assert_has_calls([
|
||||
mock.call(self.context, self.fake_spec, [self.instance.uuid],
|
||||
@ -511,7 +513,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||
self.assertEqual(("host2", "node2", fake_limits2),
|
||||
self.task._find_destination())
|
||||
# Should have removed allocations for the first host.
|
||||
mock_remove.assert_called_once_with('host1', 'node1')
|
||||
mock_remove.assert_called_once_with(fake_selection1.compute_node_uuid)
|
||||
mock_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||
mock_select.assert_has_calls([
|
||||
mock.call(self.context, self.fake_spec, [self.instance.uuid],
|
||||
@ -539,7 +541,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||
self.assertEqual(("host2", "node2", fake_limits2),
|
||||
self.task._find_destination())
|
||||
# Should have removed allocations for the first host.
|
||||
mock_remove.assert_called_once_with('host1', 'node1')
|
||||
mock_remove.assert_called_once_with(fake_selection1.compute_node_uuid)
|
||||
mock_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||
mock_select.assert_has_calls([
|
||||
mock.call(self.context, self.fake_spec, [self.instance.uuid],
|
||||
@ -567,7 +569,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||
self.assertEqual('failed', self.task.migration.status)
|
||||
mock_save.assert_called_once_with()
|
||||
# Should have removed allocations for the first host.
|
||||
mock_remove.assert_called_once_with('host1', 'node1')
|
||||
mock_remove.assert_called_once_with(fake_selection1.compute_node_uuid)
|
||||
mock_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||
mock_select.assert_called_once_with(
|
||||
self.context, self.fake_spec, [self.instance.uuid],
|
||||
@ -673,17 +675,12 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||
mock_get.assert_called_once_with(
|
||||
self.task.context, self.task.destination)
|
||||
|
||||
@mock.patch.object(objects.ComputeNode, 'get_by_host_and_nodename',
|
||||
side_effect=exception.ComputeHostNotFound(host='host'))
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'remove_provider_tree_from_instance_allocation')
|
||||
def test_remove_host_allocations_compute_host_not_found(
|
||||
self, remove_provider, get_cn):
|
||||
"""Tests that failing to find a ComputeNode will not blow up
|
||||
the _remove_host_allocations method.
|
||||
"""
|
||||
self.task._remove_host_allocations('host', 'node')
|
||||
remove_provider.assert_not_called()
|
||||
def test_remove_host_allocations(self, remove_provider):
|
||||
self.task._remove_host_allocations(uuids.cn)
|
||||
remove_provider.assert_called_once_with(
|
||||
self.task.context, self.task.instance.uuid, uuids.cn)
|
||||
|
||||
def test_check_can_migrate_pci(self):
|
||||
"""Tests that _check_can_migrate_pci() allows live-migration if
|
||||
|
Loading…
Reference in New Issue
Block a user