openstack/nova
Code Issues Proposed changes

Restrict live migration to same cell

Browse Source 
We do not yet support live migrating an instance
across cells. This change handles two cases for
live migration:

1. The destination host is forced so the scheduler
   is bypassed. In this case we directly compare the
   source cell against the destination cell and fail
   if they are not the same with a MigrationPreCheckError.

2. If no destination host is specified, or it's not forced,
   we update the RequestSpec sent to the scheduler so it
   will restrict the compute nodes it pulls from the same
   cell that the instance lives in. If a host is requested
   in this case but it's in a different cell, it would result
   in a NoValidHost error from the scheduler.

Change-Id: I66fc72d402ac118270a835cf929fe1ea387d78cd
Closes-Bug: #1712008
This commit is contained in:
Matt Riedemann 2017-08-22 16:54:44 -04:00
parent 72728ed01f
commit 8fc789deb7
2 changed files with 114 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
nova/conductor/tasks/live_migrate.py
View File

@ -117,6 +117,16 @@ class LiveMigrationTask(base.TaskBase):
source_node, dest_node = self._check_compatible_with_source_hypervisor(
self.destination)
self._call_livem_checks_on_host(self.destination)
# Make sure the forced destination host is in the same cell that the
# instance currently lives in.
# NOTE(mriedem): This can go away if/when the forced destination host
# case calls select_destinations.
source_cell_mapping = self._get_source_cell_mapping()
dest_cell_mapping = self._get_destination_cell_mapping()
if source_cell_mapping.uuid != dest_cell_mapping.uuid:
raise exception.MigrationPreCheckError(
reason=(_('Unable to force live migrate instance %s '
'across cells.') % self.instance.uuid))
return source_node, dest_node
def _claim_resources_on_destination(self, source_node, dest_node):
@ -246,6 +256,36 @@ class LiveMigrationTask(base.TaskBase):
"%s") % destination
raise exception.MigrationPreCheckError(msg)
def _get_source_cell_mapping(self):
"""Returns the CellMapping for the cell in which the instance lives
:returns: nova.objects.CellMapping record for the cell where
the instance currently lives.
:raises: MigrationPreCheckError - in case a mapping is not found
"""
try:
return objects.InstanceMapping.get_by_instance_uuid(
self.context, self.instance.uuid).cell_mapping
except exception.InstanceMappingNotFound:
raise exception.MigrationPreCheckError(
reason=(_('Unable to determine in which cell '
'instance %s lives.') % self.instance.uuid))
def _get_destination_cell_mapping(self):
"""Returns the CellMapping for the destination host
:returns: nova.objects.CellMapping record for the cell where
the destination host is mapped.
:raises: MigrationPreCheckError - in case a mapping is not found
"""
try:
return objects.HostMapping.get_by_host(
self.context, self.destination).cell_mapping
except exception.HostMappingNotFound:
raise exception.MigrationPreCheckError(
reason=(_('Unable to determine in which cell '
'destination host %s lives.') % self.destination))
def _find_destination(self):
# TODO(johngarbutt) this retry loop should be shared
attempted_hosts = [self.source]
@ -269,6 +309,21 @@ class LiveMigrationTask(base.TaskBase):
# is not forced to be the original host
request_spec.reset_forced_destinations()
scheduler_utils.setup_instance_group(self.context, request_spec)
# We currently only support live migrating to hosts in the same
# cell that the instance lives in, so we need to tell the scheduler
# to limit the applicable hosts based on cell.
cell_mapping = self._get_source_cell_mapping()
LOG.debug('Requesting cell %(cell)s while live migrating',
{'cell': cell_mapping.identity},
instance=self.instance)
if ('requested_destination' in request_spec and
request_spec.requested_destination):
request_spec.requested_destination.cell = cell_mapping
else:
request_spec.requested_destination = objects.Destination(
cell=cell_mapping)
host = None
while host is None:
self._check_not_over_max_retries(attempted_hosts)

59
nova/tests/unit/conductor/tasks/test_live_migrate.py
View File

@ -12,6 +12,7 @@
import mock
import oslo_messaging as messaging
import six
from nova.compute import power_state
from nova.compute import rpcapi as compute_rpcapi
@ -246,6 +247,33 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
mock.call(self.destination)],
mock_get_info.call_args_list)
@mock.patch.object(objects.Service, 'get_by_compute_host')
@mock.patch.object(live_migrate.LiveMigrationTask, '_get_compute_info')
@mock.patch.object(servicegroup.API, 'service_is_up')
@mock.patch.object(compute_rpcapi.ComputeAPI,
'check_can_live_migrate_destination')
@mock.patch.object(objects.HostMapping, 'get_by_host',
return_value=objects.HostMapping(
cell_mapping=objects.CellMapping(
uuid=uuids.different)))
def test_check_requested_destination_fails_different_cells(
self, mock_get_host_mapping, mock_check, mock_is_up,
mock_get_info, mock_get_host):
mock_get_host.return_value = "service"
mock_is_up.return_value = True
hypervisor_details = objects.ComputeNode(
hypervisor_type="a",
hypervisor_version=6.1,
free_ram_mb=513,
memory_mb=512,
ram_allocation_ratio=1.0)
mock_get_info.return_value = hypervisor_details
mock_check.return_value = "migrate_data"
ex = self.assertRaises(exception.MigrationPreCheckError,
self.task._check_requested_destination)
self.assertIn('across cells', six.text_type(ex))
def test_find_destination_works(self):
self.mox.StubOutWithMock(utils, 'get_image_from_system_metadata')
self.mox.StubOutWithMock(scheduler_utils, 'setup_instance_group')
@ -271,6 +299,10 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
self.mox.ReplayAll()
self.assertEqual("host1", self.task._find_destination())
# Make sure the request_spec was updated to include the cell
# mapping.
self.assertIsNotNone(self.fake_spec.requested_destination.cell)
def test_find_destination_works_with_no_request_spec(self):
task = live_migrate.LiveMigrationTask(
self.context, self.instance, self.destination,
@ -300,6 +332,9 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
setup_ig.assert_called_once_with(self.context, another_spec)
select_dest.assert_called_once_with(self.context, another_spec,
[self.instance.uuid])
# Make sure the request_spec was updated to include the cell
# mapping.
self.assertIsNotNone(another_spec.requested_destination.cell)
check_compat.assert_called_once_with("host1")
call_livem_checks.assert_called_once_with("host1")
do_test()
@ -577,3 +612,27 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
self.instance.project_id, self.instance.user_id)
test()
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid',
side_effect=exception.InstanceMappingNotFound(
uuid=uuids.instance))
def test_get_source_cell_mapping_not_found(self, mock_get):
"""Negative test where InstanceMappingNotFound is raised and converted
to MigrationPreCheckError.
"""
self.assertRaises(exception.MigrationPreCheckError,
self.task._get_source_cell_mapping)
mock_get.assert_called_once_with(
self.task.context, self.task.instance.uuid)
@mock.patch.object(objects.HostMapping, 'get_by_host',
side_effect=exception.HostMappingNotFound(
name='destination'))
def test_get_destination_cell_mapping_not_found(self, mock_get):
"""Negative test where HostMappingNotFound is raised and converted
to MigrationPreCheckError.
"""
self.assertRaises(exception.MigrationPreCheckError,
self.task._get_destination_cell_mapping)
mock_get.assert_called_once_with(
self.task.context, self.task.destination)