Improve logs when hard linking images fails

When hard linking images fail in ironic the logs doesn't have
enough related path information to resolve the underlying cause
of the exception.

Change-Id: I3331c4eca2deb9383f448f1efa93cbef53019f9c
Story: #2004303
Task: #27863
(cherry picked from commit 46a5899543bce73e1d40f6f363031f9ac1a472e8)

3 files changed, 28 insertions, 0 deletions

diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py
index 76b55d51d..10f73253c 100644
--- a/ironic/drivers/modules/image_cache.py
+++ b/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)
 
diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py
index b50d3fe40..5bd4c8d33 100644
--- a/ironic/tests/unit/drivers/modules/test_image_cache.py
+++ b/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):
diff --git a/releasenotes/notes/invalid_cross_device_link-7ecf3543a8ada09f.yaml b/releasenotes/notes/invalid_cross_device_link-7ecf3543a8ada09f.yaml
new file mode 100644
index 000000000..d9e4cebbe
--- /dev/null
+++ b/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.