Merge "Ensure that image link points to the correct image" into stable/juno

2 files changed, 48 insertions, 8 deletions

diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py
index 2765c18c0..e50c602d1 100644
--- a/ironic/drivers/modules/image_cache.py
+++ b/ironic/drivers/modules/image_cache.py
@@ -104,11 +104,18 @@ class ImageCache(object):
         # TODO(dtantsur): lock expiration time
         with lockutils.lock(img_download_lock_name, 'ironic-'):
             if os.path.exists(dest_path):
-                LOG.debug("Destination %(dest)s already exists for "
-                            "image %(uuid)s" %
-                          {'uuid': uuid,
-                           'dest': dest_path})
-                return
+                # NOTE(vdrok): After rebuild requested image can change, so we
+                # should ensure that dest_path and master_path (if exists) are
+                # pointing to the same file
+                if (os.path.exists(master_path) and
+                        (os.stat(dest_path).st_ino ==
+                         os.stat(master_path).st_ino)):
+                    LOG.debug("Destination %(dest)s already exists for "
+                              "image %(uuid)s" %
+                              {'uuid': uuid,
+                               'dest': dest_path})
+                    return
+                os.unlink(dest_path)
 
             try:
                 # NOTE(dtantsur): ensure we're not in the middle of clean up
diff --git a/ironic/tests/drivers/test_image_cache.py b/ironic/tests/drivers/test_image_cache.py
index 0849b8285..0991b48fc 100644
--- a/ironic/tests/drivers/test_image_cache.py
+++ b/ironic/tests/drivers/test_image_cache.py
@@ -56,16 +56,49 @@ class TestImageCacheFetch(base.TestCase):
             None, 'uuid', self.dest_path, None)
         self.assertFalse(mock_clean_up.called)
 
+    @mock.patch.object(os, 'unlink')
     @mock.patch.object(image_cache.ImageCache, 'clean_up')
     @mock.patch.object(image_cache.ImageCache, '_download_image')
-    def test_fetch_image_dest_exists(self, mock_download, mock_clean_up,
-                                     mock_fetch_to_raw):
+    def test_fetch_image_dest_and_master_exist_uptodate(self, mock_download,
+            mock_clean_up, mock_unlink, mock_fetch):
+        touch(self.master_path)
+        os.link(self.master_path, self.dest_path)
+        self.cache.fetch_image(self.uuid, self.dest_path)
+        self.assertFalse(mock_unlink.called)
+        self.assertFalse(mock_download.called)
+        self.assertFalse(mock_fetch.called)
+        self.assertFalse(mock_clean_up.called)
+
+    @mock.patch.object(image_cache.ImageCache, 'clean_up')
+    @mock.patch.object(image_cache.ImageCache, '_download_image')
+    def test_fetch_image_dest_and_master_exist_outdated(self, mock_download,
+            mock_clean_up, mock_fetch):
+        touch(self.master_path)
         touch(self.dest_path)
+        self.assertNotEqual(os.stat(self.dest_path).st_ino,
+                            os.stat(self.master_path).st_ino)
         self.cache.fetch_image(self.uuid, self.dest_path)
         self.assertFalse(mock_download.called)
-        self.assertFalse(mock_fetch_to_raw.called)
+        self.assertFalse(mock_fetch.called)
+        self.assertTrue(os.path.isfile(self.dest_path))
+        self.assertEqual(os.stat(self.dest_path).st_ino,
+                         os.stat(self.master_path).st_ino)
         self.assertFalse(mock_clean_up.called)
 
+    @mock.patch.object(os, 'unlink')
+    @mock.patch.object(image_cache.ImageCache, 'clean_up')
+    @mock.patch.object(image_cache.ImageCache, '_download_image')
+    def test_fetch_image_only_dest_exists(self, mock_download,
+            mock_clean_up, mock_unlink, mock_fetch):
+        touch(self.dest_path)
+        self.cache.fetch_image(self.uuid, self.dest_path)
+        mock_unlink.assert_called_once_with(self.dest_path)
+        self.assertFalse(mock_fetch.called)
+        mock_download.assert_called_once_with(
+            self.uuid, self.master_path, self.dest_path,
+            ctx=None)
+        self.assertTrue(mock_clean_up.called)
+
     @mock.patch.object(image_cache.ImageCache, 'clean_up')
     @mock.patch.object(image_cache.ImageCache, '_download_image')
     def test_fetch_image_master_exists(self, mock_download, mock_clean_up,