Fix volume creation from image with allowed_direct_url_schemes

When CONF.allowed_direct_url_schemes is set to ['file'] and
Glance image has direct_url or locations metadata with 'file:///'
scheme, cinder tries to directly read the image data from the
local file. However, currently the code is broken because return
value from get_location() has been changed to a tuple.
This fixes the code so that it can handle the metadata correctly.

Change-Id: I39a12a31fbfbd3a9824c67391096f74406d8a749
Closes-Bug: #1437477

diff --git a/cinder/image/glance.py b/cinder/image/glance.py
index 2a248d9bf1b510487fc46a614adeb545ad8a0aab..ecc3314c6f75d4e5a96ff5d06f2ac74cfde905c7 100644 (file)
--- a/cinder/image/glance.py
+++ b/cinder/image/glance.py
@@ -242,8 +242,9 @@ class GlanceImageService(object):
         return base_image_meta
 
     def get_location(self, context, image_id):
-        """Returns the direct url representing the backend storage location,
-        or None if this attribute is not shown by Glance.
+        """Returns a tuple of the direct url and locations representing the
+        backend storage location, or (None, None) if these attributes are not
+        shown by Glance.
         """
         if CONF.glance_api_version == 1:
             # image location not available in v1
@@ -266,16 +267,20 @@ class GlanceImageService(object):
 
     def download(self, context, image_id, data=None):
         """Calls out to Glance for data and writes data."""
-        if 'file' in CONF.allowed_direct_url_schemes:
-            location = self.get_location(context, image_id)
-            o = urlparse.urlparse(location)
-            if o.scheme == "file":
-                with open(o.path, "r") as f:
+        if data and 'file' in CONF.allowed_direct_url_schemes:
+            direct_url, locations = self.get_location(context, image_id)
+            urls = [direct_url] + [loc.get('url') for loc in locations or []]
+            for url in urls:
+                if url is None:
+                    continue
+                parsed_url = urlparse.urlparse(url)
+                if parsed_url.scheme == "file":
                     # a system call to cp could have significant performance
                     # advantages, however we do not have the path to files at
                     # this point in the abstraction.
-                    shutil.copyfileobj(f, data)
-                return
+                    with open(parsed_url.path, "r") as f:
+                        shutil.copyfileobj(f, data)
+                    return
 
         try:
             image_chunks = self._client.call(context, 'data', image_id)

diff --git a/cinder/tests/unit/image/test_glance.py b/cinder/tests/unit/image/test_glance.py
index f4032c7bf6a0f6ee3bdc64ef149a8ca0e177a4ea..8f043e32f0828b4334b6680844d5944bc3d90d46 100644 (file)
--- a/cinder/tests/unit/image/test_glance.py
+++ b/cinder/tests/unit/image/test_glance.py
@@ -518,6 +518,31 @@ class TestGlanceImageService(test.TestCase):
         self.assertRaises(exception.ImageNotFound, service.download,
                           self.context, image_id, writer)
 
+    @mock.patch('__builtin__.open')
+    @mock.patch('shutil.copyfileobj')
+    def test_download_from_direct_file(self, mock_copyfileobj, mock_open):
+        fixture = self._make_fixture(name='test image',
+                                     locations=[{'url': 'file:///tmp/test'}])
+        image_id = self.service.create(self.context, fixture)['id']
+        writer = NullWriter()
+        self.flags(allowed_direct_url_schemes=['file'])
+        self.flags(glance_api_version=2)
+        self.service.download(self.context, image_id, writer)
+        mock_copyfileobj.assert_called_once_with(mock.ANY, writer)
+
+    @mock.patch('__builtin__.open')
+    @mock.patch('shutil.copyfileobj')
+    def test_download_from_direct_file_non_file(self,
+                                                mock_copyfileobj, mock_open):
+        fixture = self._make_fixture(name='test image',
+                                     direct_url='swift+http://test/image')
+        image_id = self.service.create(self.context, fixture)['id']
+        writer = NullWriter()
+        self.flags(allowed_direct_url_schemes=['file'])
+        self.flags(glance_api_version=2)
+        self.service.download(self.context, image_id, writer)
+        self.assertEqual(None, mock_copyfileobj.call_args)
+
     def test_glance_client_image_id(self):
         fixture = self._make_fixture(name='test image')
         image_id = self.service.create(self.context, fixture)['id']