Claims: Make sure move claims create a migration context records
This way we will be able to keep track of the resources that were actually claimed for those resources (such as NUMA topology) which are not just a simple sum but actually have state. This puts in the infra for tracking the claimed resources, however 2 additional steps are needed to fully use this and make resource tracking work for all instances when migrating: 1) We don't use the resources claimed and stashed in the Migration context to do resource tracking. This will be a follow-on fix in the resource tracker. 2) We still don't properly assign the newly claimed topology to the instance once the migration is confirmed - this will be done in a follow up patch that builds on this work. Change-Id: Ib0532f0cbebc5568d9b94b74b7a1555fcf871df5 Partial-bug: #1417667 Related-blueprint: migration-fix-resource-tracking
This commit is contained in:
parent
9f61d1eb64
commit
c458921729
|
@ -293,3 +293,17 @@ class MoveClaim(Claim):
|
|||
self.context,
|
||||
self.instance, instance_type=self.instance_type,
|
||||
image_meta=self.image_meta)
|
||||
|
||||
def create_migration_context(self):
|
||||
if not self.migration:
|
||||
# FIXME(ndipanov): Move this to a LOG.warn once Mitaka opens up
|
||||
LOG.debug("Can't create a migration_context record without a "
|
||||
"migration object specified.")
|
||||
return
|
||||
|
||||
mig_context = objects.MigrationContext(
|
||||
context=self.context, instance_uuid=self.instance.uuid,
|
||||
migration_id=self.migration.id,
|
||||
old_numa_topology=self.instance.numa_topology,
|
||||
new_numa_topology=self.claimed_numa_topology)
|
||||
return mig_context
|
||||
|
|
|
@ -238,6 +238,8 @@ class ResourceTracker(object):
|
|||
image_meta, self, self.compute_node,
|
||||
overhead=overhead, limits=limits)
|
||||
claim.migration = migration
|
||||
instance.migration_context = claim.create_migration_context()
|
||||
instance.save()
|
||||
|
||||
# Mark the resources in-use for the resize landing on this
|
||||
# compute host:
|
||||
|
@ -339,6 +341,8 @@ class ResourceTracker(object):
|
|||
ctxt = context.elevated()
|
||||
self._update(ctxt)
|
||||
|
||||
instance.drop_migration_context()
|
||||
|
||||
@utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)
|
||||
def update_usage(self, context, instance):
|
||||
"""Update the resource usage and stats after a change in an
|
||||
|
|
|
@ -415,3 +415,25 @@ class MoveClaimTestCase(ClaimTestCase):
|
|||
def test_abort(self, mock_get):
|
||||
claim = self._abort()
|
||||
self.assertTrue(claim.tracker.rcalled)
|
||||
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
def test_create_migration_context(self, mock_get):
|
||||
numa_topology = objects.InstanceNUMATopology(
|
||||
cells=[objects.InstanceNUMACell(
|
||||
id=1, cpuset=set([1, 2]), memory=512)])
|
||||
self.instance.numa_topology = None
|
||||
claim = self._claim(numa_topology=numa_topology)
|
||||
migration = objects.Migration(context=self.context, id=42)
|
||||
claim.migration = migration
|
||||
fake_mig_context = mock.Mock(spec=objects.MigrationContext)
|
||||
with mock.patch('nova.objects.MigrationContext',
|
||||
return_value=fake_mig_context) as ctxt_mock:
|
||||
claim.create_migration_context()
|
||||
ctxt_mock.assert_called_once_with(
|
||||
context=self.context, instance_uuid=self.instance.uuid,
|
||||
migration_id=42, old_numa_topology=None,
|
||||
new_numa_topology=mock.ANY)
|
||||
self.assertIsInstance(ctxt_mock.call_args[1]['new_numa_topology'],
|
||||
objects.InstanceNUMATopology)
|
||||
self.assertEqual(migration, claim.migration)
|
||||
|
|
|
@ -1090,9 +1090,10 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
|
|||
self.instance_type = self._fake_flavor_create()
|
||||
self.claim_method = self.tracker._move_claim
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
def test_abort(self, mock_get):
|
||||
def test_abort(self, mock_get, mock_save):
|
||||
try:
|
||||
with self.claim_method(self.context, self.instance,
|
||||
self.instance_type, limits=self.limits):
|
||||
|
@ -1104,10 +1105,12 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
|
|||
self._assert(0, 'local_gb_used')
|
||||
self._assert(0, 'vcpus_used')
|
||||
self.assertEqual(0, len(self.tracker.tracked_migrations))
|
||||
mock_save.assert_called_once_with()
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
def test_additive_claims(self, mock_get):
|
||||
def test_additive_claims(self, mock_get, mock_save):
|
||||
|
||||
limits = self._limits(
|
||||
2 * FAKE_VIRT_MEMORY_WITH_OVERHEAD,
|
||||
|
@ -1115,20 +1118,25 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
|
|||
2 * FAKE_VIRT_VCPUS)
|
||||
self.claim_method(
|
||||
self.context, self.instance, self.instance_type, limits=limits)
|
||||
mock_save.assert_called_once_with()
|
||||
mock_save.reset_mock()
|
||||
instance2 = self._fake_instance_obj()
|
||||
self.claim_method(
|
||||
self.context, instance2, self.instance_type, limits=limits)
|
||||
mock_save.assert_called_once_with()
|
||||
|
||||
self._assert(2 * FAKE_VIRT_MEMORY_WITH_OVERHEAD, 'memory_mb_used')
|
||||
self._assert(2 * FAKE_VIRT_LOCAL_GB, 'local_gb_used')
|
||||
self._assert(2 * FAKE_VIRT_VCPUS, 'vcpus_used')
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
def test_revert(self, mock_get):
|
||||
def test_revert(self, mock_get, mock_save):
|
||||
self.claim_method(
|
||||
self.context, self.instance, self.instance_type,
|
||||
image_meta={}, limits=self.limits)
|
||||
mock_save.assert_called_once_with()
|
||||
self.tracker.drop_move_claim(self.context, self.instance)
|
||||
|
||||
self.assertEqual(0, len(self.tracker.tracked_instances))
|
||||
|
@ -1137,20 +1145,23 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
|
|||
self._assert(0, 'local_gb_used')
|
||||
self._assert(0, 'vcpus_used')
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance_uuid',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
def test_move_type_not_tracked(self, mock_get):
|
||||
def test_move_type_not_tracked(self, mock_get, mock_save):
|
||||
self.claim_method(self.context, self.instance,
|
||||
self.instance_type, limits=self.limits, move_type="evacuation")
|
||||
mock_save.assert_called_once_with()
|
||||
|
||||
self._assert(0, 'memory_mb_used')
|
||||
self._assert(0, 'local_gb_used')
|
||||
self._assert(0, 'vcpus_used')
|
||||
self.assertEqual(0, len(self.tracker.tracked_migrations))
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch.object(objects.Migration, 'save')
|
||||
def test_existing_migration(self, save_mock):
|
||||
migration = objects.Migration(self.context,
|
||||
def test_existing_migration(self, save_mock, save_inst_mock):
|
||||
migration = objects.Migration(self.context, id=42,
|
||||
instance_uuid=self.instance.uuid,
|
||||
status='accepted',
|
||||
migration_type='evacuation')
|
||||
|
@ -1161,6 +1172,7 @@ class _MoveClaimTestCase(BaseTrackerTestCase):
|
|||
self.assertEqual("pre-migrating", migration.status)
|
||||
self.assertEqual(0, len(self.tracker.tracked_migrations))
|
||||
save_mock.assert_called_once_with()
|
||||
save_inst_mock.assert_called_once_with()
|
||||
|
||||
|
||||
class ResizeClaimTestCase(_MoveClaimTestCase):
|
||||
|
|
|
@ -304,6 +304,19 @@ _MIGRATION_INSTANCE_FIXTURES = {
|
|||
),
|
||||
}
|
||||
|
||||
_MIGRATION_CONTEXT_FIXTURES = {
|
||||
'f4f0bfea-fe7e-4264-b598-01cb13ef1997': objects.MigrationContext(
|
||||
instance_uuid='f4f0bfea-fe7e-4264-b598-01cb13ef1997',
|
||||
migration_id=3,
|
||||
new_numa_topology=None,
|
||||
old_numa_topology=None),
|
||||
'c17741a5-6f3d-44a8-ade8-773dc8c29124': objects.MigrationContext(
|
||||
instance_uuid='c17741a5-6f3d-44a8-ade8-773dc8c29124',
|
||||
migration_id=3,
|
||||
new_numa_topology=None,
|
||||
old_numa_topology=None),
|
||||
}
|
||||
|
||||
|
||||
def overhead_zero(instance):
|
||||
# Emulate that the driver does not adjust the memory
|
||||
|
@ -1163,6 +1176,7 @@ class TestInstanceClaim(BaseTestCase):
|
|||
self.assertEqualNUMAHostTopology(expected_numa, new_numa)
|
||||
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
|
||||
@mock.patch('nova.objects.Instance.get_by_uuid')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
|
||||
|
@ -1195,11 +1209,12 @@ class TestMoveClaim(BaseTestCase):
|
|||
self.rt.update_available_resource(self.ctx)
|
||||
|
||||
def register_mocks(self, pci_mock, inst_list_mock, inst_by_uuid,
|
||||
migr_mock):
|
||||
migr_mock, inst_save_mock):
|
||||
pci_mock.return_value = objects.InstancePCIRequests(requests=[])
|
||||
self.inst_list_mock = inst_list_mock
|
||||
self.inst_by_uuid = inst_by_uuid
|
||||
self.migr_mock = migr_mock
|
||||
self.inst_save_mock = inst_save_mock
|
||||
|
||||
def audit(self, rt, instances, migrations, migr_inst):
|
||||
self.inst_list_mock.return_value = \
|
||||
|
@ -1232,62 +1247,83 @@ class TestMoveClaim(BaseTestCase):
|
|||
|
||||
@mock.patch('nova.objects.Flavor.get_by_id')
|
||||
def test_claim(self, flavor_mock, pci_mock, inst_list_mock, inst_by_uuid,
|
||||
migr_mock):
|
||||
migr_mock, inst_save_mock):
|
||||
"""Resize self.instance and check that the expected quantities of each
|
||||
resource have been consumed.
|
||||
"""
|
||||
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
|
||||
inst_save_mock)
|
||||
self.driver_mock.get_host_ip_addr.return_value = "fake-ip"
|
||||
flavor_mock.return_value = objects.Flavor(**self.flavor)
|
||||
mig_context_obj = _MIGRATION_CONTEXT_FIXTURES[self.instance.uuid]
|
||||
|
||||
expected = copy.deepcopy(self.rt.compute_node)
|
||||
self.adjust_expected(expected, self.flavor)
|
||||
|
||||
with mock.patch.object(self.rt, '_create_migration') as migr_mock:
|
||||
create_mig_mock = mock.patch.object(self.rt, '_create_migration')
|
||||
mig_ctxt_mock = mock.patch('nova.objects.MigrationContext',
|
||||
return_value=mig_context_obj)
|
||||
with create_mig_mock as migr_mock, mig_ctxt_mock as ctxt_mock:
|
||||
migr_mock.return_value = _MIGRATION_FIXTURES['source-only']
|
||||
claim = self.rt.resize_claim(
|
||||
self.ctx, self.instance, self.flavor, None)
|
||||
self.assertEqual(1, ctxt_mock.call_count)
|
||||
|
||||
self.assertIsInstance(claim, claims.MoveClaim)
|
||||
inst_save_mock.assert_called_once_with()
|
||||
self.assertTrue(obj_base.obj_equal_prims(expected,
|
||||
self.rt.compute_node))
|
||||
|
||||
def test_same_host(self, pci_mock, inst_list_mock, inst_by_uuid,
|
||||
migr_mock):
|
||||
migr_mock, inst_save_mock):
|
||||
"""Resize self.instance to the same host but with a different flavor.
|
||||
Then abort the claim. Check that the same amount of resources are
|
||||
available afterwards as we started with.
|
||||
"""
|
||||
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
|
||||
inst_save_mock)
|
||||
migr_obj = _MIGRATION_FIXTURES['source-and-dest']
|
||||
self.instance = _MIGRATION_INSTANCE_FIXTURES[migr_obj['instance_uuid']]
|
||||
self.instance._context = self.ctx
|
||||
mig_context_obj = _MIGRATION_CONTEXT_FIXTURES[self.instance.uuid]
|
||||
|
||||
with mock.patch.object(self.instance, 'save'):
|
||||
self.rt.instance_claim(self.ctx, self.instance, None)
|
||||
expected = copy.deepcopy(self.rt.compute_node)
|
||||
|
||||
with mock.patch.object(self.rt, '_create_migration') as migr_mock:
|
||||
create_mig_mock = mock.patch.object(self.rt, '_create_migration')
|
||||
mig_ctxt_mock = mock.patch('nova.objects.MigrationContext',
|
||||
return_value=mig_context_obj)
|
||||
|
||||
with create_mig_mock as migr_mock, mig_ctxt_mock as ctxt_mock:
|
||||
migr_mock.return_value = migr_obj
|
||||
claim = self.rt.resize_claim(self.ctx, self.instance,
|
||||
_INSTANCE_TYPE_OBJ_FIXTURES[1], None)
|
||||
self.assertEqual(1, ctxt_mock.call_count)
|
||||
|
||||
self.audit(self.rt, [self.instance], [migr_obj], self.instance)
|
||||
inst_save_mock.assert_called_once_with()
|
||||
self.assertNotEqual(expected, self.rt.compute_node)
|
||||
|
||||
claim.abort()
|
||||
self.assertTrue(obj_base.obj_equal_prims(expected,
|
||||
self.rt.compute_node))
|
||||
claim.instance.migration_context = mig_context_obj
|
||||
with mock.patch('nova.objects.MigrationContext._destroy') as destroy_m:
|
||||
claim.abort()
|
||||
self.assertTrue(obj_base.obj_equal_prims(expected,
|
||||
self.rt.compute_node))
|
||||
destroy_m.assert_called_once_with(self.ctx, claim.instance.uuid)
|
||||
|
||||
def test_revert_reserve_source(
|
||||
self, pci_mock, inst_list_mock, inst_by_uuid, migr_mock):
|
||||
self, pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
|
||||
inst_save_mock):
|
||||
"""Check that the source node of an instance migration reserves
|
||||
resources until the migration has completed, even if the migration is
|
||||
reverted.
|
||||
"""
|
||||
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
|
||||
inst_save_mock)
|
||||
|
||||
# Get our migrations, instances and itypes in a row
|
||||
src_migr = _MIGRATION_FIXTURES['source-only']
|
||||
|
@ -1342,8 +1378,9 @@ class TestMoveClaim(BaseTestCase):
|
|||
src_rt.compute_node))
|
||||
|
||||
def test_dupe_filter(self, pci_mock, inst_list_mock, inst_by_uuid,
|
||||
migr_mock):
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock)
|
||||
migr_mock, inst_save_mock):
|
||||
self.register_mocks(pci_mock, inst_list_mock, inst_by_uuid, migr_mock,
|
||||
inst_save_mock)
|
||||
|
||||
migr_obj = _MIGRATION_FIXTURES['source-and-dest']
|
||||
# This is good enough to prevent a lazy-load; value is unimportant
|
||||
|
|
Loading…
Reference in New Issue