From f8919c97057ea1d6f0a62c03e0876eec70b7902d Mon Sep 17 00:00:00 2001 From: Michael Still Date: Wed, 20 Mar 2019 20:23:05 +0000 Subject: [PATCH] Remove write_to_file. It only really existed to make unit testing easier back in the day, and is trivial to move to its two callers. Change-Id: I06c4408995d4bad0a4560e8e9cd5298d4bb6b043 --- nova/tests/unit/virt/libvirt/test_driver.py | 60 +++++++++++---------- nova/tests/unit/virt/libvirt/test_utils.py | 19 ++----- nova/virt/libvirt/driver.py | 7 +-- nova/virt/libvirt/utils.py | 11 ---- 4 files changed, 40 insertions(+), 57 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index a2bab72e38ea..342e985b5ca1 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -14363,19 +14363,21 @@ class LibvirtConnTestCase(test.NoDBTestCase, disk_info_path = os.path.join(instance_path, 'disk.info') with test.nested( + mock.patch('builtins.open', new_callable=mock.mock_open), mock.patch.object(os, 'mkdir'), - mock.patch('nova.virt.libvirt.utils.write_to_file'), mock.patch.object(drvr, '_create_images_and_backing') ) as ( - mkdir, write_to_file, create_images_and_backing + mock_open, mkdir, create_images_and_backing ): drvr.pre_live_migration(self.context, instance, block_device_info=None, network_info=[], disk_info=jsonutils.dumps(disk_info), migrate_data=migrate_data) - write_to_file.assert_called_with(disk_info_path, - jsonutils.dumps(image_disk_info)) + + self.assertIn(mock.call(disk_info_path, 'w'), mock_open.mock_calls) + mock_open.return_value.write.assert_called_with( + jsonutils.dumps(image_disk_info)) @mock.patch('nova.virt.libvirt.utils.file_open', side_effect=[io.BytesIO(b''), io.BytesIO(b'')]) @@ -16292,16 +16294,16 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('nova.virt.libvirt.LibvirtDriver._create_images_and_backing') @mock.patch('nova.virt.libvirt.LibvirtDriver.' '_get_instance_disk_info_from_config') - @mock.patch('nova.virt.libvirt.utils.write_to_file') @mock.patch('nova.virt.libvirt.utils.get_instance_path') @mock.patch('nova.virt.libvirt.LibvirtDriver._get_guest_config') @mock.patch('nova.virt.libvirt.blockinfo.get_disk_info') @mock.patch('nova.virt.libvirt.LibvirtDriver._destroy') @mock.patch('nova.virt.libvirt.LibvirtDriver.' '_get_all_assigned_mediated_devices') + @mock.patch('builtins.open', new=mock.mock_open()) def test_hard_reboot_does_not_call_glance_show(self, mock_get_mdev, mock_destroy, mock_get_disk_info, - mock_get_guest_config, mock_get_instance_path, mock_write_to_file, + mock_get_guest_config, mock_get_instance_path, mock_get_instance_disk_info, mock_create_images_and_backing, mock_create_domand_and_network, mock_prepare_pci_devices_for_use, mock_get_instance_pci_devs, mock_looping_call, mock_ensure_tree): @@ -22375,18 +22377,17 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): @mock.patch.object(libvirt_driver.LibvirtDriver, '_create_guest_with_network') @mock.patch.object(libvirt_driver.LibvirtDriver, '_disk_raw_to_qcow2') - # Don't write libvirt xml to disk - @mock.patch.object(libvirt_utils, 'write_to_file') # NOTE(mdbooth): The following 4 mocks are required to execute # get_guest_xml(). @mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled') @mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata') @mock.patch('nova.privsep.utils.supports_direct_io') @mock.patch('nova.api.metadata.base.InstanceMetadata') + @mock.patch('builtins.open', new=mock.mock_open()) def _test_finish_migration(self, mock_instance_metadata, mock_supports_direct_io, mock_build_device_metadata, - mock_set_host_enabled, mock_write_to_file, + mock_set_host_enabled, mock_raw_to_qcow2, mock_create_guest_with_network, mock_get_info, mock_inject_data, @@ -23761,18 +23762,19 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): @mock.patch('nova.objects.image_meta.ImageMeta.from_image_ref') @mock.patch('nova.virt.libvirt.LibvirtDriver.' '_get_all_assigned_mediated_devices') - @mock.patch('nova.virt.libvirt.utils.write_to_file') # NOTE(mdbooth): The following 4 mocks are required to execute # get_guest_xml(). @mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled') @mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata') @mock.patch('nova.privsep.utils.supports_direct_io') @mock.patch('nova.api.metadata.base.InstanceMetadata') - def _test_rescue(self, instance, mock_instance_metadata, - mock_supports_direct_io, mock_build_device_metadata, - mock_set_host_enabled, mock_write_to_file, mock_get_mdev, - mock_get_image_meta_by_ref, image_meta_dict=None, exists=None, - instance_image_meta_dict=None, block_device_info=None): + @mock.patch('builtins.open', new=mock.mock_open()) + def _test_rescue( + self, instance, mock_instance_metadata, mock_supports_direct_io, + mock_build_device_metadata, mock_set_host_enabled, mock_get_mdev, + mock_get_image_meta_by_ref, image_meta_dict=None, exists=None, + instance_image_meta_dict=None, block_device_info=None, + ): self.flags(instances_path=self.useFixture(fixtures.TempDir()).path) mock_build_device_metadata.return_value = None @@ -23929,9 +23931,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): self.assertEqual(expected_kernel_ramdisk_paths, kernel_ramdisk_paths) - @mock.patch('nova.virt.libvirt.utils.write_to_file') - def test_rescue_stable_device_unsupported_virt_types(self, - mock_libvirt_write_to_file): + @mock.patch('builtins.open', new=mock.mock_open()) + def test_rescue_stable_device_unsupported_virt_types(self): network_info = _fake_network_info(self, 1) instance = self._create_instance({'config_drive': str(True)}) rescue_image_meta_dict = {'id': uuids.rescue_image_id, @@ -24087,15 +24088,16 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mock.patch.object(drvr, '_get_guest_xml'), mock.patch.object(drvr, '_create_image'), mock.patch.object(drvr, '_get_existing_domain_xml'), - mock.patch.object(libvirt_utils, 'write_to_file'), mock.patch.object(libvirt_utils, 'get_instance_path'), mock.patch('nova.virt.libvirt.blockinfo.get_disk_info'), mock.patch('nova.image.glance.API.get'), - mock.patch('nova.objects.image_meta.ImageMeta.from_dict') - ) as (mock_create, mock_destroy, mock_get_guest_xml, mock_create_image, - mock_get_existing_xml, mock_write, mock_inst_path, - mock_get_disk_info, mock_image_get, mock_from_dict): - + mock.patch('nova.objects.image_meta.ImageMeta.from_dict'), + mock.patch('builtins.open', new_callable=mock.mock_open), + ) as ( + mock_create, mock_destroy, mock_get_guest_xml, mock_create_image, + mock_get_existing_xml, mock_inst_path, mock_get_disk_info, + mock_image_get, mock_from_dict, mock_open, + ): self.flags(virt_type='kvm', group='libvirt') mock_image_get.return_value = mock.sentinel.bdm_image_meta_dict mock_from_dict.return_value = mock.sentinel.bdm_image_meta @@ -24185,8 +24187,11 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): @mock.patch.object(libvirt_utils, 'get_instance_path') @mock.patch.object(libvirt_utils, 'load_file') @mock.patch.object(host.Host, '_get_domain') - def _test_unrescue(self, instance, mock_get_domain, mock_load_file, - mock_get_instance_path): + @mock.patch('builtins.open', new=mock.mock_open()) + def _test_unrescue( + self, instance, mock_get_domain, mock_load_file, + mock_get_instance_path, + ): dummyxml = ("instance-0000000a" "" "" @@ -24210,7 +24215,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) with test.nested( - mock.patch.object(libvirt_utils, 'write_to_file'), mock.patch.object(drvr, '_destroy'), mock.patch.object(drvr, '_create_guest'), mock.patch.object(os, 'unlink'), @@ -24222,7 +24226,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mock.patch.object(lvm, 'remove_volumes'), mock.patch.object(glob, 'iglob', return_value=[rescue_file, rescue_dir]) - ) as (mock_write, mock_destroy, mock_create, mock_del, + ) as (mock_destroy, mock_create, mock_del, mock_rmtree, mock_isdir, mock_lvm_disks, mock_remove_volumes, mock_glob): drvr.unrescue(self.context, instance) diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 0caf55df216c..ccb96d38c2fa 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -248,17 +248,6 @@ class LibvirtUtilsTestCase(test.NoDBTestCase): finally: os.unlink(dst_path) - def test_write_to_file(self): - dst_fd, dst_path = tempfile.mkstemp() - try: - os.close(dst_fd) - - libvirt_utils.write_to_file(dst_path, 'hello') - with open(dst_path, 'r') as fp: - self.assertEqual(fp.read(), 'hello') - finally: - os.unlink(dst_path) - @mock.patch.object(compute_utils, 'disk_ops_semaphore') @mock.patch('nova.privsep.utils.supports_direct_io', return_value=False) @mock.patch('oslo_concurrency.processutils.execute') @@ -330,8 +319,8 @@ class LibvirtUtilsTestCase(test.NoDBTestCase): try: os.close(dst_fd) - # We have a test for write_to_file. If that is sound, this suffices - libvirt_utils.write_to_file(dst_path, 'hello') + with open(dst_path, 'w') as f: + f.write('hello') self.assertEqual(libvirt_utils.load_file(dst_path), 'hello') finally: os.unlink(dst_path) @@ -341,8 +330,8 @@ class LibvirtUtilsTestCase(test.NoDBTestCase): try: os.close(dst_fd) - # We have a test for write_to_file. If that is sound, this suffices - libvirt_utils.write_to_file(dst_path, 'hello') + with open(dst_path, 'w') as f: + f.write('hello') with libvirt_utils.file_open(dst_path, 'r') as fp: self.assertEqual(fp.read(), 'hello') finally: diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 7097491301e4..55ff6861877c 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -3590,7 +3590,8 @@ class LibvirtDriver(driver.ComputeDriver): instance_dir = libvirt_utils.get_instance_path(instance) unrescue_xml = self._get_existing_domain_xml(instance, network_info) unrescue_xml_path = os.path.join(instance_dir, 'unrescue.xml') - libvirt_utils.write_to_file(unrescue_xml_path, unrescue_xml) + with open(unrescue_xml_path, 'w') as f: + f.write(unrescue_xml) rescue_image_id = None rescue_image_meta = None @@ -9707,8 +9708,8 @@ class LibvirtDriver(driver.ComputeDriver): image_disk_info_path = os.path.join(instance_dir, 'disk.info') - libvirt_utils.write_to_file(image_disk_info_path, - jsonutils.dumps(image_disk_info)) + with open(image_disk_info_path, 'w') as f: + f.write(jsonutils.dumps(image_disk_info)) if not is_shared_block_storage: # Ensure images and backing files are present. diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index 5a6090257393..edc85835168d 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -306,17 +306,6 @@ def copy_image( compression=compression) -# TODO(stephenfin): This is dumb; remove it. -def write_to_file(path: str, contents: str) -> None: - """Write the given contents to a file - - :param path: Destination file - :param contents: Desired contents of the file - """ - with open(path, 'w') as f: - f.write(contents) - - def chown_for_id_maps( path: str, id_maps: ty.List[vconfig.LibvirtConfigGuestIDMap], ) -> None: