Merge "Raw image size estimation improved"

6 files changed, 98 insertions, 10 deletions

diff --git a/ironic/common/images.py b/ironic/common/images.py
index 84fbd93b3..d614539cb 100644
--- a/ironic/common/images.py
+++ b/ironic/common/images.py
@@ -430,18 +430,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 c19ea5f3f..f51defd20 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 78b5e02b4..10d3328d1 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 f5cb8ac83..f89ad655c 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 896a1900b..0711fedec 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 000000000..7c927a0df
--- /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