Set migration object attributes for source/dest during live migrate
During live migration, the Migration object is created in the conductor task manager but the node fields are not yet set, and dest_compute wouldn't be set if a host was not specified in the API. This change makes sure that the dest_compute, dest_node and source_node attributes are set on the Migration object in the LiveMigrationTask before casting to the source compute. Change-Id: Idad5cdbb2c5647c469e4ad5e9393564255df0f7f
This commit is contained in:
Matt Riedemann
parent
5f05d02aaf
commit
dc64366a77
|
|
@ -53,9 +53,7 @@ class LiveMigrationTask(base.TaskBase):
|
|||
# wants the scheduler to pick a destination host, or a host was
|
||||
# specified but is not forcing it, so they want the scheduler
|
||||
# filters to run on the specified host, like a scheduler hint.
|
||||
self.destination = self._find_destination()
|
||||
self.migration.dest_compute = self.destination
|
||||
self.migration.save()
|
||||
self.destination, dest_node = self._find_destination()
|
||||
else:
|
||||
# This is the case that the user specified the 'force' flag when
|
||||
# live migrating with a specific destination host so the scheduler
|
||||
|
|
@ -76,6 +74,14 @@ class LiveMigrationTask(base.TaskBase):
|
|||
scheduler_utils.claim_resources_on_destination(
|
||||
self.scheduler_client.reportclient, self.instance,
|
||||
source_node, dest_node)
|
||||
# dest_node is a ComputeNode object, so we need to get the actual
|
||||
# node name off it to set in the Migration object below.
|
||||
dest_node = dest_node.hypervisor_hostname
|
||||
|
||||
self.migration.source_node = self.instance.node
|
||||
self.migration.dest_node = dest_node
|
||||
self.migration.dest_compute = self.destination
|
||||
self.migration.save()
|
||||
|
||||
# TODO(johngarbutt) need to move complexity out of compute manager
|
||||
# TODO(johngarbutt) disk_over_commit?
|
||||
|
|
@ -305,7 +311,7 @@ class LiveMigrationTask(base.TaskBase):
|
|||
# those before moving on.
|
||||
self._remove_host_allocations(host, hoststate['nodename'])
|
||||
host = None
|
||||
return host
|
||||
return host, hoststate['nodename']
|
||||
|
||||
def _remove_host_allocations(self, host, node):
|
||||
"""Removes instance allocations against the given host from Placement
|
||||
|
|
|
|||
|
|
@ -61,15 +61,17 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
self.fake_spec)
|
||||
|
||||
def test_execute_with_destination(self):
|
||||
dest_node = objects.ComputeNode(hypervisor_hostname='dest_node')
|
||||
with test.nested(
|
||||
mock.patch.object(self.task, '_check_host_is_up'),
|
||||
mock.patch.object(self.task, '_check_requested_destination',
|
||||
return_value=(mock.sentinel.source_node,
|
||||
mock.sentinel.dest_node)),
|
||||
dest_node)),
|
||||
mock.patch.object(scheduler_utils,
|
||||
'claim_resources_on_destination'),
|
||||
mock.patch.object(self.migration, 'save'),
|
||||
mock.patch.object(self.task.compute_rpcapi, 'live_migration'),
|
||||
) as (mock_check_up, mock_check_dest, mock_claim, mock_mig):
|
||||
) as (mock_check_up, mock_check_dest, mock_claim, mock_save, mock_mig):
|
||||
mock_mig.return_value = "bob"
|
||||
|
||||
self.assertEqual("bob", self.task.execute())
|
||||
|
|
@ -77,7 +79,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
mock_check_dest.assert_called_once_with()
|
||||
mock_claim.assert_called_once_with(
|
||||
self.task.scheduler_client.reportclient, self.instance,
|
||||
mock.sentinel.source_node, mock.sentinel.dest_node)
|
||||
mock.sentinel.source_node, dest_node)
|
||||
mock_mig.assert_called_once_with(
|
||||
self.context,
|
||||
host=self.instance_host,
|
||||
|
|
@ -86,6 +88,13 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
block_migration=self.block_migration,
|
||||
migration=self.migration,
|
||||
migrate_data=None)
|
||||
self.assertTrue(mock_save.called)
|
||||
# make sure the source/dest fields were set on the migration object
|
||||
self.assertEqual(self.instance.node, self.migration.source_node)
|
||||
self.assertEqual(dest_node.hypervisor_hostname,
|
||||
self.migration.dest_node)
|
||||
self.assertEqual(self.task.destination,
|
||||
self.migration.dest_compute)
|
||||
|
||||
def test_execute_without_destination(self):
|
||||
self.destination = None
|
||||
|
|
@ -98,7 +107,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
mock.patch.object(self.task.compute_rpcapi, 'live_migration'),
|
||||
mock.patch.object(self.migration, 'save')
|
||||
) as (mock_check, mock_find, mock_mig, mock_save):
|
||||
mock_find.return_value = "found_host"
|
||||
mock_find.return_value = ("found_host", "found_node")
|
||||
mock_mig.return_value = "bob"
|
||||
|
||||
self.assertEqual("bob", self.task.execute())
|
||||
|
|
@ -113,6 +122,8 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
migrate_data=None)
|
||||
self.assertTrue(mock_save.called)
|
||||
self.assertEqual('found_host', self.migration.dest_compute)
|
||||
self.assertEqual('found_node', self.migration.dest_node)
|
||||
self.assertEqual(self.instance.node, self.migration.source_node)
|
||||
|
||||
def test_check_instance_is_active_passes_when_paused(self):
|
||||
self.task.instance['power_state'] = power_state.PAUSED
|
||||
|
|
@ -294,12 +305,12 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
self.fake_spec.reset_forced_destinations()
|
||||
self.task.scheduler_client.select_destinations(
|
||||
self.context, self.fake_spec, [self.instance.uuid]).AndReturn(
|
||||
[{'host': 'host1'}])
|
||||
[{'host': 'host1', 'nodename': 'node1'}])
|
||||
self.task._check_compatible_with_source_hypervisor("host1")
|
||||
self.task._call_livem_checks_on_host("host1")
|
||||
|
||||
self.mox.ReplayAll()
|
||||
self.assertEqual("host1", self.task._find_destination())
|
||||
self.assertEqual(("host1", "node1"), self.task._find_destination())
|
||||
|
||||
# Make sure the request_spec was updated to include the cell
|
||||
# mapping.
|
||||
|
|
@ -326,9 +337,9 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
check_compat, call_livem_checks):
|
||||
get_image.return_value = "image"
|
||||
from_components.return_value = another_spec
|
||||
select_dest.return_value = [{'host': 'host1'}]
|
||||
select_dest.return_value = [{'host': 'host1', 'nodename': 'node1'}]
|
||||
|
||||
self.assertEqual("host1", task._find_destination())
|
||||
self.assertEqual(("host1", "node1"), task._find_destination())
|
||||
|
||||
get_image.assert_called_once_with(self.instance.system_metadata)
|
||||
setup_ig.assert_called_once_with(self.context, another_spec)
|
||||
|
|
@ -354,12 +365,12 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
scheduler_utils.setup_instance_group(self.context, self.fake_spec)
|
||||
self.task.scheduler_client.select_destinations(self.context,
|
||||
self.fake_spec, [self.instance.uuid]).AndReturn(
|
||||
[{'host': 'host1'}])
|
||||
[{'host': 'host1', 'nodename': 'node1'}])
|
||||
self.task._check_compatible_with_source_hypervisor("host1")
|
||||
self.task._call_livem_checks_on_host("host1")
|
||||
|
||||
self.mox.ReplayAll()
|
||||
self.assertEqual("host1", self.task._find_destination())
|
||||
self.assertEqual(("host1", "node1"), self.task._find_destination())
|
||||
|
||||
def _test_find_destination_retry_hypervisor_raises(self, error):
|
||||
self.mox.StubOutWithMock(utils, 'get_image_from_system_metadata')
|
||||
|
|
@ -381,14 +392,14 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
|
||||
self.task.scheduler_client.select_destinations(self.context,
|
||||
self.fake_spec, [self.instance.uuid]).AndReturn(
|
||||
[{'host': 'host2'}])
|
||||
[{'host': 'host2', 'nodename': 'node2'}])
|
||||
self.task._check_compatible_with_source_hypervisor("host2")
|
||||
self.task._call_livem_checks_on_host("host2")
|
||||
|
||||
self.mox.ReplayAll()
|
||||
with mock.patch.object(self.task,
|
||||
'_remove_host_allocations') as remove_allocs:
|
||||
self.assertEqual("host2", self.task._find_destination())
|
||||
self.assertEqual(("host2", "node2"), self.task._find_destination())
|
||||
# Should have removed allocations for the first host.
|
||||
remove_allocs.assert_called_once_with('host1', 'node1')
|
||||
|
||||
|
|
@ -422,14 +433,14 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
|
||||
self.task.scheduler_client.select_destinations(self.context,
|
||||
self.fake_spec, [self.instance.uuid]).AndReturn(
|
||||
[{'host': 'host2'}])
|
||||
[{'host': 'host2', 'nodename': 'node2'}])
|
||||
self.task._check_compatible_with_source_hypervisor("host2")
|
||||
self.task._call_livem_checks_on_host("host2")
|
||||
|
||||
self.mox.ReplayAll()
|
||||
with mock.patch.object(self.task,
|
||||
'_remove_host_allocations') as remove_allocs:
|
||||
self.assertEqual("host2", self.task._find_destination())
|
||||
self.assertEqual(("host2", "node2"), self.task._find_destination())
|
||||
# Should have removed allocations for the first host.
|
||||
remove_allocs.assert_called_once_with('host1', 'node1')
|
||||
|
||||
|
|
@ -455,14 +466,14 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
|
|||
|
||||
self.task.scheduler_client.select_destinations(self.context,
|
||||
self.fake_spec, [self.instance.uuid]).AndReturn(
|
||||
[{'host': 'host2'}])
|
||||
[{'host': 'host2', 'nodename': 'node2'}])
|
||||
self.task._check_compatible_with_source_hypervisor("host2")
|
||||
self.task._call_livem_checks_on_host("host2")
|
||||
|
||||
self.mox.ReplayAll()
|
||||
with mock.patch.object(self.task,
|
||||
'_remove_host_allocations') as remove_allocs:
|
||||
self.assertEqual("host2", self.task._find_destination())
|
||||
self.assertEqual(("host2", "node2"), self.task._find_destination())
|
||||
# Should have removed allocations for the first host.
|
||||
remove_allocs.assert_called_once_with('host1', 'node1')
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue