Merge "Always update cache for HTTP images if Last Modified is unknown" into bugfix/18.1

4 files changed, 28 insertions, 15 deletions

diff --git a/doc/source/install/standalone/enrollment.rst b/doc/source/install/standalone/enrollment.rst
index aa9878209..305091070 100644
--- a/doc/source/install/standalone/enrollment.rst
+++ b/doc/source/install/standalone/enrollment.rst
@@ -48,6 +48,9 @@ There are however some limitations for different hardware interfaces:
    modification time, Ironic will re-download the content. For "file://"
    images, the file system modification time is used.
 
+   If the HTTP server does not provide the last modification date and time,
+   the image will be redownloaded every time it is used.
+
 .. _hashlib: https://docs.python.org/3/library/hashlib.html
 
 Enrolling nodes
diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py
index 10d3328d1..fba6fe77e 100644
--- a/ironic/drivers/modules/image_cache.py
+++ b/ironic/drivers/modules/image_cache.py
@@ -414,20 +414,22 @@ def _delete_master_path_if_stale(master_path, href, ctx):
         img_mtime = img_service.show(href).get('updated_at')
         if not img_mtime:
             # This means that href is not a glance image and doesn't have an
-            # updated_at attribute
+            # updated_at attribute. To play on the safe side, redownload the
+            # master copy of the image.
             LOG.warning("Image service couldn't determine last "
-                        "modification time of %(href)s, considering "
-                        "cached image up to date.", {'href': href})
-            return True
-        master_mtime = utils.unix_file_modification_datetime(master_path)
-        if img_mtime <= master_mtime:
-            return True
-        # Delete image from cache as it is outdated
-        LOG.info('Image %(href)s was last modified at %(remote_time)s. '
-                 'Deleting the cached copy "%(cached_file)s since it was '
-                 'last modified at %(local_time)s and may be outdated.',
-                 {'href': href, 'remote_time': img_mtime,
-                  'local_time': master_mtime, 'cached_file': master_path})
+                        "modification time of %(href)s, updating "
+                        "the cached copy %(cached_file)s.",
+                        {'href': href, 'cached_file': master_path})
+        else:
+            master_mtime = utils.unix_file_modification_datetime(master_path)
+            if img_mtime <= master_mtime:
+                return True
+            # Delete image from cache as it is outdated
+            LOG.info('Image %(href)s was last modified at %(remote_time)s. '
+                     'Deleting the cached copy "%(cached_file)s since it was '
+                     'last modified at %(local_time)s and may be outdated.',
+                     {'href': href, 'remote_time': img_mtime,
+                      'local_time': master_mtime, 'cached_file': master_path})
 
         os.unlink(master_path)
     return False
diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py
index 994d3a214..51e36cd19 100644
--- a/ironic/tests/unit/drivers/modules/test_image_cache.py
+++ b/ironic/tests/unit/drivers/modules/test_image_cache.py
@@ -298,8 +298,8 @@ class TestUpdateImages(base.TestCase):
         res = image_cache._delete_master_path_if_stale(self.master_path, href,
                                                        None)
         mock_gis.assert_called_once_with(href, context=None)
-        self.assertFalse(mock_unlink.called)
-        self.assertTrue(res)
+        mock_unlink.assert_called_once_with(self.master_path)
+        self.assertFalse(res)
 
     @mock.patch.object(image_service, 'get_image_service', autospec=True)
     def test__delete_master_path_if_stale_master_up_to_date(self, mock_gis,
diff --git a/releasenotes/notes/image-cache-4082178dabd64249.yaml b/releasenotes/notes/image-cache-4082178dabd64249.yaml
new file mode 100644
index 000000000..b63501570
--- /dev/null
+++ b/releasenotes/notes/image-cache-4082178dabd64249.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    When an ``http(s)://`` image is used, the cached copy of the image will always
+    be updated if the HTTP server does not provide the last modification date
+    and time. Previously the cached image would be considered up-to-date, which
+    could cause invalid behavior if the image is generated on fly or was modified
+    while being served.