Skip to content

Commit f8919c9

Browse files
mikalstillstephenfin
authored andcommitted
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
1 parent 19cb983 commit f8919c9

File tree

4 files changed

+40
-57
lines changed

4 files changed

+40
-57
lines changed

nova/tests/unit/virt/libvirt/test_driver.py

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14363,19 +14363,21 @@ def test_pre_live_migration_recreate_disk_info(self):
1436314363
disk_info_path = os.path.join(instance_path, 'disk.info')
1436414364

1436514365
with test.nested(
14366+
mock.patch('builtins.open', new_callable=mock.mock_open),
1436614367
mock.patch.object(os, 'mkdir'),
14367-
mock.patch('nova.virt.libvirt.utils.write_to_file'),
1436814368
mock.patch.object(drvr, '_create_images_and_backing')
1436914369
) as (
14370-
mkdir, write_to_file, create_images_and_backing
14370+
mock_open, mkdir, create_images_and_backing
1437114371
):
1437214372
drvr.pre_live_migration(self.context, instance,
1437314373
block_device_info=None,
1437414374
network_info=[],
1437514375
disk_info=jsonutils.dumps(disk_info),
1437614376
migrate_data=migrate_data)
14377-
write_to_file.assert_called_with(disk_info_path,
14378-
jsonutils.dumps(image_disk_info))
14377+
14378+
self.assertIn(mock.call(disk_info_path, 'w'), mock_open.mock_calls)
14379+
mock_open.return_value.write.assert_called_with(
14380+
jsonutils.dumps(image_disk_info))
1437914381

