Merge "Resolve TODO in _remove_host_allocations"
This commit is contained in:
commit
7aa88029bb
|
@ -489,38 +489,23 @@ class LiveMigrationTask(base.TaskBase):
|
||||||
# The scheduler would have created allocations against the
|
# The scheduler would have created allocations against the
|
||||||
# selected destination host in Placement, so we need to remove
|
# selected destination host in Placement, so we need to remove
|
||||||
# those before moving on.
|
# those before moving on.
|
||||||
self._remove_host_allocations(host, selection.nodename)
|
self._remove_host_allocations(selection.compute_node_uuid)
|
||||||
host = None
|
host = None
|
||||||
# TODO(artom) We should probably just return the whole selection object
|
# TODO(artom) We should probably just return the whole selection object
|
||||||
# at this point.
|
# at this point.
|
||||||
return (selection.service_host, selection.nodename, selection.limits)
|
return (selection.service_host, selection.nodename, selection.limits)
|
||||||
|
|
||||||
def _remove_host_allocations(self, host, node):
|
def _remove_host_allocations(self, compute_node_uuid):
|
||||||
"""Removes instance allocations against the given host from Placement
|
"""Removes instance allocations against the given node from Placement
|
||||||
|
|
||||||
:param host: The name of the host.
|
:param compute_node_uuid: UUID of ComputeNode resource provider
|
||||||
:param node: The name of the node.
|
|
||||||
"""
|
"""
|
||||||
# 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.
|
# Now remove the allocations for our instance against that node.
|
||||||
# Note that this does not remove allocations against any other node
|
# Note that this does not remove allocations against any other node
|
||||||
# or shared resource provider, it's just undoing what the scheduler
|
# or shared resource provider, it's just undoing what the scheduler
|
||||||
# allocated for the given (destination) node.
|
# allocated for the given (destination) node.
|
||||||
self.report_client.remove_provider_tree_from_instance_allocation(
|
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):
|
def _check_not_over_max_retries(self, attempted_hosts):
|
||||||
if CONF.migrate_max_retries == -1:
|
if CONF.migrate_max_retries == -1:
|
||||||
|
|
|
@ -33,10 +33,12 @@ from nova.tests.unit import fake_instance
|
||||||
|
|
||||||
fake_limits1 = objects.SchedulerLimits()
|
fake_limits1 = objects.SchedulerLimits()
|
||||||
fake_selection1 = objects.Selection(service_host="host1", nodename="node1",
|
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_limits2 = objects.SchedulerLimits()
|
||||||
fake_selection2 = objects.Selection(service_host="host2", nodename="node2",
|
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):
|
class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||||
|
@ -476,7 +478,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||||
self.assertEqual(("host2", "node2", fake_limits2),
|
self.assertEqual(("host2", "node2", fake_limits2),
|
||||||
self.task._find_destination())
|
self.task._find_destination())
|
||||||
# Should have removed allocations for the first host.
|
# 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_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||||
mock_select.assert_has_calls([
|
mock_select.assert_has_calls([
|
||||||
mock.call(self.context, self.fake_spec, [self.instance.uuid],
|
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.assertEqual(("host2", "node2", fake_limits2),
|
||||||
self.task._find_destination())
|
self.task._find_destination())
|
||||||
# Should have removed allocations for the first host.
|
# 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_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||||
mock_select.assert_has_calls([
|
mock_select.assert_has_calls([
|
||||||
mock.call(self.context, self.fake_spec, [self.instance.uuid],
|
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.assertEqual(("host2", "node2", fake_limits2),
|
||||||
self.task._find_destination())
|
self.task._find_destination())
|
||||||
# Should have removed allocations for the first host.
|
# 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_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||||
mock_select.assert_has_calls([
|
mock_select.assert_has_calls([
|
||||||
mock.call(self.context, self.fake_spec, [self.instance.uuid],
|
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)
|
self.assertEqual('failed', self.task.migration.status)
|
||||||
mock_save.assert_called_once_with()
|
mock_save.assert_called_once_with()
|
||||||
# Should have removed allocations for the first host.
|
# 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_setup.assert_called_once_with(self.context, self.fake_spec)
|
||||||
mock_select.assert_called_once_with(
|
mock_select.assert_called_once_with(
|
||||||
self.context, self.fake_spec, [self.instance.uuid],
|
self.context, self.fake_spec, [self.instance.uuid],
|
||||||
|
@ -673,17 +675,12 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
||||||
mock_get.assert_called_once_with(
|
mock_get.assert_called_once_with(
|
||||||
self.task.context, self.task.destination)
|
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.'
|
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||||
'remove_provider_tree_from_instance_allocation')
|
'remove_provider_tree_from_instance_allocation')
|
||||||
def test_remove_host_allocations_compute_host_not_found(
|
def test_remove_host_allocations(self, remove_provider):
|
||||||
self, remove_provider, get_cn):
|
self.task._remove_host_allocations(uuids.cn)
|
||||||
"""Tests that failing to find a ComputeNode will not blow up
|
remove_provider.assert_called_once_with(
|
||||||
the _remove_host_allocations method.
|
self.task.context, self.task.instance.uuid, uuids.cn)
|
||||||
"""
|
|
||||||
self.task._remove_host_allocations('host', 'node')
|
|
||||||
remove_provider.assert_not_called()
|
|
||||||
|
|
||||||
def test_check_can_migrate_pci(self):
|
def test_check_can_migrate_pci(self):
|
||||||
"""Tests that _check_can_migrate_pci() allows live-migration if
|
"""Tests that _check_can_migrate_pci() allows live-migration if
|
||||||
|
|
Loading…
Reference in New Issue