diff --git a/ironic/common/images.py b/ironic/common/images.py index 1bd1755fdb..f1dc7ad15a 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -414,22 +414,36 @@ def fetch(context, image_href, path, force_raw=False): image_to_raw(image_href, path, "%s.part" % path) +def force_raw_get_source_format(image_href, path): + data = disk_utils.qemu_img_info(path) + + fmt = data.file_format + if fmt is None: + raise exception.ImageUnacceptable( + reason=_("'qemu-img info' parsing failed."), + image_id=image_href) + + backing_file = data.backing_file + if backing_file is not None: + raise exception.ImageUnacceptable( + image_id=image_href, + reason=_("fmt=%(fmt)s backed by: %(backing_file)s") % + {'fmt': fmt, 'backing_file': backing_file}) + + return fmt + + +def force_raw_will_convert(image_href, path_tmp): + with fileutils.remove_path_on_error(path_tmp): + fmt = force_raw_get_source_format(image_href, path_tmp) + if fmt != "raw": + return True + return False + + def image_to_raw(image_href, path, path_tmp): with fileutils.remove_path_on_error(path_tmp): - data = disk_utils.qemu_img_info(path_tmp) - - fmt = data.file_format - if fmt is None: - raise exception.ImageUnacceptable( - reason=_("'qemu-img info' parsing failed."), - image_id=image_href) - - backing_file = data.backing_file - if backing_file is not None: - raise exception.ImageUnacceptable( - image_id=image_href, - reason=_("fmt=%(fmt)s backed by: %(backing_file)s") % - {'fmt': fmt, 'backing_file': backing_file}) + fmt = force_raw_get_source_format(image_href, path_tmp) if fmt != "raw": staged = "%s.converted" % path diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py index b7ca62fc3a..78b5e02b49 100644 --- a/ironic/drivers/modules/image_cache.py +++ b/ironic/drivers/modules/image_cache.py @@ -313,9 +313,10 @@ def _fetch(context, image_href, path, force_raw=False): # Notes(yjiang5): If glance can provide the virtual size information, # then we can firstly clean cache and then invoke images.fetch(). if force_raw: - required_space = images.converted_size(path_tmp) - directory = os.path.dirname(path_tmp) - _clean_up_caches(directory, required_space) + if images.force_raw_will_convert(image_href, path_tmp): + required_space = images.converted_size(path_tmp) + directory = os.path.dirname(path_tmp) + _clean_up_caches(directory, required_space) images.image_to_raw(image_href, path, path_tmp) else: os.rename(path_tmp, path) diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py index b70960ec13..896a1900b5 100644 --- a/ironic/tests/unit/drivers/modules/test_image_cache.py +++ b/ironic/tests/unit/drivers/modules/test_image_cache.py @@ -733,8 +733,12 @@ class TestFetchCleanup(base.TestCase): @mock.patch.object(images, 'converted_size', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @mock.patch.object(images, 'image_to_raw', autospec=True) + @mock.patch.object(images, 'force_raw_will_convert', autospec=True, + return_value=True) @mock.patch.object(image_cache, '_clean_up_caches', autospec=True) - def test__fetch(self, mock_clean, mock_raw, mock_fetch, mock_size): + def test__fetch( + self, mock_clean, mock_will_convert, mock_raw, mock_fetch, + mock_size): mock_size.return_value = 100 image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True) mock_fetch.assert_called_once_with('fake', 'fake-uuid', @@ -742,3 +746,22 @@ class TestFetchCleanup(base.TestCase): mock_clean.assert_called_once_with('/foo', 100) mock_raw.assert_called_once_with('fake-uuid', '/foo/bar', '/foo/bar.part') + mock_will_convert.assert_called_once_with('fake-uuid', '/foo/bar.part') + + @mock.patch.object(images, 'converted_size', autospec=True) + @mock.patch.object(images, 'fetch', autospec=True) + @mock.patch.object(images, 'image_to_raw', autospec=True) + @mock.patch.object(images, 'force_raw_will_convert', autospec=True, + return_value=False) + @mock.patch.object(image_cache, '_clean_up_caches', autospec=True) + def test__fetch_already_raw( + self, mock_clean, mock_will_convert, mock_raw, mock_fetch, + mock_size): + image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True) + mock_fetch.assert_called_once_with('fake', 'fake-uuid', + '/foo/bar.part', force_raw=False) + mock_clean.assert_not_called() + mock_size.assert_not_called() + mock_raw.assert_called_once_with('fake-uuid', '/foo/bar', + '/foo/bar.part') + mock_will_convert.assert_called_once_with('fake-uuid', '/foo/bar.part') diff --git a/releasenotes/notes/dont-cleanup-cache-twice-0395a50ad723bca8.yaml b/releasenotes/notes/dont-cleanup-cache-twice-0395a50ad723bca8.yaml new file mode 100644 index 0000000000..a9f8327864 --- /dev/null +++ b/releasenotes/notes/dont-cleanup-cache-twice-0395a50ad723bca8.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Ironic now does not try to allocate the space needed for instance image + conversion to raw format if it is already raw.