diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py index 8cb88fda79..a4d9be654b 100644 --- a/ironic/drivers/modules/image_cache.py +++ b/ironic/drivers/modules/image_cache.py @@ -19,6 +19,7 @@ Utility for caching master images. """ import os +import shutil import tempfile import threading import time @@ -167,7 +168,17 @@ class ImageCache(object): if cache_up_to_date: # NOTE(dtantsur): ensure we're not in the middle of clean up with lockutils.lock('master_image'): - os.link(master_path, dest_path) + try: + os.link(master_path, dest_path) + except OSError as exc: + LOG.debug( + "Could not hardlink image file %(image)s to " + "the cache location %(dest_path)s (will copy it " + "over): %(error)s", { + 'image': master_path, + 'dest_path': dest_path, + 'error': exc}) + shutil.copyfile(master_path, dest_path) LOG.debug("Master cache hit for image %(href)s", {'href': href}) return diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py index c183e3e7a5..201ced77a4 100644 --- a/ironic/tests/unit/drivers/modules/test_image_cache.py +++ b/ironic/tests/unit/drivers/modules/test_image_cache.py @@ -18,6 +18,7 @@ import datetime import os +import shutil import tempfile import time from unittest import mock @@ -238,6 +239,34 @@ class TestImageCacheFetch(BaseTest): mock_clean_up.assert_not_called() mock_image_service.assert_not_called() + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(image_cache, '_delete_dest_path_if_stale', + return_value=False, autospec=True) + @mock.patch.object(image_cache, '_delete_master_path_if_stale', + return_value=True, autospec=True) + def test_fetch_image_hardlink_fails_fallback_to_copy( + self, mock_cache_upd, mock_dest_upd, mock_link, mock_copyfile, + mock_download, mock_clean_up, mock_image_service): + mock_link.side_effect = OSError("Invalid cross-device link") + + self.cache.fetch_image(self.uuid, self.dest_path) + + mock_link.assert_called_once_with(self.master_path, self.dest_path) + + mock_copyfile.assert_called_once_with(self.master_path, self.dest_path) + + mock_cache_upd.assert_called_once_with( + self.master_path, self.uuid, + mock_image_service.return_value.show.return_value) + mock_dest_upd.assert_called_once_with(self.master_path, self.dest_path) + + mock_download.assert_not_called() + mock_clean_up.assert_not_called() + + mock_image_service.assert_called_once_with(self.uuid, context=None) + mock_image_service.return_value.show.assert_called_once_with(self.uuid) + @mock.patch.object(image_cache, '_fetch', autospec=True) class TestImageCacheDownload(BaseTest): diff --git a/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml b/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml new file mode 100644 index 0000000000..920ee6ef78 --- /dev/null +++ b/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + When caching an image between different file systems, the hard link + operation would fail. This is fixed by falling back to a copy + operation.