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

@@ -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

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)

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.