Merge "Stop allocating double space for raw images"

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 = force_raw_get_source_format(image_href, 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})
 
         if fmt != "raw":
             staged = "%s.converted" % path

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)
+        if images.force_raw_will_convert(image_href, path_tmp):
-        directory = os.path.dirname(path_tmp)
+            required_space = images.converted_size(path_tmp)
-        _clean_up_caches(directory, required_space)
+            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)

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')

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.