openstack
/
ironic
Code Issues Proposed changes

Avoid double file removal in create_boot_iso 

The images.fetch call removes the provided destination file on error.
The create_boot_iso function calls it with a temporary file, which
results in another attempt to remove it. This attempt fails and masks
the actual exception (e.g. validation failure).

I doubt this problem is exposed to end users, but fixing it anyway.

Change-Id: I77ef1cabbcea9808771613fa37b27978fd0ce770
This commit is contained in:
Dmitry Tantsur 2021-07-06 19:08:38 +02:00
parent 7dd26522b0
commit c830861ee8
2 changed files with 8 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ironic/common/images.py
View File

@ -357,13 +357,16 @@ def fetch_into(context, image_href, image_file):
{'image_service': image_service.__class__,
'image_href': image_href})
image_service.download(image_href, image_file)
if isinstance(image_file, str):
with open(image_file, "wb") as image_file_obj:
image_service.download(image_href, image_file_obj)
else:
image_service.download(image_href, image_file)
def fetch(context, image_href, path, force_raw=False):
with fileutils.remove_path_on_error(path):
with open(path, "wb") as image_file:
fetch_into(context, image_href, image_file)
fetch_into(context, image_href, path)
if force_raw:
image_to_raw(image_href, path, "%s.part" % path)
@ -527,7 +530,7 @@ def create_boot_iso(context, output_filename, kernel_href,
# to perform the massaging of the image, because oddly enough
# we need to do all the same basic things, just a little
# differently.
fetch(context, base_iso, output_filename)
fetch_into(context, base_iso, output_filename)
# Temporary, return to the caller until we support the combined
# operation.
return

2
ironic/tests/unit/common/test_images.py
View File

@ -920,7 +920,7 @@ class FsImageTestCase(base.TestCase):
kernel_params=params, inject_files=None)
@mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True)
@mock.patch.object(images, 'fetch', autospec=True)
@mock.patch.object(images, 'fetch_into', autospec=True)
@mock.patch.object(utils, 'tempdir', autospec=True)
def test_create_boot_iso_for_existing_iso(self, tempdir_mock,
fetch_images_mock,