Merge "Fix volume creation from image with allowed_direct_url_schemes"

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,
+        """Returns a tuple of the direct url and locations representing the
-        or None if this attribute is not shown by Glance.
+        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:
+        if data and 'file' in CONF.allowed_direct_url_schemes:
-            location = self.get_location(context, image_id)
+            direct_url, locations = self.get_location(context, image_id)
-            o = urlparse.urlparse(location)
+            urls = [direct_url] + [loc.get('url') for loc in locations or []]
-            if o.scheme == "file":
+            for url in urls:
-                with open(o.path, "r") as f:
+                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)
+                    with open(parsed_url.path, "r") as f:
-                return
+                        shutil.copyfileobj(f, data)
+                    return
 
         try:
             image_chunks = self._client.call(context, 'data', image_id)

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