openstack/nova
Code Issues Proposed changes

Merge "Move confirm resize under semaphore"

Browse Source
This commit is contained in:
Zuul
2020-09-10 18:49:19 +00:00
committed by Gerrit Code Review
parent ee9ceca416 a57800d382
commit 65631f257b
7 changed files with 56 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
nova/compute/manager.py
View File

@ -4356,6 +4356,8 @@ class ComputeManager(manager.Manager):
# NOTE(tr3buchet): tear down networks on source host
self.network_api.setup_networks_on_host(context, instance,
migration.source_compute, teardown=True)
# TODO(stephenfin): These next three calls should be bundled
network_info = self.network_api.get_instance_nw_info(context,
instance)
@ -4369,17 +4371,8 @@ class ComputeManager(manager.Manager):
self.driver.confirm_migration(context, migration, instance,
network_info)
migration.status = 'confirmed'
migration.save()
# NOTE(mriedem): drop_move_claim relies on
# instance.migration_context so make sure to not call
# instance.drop_migration_context() until after drop_move_claim
# is called.
self.rt.drop_move_claim(
context, instance, migration.source_node, instance.old_flavor,
prefix='old_')
instance.drop_migration_context()
# Free up the old_flavor usage from the resource tracker for this host.
self.rt.drop_move_claim_at_source(context, instance, migration)
# NOTE(mriedem): The old_vm_state could be STOPPED but the user
# might have manually powered up the instance to confirm the
@ -4537,13 +4530,7 @@ class ComputeManager(manager.Manager):
self._delete_volume_attachments(ctxt, instance.get_bdms())
# Free up the old_flavor usage from the resource tracker for this host.
self.rt.drop_move_claim(
ctxt, instance, migration.source_node, instance.old_flavor,
prefix='old_')
instance.drop_migration_context()
migration.status = 'confirmed'
migration.save()
self.rt.drop_move_claim_at_source(ctxt, instance, migration)
def _confirm_snapshot_based_resize_delete_port_bindings(
self, ctxt, instance):

22
nova/compute/resource_tracker.py
View File

@ -551,9 +551,31 @@ class ResourceTracker(object):
dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj()
self.compute_nodes[nodename].pci_device_pools = dev_pools_obj
@utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair=True)
def drop_move_claim_at_source(self, context, instance, migration):
"""Drop a move claim after confirming a resize or cold migration."""
migration.status = 'confirmed'
migration.save()
self._drop_move_claim(
context, instance, migration.source_node, instance.old_flavor,
prefix='old_')
# NOTE(stephenfin): Unsetting this is unnecessary for cross-cell
# resize, since the source and dest instance objects are different and
# the source instance will be deleted soon. It's easier to just do it
# though.
instance.drop_migration_context()
@utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair=True)
def drop_move_claim(self, context, instance, nodename,
instance_type=None, prefix='new_'):
self._drop_move_claim(
context, instance, nodename, instance_type, prefix='new_')
def _drop_move_claim(
self, context, instance, nodename, instance_type=None, prefix='new_',
):
"""Remove usage for an incoming/outgoing migration.
:param context: Security context.

2
nova/conductor/tasks/cross_cell_migrate.py
View File

@ -985,7 +985,7 @@ class ConfirmResizeTask(base.TaskBase):
"""
LOG.debug('Updating migration and instance status in target cell DB.',
instance=self.instance)
# Complete the migration confirmation.
# Update the target cell migration.
self.migration.status = 'confirmed'
self.migration.save()
# Update the target cell instance.

16
nova/tests/functional/regressions/test_bug_1879878.py
View File

@ -70,7 +70,7 @@ class TestSameCell(integrated_helpers._IntegratedTestBase):
self.assertUsage(src_host, 1)
self.assertUsage(dst_host, 0)
orig_drop_claim = rt.ResourceTracker.drop_move_claim
orig_drop_claim = rt.ResourceTracker.drop_move_claim_at_source
def fake_drop_move_claim(*args, **kwargs):
# run periodics after marking the migration confirmed, simulating a
@ -83,15 +83,14 @@ class TestSameCell(integrated_helpers._IntegratedTestBase):
if drop_race:
self._run_periodics()
# FIXME(stephenfin): the periodic should not have dropped the
# records for the src yet
self.assertUsage(src_host, 0)
self.assertUsage(dst_host, 1)
self.assertUsage(src_host, 1)
self.assertUsage(dst_host, 1)
return orig_drop_claim(*args, **kwargs)
self.stub_out(
'nova.compute.resource_tracker.ResourceTracker.drop_move_claim',
'nova.compute.resource_tracker.ResourceTracker.'
'drop_move_claim_at_source',
fake_drop_move_claim,
)
@ -107,10 +106,7 @@ class TestSameCell(integrated_helpers._IntegratedTestBase):
# migration is now confirmed so we should once again only have usage on
# one host
# FIXME(stephenfin): Our usage here should be 0 and 1 for source and
# dest respectively when confirming, but that won't happen until we run
# the periodic and rebuild our inventory from scratch
self.assertUsage(src_host, -1 if drop_race else 0)
self.assertUsage(src_host, 0)
self.assertUsage(dst_host, 1)
# running periodics shouldn't change things

