diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py index f4226c395d..358a21581e 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 @@ -153,6 +154,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 @@ -165,6 +169,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 68a1d95e01..37ae008ecf 100644 --- a/ironic/tests/unit/drivers/modules/test_image_cache.py +++ b/ironic/tests/unit/drivers/modules/test_image_cache.py @@ -211,6 +211,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 0000000000..d9e4cebbe8 --- /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.