From fde77d49ff550b73f5f1671edc7366c9b7646200 Mon Sep 17 00:00:00 2001 From: Rajesh Tailor Date: Thu, 5 Feb 2015 02:57:59 -0800 Subject: [PATCH] Delete instance files from dest host in revert-resize When revert-resize call is finished, it doesn't clear instance files from destination node for non-shared instance storage. The driver call, which is responsible for destroying destination host instance is not deleting instance files, because it finds that both source and destination host shares the same instance storage. The check_instance_shared_storage rpc-call had no host information, hence the manager call for check_instance_shared_storage was checking instance storage on destination host itself and returning True even if it is non-shared storage. Added host parameter to check_instance_shared_storage rpc-call, so that in manager call check_instance_shared_storage method get executed on source compute node and returns True/False based on shared/non-shared storage. Closes-Bug: #1418855 Change-Id: Ic529b1c2c1cfd914facb14941e4bb641db6a0e82 --- nova/compute/manager.py | 8 +++---- nova/compute/rpcapi.py | 4 ++-- nova/tests/unit/compute/test_compute.py | 4 +++- nova/tests/unit/compute/test_compute_mgr.py | 23 ++++++++++++++------- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6da40af83e75..e5946432439c 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -774,7 +774,7 @@ class ComputeManager(manager.Manager): network_info, bdi, destroy_disks) - def _is_instance_storage_shared(self, context, instance): + def _is_instance_storage_shared(self, context, instance, host=None): shared_storage = True data = None try: @@ -783,7 +783,7 @@ class ComputeManager(manager.Manager): if data: shared_storage = (self.compute_rpcapi. check_instance_shared_storage(context, - instance, data)) + instance, data, host=host)) except NotImplementedError: LOG.warning(_LW('Hypervisor driver does not support ' 'instance shared storage check, ' @@ -3573,8 +3573,8 @@ class ComputeManager(manager.Manager): block_device_info = self._get_instance_block_device_info( context, instance, bdms=bdms) - destroy_disks = not self._is_instance_storage_shared(context, - instance) + destroy_disks = not self._is_instance_storage_shared( + context, instance, host=migration.source_compute) self.driver.destroy(context, instance, network_info, block_device_info, destroy_disks) diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index b6b56b77f0d8..b672dfe07afb 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -403,13 +403,13 @@ class ComputeAPI(object): instance=instance, dest_check_data=dest_check_data) - def check_instance_shared_storage(self, ctxt, instance, data): + def check_instance_shared_storage(self, ctxt, instance, data, host=None): if self.client.can_send_version('3.29'): version = '3.29' else: version = '3.0' instance = jsonutils.to_primitive(instance) - cctxt = self.client.prepare(server=_compute_host(None, instance), + cctxt = self.client.prepare(server=_compute_host(host, instance), version=version) return cctxt.call(ctxt, 'check_instance_shared_storage', instance=instance, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index d59552527b4d..8dac170bbb3d 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -4241,6 +4241,8 @@ class ComputeTestCase(BaseTestCase): self._stub_out_resize_network_methods() instance = self._create_fake_instance_obj() for operation in actions: + if 'revert_resize' in operation: + migration.source_compute = 'fake-mini' def fake_migration_save(*args, **kwargs): raise test.TestingException() @@ -6600,7 +6602,7 @@ class ComputeTestCase(BaseTestCase): evacuated_instance).AndReturn({'filename': 'tmpfilename'}) self.compute.compute_rpcapi.check_instance_shared_storage(fake_context, evacuated_instance, - {'filename': 'tmpfilename'}).AndReturn(False) + {'filename': 'tmpfilename'}, host=None).AndReturn(False) self.compute.driver.check_instance_shared_storage_cleanup(fake_context, {'filename': 'tmpfilename'}) self.compute.driver.destroy(fake_context, evacuated_instance, diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 59fc51494694..9ef46082687f 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -3362,7 +3362,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): self.assertEqual([mock.call(), mock.call()], migration_obj_as_admin.mock_calls) - def test_revert_resize_instance_destroy_disks(self): + def _test_revert_resize_instance_destroy_disks(self, is_shared=False): # This test asserts that _is_instance_storage_shared() is called from # revert_resize() and the return value is passed to driver.destroy(). @@ -3394,20 +3394,27 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): self.migration.source_compute = self.instance['host'] - # inform compute that this instance uses shared storage - _is_instance_storage_shared.return_value = True + # Inform compute that instance uses non-shared or shared storage + _is_instance_storage_shared.return_value = is_shared self.compute.revert_resize(context=self.context, migration=self.migration, instance=self.instance, reservations=None) - _is_instance_storage_shared.assert_called_once_with(self.context, - self.instance) + _is_instance_storage_shared.assert_called_once_with( + self.context, self.instance, + host=self.migration.source_compute) - # since shared storage is used, we should not be instructed to - # destroy disks here + # If instance storage is shared, driver destroy method + # should not destroy disks otherwise it should destroy disks. destroy.assert_called_once_with(self.context, self.instance, - mock.ANY, mock.ANY, False) + mock.ANY, mock.ANY, not is_shared) do_test() + + def test_revert_resize_instance_destroy_disks_shared_storage(self): + self._test_revert_resize_instance_destroy_disks(is_shared=True) + + def test_revert_resize_instance_destroy_disks_non_shared_storage(self): + self._test_revert_resize_instance_destroy_disks(is_shared=False)