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
This commit is contained in:
Michael Still 2019-03-20 20:23:05 +00:00 committed by Stephen Finucane
parent 19cb983800
commit f8919c9705
4 changed files with 40 additions and 57 deletions

View File

@ -14363,19 +14363,21 @@ class LibvirtConnTestCase(test.NoDBTestCase,
disk_info_path = os.path.join(instance_path, 'disk.info') disk_info_path = os.path.join(instance_path, 'disk.info')
with test.nested( with test.nested(
mock.patch('builtins.open', new_callable=mock.mock_open),
mock.patch.object(os, 'mkdir'), mock.patch.object(os, 'mkdir'),
mock.patch('nova.virt.libvirt.utils.write_to_file'),
mock.patch.object(drvr, '_create_images_and_backing') mock.patch.object(drvr, '_create_images_and_backing')
) as ( ) as (
mkdir, write_to_file, create_images_and_backing mock_open, mkdir, create_images_and_backing
): ):
drvr.pre_live_migration(self.context, instance, drvr.pre_live_migration(self.context, instance,
block_device_info=None, block_device_info=None,
network_info=[], network_info=[],
disk_info=jsonutils.dumps(disk_info), disk_info=jsonutils.dumps(disk_info),
migrate_data=migrate_data) 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', @mock.patch('nova.virt.libvirt.utils.file_open',
side_effect=[io.BytesIO(b''), io.BytesIO(b'')]) 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._create_images_and_backing')
@mock.patch('nova.virt.libvirt.LibvirtDriver.' @mock.patch('nova.virt.libvirt.LibvirtDriver.'
'_get_instance_disk_info_from_config') '_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.utils.get_instance_path')
@mock.patch('nova.virt.libvirt.LibvirtDriver._get_guest_config') @mock.patch('nova.virt.libvirt.LibvirtDriver._get_guest_config')
@mock.patch('nova.virt.libvirt.blockinfo.get_disk_info') @mock.patch('nova.virt.libvirt.blockinfo.get_disk_info')
@mock.patch('nova.virt.libvirt.LibvirtDriver._destroy') @mock.patch('nova.virt.libvirt.LibvirtDriver._destroy')
@mock.patch('nova.virt.libvirt.LibvirtDriver.' @mock.patch('nova.virt.libvirt.LibvirtDriver.'
'_get_all_assigned_mediated_devices') '_get_all_assigned_mediated_devices')
@mock.patch('builtins.open', new=mock.mock_open())
def test_hard_reboot_does_not_call_glance_show(self, def test_hard_reboot_does_not_call_glance_show(self,
mock_get_mdev, mock_destroy, mock_get_disk_info, 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_get_instance_disk_info, mock_create_images_and_backing,
mock_create_domand_and_network, mock_prepare_pci_devices_for_use, mock_create_domand_and_network, mock_prepare_pci_devices_for_use,
mock_get_instance_pci_devs, mock_looping_call, mock_ensure_tree): 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, @mock.patch.object(libvirt_driver.LibvirtDriver,
'_create_guest_with_network') '_create_guest_with_network')
@mock.patch.object(libvirt_driver.LibvirtDriver, '_disk_raw_to_qcow2') @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 # NOTE(mdbooth): The following 4 mocks are required to execute
# get_guest_xml(). # get_guest_xml().
@mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled') @mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled')
@mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata') @mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata')
@mock.patch('nova.privsep.utils.supports_direct_io') @mock.patch('nova.privsep.utils.supports_direct_io')
@mock.patch('nova.api.metadata.base.InstanceMetadata') @mock.patch('nova.api.metadata.base.InstanceMetadata')
@mock.patch('builtins.open', new=mock.mock_open())
def _test_finish_migration(self, mock_instance_metadata, def _test_finish_migration(self, mock_instance_metadata,
mock_supports_direct_io, mock_supports_direct_io,
mock_build_device_metadata, mock_build_device_metadata,
mock_set_host_enabled, mock_write_to_file, mock_set_host_enabled,
mock_raw_to_qcow2, mock_raw_to_qcow2,
mock_create_guest_with_network, mock_create_guest_with_network,
mock_get_info, mock_inject_data, 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.objects.image_meta.ImageMeta.from_image_ref')
@mock.patch('nova.virt.libvirt.LibvirtDriver.' @mock.patch('nova.virt.libvirt.LibvirtDriver.'
'_get_all_assigned_mediated_devices') '_get_all_assigned_mediated_devices')
@mock.patch('nova.virt.libvirt.utils.write_to_file')
# NOTE(mdbooth): The following 4 mocks are required to execute # NOTE(mdbooth): The following 4 mocks are required to execute
# get_guest_xml(). # get_guest_xml().
@mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled') @mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled')
@mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata') @mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata')
@mock.patch('nova.privsep.utils.supports_direct_io') @mock.patch('nova.privsep.utils.supports_direct_io')
@mock.patch('nova.api.metadata.base.InstanceMetadata') @mock.patch('nova.api.metadata.base.InstanceMetadata')
def _test_rescue(self, instance, mock_instance_metadata, @mock.patch('builtins.open', new=mock.mock_open())
mock_supports_direct_io, mock_build_device_metadata, def _test_rescue(
mock_set_host_enabled, mock_write_to_file, mock_get_mdev, self, instance, mock_instance_metadata, mock_supports_direct_io,
mock_get_image_meta_by_ref, image_meta_dict=None, exists=None, mock_build_device_metadata, mock_set_host_enabled, mock_get_mdev,
instance_image_meta_dict=None, block_device_info=None): 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) self.flags(instances_path=self.useFixture(fixtures.TempDir()).path)
mock_build_device_metadata.return_value = None mock_build_device_metadata.return_value = None
@ -23929,9 +23931,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
self.assertEqual(expected_kernel_ramdisk_paths, self.assertEqual(expected_kernel_ramdisk_paths,
kernel_ramdisk_paths) kernel_ramdisk_paths)
@mock.patch('nova.virt.libvirt.utils.write_to_file') @mock.patch('builtins.open', new=mock.mock_open())
def test_rescue_stable_device_unsupported_virt_types(self, def test_rescue_stable_device_unsupported_virt_types(self):
mock_libvirt_write_to_file):
network_info = _fake_network_info(self, 1) network_info = _fake_network_info(self, 1)
instance = self._create_instance({'config_drive': str(True)}) instance = self._create_instance({'config_drive': str(True)})
rescue_image_meta_dict = {'id': uuids.rescue_image_id, 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, '_get_guest_xml'),
mock.patch.object(drvr, '_create_image'), mock.patch.object(drvr, '_create_image'),
mock.patch.object(drvr, '_get_existing_domain_xml'), 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.object(libvirt_utils, 'get_instance_path'),
mock.patch('nova.virt.libvirt.blockinfo.get_disk_info'), mock.patch('nova.virt.libvirt.blockinfo.get_disk_info'),
mock.patch('nova.image.glance.API.get'), mock.patch('nova.image.glance.API.get'),
mock.patch('nova.objects.image_meta.ImageMeta.from_dict') mock.patch('nova.objects.image_meta.ImageMeta.from_dict'),
) as (mock_create, mock_destroy, mock_get_guest_xml, mock_create_image, mock.patch('builtins.open', new_callable=mock.mock_open),
mock_get_existing_xml, mock_write, mock_inst_path, ) as (
mock_get_disk_info, mock_image_get, mock_from_dict): 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') self.flags(virt_type='kvm', group='libvirt')
mock_image_get.return_value = mock.sentinel.bdm_image_meta_dict mock_image_get.return_value = mock.sentinel.bdm_image_meta_dict
mock_from_dict.return_value = mock.sentinel.bdm_image_meta 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, 'get_instance_path')
@mock.patch.object(libvirt_utils, 'load_file') @mock.patch.object(libvirt_utils, 'load_file')
@mock.patch.object(host.Host, '_get_domain') @mock.patch.object(host.Host, '_get_domain')
def _test_unrescue(self, instance, mock_get_domain, mock_load_file, @mock.patch('builtins.open', new=mock.mock_open())
mock_get_instance_path): def _test_unrescue(
self, instance, mock_get_domain, mock_load_file,
mock_get_instance_path,
):
dummyxml = ("<domain type='kvm'><name>instance-0000000a</name>" dummyxml = ("<domain type='kvm'><name>instance-0000000a</name>"
"<devices>" "<devices>"
"<disk type='block' device='disk'>" "<disk type='block' device='disk'>"
@ -24210,7 +24215,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
with test.nested( with test.nested(
mock.patch.object(libvirt_utils, 'write_to_file'),
mock.patch.object(drvr, '_destroy'), mock.patch.object(drvr, '_destroy'),
mock.patch.object(drvr, '_create_guest'), mock.patch.object(drvr, '_create_guest'),
mock.patch.object(os, 'unlink'), mock.patch.object(os, 'unlink'),
@ -24222,7 +24226,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
mock.patch.object(lvm, 'remove_volumes'), mock.patch.object(lvm, 'remove_volumes'),
mock.patch.object(glob, 'iglob', mock.patch.object(glob, 'iglob',
return_value=[rescue_file, rescue_dir]) 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_rmtree, mock_isdir, mock_lvm_disks,
mock_remove_volumes, mock_glob): mock_remove_volumes, mock_glob):
drvr.unrescue(self.context, instance) drvr.unrescue(self.context, instance)

View File

@ -248,17 +248,6 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
finally: finally:
os.unlink(dst_path) 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.object(compute_utils, 'disk_ops_semaphore')
@mock.patch('nova.privsep.utils.supports_direct_io', return_value=False) @mock.patch('nova.privsep.utils.supports_direct_io', return_value=False)
@mock.patch('oslo_concurrency.processutils.execute') @mock.patch('oslo_concurrency.processutils.execute')
@ -330,8 +319,8 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
try: try:
os.close(dst_fd) os.close(dst_fd)
# We have a test for write_to_file. If that is sound, this suffices with open(dst_path, 'w') as f:
libvirt_utils.write_to_file(dst_path, 'hello') f.write('hello')
self.assertEqual(libvirt_utils.load_file(dst_path), 'hello') self.assertEqual(libvirt_utils.load_file(dst_path), 'hello')
finally: finally:
os.unlink(dst_path) os.unlink(dst_path)
@ -341,8 +330,8 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
try: try:
os.close(dst_fd) os.close(dst_fd)
# We have a test for write_to_file. If that is sound, this suffices with open(dst_path, 'w') as f:
libvirt_utils.write_to_file(dst_path, 'hello') f.write('hello')
with libvirt_utils.file_open(dst_path, 'r') as fp: with libvirt_utils.file_open(dst_path, 'r') as fp:
self.assertEqual(fp.read(), 'hello') self.assertEqual(fp.read(), 'hello')
finally: finally:

View File

@ -3590,7 +3590,8 @@ class LibvirtDriver(driver.ComputeDriver):
instance_dir = libvirt_utils.get_instance_path(instance) instance_dir = libvirt_utils.get_instance_path(instance)
unrescue_xml = self._get_existing_domain_xml(instance, network_info) unrescue_xml = self._get_existing_domain_xml(instance, network_info)
unrescue_xml_path = os.path.join(instance_dir, 'unrescue.xml') 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_id = None
rescue_image_meta = None rescue_image_meta = None
@ -9707,8 +9708,8 @@ class LibvirtDriver(driver.ComputeDriver):
image_disk_info_path = os.path.join(instance_dir, image_disk_info_path = os.path.join(instance_dir,
'disk.info') 'disk.info')
libvirt_utils.write_to_file(image_disk_info_path, with open(image_disk_info_path, 'w') as f:
jsonutils.dumps(image_disk_info)) f.write(jsonutils.dumps(image_disk_info))
if not is_shared_block_storage: if not is_shared_block_storage:
# Ensure images and backing files are present. # Ensure images and backing files are present.

View File

@ -306,17 +306,6 @@ def copy_image(
compression=compression) 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( def chown_for_id_maps(
path: str, id_maps: ty.List[vconfig.LibvirtConfigGuestIDMap], path: str, id_maps: ty.List[vconfig.LibvirtConfigGuestIDMap],
) -> None: ) -> None: