diff --git a/nova/compute/manager.py b/nova/compute/manager.py index e89199cd4bf..e29634c1a8b 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -522,6 +522,10 @@ def update_compute_provider_status(self, context, rp_uuid, enabled): self.reportclient.set_traits_for_provider( context, rp_uuid, new_traits, generation=trait_info.generation) + def is_instance_storage_shared(self, context, instance, host=None): + self._compute._is_instance_storage_shared( + self, context, instance, host) + class ComputeManager(manager.Manager): """Manages the running instances from creation to destruction.""" diff --git a/nova/tests/unit/compute/test_virtapi.py b/nova/tests/unit/compute/test_virtapi.py index e477bf911cc..ce8a09f3d59 100644 --- a/nova/tests/unit/compute/test_virtapi.py +++ b/nova/tests/unit/compute/test_virtapi.py @@ -56,6 +56,12 @@ def test_update_compute_provider_status(self): nova_context.get_admin_context(), uuids.rp_uuid, enabled=False) + def test_is_instance_storage_shared(self): + self.assertExpected('is_instance_storage_shared', + nova_context.get_admin_context(), + mock.sentinel.instance, + mock.sentinel.host) + class FakeVirtAPITest(VirtAPIBaseTest): @@ -74,6 +80,8 @@ def assertExpected(self, method, *args, **kwargs): self.virtapi.exit_wait_early(*args, **kwargs) elif method == 'update_compute_provider_status': self.virtapi.update_compute_provider_status(*args, **kwargs) + elif method == 'is_instance_storage_shared': + self.virtapi.is_instance_storage_shared(*args, **kwargs) else: self.fail("Unhandled FakeVirtAPI method: %s" % method) @@ -116,6 +124,9 @@ def _prepare_for_instance_event(self, instance, name, tag): self._events.append(m) return m + def _is_instance_storage_shared(self, context, instance, host): + return + class ComputeVirtAPITest(VirtAPIBaseTest): @@ -230,3 +241,16 @@ def test_update_compute_provider_status(self): self.virtapi.update_compute_provider_status( ctxt, uuids.rp_uuid, enabled=True) self.assertEqual(set(), self.compute.provider_traits[uuids.rp_uuid]) + + def test_is_instance_storage_shared(self): + ctxt = nova_context.get_admin_context() + + with mock.patch.object(self.compute, + '_is_instance_storage_shared') as mock_shared: + self.virtapi.is_instance_storage_shared(ctxt, + mock.sentinel.instance, + mock.sentinel.host) + mock_shared.assert_called_once_with(self.virtapi, + ctxt, + mock.sentinel.instance, + mock.sentinel.host) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 23b7b060907..c47ee950083 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -18512,28 +18512,15 @@ def test_set_cache_mode_invalid_object(self): drvr._set_cache_mode(fake_conf) self.assertEqual(fake_conf.driver_cache, 'fake') - @mock.patch('os.unlink') - @mock.patch.object(os.path, 'exists') + @mock.patch.object(fake.FakeVirtAPI, 'is_instance_storage_shared') def _test_shared_storage_detection(self, is_same, - mock_exists, mock_unlink): + mock_is_instance_storage_shared): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) drvr.get_host_ip_addr = mock.MagicMock(return_value='bar') - mock_exists.return_value = is_same - with test.nested( - mock.patch.object(drvr._remotefs, 'create_file'), - mock.patch.object(drvr._remotefs, 'remove_file') - ) as (mock_rem_fs_create, mock_rem_fs_remove): - result = drvr._is_path_shared_with('host', '/path') - mock_rem_fs_create.assert_any_call('host', mock.ANY) - create_args, create_kwargs = mock_rem_fs_create.call_args - self.assertTrue(create_args[1].startswith('/path')) - if is_same: - mock_unlink.assert_called_once_with(mock.ANY) - else: - mock_rem_fs_remove.assert_called_with('host', mock.ANY) - remove_args, remove_kwargs = mock_rem_fs_remove.call_args - self.assertTrue(remove_args[1].startswith('/path')) - return result + mock_is_instance_storage_shared.return_value = is_same + return drvr._is_instance_storage_shared(None, + mock.sentinel.instance, + 'host') def test_shared_storage_detection_same_host(self): self.assertTrue(self._test_shared_storage_detection(True)) @@ -18541,19 +18528,16 @@ def test_shared_storage_detection_same_host(self): def test_shared_storage_detection_different_host(self): self.assertFalse(self._test_shared_storage_detection(False)) - @mock.patch.object(os, 'unlink') - @mock.patch.object(os.path, 'exists') - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch.object(fake.FakeVirtAPI, 'is_instance_storage_shared') @mock.patch.object(libvirt_driver.LibvirtDriver, 'get_host_ip_addr', return_value='foo') - def test_shared_storage_detection_easy(self, mock_get, mock_exec, - mock_exists, mock_unlink): + def test_shared_storage_detection_easy(self, mock_get, + mock_is_instance_storage_shared): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) - self.assertTrue(drvr._is_path_shared_with('foo', '/path')) + self.assertTrue(drvr._is_instance_storage_shared( + None, mock.sentinel.instance, 'foo')) mock_get.assert_called_once_with() - mock_exec.assert_not_called() - mock_exists.assert_not_called() - mock_unlink.assert_not_called() + mock_is_instance_storage_shared.assert_not_called() def test_store_pid_remove_pid(self): instance = objects.Instance(**self.test_instance) @@ -21585,7 +21569,7 @@ def _create_instance(self, params=None): @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_host_ip_addr', return_value='10.0.0.1') @mock.patch(('nova.virt.libvirt.driver.LibvirtDriver.' - '_is_path_shared_with'), return_value=False) + '_is_instance_storage_shared'), return_value=False) @mock.patch('os.rename') @mock.patch('os.path.exists', return_value=True) @mock.patch('oslo_concurrency.processutils.execute', @@ -21616,7 +21600,7 @@ def test_migrate_disk_and_power_off_exception( @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_host_ip_addr', return_value='10.0.0.1') @mock.patch(('nova.virt.libvirt.driver.LibvirtDriver.' - '_is_path_shared_with'), return_value=False) + '_is_instance_storage_shared'), return_value=False) @mock.patch('os.rename') @mock.patch('os.path.exists', return_value=True) @mock.patch('oslo_concurrency.processutils.execute') @@ -21656,6 +21640,7 @@ def _test_migrate_disk_and_power_off( mock_vtpm.assert_called_with( instance.uuid, mock.ANY, mock.ANY, '10.0.0.1', mock.ANY, mock.ANY) mock_unplug_vifs.assert_called_once() + return instance def test_migrate_disk_and_power_off(self): flavor = {'root_gb': 10, 'ephemeral_gb': 20} @@ -21768,14 +21753,14 @@ def _test_migrate_disk_and_power_off_resize_check(self, expected_exc): """Test for nova.virt.libvirt.libvirt_driver.LibvirtConnection .migrate_disk_and_power_off. """ - instance = self._create_instance() + self.instance = self._create_instance() flavor = {'root_gb': 10, 'ephemeral_gb': 20} flavor_obj = objects.Flavor(**flavor) # Migration is not implemented for LVM backed instances self.assertRaises(expected_exc, self.drvr.migrate_disk_and_power_off, - None, instance, '10.0.0.1', flavor_obj, None) + None, self.instance, '10.0.0.1', flavor_obj, None) @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.unplug_vifs', new=mock.Mock()) @@ -21786,7 +21771,7 @@ def _test_migrate_disk_and_power_off_resize_check(self, expected_exc): @mock.patch('nova.virt.libvirt.driver.LibvirtDriver' '._get_instance_disk_info') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver' - '._is_path_shared_with') + '._is_instance_storage_shared') def _test_migrate_disk_and_power_off_backing_file(self, shared_storage, mock_is_shared_storage, @@ -21839,7 +21824,7 @@ def test_migrate_disk_and_power_off_lvm(self): self._test_migrate_disk_and_power_off_resize_check(expected_exc) @mock.patch.object(libvirt_driver.LibvirtDriver, - '_is_path_shared_with', return_value=False) + '_is_instance_storage_shared', return_value=False) def test_migrate_disk_and_power_off_resize_cannot_ssh(self, mock_is_shared): def fake_execute(*args, **kwargs): @@ -21848,7 +21833,7 @@ def fake_execute(*args, **kwargs): expected_exc = exception.InstanceFaultRollback self._test_migrate_disk_and_power_off_resize_check(expected_exc) - mock_is_shared.assert_called_once_with('10.0.0.1', test.MatchType(str)) + mock_is_shared.assert_called_once_with(None, self.instance, '10.0.0.1') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver' '._get_instance_disk_info') @@ -21968,7 +21953,7 @@ def test_migrate_disk_and_power_off_resize_error_eph(self, mock_get, @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._destroy') @mock.patch('nova.virt.libvirt.utils.get_instance_path') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver' - '._is_path_shared_with') + '._is_instance_storage_shared') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver' '._get_instance_disk_info') def test_migrate_disk_and_power_off_resize_copy_disk_info( diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 11be13ef9ef..8b1659c6940 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -670,6 +670,9 @@ def exit_wait_early(self, events): def update_compute_provider_status(self, context, rp_uuid, enabled): pass + def is_instance_storage_shared(self, context, instance, host=None): + pass + class FakeComputeVirtAPI(FakeVirtAPI): def __init__(self, compute): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 85a4b463ed2..61f9d52878f 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -11036,26 +11036,16 @@ def _get_disk_size_reserved_for_image_cache(self): return compute_utils.convert_mb_to_ceil_gb( self.image_cache_manager.get_disk_usage() / 1024.0 / 1024.0) - def _is_path_shared_with(self, dest, path): + def _is_instance_storage_shared(self, context, instance, dest): # NOTE (rmk): There are two methods of determining whether we are # on the same filesystem: the source and dest IP are the - # same, or we create a file on the dest system via SSH - # and check whether the source system can also see it. - shared_path = (dest == self.get_host_ip_addr()) - if not shared_path: - tmp_file = uuidutils.generate_uuid(dashed=False) + '.tmp' - tmp_path = os.path.join(path, tmp_file) + # same... + if dest == self.get_host_ip_addr(): + return True - try: - self._remotefs.create_file(dest, tmp_path) - if os.path.exists(tmp_path): - shared_path = True - os.unlink(tmp_path) - else: - self._remotefs.remove_file(dest, tmp_path) - except Exception: - pass - return shared_path + # NOTE (fwiesel): Or we rely on the drivers api pair + # check_instance_shared_storage_local / check_instance_shared_storage + return self.virtapi.is_instance_storage_shared(context, instance, dest) def migrate_disk_and_power_off(self, context, instance, dest, flavor, network_info, @@ -11095,7 +11085,9 @@ def migrate_disk_and_power_off(self, context, instance, dest, # shared storage for instance dir (eg. NFS). inst_base = libvirt_utils.get_instance_path(instance) inst_base_resize = inst_base + "_resize" - shared_instance_path = self._is_path_shared_with(dest, inst_base) + shared_instance_path = self._is_instance_storage_shared(context, + instance, + dest) # try to create the directory on the remote compute node # if this fails we pass the exception up the stack so we can catch diff --git a/nova/virt/virtapi.py b/nova/virt/virtapi.py index dad1197d8cc..eec9ffd3799 100644 --- a/nova/virt/virtapi.py +++ b/nova/virt/virtapi.py @@ -34,3 +34,6 @@ def update_compute_provider_status(self, context, rp_uuid, enabled): the trait would be added. """ raise NotImplementedError() + + def is_instance_storage_shared(self, context, instance, host=None): + raise NotImplementedError()