cinder-volume: Stop masking IOError different than ENOSPC

When glanceclient raises an IOError with a different errno than ENOSPC,
cinder-volume silently masked it and continued its volume creation
process. The result was volumes with invalid content being successfuly
created.

With the patch, an ImageDownloadFailed exception is raised in this case,
which makes the volume creation process fail and gives enough
information to operators for troubleshooting.

As explained in detail below, this patch is a squash of four cherry
picks to fix Bug #1799221.  The cherry-picks are being squashed
instead of done separately per normal cinder practice because they
are dependent; this will make sure that one of them isn't further
backported without the supporting patches.

Here's the relationship between the four cherry-picks:
(1) The purpose of this backport is to fix Bug #1799221, which was
    introduced in Pike.  It is fixed in Stein and Rocky by
    Ic011fe30b4840e5098db1a594ea276ec98768bff
(2) That change requires an exception introduced in Stein and backported
    to Rocky by If7c22ac4516f8c2a6ccd8bf6b6ed98409312b138 to fix Bug
    #1798147 (which defect is also present in Queens and Pike)
(3) The change in (2) introduced Bug #1808443 which was fixed in Stein
    and Rocky by I6d8dedfd056add3414f8f4bf7f7279eae4763286
(4) The change in (2) also introduced Bug #1811184, which is fixed by
    I6d8dedfd056add3414f8f4bf7f7279eae4763286 in Stein and Rocky, and
    which adds a unit test for Bug #1798147.
In short, in order to backport the fix for (1), we need to backport (2),
but in order to backport (2) we need to follow up immediately with
backports of (3) and (4) to fix the defects (2) introduces.

The attentive reader will note that this patch smuggles in the fix for
Bug #1798147.  We could have left this out, but it's a very small
isolated change, the defect is present in Queens and Pike (remember that
Bug #1799221, the subject of this patch, was introduced in Pike), and it
has a unit test (see (4), above).  Finally, leaving out the fix for
Bug #1798147 and backporting only the exception would still require
backporting the fixes for Bug #1808443 and Bug #1811184, so it really
would not simplify this patch.

To summarize what's being included here:
commit   changeId                                    fixed bug
805368e  If7c22ac4516f8c2a6ccd8bf6b6ed98409312b138   Bug #1798147
9c696ce  I2aa56da73660794c6dedcbb8a66e84bcec511a9c   Bug #1808443
844b627  I6d8dedfd056add3414f8f4bf7f7279eae4763286   Bug #1811184
bf89f76  Ic011fe30b4840e5098db1a594ea276ec98768bff   Bug #1799221

Change-Id: Ic011fe30b4840e5098db1a594ea276ec98768bff
Closes-Bug: #1799221
(cherry picked from commit 864c074ff1)
(cherry picked from commit bf89f76fb1)
Related-Bug: #1798147
(cherry picked from commit d3afc39467)
(cherry picked from commit 805368e902)
Related-Bug: #1808443
(cherry picked from commit 80fdc0a71b)
(cherry picked from commit 9c696ce29f)
Related-bug: #1811184
(cherry picked from commit 16b4346171)
(cherry picked from commit 844b627c38)

cinder/exception.py

@@ -724,6 +724,10 @@ class GlanceMetadataNotFound(NotFound):
     message = _("Glance metadata for volume/snapshot %(id)s cannot be found.")
 
 
+class ImageDownloadFailed(CinderException):
+    message = _("Failed to download image %(image_href)s, reason: %(reason)s")
+
+
 class ExportFailure(Invalid):
     message = _("Failed to export for volume: %(reason)s")
 

cinder/image/glance.py

@@ -323,6 +323,10 @@ class GlanceImageService(object):
         except Exception:
             _reraise_translated_image_exception(image_id)
 
+        if image_chunks is None:
+            raise exception.ImageDownloadFailed(
+                image_href=image_id, reason=_('image contains no data.'))
+
         if not data:
             return image_chunks
         else:

cinder/image/image_utils.py

@@ -313,6 +313,12 @@ def fetch(context, image_service, image_id, path, _user_id, _project_id):
                     raise exception.ImageTooBig(image_id=image_id,
                                                 reason=reason)
 
+                reason = ("IOError: %(errno)s %(strerror)s" %
+                          {'errno': e.errno, 'strerror': e.strerror})
+                LOG.error(reason)
+                raise exception.ImageDownloadFailed(image_href=image_id,
+                                                    reason=reason)
+
     duration = timeutils.delta_seconds(start_time, timeutils.utcnow())
 
     # NOTE(jdg): use a default of 1, mostly for unit test, but in

cinder/tests/unit/image/test_glance.py

@@ -16,6 +16,7 @@
 
 import datetime
 import itertools
+import six
 
 import ddt
 import glanceclient.exc
@@ -528,6 +529,20 @@ class TestGlanceImageService(test.TestCase):
         self.flags(glance_num_retries=1)
         service.download(self.context, image_id, writer)
 
+    def test_download_no_data(self):
+        class MyGlanceStubClient(glance_stubs.StubGlanceClient):
+            """Returns None instead of an iterator."""
+            def data(self, image_id):
+                return None
+
+        client = MyGlanceStubClient()
+        service = self._create_image_service(client)
+        image_id = 'fake-image-uuid'
+        e = self.assertRaises(exception.ImageDownloadFailed, service.download,
+                              self.context, image_id)
+        self.assertIn('image contains no data', six.text_type(e))
+        self.assertIn(image_id, six.text_type(e))
+
     def test_client_forbidden_converts_to_imagenotauthed(self):
         class MyGlanceStubClient(glance_stubs.StubGlanceClient):
             """A client that raises a Forbidden exception."""

cinder/tests/unit/test_image_utils.py

@@ -346,6 +346,26 @@ class TestFetch(test.TestCase):
                               context, image_service, image_id, path,
                               _user_id, _project_id)
 
+    def test_fetch_ioerror(self):
+        context = mock.sentinel.context
+        image_service = mock.Mock()
+        image_id = mock.sentinel.image_id
+        e = IOError()
+        e.errno = errno.ECONNRESET
+        e.strerror = 'Some descriptive message'
+        image_service.download.side_effect = e
+        path = '/test_path'
+        _user_id = mock.sentinel._user_id
+        _project_id = mock.sentinel._project_id
+
+        with mock.patch('cinder.image.image_utils.open',
+                        new=mock.mock_open(), create=True):
+            self.assertRaisesRegex(exception.ImageDownloadFailed,
+                                   e.strerror,
+                                   image_utils.fetch,
+                                   context, image_service, image_id, path,
+                                   _user_id, _project_id)
+
 
 class TestVerifyImage(test.TestCase):
     @mock.patch('cinder.image.image_utils.qemu_img_info')

releasenotes/notes/bug-1799221-fix-truncated-volumes-in-case-of-glance-errors-6cae19218249c3cf.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixed a bug which could create volumes with invalid content in case of
+    unhandled errors from glance client
+    (Bug `#1799221 <https://bugs.launchpad.net/cinder/+bug/1799221>`_).