From 70039cbe607195f66872d8640976c13284f75b79 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Thu, 4 Jan 2018 16:47:20 +0100
Subject: [PATCH] Handle case when a glance image contains no data

In my experience, it happens when Swift storage backing Glance was
purged, but it can probably also happen if an image never had data
at all. This patch raises a correct exception instead of TypeError.

Switch relevant unit tests to Glance V2, as V1 is long deprecated.

Change-Id: I75e5ae7f46ce3a1506ed6108ff05df3083fd5084
Closes-Bug: #1741223
---
 .../common/glance_service/base_image_service.py   |  8 ++++++++
 ironic/tests/unit/common/test_glance_service.py   | 15 +++++++++++++--
 ironic/tests/unit/stubs.py                        | 12 +++++++++++-
 .../notes/image-no-data-c281f638d3dedfb2.yaml     |  6 ++++++
 4 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/image-no-data-c281f638d3dedfb2.yaml

diff --git a/ironic/common/glance_service/base_image_service.py b/ironic/common/glance_service/base_image_service.py
index 1768b2a118..85aae5734a 100644
--- a/ironic/common/glance_service/base_image_service.py
+++ b/ironic/common/glance_service/base_image_service.py
@@ -217,6 +217,14 @@ class BaseImageService(object):
                 return
 
         image_chunks = self.call(method, image_id)
+        # NOTE(dtantsur): when using Glance V2, image_chunks is a wrapper
+        # around real data, so we have to check the wrapped data for None.
+        # Glance V1 returns HTTP 404 in this case, so no need to fix it.
+        # TODO(dtantsur): remove the hasattr check when we no longer support
+        # Glance V1.
+        if hasattr(image_chunks, 'wrapped') and image_chunks.wrapped is None:
+            raise exception.ImageDownloadFailed(
+                image_href=image_href, reason=_('image contains no data.'))
 
         if data is None:
             return image_chunks
diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py
index 7e135fb403..71c9846fc1 100644
--- a/ironic/tests/unit/common/test_glance_service.py
+++ b/ironic/tests/unit/common/test_glance_service.py
@@ -92,11 +92,11 @@ class TestGlanceImageService(base.TestCase):
 
     def setUp(self):
         super(TestGlanceImageService, self).setUp()
-        client = stubs.StubGlanceClient()
+        self.client = stubs.StubGlanceClient()
         self.context = context.RequestContext(auth_token=True)
         self.context.user_id = 'fake'
         self.context.project_id = 'fake'
-        self.service = service.GlanceImageService(client, 1, self.context)
+        self.service = service.GlanceImageService(self.client, 2, self.context)
 
         self.config(glance_api_servers=['http://localhost'], group='glance')
         self.config(auth_strategy='keystone', group='glance')
@@ -203,6 +203,17 @@ class TestGlanceImageService(base.TestCase):
         stub_service.download(image_id, writer)
         self.assertTrue(mock_sleep.called)
 
+    def test_download_no_data(self):
+        self.client.fake_wrapped = None
+        image_id = uuidutils.generate_uuid()
+
+        image = self._make_datetime_fixture()
+        with mock.patch.object(self.client, 'get', return_value=image,
+                               autospec=True):
+            self.assertRaisesRegex(exception.ImageDownloadFailed,
+                                   'image contains no data',
+                                   self.service.download, image_id)
+
     @mock.patch('sendfile.sendfile', autospec=True)
     @mock.patch('os.path.getsize', autospec=True)
     @mock.patch('%s.open' % __name__, new=mock.mock_open(), create=True)
diff --git a/ironic/tests/unit/stubs.py b/ironic/tests/unit/stubs.py
index d639ad95d5..ba66e15f22 100644
--- a/ironic/tests/unit/stubs.py
+++ b/ironic/tests/unit/stubs.py
@@ -18,8 +18,18 @@ from glanceclient import exc as glance_exc
 NOW_GLANCE_FORMAT = "2010-10-11T10:30:22"
 
 
+class _GlanceWrapper(object):
+    def __init__(self, wrapped):
+        self.wrapped = wrapped
+
+    def __iter__(self):
+        return iter(())
+
+
 class StubGlanceClient(object):
 
+    fake_wrapped = object()
+
     def __init__(self, images=None):
         self._images = []
         _images = images or []
@@ -38,7 +48,7 @@ class StubGlanceClient(object):
 
     def data(self, image_id):
         self.get(image_id)
-        return []
+        return _GlanceWrapper(self.fake_wrapped)
 
 
 class FakeImage(object):
diff --git a/releasenotes/notes/image-no-data-c281f638d3dedfb2.yaml b/releasenotes/notes/image-no-data-c281f638d3dedfb2.yaml
new file mode 100644
index 0000000000..66e1165b19
--- /dev/null
+++ b/releasenotes/notes/image-no-data-c281f638d3dedfb2.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fails deployment with the correct error message in a node's ``last_error``
+    field if an image from the Image service doesn't contain any data.
+    See `bug 1741223 <https://bugs.launchpad.net/bugs/1741223>`_ for details.