6
nova/tests/unit/compute/test_compute.py
View File

@ -8151,14 +8151,10 @@ class ComputeTestCase(BaseTestCase,
instance.new_flavor = new_type
instance.migration_context = objects.MigrationContext()
def fake_drop_move_claim(*args, **kwargs):
pass
def fake_setup_networks_on_host(self, *args, **kwargs):
pass
self._mock_rt(
drop_move_claim=mock.Mock(side_effect=fake_drop_move_claim))
self._mock_rt()
with test.nested(
mock.patch.object(self.compute.network_api,

28
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -8800,14 +8800,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
mock_mig_save, mock_mig_get, mock_inst_get,
mock_delete_scheduler_info):
def fake_drop_move_claim(*args, **kwargs):
# RT.drop_move_claim must be called before
# instance.drop_migration_context.
mock_drop.assert_not_called()
mock_rt = self._mock_rt()
# Enforce order of drop_move_claim/drop_migration_context calls.
mock_rt.drop_move_claim.side_effect = fake_drop_move_claim
self._mock_rt()
self.instance.migration_context = objects.MigrationContext(
new_pci_devices=None,
old_pci_devices=None)
@ -11592,22 +11585,22 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
'_confirm_snapshot_based_resize_delete_port_bindings')
@mock.patch('nova.compute.manager.ComputeManager.'
'_delete_volume_attachments')
@mock.patch('nova.objects.Instance.drop_migration_context')
def test_confirm_snapshot_based_resize_at_source(
self, mock_drop_mig_ctx, mock_delete_vols, mock_delete_bindings,
self, mock_delete_vols, mock_delete_bindings,
mock_delete_allocs, mock_get_bdms):
"""Happy path test for confirm_snapshot_based_resize_at_source."""
self.instance.old_flavor = objects.Flavor()
with test.nested(
mock.patch.object(self.compute, 'network_api'),
mock.patch.object(self.compute.driver, 'cleanup'),
mock.patch.object(self.compute.rt, 'drop_move_claim')
mock.patch.object(self.compute, 'network_api'),
mock.patch.object(self.compute.driver, 'cleanup'),
mock.patch.object(self.compute.rt, 'drop_move_claim_at_source')
) as (
mock_network_api, mock_cleanup, mock_drop_claim
mock_network_api, mock_cleanup, mock_drop_claim,
):
# Run the code.
self.compute.confirm_snapshot_based_resize_at_source(
self.context, self.instance, self.migration)
# Assert the mocks.
mock_network_api.get_instance_nw_info.assert_called_once_with(
self.context, self.instance)
@ -11623,12 +11616,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
self.context, mock_get_bdms.return_value)
# Move claim and migration context were dropped.
mock_drop_claim.assert_called_once_with(
self.context, self.instance, self.migration.source_node,
self.instance.old_flavor, prefix='old_')
mock_drop_mig_ctx.assert_called_once_with()
# The migration was updated.
self.assertEqual('confirmed', self.migration.status)
self.migration.save.assert_called_once_with()
self.context, self.instance, self.migration)
mock_delete_allocs.assert_called_once_with(
self.context, self.instance, self.migration)

22
nova/tests/unit/compute/test_resource_tracker.py
View File

@ -2541,6 +2541,7 @@ class TestResize(BaseTestCase):
mig_context_obj.new_resources = objects.ResourceList(
objects=[self.resource_1, self.resource_2])
instance.migration_context = mig_context_obj
instance.system_metadata = {}
migration = objects.Migration(
id=3,
@ -2582,15 +2583,18 @@ class TestResize(BaseTestCase):
len(self.rt.assigned_resources[cn.uuid][rc]))
# Confirm or revert resize
if revert:
flavor = new_flavor
prefix = 'new_'
else:
flavor = old_flavor
prefix = 'old_'
self.rt.drop_move_claim(ctx, instance, _NODENAME, flavor,
prefix=prefix)
with test.nested(
mock.patch('nova.objects.Migration.save'),
mock.patch('nova.objects.Instance.drop_migration_context'),
):
if revert:
flavor = new_flavor
prefix = 'new_'
self.rt.drop_move_claim(
ctx, instance, _NODENAME, flavor, prefix=prefix)
else: # confirm
flavor = old_flavor
self.rt.drop_move_claim_at_source(ctx, instance, migration)
expected = compute_update_usage(expected, flavor, sign=-1)
self.assertTrue(obj_base.obj_equal_prims(