Merge "Improve logs when hard linking images fails" into stable/queens

ironic/drivers/modules/image_cache.py

@@ -30,6 +30,7 @@ import six
 
 from ironic.common import exception
 from ironic.common.glance_service import service_utils
+from ironic.common.i18n import _
 from ironic.common import image_service
 from ironic.common import images
 from ironic.common import utils
@@ -148,6 +149,9 @@ class ImageCache(object):
         :param ctx: context
         :param force_raw: boolean value, whether to convert the image to raw
                           format
+        :raise ImageDownloadFailed: when the image cache and the image HTTP or
+                                    TFTP location are on different file system,
+                                    causing hard link to fail.
         """
         # TODO(ghe): timeout and retry for downloads
         # TODO(ghe): logging when image cannot be created
@@ -160,6 +164,13 @@ class ImageCache(object):
             # will have link count >1 at any moment, so won't be cleaned up
             os.link(tmp_path, master_path)
             os.link(master_path, dest_path)
+        except OSError as exc:
+            msg = (_("Could not link image %(img_href)s from %(src_path)s "
+                     "to %(dst_path)s, error: %(exc)s") %
+                   {'img_href': href, 'src_path': master_path,
+                    'dst_path': dest_path, 'exc': exc})
+            LOG.error(msg)
+            raise exception.ImageDownloadFailed(msg)
         finally:
             utils.rmtree_without_raise(tmp_dir)
 

ironic/tests/unit/drivers/modules/test_image_cache.py

@@ -174,6 +174,18 @@ class TestImageCacheFetch(base.TestCase):
         with open(self.dest_path) as fp:
             self.assertEqual("TEST", fp.read())
 
+    @mock.patch.object(image_cache, '_fetch', autospec=True)
+    @mock.patch.object(image_cache, 'LOG', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    def test__download_image_linkfail(self, mock_link, mock_log, mock_fetch):
+        mock_link.side_effect = [None, OSError]
+        self.assertRaises(exception.ImageDownloadFailed,
+                          self.cache._download_image,
+                          self.uuid, self.master_path, self.dest_path)
+        self.assertTrue(mock_fetch.called)
+        self.assertEqual(2, mock_link.call_count)
+        self.assertTrue(mock_log.error.called)
+
 
 @mock.patch.object(os, 'unlink', autospec=True)
 class TestUpdateImages(base.TestCase):

releasenotes/notes/invalid_cross_device_link-7ecf3543a8ada09f.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Properly reports an error when the image cache and the image HTTP or TFTP
+    location are on different file system, causing hard link to fail.