TemporaryImages to inspect image before conversion

This intruduces TemporaryImages utility class which enables
download images to temporary file for inspection before they are
used for format conversion. Using this utility, we can get image
information such as virtual size, which is required to implement
image cache, before calling fetch_to_raw() etc., without
downloading the image twice nor changing APIs of image_utils.

Change-Id: I6c2d48bca60e5b9e1615bbe10602d925ae6d75ff
Partially-implements: blueprint image-volume-cache

diff --git a/cinder/image/glance.py b/cinder/image/glance.py
index 0a2250d293483cdf978a5f2b8bb3ca3eada47f3e..444fb50c69a881be0f907fd09907c8534eba4477 100644 (file)
--- a/cinder/image/glance.py
+++ b/cinder/image/glance.py
@@ -200,6 +200,7 @@ class GlanceImageService(object):
     def __init__(self, client=None):
         self._client = client or GlanceClientWrapper()
         self._image_schema = None
+        self.temp_images = None
 
     def detail(self, context, **kwargs):
         """Calls out to Glance for a list of detailed image information."""

diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py
index 05477f59798c8efabe050efd1ac6f67727086b49..81f521d79b542c0529cfff861da5652acd861b2b 100644 (file)
--- a/cinder/image/image_utils.py
+++ b/cinder/image/image_utils.py
@@ -264,7 +264,12 @@ def fetch_to_volume_format(context, image_service,
                              "can be used if qemu-img is not installed."),
                     image_id=image_id)
 
-        fetch(context, image_service, image_id, tmp, user_id, project_id)
+        tmp_images = TemporaryImages.for_image_service(image_service)
+        tmp_image = tmp_images.get(context, image_id)
+        if tmp_image:
+            tmp = tmp_image
+        else:
+            fetch(context, image_service, image_id, tmp, user_id, project_id)
 
         if is_xenserver_image(context, image_service, image_id):
             replace_xenserver_image_with_coalesced_vhd(tmp)
@@ -486,3 +491,48 @@ def replace_xenserver_image_with_coalesced_vhd(image_file):
         coalesced = coalesce_chain(chain)
         fileutils.delete_if_exists(image_file)
         os.rename(coalesced, image_file)
+
+
+class TemporaryImages(object):
+    """Manage temporarily downloaded images to avoid downloading it twice.
+
+    In the 'with TemporaryImages.fetch(image_service, ctx, image_id) as tmp'
+    clause, 'tmp' can be used as the downloaded image path. In addition,
+    image_utils.fetch() will use the pre-fetched image by the TemporaryImages.
+    This is useful to inspect image contents before conversion.
+    """
+
+    def __init__(self, image_service):
+        self.temporary_images = {}
+        self.image_service = image_service
+        image_service.temp_images = self
+
+    @staticmethod
+    def for_image_service(image_service):
+        instance = image_service.temp_images
+        if instance:
+            return instance
+        return TemporaryImages(image_service)
+
+    @classmethod
+    @contextlib.contextmanager
+    def fetch(cls, image_service, context, image_id):
+        tmp_images = cls.for_image_service(image_service).temporary_images
+        with temporary_file() as tmp:
+            fetch_verify_image(context, image_service, image_id, tmp)
+            user = context.user_id
+            if not tmp_images.get(user):
+                tmp_images[user] = {}
+            tmp_images[user][image_id] = tmp
+            LOG.debug("Temporary image %(id)s is fetched for user %(user)s.",
+                      {'id': image_id, 'user': user})
+            yield tmp
+            del tmp_images[user][image_id]
+        LOG.debug("Temporary image %(id)s for user %(user)s is deleted.",
+                  {'id': image_id, 'user': user})
+
+    def get(self, context, image_id):
+        user = context.user_id
+        if not self.temporary_images.get(user):
+            return None
+        return self.temporary_images[user].get(image_id)

diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py
index 58027017462f525a309d5770183d96e8f933b66b..6bce1140fec86739c0ddfa066c8a7fd3413587e8 100644 (file)
--- a/cinder/tests/unit/test_image_utils.py
+++ b/cinder/tests/unit/test_image_utils.py
@@ -578,7 +578,8 @@ class TestFetchToVolumeFormat(test.TestCase):
     def test_defaults(self, mock_conf, mock_temp, mock_info, mock_fetch,
                       mock_is_xen, mock_repl_xen, mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        ctxt.user_id = mock.sentinel.user_id
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
@@ -621,12 +622,12 @@ class TestFetchToVolumeFormat(test.TestCase):
     def test_kwargs(self, mock_conf, mock_temp, mock_info, mock_fetch,
                     mock_is_xen, mock_repl_xen, mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root
@@ -656,6 +657,58 @@ class TestFetchToVolumeFormat(test.TestCase):
         mock_convert.assert_called_once_with(tmp, dest, volume_format,
                                              run_as_root=run_as_root)
 
+    @mock.patch('cinder.image.image_utils.convert_image')
+    @mock.patch('cinder.image.image_utils.volume_utils.copy_volume')
+    @mock.patch(
+        'cinder.image.image_utils.replace_xenserver_image_with_coalesced_vhd')
+    @mock.patch('cinder.image.image_utils.is_xenserver_image',
+                return_value=False)
+    @mock.patch('cinder.image.image_utils.fetch')
+    @mock.patch('cinder.image.image_utils.qemu_img_info')
+    @mock.patch('cinder.image.image_utils.temporary_file')
+    @mock.patch('cinder.image.image_utils.CONF')
+    def test_temporary_images(self, mock_conf, mock_temp, mock_info,
+                              mock_fetch, mock_is_xen, mock_repl_xen,
+                              mock_copy, mock_convert):
+        ctxt = mock.sentinel.context
+        ctxt.user_id = mock.sentinel.user_id
+        image_service = mock.Mock(temp_images=None)
+        image_id = mock.sentinel.image_id
+        dest = mock.sentinel.dest
+        volume_format = mock.sentinel.volume_format
+        blocksize = mock.sentinel.blocksize
+
+        data = mock_info.return_value
+        data.file_format = volume_format
+        data.backing_file = None
+        data.virtual_size = 1234
+        tmp = mock.sentinel.tmp
+        dummy = mock.sentinel.dummy
+        mock_temp.return_value.__enter__.side_effect = [tmp, dummy]
+
+        with image_utils.TemporaryImages.fetch(image_service, ctxt,
+                                               image_id) as tmp_img:
+            self.assertEqual(tmp_img, tmp)
+            output = image_utils.fetch_to_volume_format(ctxt, image_service,
+                                                        image_id, dest,
+                                                        volume_format,
+                                                        blocksize)
+
+        self.assertIsNone(output)
+        image_service.show.assert_called_once_with(ctxt, image_id)
+        self.assertEqual(2, mock_temp.call_count)
+        mock_info.assert_has_calls([
+            mock.call(tmp, run_as_root=True),
+            mock.call(dummy, run_as_root=True),
+            mock.call(tmp, run_as_root=True),
+            mock.call(dest, run_as_root=True)])
+        mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
+                                           tmp, None, None)
+        self.assertFalse(mock_repl_xen.called)
+        self.assertFalse(mock_copy.called)
+        mock_convert.assert_called_once_with(tmp, dest, volume_format,
+                                             run_as_root=True)
+
     @mock.patch('cinder.image.image_utils.convert_image')
     @mock.patch('cinder.image.image_utils.volume_utils.copy_volume')
     @mock.patch(
@@ -671,12 +724,12 @@ class TestFetchToVolumeFormat(test.TestCase):
                                     mock_fetch, mock_is_xen, mock_repl_xen,
                                     mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root
@@ -760,12 +813,12 @@ class TestFetchToVolumeFormat(test.TestCase):
                                      mock_fetch, mock_is_xen, mock_repl_xen,
                                      mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root
@@ -801,12 +854,12 @@ class TestFetchToVolumeFormat(test.TestCase):
     def test_size_error(self, mock_conf, mock_temp, mock_info, mock_fetch,
                         mock_is_xen, mock_repl_xen, mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 1234
         run_as_root = mock.sentinel.run_as_root
@@ -849,12 +902,12 @@ class TestFetchToVolumeFormat(test.TestCase):
                                   mock_fetch, mock_is_xen, mock_repl_xen,
                                   mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root
@@ -897,12 +950,12 @@ class TestFetchToVolumeFormat(test.TestCase):
                                 mock_fetch, mock_is_xen, mock_repl_xen,
                                 mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root
@@ -946,12 +999,12 @@ class TestFetchToVolumeFormat(test.TestCase):
                                    mock_copy, mock_convert,
                                    legacy_format_name=False):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = 'vhd'
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root
@@ -1010,12 +1063,12 @@ class TestFetchToVolumeFormat(test.TestCase):
                               mock_fetch, mock_is_xen, mock_repl_xen,
                               mock_copy, mock_convert):
         ctxt = mock.sentinel.context
-        image_service = mock.Mock()
+        image_service = mock.Mock(temp_images=None)
         image_id = mock.sentinel.image_id
         dest = mock.sentinel.dest
         volume_format = mock.sentinel.volume_format
         blocksize = mock.sentinel.blocksize
-        user_id = mock.sentinel.user_id
+        ctxt.user_id = user_id = mock.sentinel.user_id
         project_id = mock.sentinel.project_id
         size = 4321
         run_as_root = mock.sentinel.run_as_root