1438014382
@mock.patch('nova.virt.libvirt.utils.file_open',
1438114383
side_effect=[io.BytesIO(b''), io.BytesIO(b'')])
@@ -16292,16 +16294,16 @@ def test_hard_reboot(self, mock_get_mdev, mock_destroy, mock_get_disk_info,
1629216294
@mock.patch('nova.virt.libvirt.LibvirtDriver._create_images_and_backing')
1629316295
@mock.patch('nova.virt.libvirt.LibvirtDriver.'
1629416296
'_get_instance_disk_info_from_config')
16295-
@mock.patch('nova.virt.libvirt.utils.write_to_file')
1629616297
@mock.patch('nova.virt.libvirt.utils.get_instance_path')
1629716298
@mock.patch('nova.virt.libvirt.LibvirtDriver._get_guest_config')
1629816299
@mock.patch('nova.virt.libvirt.blockinfo.get_disk_info')
1629916300
@mock.patch('nova.virt.libvirt.LibvirtDriver._destroy')
1630016301
@mock.patch('nova.virt.libvirt.LibvirtDriver.'
1630116302
'_get_all_assigned_mediated_devices')
16303+
@mock.patch('builtins.open', new=mock.mock_open())
1630216304
def test_hard_reboot_does_not_call_glance_show(self,
1630316305
mock_get_mdev, mock_destroy, mock_get_disk_info,
16304-
mock_get_guest_config, mock_get_instance_path, mock_write_to_file,
16306+
mock_get_guest_config, mock_get_instance_path,
1630516307
mock_get_instance_disk_info, mock_create_images_and_backing,
1630616308
mock_create_domand_and_network, mock_prepare_pci_devices_for_use,
1630716309
mock_get_instance_pci_devs, mock_looping_call, mock_ensure_tree):
@@ -22375,18 +22377,17 @@ def test_disk_raw_to_qcow2_no_directio(self, mock_rename, mock_execute,
2237522377
@mock.patch.object(libvirt_driver.LibvirtDriver,
2237622378
'_create_guest_with_network')
2237722379
@mock.patch.object(libvirt_driver.LibvirtDriver, '_disk_raw_to_qcow2')
22378-
# Don't write libvirt xml to disk
22379-
@mock.patch.object(libvirt_utils, 'write_to_file')
2238022380
# NOTE(mdbooth): The following 4 mocks are required to execute
2238122381
# get_guest_xml().
2238222382
@mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled')
2238322383
@mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata')
2238422384
@mock.patch('nova.privsep.utils.supports_direct_io')
2238522385
@mock.patch('nova.api.metadata.base.InstanceMetadata')
22386+
@mock.patch('builtins.open', new=mock.mock_open())
2238622387
def _test_finish_migration(self, mock_instance_metadata,
2238722388
mock_supports_direct_io,
2238822389
mock_build_device_metadata,
22389-
mock_set_host_enabled, mock_write_to_file,
22390+
mock_set_host_enabled,
2239022391
mock_raw_to_qcow2,
2239122392
mock_create_guest_with_network,
2239222393
mock_get_info, mock_inject_data,
@@ -23761,18 +23762,19 @@ def test_detach_interface_device_with_same_mac_address(
2376123762
@mock.patch('nova.objects.image_meta.ImageMeta.from_image_ref')
2376223763
@mock.patch('nova.virt.libvirt.LibvirtDriver.'
2376323764
'_get_all_assigned_mediated_devices')
23764-
@mock.patch('nova.virt.libvirt.utils.write_to_file')
2376523765
# NOTE(mdbooth): The following 4 mocks are required to execute
2376623766
# get_guest_xml().
2376723767
@mock.patch.object(libvirt_driver.LibvirtDriver, '_set_host_enabled')
2376823768
@mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata')
2376923769
@mock.patch('nova.privsep.utils.supports_direct_io')
2377023770
@mock.patch('nova.api.metadata.base.InstanceMetadata')
23771-
def _test_rescue(self, instance, mock_instance_metadata,
23772-
mock_supports_direct_io, mock_build_device_metadata,
23773-
mock_set_host_enabled, mock_write_to_file, mock_get_mdev,
23774-
mock_get_image_meta_by_ref, image_meta_dict=None, exists=None,
23775-
instance_image_meta_dict=None, block_device_info=None):
23771+
@mock.patch('builtins.open', new=mock.mock_open())
23772+
def _test_rescue(
23773+
self, instance, mock_instance_metadata, mock_supports_direct_io,
23774+
mock_build_device_metadata, mock_set_host_enabled, mock_get_mdev,
23775+
mock_get_image_meta_by_ref, image_meta_dict=None, exists=None,
23776+
instance_image_meta_dict=None, block_device_info=None,
23777+
):
2377623778

2377723779
self.flags(instances_path=self.useFixture(fixtures.TempDir()).path)
2377823780
mock_build_device_metadata.return_value = None
@@ -23929,9 +23931,8 @@ def test_rescue_config_drive(self, mock_mkisofs):
2392923931
self.assertEqual(expected_kernel_ramdisk_paths,
2393023932
kernel_ramdisk_paths)
2393123933

23932-
@mock.patch('nova.virt.libvirt.utils.write_to_file')
23933-
def test_rescue_stable_device_unsupported_virt_types(self,
23934-
mock_libvirt_write_to_file):
23934+
@mock.patch('builtins.open', new=mock.mock_open())
23935+
def test_rescue_stable_device_unsupported_virt_types(self):
2393523936
network_info = _fake_network_info(self, 1)
2393623937
instance = self._create_instance({'config_drive': str(True)})
2393723938
rescue_image_meta_dict = {'id': uuids.rescue_image_id,
@@ -24087,15 +24088,16 @@ def test_rescue_stable_device_bfv_without_instance_image_ref(self):
2408724088
mock.patch.object(drvr, '_get_guest_xml'),
2408824089
mock.patch.object(drvr, '_create_image'),
2408924090
mock.patch.object(drvr, '_get_existing_domain_xml'),
24090-
mock.patch.object(libvirt_utils, 'write_to_file'),
2409124091
mock.patch.object(libvirt_utils, 'get_instance_path'),
2409224092
mock.patch('nova.virt.libvirt.blockinfo.get_disk_info'),
2409324093
mock.patch('nova.image.glance.API.get'),
24094-
mock.patch('nova.objects.image_meta.ImageMeta.from_dict')
24095-
) as (mock_create, mock_destroy, mock_get_guest_xml, mock_create_image,
24096-
mock_get_existing_xml, mock_write, mock_inst_path,
24097-
mock_get_disk_info, mock_image_get, mock_from_dict):
24098-
24094+
mock.patch('nova.objects.image_meta.ImageMeta.from_dict'),
24095+
mock.patch('builtins.open', new_callable=mock.mock_open),
24096+
) as (
24097+
mock_create, mock_destroy, mock_get_guest_xml, mock_create_image,
24098+
mock_get_existing_xml, mock_inst_path, mock_get_disk_info,
24099+
mock_image_get, mock_from_dict, mock_open,
24100+
):
2409924101
self.flags(virt_type='kvm', group='libvirt')
2410024102
mock_image_get.return_value = mock.sentinel.bdm_image_meta_dict
2410124103
mock_from_dict.return_value = mock.sentinel.bdm_image_meta
@@ -24185,8 +24187,11 @@ def test_rescue_stable_device_bfv(self):
2418524187
@mock.patch.object(libvirt_utils, 'get_instance_path')
2418624188
@mock.patch.object(libvirt_utils, 'load_file')
2418724189
@mock.patch.object(host.Host, '_get_domain')
24188-
def _test_unrescue(self, instance, mock_get_domain, mock_load_file,
24189-
mock_get_instance_path):
24190+
@mock.patch('builtins.open', new=mock.mock_open())
24191+
def _test_unrescue(
24192+
self, instance, mock_get_domain, mock_load_file,
24193+
mock_get_instance_path,
24194+
):
2419024195
dummyxml = ("<domain type='kvm'><name>instance-0000000a</name>"
2419124196
"<devices>"
2419224197
"<disk type='block' device='disk'>"
@@ -24210,7 +24215,6 @@ def isdir_sideeffect(*args, **kwargs):
2421024215

2421124216
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
2421224217
with test.nested(
24213-
mock.patch.object(libvirt_utils, 'write_to_file'),
2421424218
mock.patch.object(drvr, '_destroy'),
2421524219
mock.patch.object(drvr, '_create_guest'),
2421624220
mock.patch.object(os, 'unlink'),
@@ -24222,7 +24226,7 @@ def isdir_sideeffect(*args, **kwargs):
2422224226
mock.patch.object(lvm, 'remove_volumes'),
2422324227
mock.patch.object(glob, 'iglob',
2422424228
return_value=[rescue_file, rescue_dir])
24225-
) as (mock_write, mock_destroy, mock_create, mock_del,
24229+
) as (mock_destroy, mock_create, mock_del,
2422624230
mock_rmtree, mock_isdir, mock_lvm_disks,
2422724231
mock_remove_volumes, mock_glob):
2422824232
drvr.unrescue(self.context, instance)

nova/tests/unit/virt/libvirt/test_utils.py

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -248,17 +248,6 @@ def test_copy_image(self):
248248
finally:
249249
os.unlink(dst_path)
250250

251-
def test_write_to_file(self):
252-
dst_fd, dst_path = tempfile.mkstemp()
253-
try:
254-
os.close(dst_fd)
255-
256-
libvirt_utils.write_to_file(dst_path, 'hello')
257-
with open(dst_path, 'r') as fp:
258-
self.assertEqual(fp.read(), 'hello')
259-
finally:
260-
os.unlink(dst_path)
261-
262251
@mock.patch.object(compute_utils, 'disk_ops_semaphore')
263252
@mock.patch('nova.privsep.utils.supports_direct_io', return_value=False)
264253
@mock.patch('oslo_concurrency.processutils.execute')
@@ -330,8 +319,8 @@ def test_load_file(self):
330319
try:
331320
os.close(dst_fd)
332321

333-
# We have a test for write_to_file. If that is sound, this suffices
334-
libvirt_utils.write_to_file(dst_path, 'hello')
322+
with open(dst_path, 'w') as f:
323+
f.write('hello')
335324
self.assertEqual(libvirt_utils.load_file(dst_path), 'hello')
336325
finally:
337326
os.unlink(dst_path)
@@ -341,8 +330,8 @@ def test_file_open(self):
341330
try:
342331
os.close(dst_fd)
343332

344-
# We have a test for write_to_file. If that is sound, this suffices
345-
libvirt_utils.write_to_file(dst_path, 'hello')
333+
with open(dst_path, 'w') as f:
334+
f.write('hello')
346335
with libvirt_utils.file_open(dst_path, 'r') as fp:
347336
self.assertEqual(fp.read(), 'hello')
348337
finally:

nova/virt/libvirt/driver.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,7 +3590,8 @@ def rescue(self, context, instance, network_info, image_meta,
35903590
instance_dir = libvirt_utils.get_instance_path(instance)
35913591
unrescue_xml = self._get_existing_domain_xml(instance, network_info)
35923592
unrescue_xml_path = os.path.join(instance_dir, 'unrescue.xml')
3593-
libvirt_utils.write_to_file(unrescue_xml_path, unrescue_xml)
3593+
with open(unrescue_xml_path, 'w') as f:
3594+
f.write(unrescue_xml)
35943595

35953596
rescue_image_id = None
35963597
rescue_image_meta = None
@@ -9707,8 +9708,8 @@ def pre_live_migration(self, context, instance, block_device_info,
97079708

97089709
image_disk_info_path = os.path.join(instance_dir,
97099710
'disk.info')
9710-
libvirt_utils.write_to_file(image_disk_info_path,
9711-
jsonutils.dumps(image_disk_info))
9711+
with open(image_disk_info_path, 'w') as f:
9712+
f.write(jsonutils.dumps(image_disk_info))
97129713

97139714
if not is_shared_block_storage:
97149715
# Ensure images and backing files are present.

nova/virt/libvirt/utils.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -306,17 +306,6 @@ def copy_image(
306306
compression=compression)
307307

308308

309-
# TODO(stephenfin): This is dumb; remove it.
310-
def write_to_file(path: str, contents: str) -> None:
311-
"""Write the given contents to a file
312-
313-
:param path: Destination file
314-
:param contents: Desired contents of the file
315-
"""
316-
with open(path, 'w') as f:
317-
f.write(contents)
318-
319-
320309
def chown_for_id_maps(
321310
path: str, id_maps: ty.List[vconfig.LibvirtConfigGuestIDMap],
322311
) -> None:

0 commit comments

Comments
 (0)