From fbebcc3201cedc37515a811a183d9806c8f5618a Mon Sep 17 00:00:00 2001 From: Vladyslav Drok Date: Wed, 17 Jun 2020 23:32:27 +0200 Subject: [PATCH] Stop allocating double space for raw images There are two possibilities when ironic conductor will be converting instance image to raw format: 1. when iscsi deploy is used and [DEFAULT]force_raw_images config option is set to True; 2. when direct deploy is used, [agent]image_download_source config option is set to "http" and [DEFAULT]force_raw_images and [agent]stream_raw_images config options are set to True. If one of the above conditions is met, ironic tries to allocate the disk space needed for conversion even if the image is already raw, basically doubling the disk space requirements for each image. This change adds a check that skips the unnecessary conductor disk space allocation attempt if the image is already raw. Story: 2007830 Task: 40112 Change-Id: I730f9ec89ff1598d42777e4a503f46316eb85cad --- ironic/common/images.py | 42 ++++++++++++------- ironic/drivers/modules/image_cache.py | 7 ++-- .../unit/drivers/modules/test_image_cache.py | 25 ++++++++++- ...-cleanup-cache-twice-0395a50ad723bca8.yaml | 5 +++ 4 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/dont-cleanup-cache-twice-0395a50ad723bca8.yaml 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.