From 5f2bdd85c24157ca627dca4a97a087fe50a3d2fb Mon Sep 17 00:00:00 2001 From: Tomoki Sekiyama Date: Fri, 14 Aug 2015 14:13:10 -0400 Subject: [PATCH] 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 --- cinder/image/glance.py | 1 + cinder/image/image_utils.py | 52 +++++++++++++++- cinder/tests/unit/test_image_utils.py | 87 +++++++++++++++++++++------ 3 files changed, 122 insertions(+), 18 deletions(-) diff --git a/cinder/image/glance.py b/cinder/image/glance.py index 0a2250d29..444fb50c6 100644 --- 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 05477f597..81f521d79 100644 --- 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 580270174..6bce1140f 100644 --- 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 -- 2.45.2