From 71ccbf5955e727b512d669a83acef1e3f676c4d5 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 7 Oct 2020 14:04:51 +1300 Subject: [PATCH] Raw image size estimation improved Adds the `[DEFAULT]raw_image_growth_factor` configuration option which is a scale factor used for estimating the size of a raw image converted from compact image formats such as QCOW2. By default this is set to 2.0. When clearing the cache to make space for a converted raw image, the full virtual size is attempted first, and if not enough space is available a second attempt is made with the (smaller) estimated size. Story: 1750515 Task: 9791 Change-Id: Id86e7641329a95f71ac005ee448b0ff4d7d0bbcd --- ironic/common/images.py | 15 ++++++--- ironic/conf/default.py | 7 +++++ ironic/drivers/modules/image_cache.py | 18 +++++++++-- ironic/tests/unit/common/test_images.py | 31 ++++++++++++++++--- .../unit/drivers/modules/test_image_cache.py | 27 ++++++++++++++++ ..._image_growth_factor-cba37029650e67db.yaml | 10 ++++++ 6 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/raw_image_growth_factor-cba37029650e67db.yaml diff --git a/ironic/common/images.py b/ironic/common/images.py index 778f9a12df..c48317e88b 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -426,18 +426,25 @@ def download_size(context, image_href, image_service=None): return image_show(context, image_href, image_service)['size'] -def converted_size(path): +def converted_size(path, estimate=False): """Get size of converted raw image. The size of image converted to raw format can be growing up to the virtual size of the image. :param path: path to the image file. - :returns: virtual size of the image or 0 if conversion not needed. - + :param estimate: Whether to estimate the size by scaling the + original size + :returns: For `estimate=False`, return the size of the + raw image file. For `estimate=True`, return the size of + the original image scaled by the configuration value + `raw_image_growth_factor`. """ data = disk_utils.qemu_img_info(path) - return data.virtual_size + if not estimate: + return data.virtual_size + growth_factor = CONF.raw_image_growth_factor + return int(min(data.disk_size * growth_factor, data.virtual_size)) def get_image_properties(context, image_href, properties="all"): diff --git a/ironic/conf/default.py b/ironic/conf/default.py index c19ea5f3f4..f51defd207 100644 --- a/ironic/conf/default.py +++ b/ironic/conf/default.py @@ -201,6 +201,13 @@ image_opts = [ mutable=True, help=_('If True, convert backing images to "raw" disk image ' 'format.')), + cfg.FloatOpt('raw_image_growth_factor', + default=2.0, + min=1.0, + help=_('The scale factor used for estimating the size of a ' + 'raw image converted from compact image ' + 'formats such as QCOW2. ' + 'Default is 2.0, must be greater than 1.0.')), cfg.StrOpt('isolinux_bin', default='/usr/lib/syslinux/isolinux.bin', help=_('Path to isolinux binary file.')), diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py index 78b5e02b49..10d3328d10 100644 --- a/ironic/drivers/modules/image_cache.py +++ b/ironic/drivers/modules/image_cache.py @@ -314,9 +314,23 @@ def _fetch(context, image_href, path, force_raw=False): # then we can firstly clean cache and then invoke images.fetch(). if force_raw: if images.force_raw_will_convert(image_href, path_tmp): - required_space = images.converted_size(path_tmp) + required_space = images.converted_size(path_tmp, estimate=False) directory = os.path.dirname(path_tmp) - _clean_up_caches(directory, required_space) + try: + _clean_up_caches(directory, required_space) + except exception.InsufficientDiskSpace: + + # try again with an estimated raw size instead of the full size + required_space = images.converted_size(path_tmp, estimate=True) + try: + _clean_up_caches(directory, required_space) + except exception.InsufficientDiskSpace: + LOG.warning('Not enough space for estimated image size. ' + 'Consider lowering ' + '[DEFAULT]raw_image_growth_factor=%s', + CONF.raw_image_growth_factor) + raise + images.image_to_raw(image_href, path, path_tmp) else: os.rename(path_tmp, path) diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index f5cb8ac835..f89ad655cd 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -177,13 +177,36 @@ class IronicImagesTestCase(base.TestCase): 'image_service') @mock.patch.object(disk_utils, 'qemu_img_info', autospec=True) - def test_converted_size(self, qemu_img_info_mock): + def test_converted_size_estimate_default(self, qemu_img_info_mock): info = self.FakeImgInfo() - info.virtual_size = 1 + info.disk_size = 2 + info.virtual_size = 10 ** 10 qemu_img_info_mock.return_value = info - size = images.converted_size('path') + size = images.converted_size('path', estimate=True) qemu_img_info_mock.assert_called_once_with('path') - self.assertEqual(1, size) + self.assertEqual(4, size) + + @mock.patch.object(disk_utils, 'qemu_img_info', autospec=True) + def test_converted_size_estimate_custom(self, qemu_img_info_mock): + CONF.set_override('raw_image_growth_factor', 3) + info = self.FakeImgInfo() + info.disk_size = 2 + info.virtual_size = 10 ** 10 + qemu_img_info_mock.return_value = info + size = images.converted_size('path', estimate=True) + qemu_img_info_mock.assert_called_once_with('path') + self.assertEqual(6, size) + + @mock.patch.object(disk_utils, 'qemu_img_info', autospec=True) + def test_converted_size_estimate_raw_smaller(self, qemu_img_info_mock): + CONF.set_override('raw_image_growth_factor', 3) + info = self.FakeImgInfo() + info.disk_size = 2 + info.virtual_size = 5 + qemu_img_info_mock.return_value = info + size = images.converted_size('path', estimate=True) + qemu_img_info_mock.assert_called_once_with('path') + self.assertEqual(5, size) @mock.patch.object(images, 'get_image_properties', autospec=True) @mock.patch.object(glance_utils, 'is_glance_image', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py index 896a1900b5..0711fedecc 100644 --- a/ironic/tests/unit/drivers/modules/test_image_cache.py +++ b/ironic/tests/unit/drivers/modules/test_image_cache.py @@ -765,3 +765,30 @@ class TestFetchCleanup(base.TestCase): 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=True) + @mock.patch.object(image_cache, '_clean_up_caches', autospec=True) + def test__fetch_estimate_fallback( + self, mock_clean, mock_will_convert, mock_raw, mock_fetch, + mock_size): + mock_size.side_effect = [100, 10] + mock_clean.side_effect = [exception.InsufficientDiskSpace(), None] + + 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_size.assert_has_calls([ + mock.call('/foo/bar.part', estimate=False), + mock.call('/foo/bar.part', estimate=True), + ]) + mock_clean.assert_has_calls([ + mock.call('/foo', 100), + mock.call('/foo', 10), + ]) + 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/raw_image_growth_factor-cba37029650e67db.yaml b/releasenotes/notes/raw_image_growth_factor-cba37029650e67db.yaml new file mode 100644 index 0000000000..7c927a0df2 --- /dev/null +++ b/releasenotes/notes/raw_image_growth_factor-cba37029650e67db.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Adds the `[DEFAULT]raw_image_growth_factor` configuration option which + is a scale factor used for estimating the size of a raw image converted + from compact image formats such as QCOW2. By default this is set to 2.0. + + When clearing the cache to make space for a converted raw image, the full + virtual size is attempted first, and if not enough space is available a + second attempt is made with the (smaller) estimated size. \ No newline at end of file