Merge "Update cached images based on update time"

doc/source/deploy/install-guide.rst

@@ -1750,10 +1750,12 @@ For iLO drivers, fields that should be provided are:
 * ``ilo_boot_iso``, ``image_source``, ``root_gb`` under ``instance_info``.
 
 .. note::
-   There is one limitation in this method - Ironic is not tracking changes of
-   content under hrefs that are specified. I.e., if the content under
-   "http://my.server.net/images/deploy.ramdisk" changes, Ironic does not know
-   about that and does not redownload the content.
+   Before Liberty release Ironic was not able to track non-Glance images'
+   content changes. Starting with Liberty, it is possible to do so using image
+   modification date. For example, for HTTP image, if 'Last-Modified' header
+   value of HEAD request to "http://my.server.net/images/deploy.ramdisk" is
+   greater than cached image modification time, Ironic will re-download the
+   content. For "file://" images, the file system modification time is used.
 
 
 Other references

ironic/common/glance_service/service_utils.py

@@ -77,10 +77,15 @@ def _extract_attributes(image):
 
 
 def _convert_timestamps_to_datetimes(image_meta):
-    """Returns image with timestamp fields converted to datetime objects."""
+    """Convert timestamps to datetime objects
+
+    Returns image metadata with timestamp fields converted to naive UTC
+    datetime objects.
+    """
     for attr in ['created_at', 'updated_at', 'deleted_at']:
         if image_meta.get(attr):
-            image_meta[attr] = timeutils.parse_isotime(image_meta[attr])
+            image_meta[attr] = timeutils.normalize_time(
+                timeutils.parse_isotime(image_meta[attr]))
     return image_meta
 
 _CONVERT_PROPS = ('block_device_mapping', 'mappings')

ironic/common/image_service.py

@@ -16,6 +16,7 @@
 
 
 import abc
+import datetime
 import os
 import shutil
 
@@ -30,6 +31,7 @@ import six.moves.urllib.parse as urlparse
 from ironic.common import exception
 from ironic.common.i18n import _
 from ironic.common import keystone
+from ironic.common import utils
 
 LOG = logging.getLogger(__name__)
 
@@ -119,7 +121,9 @@ class BaseImageService(object):
 
         :param image_href: Image reference.
         :raises: exception.ImageRefValidationFailed.
-        :returns: dictionary of image properties.
+        :returns: dictionary of image properties. It has three of them: 'size',
+            'updated_at' and 'properties'. 'updated_at' attribute is a naive
+            UTC datetime object.
         """
 
 
@@ -178,7 +182,9 @@ class HttpImageService(BaseImageService):
             * HEAD request failed;
             * HEAD request returned response code not equal to 200;
             * Content-Length header not found in response to HEAD request.
-        :returns: dictionary of image properties.
+        :returns: dictionary of image properties. It has three of them: 'size',
+            'updated_at' and 'properties'. 'updated_at' attribute is a naive
+            UTC datetime object.
         """
         response = self.validate_href(image_href)
         image_size = response.headers.get('Content-Length')
@@ -188,8 +194,26 @@ class HttpImageService(BaseImageService):
                 reason=_("Cannot determine image size as there is no "
                          "Content-Length header specified in response "
                          "to HEAD request."))
+
+        # Parse last-modified header to return naive datetime object
+        str_date = response.headers.get('Last-Modified')
+        date = None
+        if str_date:
+            http_date_format_strings = [
+                '%a, %d %b %Y %H:%M:%S GMT',  # RFC 822
+                '%A, %d-%b-%y %H:%M:%S GMT',  # RFC 850
+                '%a %b %d %H:%M:%S %Y'        # ANSI C
+            ]
+            for fmt in http_date_format_strings:
+                try:
+                    date = datetime.datetime.strptime(str_date, fmt)
+                    break
+                except ValueError:
+                    continue
+
         return {
             'size': int(image_size),
+            'updated_at': date,
             'properties': {}
         }
 
@@ -248,11 +272,15 @@ class FileImageService(BaseImageService):
         :param image_href: Image reference.
         :raises: exception.ImageRefValidationFailed if image file specified
             doesn't exist.
-        :returns: dictionary of image properties.
+        :returns: dictionary of image properties. It has three of them: 'size',
+            'updated_at' and 'properties'. 'updated_at' attribute is a naive
+            UTC datetime object.
         """
         source_image_path = self.validate_href(image_href)
         return {
             'size': os.path.getsize(source_image_path),
+            'updated_at': utils.unix_file_modification_datetime(
+                source_image_path),
             'properties': {}
         }
 

ironic/common/utils.py

@@ -19,6 +19,7 @@
 """Utilities and helper functions."""
 
 import contextlib
+import datetime
 import errno
 import hashlib
 import os
@@ -32,7 +33,9 @@ from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
+from oslo_utils import timeutils
 import paramiko
+import pytz
 import six
 
 from ironic.common import exception
@@ -658,3 +661,13 @@ def get_updated_capabilities(current_capabilities, new_capabilities):
 def is_regex_string_in_file(path, string):
     with open(path, 'r') as inf:
         return any(re.search(string, line) for line in inf.readlines())
+
+
+def unix_file_modification_datetime(file_name):
+    return timeutils.normalize_time(
+        # normalize time to be UTC without timezone
+        datetime.datetime.fromtimestamp(
+            # fromtimestamp will return local time by default, make it UTC
+            os.path.getmtime(file_name), tz=pytz.utc
+        )
+    )

ironic/drivers/modules/image_cache.py

@@ -34,6 +34,7 @@ from ironic.common.glance_service import service_utils
 from ironic.common.i18n import _
 from ironic.common.i18n import _LI
 from ironic.common.i18n import _LW
+from ironic.common import image_service
 from ironic.common import images
 from ironic.common import utils
 
@@ -75,10 +76,12 @@ class ImageCache(object):
     def fetch_image(self, href, dest_path, ctx=None, force_raw=True):
         """Fetch image by given href to the destination path.
 
-        Does nothing if destination path exists and corresponds to a file that
-        exists.
-        Only creates a link if master image for this UUID is already in cache.
-        Otherwise downloads an image and also stores it in cache.
+        Does nothing if destination path exists and is up to date with cache
+        and href contents.
+        Only creates a hard link (dest_path) to cached image if requested
+        image is already in cache and up to date with href contents.
+        Otherwise downloads an image, stores it in cache and creates a hard
+        link (dest_path) to it.
 
         :param href: image UUID or href to fetch
         :param dest_path: destination file path
@@ -115,33 +118,31 @@ class ImageCache(object):
 
         # TODO(dtantsur): lock expiration time
         with lockutils.lock(img_download_lock_name, 'ironic-'):
-            if os.path.exists(dest_path):
             # 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': href,
-                               'dest': dest_path})
-                    return
-                os.unlink(dest_path)
+            # pointing to the same file and their content is up to date
+            cache_up_to_date = _delete_master_path_if_stale(master_path, href,
+                                                            ctx)
+            dest_up_to_date = _delete_dest_path_if_stale(master_path,
+                                                         dest_path)
 
-            try:
+            if cache_up_to_date and dest_up_to_date:
+                LOG.debug("Destination %(dest)s already exists "
+                          "for image %(href)s",
+                          {'href': href, 'dest': dest_path})
+                return
+
+            if cache_up_to_date:
                 # NOTE(dtantsur): ensure we're not in the middle of clean up
                 with lockutils.lock('master_image', 'ironic-'):
                     os.link(master_path, dest_path)
-            except OSError:
-                LOG.info(_LI("Master cache miss for image %(uuid)s, "
-                             "starting download"),
-                         {'uuid': href})
-            else:
-                LOG.debug("Master cache hit for image %(uuid)s",
-                          {'uuid': href})
+                LOG.debug("Master cache hit for image %(href)s",
+                          {'href': href})
                 return
 
+            LOG.info(_LI("Master cache miss for image %(href)s, "
+                         "starting download"),
+                     {'href': href})
             self._download_image(
                 href, master_path, dest_path, ctx=ctx, force_raw=force_raw)
 
@@ -381,3 +382,59 @@ def cleanup(priority):
         return cls
 
     return _add_property_to_class_func
+
+
+def _delete_master_path_if_stale(master_path, href, ctx):
+    """Delete image from cache if it is not up to date with href contents.
+
+    :param master_path: path to an image in master cache
+    :param href: image href
+    :param ctx: context to use
+    :returns: True if master_path is up to date with href contents,
+        False if master_path was stale and was deleted or it didn't exist
+    """
+    if service_utils.is_glance_image(href):
+        # Glance image contents cannot be updated without its UUID change
+        return os.path.exists(master_path)
+    if os.path.exists(master_path):
+        img_service = image_service.get_image_service(href, context=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
+            LOG.warn(_LW("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(_LI('Image %(href)s was last modified at %(remote_time)s. '
+                     'Deleting the cached copy since it was cached at '
+                     '%(local_time)s and may be outdated.'),
+                 {'href': href, 'remote_time': img_mtime,
+                  'local_time': master_mtime})
+        os.unlink(master_path)
+    return False
+
+
+def _delete_dest_path_if_stale(master_path, dest_path):
+    """Delete dest_path if it does not point to cached image.
+
+    :param master_path: path to an image in master cache
+    :param dest_path: hard link to an image
+    :returns: True if dest_path points to master_path, False if dest_path was
+        stale and was deleted or it didn't exist
+    """
+    dest_path_exists = os.path.exists(dest_path)
+    if not dest_path_exists:
+        # Image not cached, re-download
+        return False
+    master_path_exists = os.path.exists(master_path)
+    if (not master_path_exists or
+            os.stat(master_path).st_ino != os.stat(dest_path).st_ino):
+        # Image exists in cache, but dest_path out of date
+        os.unlink(dest_path)
+        return False
+    return True

ironic/tests/drivers/test_image_cache.py

@@ -16,6 +16,7 @@
 
 """Tests for ImageCache class and helper functions."""
 
+import datetime
 import os
 import tempfile
 import time
@@ -37,7 +38,6 @@ def touch(filename):
     open(filename, 'w').close()
 
 
-@mock.patch.object(image_cache, '_fetch', autospec=True)
 class TestImageCacheFetch(base.TestCase):
 
     def setUp(self):
@@ -49,6 +49,7 @@ class TestImageCacheFetch(base.TestCase):
         self.uuid = uuidutils.generate_uuid()
         self.master_path = os.path.join(self.master_dir, self.uuid)
 
+    @mock.patch.object(image_cache, '_fetch', autospec=True)
     @mock.patch.object(image_cache.ImageCache, 'clean_up', autospec=True)
     @mock.patch.object(image_cache.ImageCache, '_download_image',
                        autospec=True)
@@ -61,94 +62,101 @@ class TestImageCacheFetch(base.TestCase):
             None, self.uuid, self.dest_path, True)
         self.assertFalse(mock_clean_up.called)
 
-    @mock.patch.object(os, 'unlink', autospec=True)
     @mock.patch.object(image_cache.ImageCache, 'clean_up', autospec=True)
     @mock.patch.object(image_cache.ImageCache, '_download_image',
                        autospec=True)
-    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)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(image_cache, '_delete_dest_path_if_stale',
+                       return_value=True, autospec=True)
+    @mock.patch.object(image_cache, '_delete_master_path_if_stale',
+                       return_value=True, autospec=True)
+    def test_fetch_image_dest_and_master_uptodate(
+            self, mock_cache_upd, mock_dest_upd, mock_link, mock_download,
+            mock_clean_up):
         self.cache.fetch_image(self.uuid, self.dest_path)
-        self.assertFalse(mock_unlink.called)
+        mock_cache_upd.assert_called_once_with(self.master_path, self.uuid,
+                                               None)
+        mock_dest_upd.assert_called_once_with(self.master_path, self.dest_path)
+        self.assertFalse(mock_link.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', autospec=True)
     @mock.patch.object(image_cache.ImageCache, '_download_image',
                        autospec=True)
-    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)
+    @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_dest_out_of_date(
+            self, mock_cache_upd, mock_dest_upd, mock_link, mock_download,
+            mock_clean_up):
         self.cache.fetch_image(self.uuid, self.dest_path)
+        mock_cache_upd.assert_called_once_with(self.master_path, self.uuid,
+                                               None)
+        mock_dest_upd.assert_called_once_with(self.master_path, self.dest_path)
+        mock_link.assert_called_once_with(self.master_path, self.dest_path)
         self.assertFalse(mock_download.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', autospec=True)
     @mock.patch.object(image_cache.ImageCache, 'clean_up', autospec=True)
     @mock.patch.object(image_cache.ImageCache, '_download_image',
                        autospec=True)
-    def test_fetch_image_only_dest_exists(
-            self, mock_download, mock_clean_up, mock_unlink, mock_fetch):
-        touch(self.dest_path)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(image_cache, '_delete_dest_path_if_stale',
+                       return_value=True, autospec=True)
+    @mock.patch.object(image_cache, '_delete_master_path_if_stale',
+                       return_value=False, autospec=True)
+    def test_fetch_image_master_out_of_date(
+            self, mock_cache_upd, mock_dest_upd, mock_link, mock_download,
+            mock_clean_up):
         self.cache.fetch_image(self.uuid, self.dest_path)
-        mock_unlink.assert_called_once_with(self.dest_path)
-        self.assertFalse(mock_fetch.called)
+        mock_cache_upd.assert_called_once_with(self.master_path, self.uuid,
+                                               None)
+        mock_dest_upd.assert_called_once_with(self.master_path, self.dest_path)
+        self.assertFalse(mock_link.called)
         mock_download.assert_called_once_with(
             self.cache, self.uuid, self.master_path, self.dest_path,
             ctx=None, force_raw=True)
-        self.assertTrue(mock_clean_up.called)
+        mock_clean_up.assert_called_once_with(self.cache)
 
     @mock.patch.object(image_cache.ImageCache, 'clean_up', autospec=True)
     @mock.patch.object(image_cache.ImageCache, '_download_image',
                        autospec=True)
-    def test_fetch_image_master_exists(self, mock_download, mock_clean_up,
-                                       mock_fetch):
-        touch(self.master_path)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(image_cache, '_delete_dest_path_if_stale',
+                       return_value=True, autospec=True)
+    @mock.patch.object(image_cache, '_delete_master_path_if_stale',
+                       return_value=False, autospec=True)
+    def test_fetch_image_both_master_and_dest_out_of_date(
+            self, mock_cache_upd, mock_dest_upd, mock_link, mock_download,
+            mock_clean_up):
         self.cache.fetch_image(self.uuid, self.dest_path)
-        self.assertFalse(mock_download.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(image_cache.ImageCache, 'clean_up', autospec=True)
-    @mock.patch.object(image_cache.ImageCache, '_download_image',
-                       autospec=True)
-    def test_fetch_image(self, mock_download, mock_clean_up,
-                         mock_fetch):
-        self.cache.fetch_image(self.uuid, self.dest_path)
-        self.assertFalse(mock_fetch.called)
+        mock_cache_upd.assert_called_once_with(self.master_path, self.uuid,
+                                               None)
+        mock_dest_upd.assert_called_once_with(self.master_path, self.dest_path)
+        self.assertFalse(mock_link.called)
         mock_download.assert_called_once_with(
             self.cache, self.uuid, self.master_path, self.dest_path,
             ctx=None, force_raw=True)
-        self.assertTrue(mock_clean_up.called)
+        mock_clean_up.assert_called_once_with(self.cache)
 
     @mock.patch.object(image_cache.ImageCache, 'clean_up', autospec=True)
     @mock.patch.object(image_cache.ImageCache, '_download_image',
                        autospec=True)
-    def test_fetch_image_not_uuid(self, mock_download, mock_clean_up,
-                                  mock_fetch):
+    def test_fetch_image_not_uuid(self, mock_download, mock_clean_up):
         href = u'http://abc.com/ubuntu.qcow2'
         href_encoded = href.encode('utf-8') if six.PY2 else href
         href_converted = str(uuid.uuid5(uuid.NAMESPACE_URL, href_encoded))
         master_path = os.path.join(self.master_dir, href_converted)
         self.cache.fetch_image(href, self.dest_path)
-        self.assertFalse(mock_fetch.called)
         mock_download.assert_called_once_with(
             self.cache, href, master_path, self.dest_path,
             ctx=None, force_raw=True)
         self.assertTrue(mock_clean_up.called)
 
+    @mock.patch.object(image_cache, '_fetch', autospec=True)
     def test__download_image(self, mock_fetch):
         def _fake_fetch(ctx, uuid, tmp_path, *args):
             self.assertEqual(self.uuid, uuid)
@@ -167,6 +175,119 @@ class TestImageCacheFetch(base.TestCase):
             self.assertEqual("TEST", fp.read())
 
 
+@mock.patch.object(os, 'unlink', autospec=True)
+class TestUpdateImages(base.TestCase):
+
+    def setUp(self):
+        super(TestUpdateImages, self).setUp()
+        self.master_dir = tempfile.mkdtemp()
+        self.dest_dir = tempfile.mkdtemp()
+        self.dest_path = os.path.join(self.dest_dir, 'dest')
+        self.uuid = uuidutils.generate_uuid()
+        self.master_path = os.path.join(self.master_dir, self.uuid)
+
+    @mock.patch.object(os.path, 'exists', return_value=False, autospec=True)
+    @mock.patch.object(image_service, 'get_image_service', autospec=True)
+    def test__delete_master_path_if_stale_glance_img_not_cached(
+            self, mock_gis, mock_path_exists, mock_unlink):
+        res = image_cache._delete_master_path_if_stale(self.master_path,
+                                                       self.uuid, None)
+        self.assertFalse(mock_gis.called)
+        self.assertFalse(mock_unlink.called)
+        mock_path_exists.assert_called_once_with(self.master_path)
+        self.assertFalse(res)
+
+    @mock.patch.object(os.path, 'exists', return_value=True, autospec=True)
+    @mock.patch.object(image_service, 'get_image_service', autospec=True)
+    def test__delete_master_path_if_stale_glance_img(
+            self, mock_gis, mock_path_exists, mock_unlink):
+        res = image_cache._delete_master_path_if_stale(self.master_path,
+                                                       self.uuid, None)
+        self.assertFalse(mock_gis.called)
+        self.assertFalse(mock_unlink.called)
+        mock_path_exists.assert_called_once_with(self.master_path)
+        self.assertTrue(res)
+
+    @mock.patch.object(image_service, 'get_image_service', autospec=True)
+    def test__delete_master_path_if_stale_no_master(self, mock_gis,
+                                                    mock_unlink):
+        res = image_cache._delete_master_path_if_stale(self.master_path,
+                                                       'http://11', None)
+        self.assertFalse(mock_gis.called)
+        self.assertFalse(mock_unlink.called)
+        self.assertFalse(res)
+
+    @mock.patch.object(image_service, 'get_image_service', autospec=True)
+    def test__delete_master_path_if_stale_no_updated_at(self, mock_gis,
+                                                        mock_unlink):
+        touch(self.master_path)
+        href = 'http://awesomefreeimages.al/img111'
+        mock_gis.return_value.show.return_value = {}
+        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.patch.object(image_service, 'get_image_service', autospec=True)
+    def test__delete_master_path_if_stale_master_up_to_date(self, mock_gis,
+                                                            mock_unlink):
+        touch(self.master_path)
+        href = 'http://awesomefreeimages.al/img999'
+        mock_gis.return_value.show.return_value = {
+            'updated_at': datetime.datetime(1999, 11, 15, 8, 12, 31)
+        }
+        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.patch.object(image_service, 'get_image_service', autospec=True)
+    def test__delete_master_path_if_stale_out_of_date(self, mock_gis,
+                                                      mock_unlink):
+        touch(self.master_path)
+        href = 'http://awesomefreeimages.al/img999'
+        mock_gis.return_value.show.return_value = {
+            'updated_at': datetime.datetime((datetime.datetime.utcnow().year
+                                             + 1), 11, 15, 8, 12, 31)
+        }
+        res = image_cache._delete_master_path_if_stale(self.master_path, href,
+                                                       None)
+        mock_gis.assert_called_once_with(href, context=None)
+        mock_unlink.assert_called_once_with(self.master_path)
+        self.assertFalse(res)
+
+    def test__delete_dest_path_if_stale_no_dest(self, mock_unlink):
+        res = image_cache._delete_dest_path_if_stale(self.master_path,
+                                                     self.dest_path)
+        self.assertFalse(mock_unlink.called)
+        self.assertFalse(res)
+
+    def test__delete_dest_path_if_stale_no_master(self, mock_unlink):
+        touch(self.dest_path)
+        res = image_cache._delete_dest_path_if_stale(self.master_path,
+                                                     self.dest_path)
+        mock_unlink.assert_called_once_with(self.dest_path)
+        self.assertFalse(res)
+
+    def test__delete_dest_path_if_stale_out_of_date(self, mock_unlink):
+        touch(self.master_path)
+        touch(self.dest_path)
+        res = image_cache._delete_dest_path_if_stale(self.master_path,
+                                                     self.dest_path)
+        mock_unlink.assert_called_once_with(self.dest_path)
+        self.assertFalse(res)
+
+    def test__delete_dest_path_if_stale_up_to_date(self, mock_unlink):
+        touch(self.master_path)
+        os.link(self.master_path, self.dest_path)
+        res = image_cache._delete_dest_path_if_stale(self.master_path,
+                                                     self.dest_path)
+        self.assertFalse(mock_unlink.called)
+        self.assertTrue(res)
+
+
 class TestImageCacheCleanUp(base.TestCase):
 
     def setUp(self):

ironic/tests/test_glance_service.py

@@ -94,12 +94,7 @@ class TestGlanceImageService(base.TestCase):
     NOW_GLANCE_OLD_FORMAT = "2010-10-11T10:30:22"
     NOW_GLANCE_FORMAT = "2010-10-11T10:30:22.000000"
 
-    class tzinfo(datetime.tzinfo):
-        @staticmethod
-        def utcoffset(*args, **kwargs):
-            return datetime.timedelta()
-
-    NOW_DATETIME = datetime.datetime(2010, 10, 11, 10, 30, 22, tzinfo=tzinfo())
+    NOW_DATETIME = datetime.datetime(2010, 10, 11, 10, 30, 22)
 
     def setUp(self):
         super(TestGlanceImageService, self).setUp()

ironic/tests/test_image_service.py

@@ -10,6 +10,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import datetime
 import os
 import shutil
 
@@ -66,11 +67,28 @@ class HttpImageServiceTestCase(base.TestCase):
         head_mock.assert_called_once_with(self.href)
 
     @mock.patch.object(requests, 'head', autospec=True)
-    def test_show(self, head_mock):
+    def _test_show(self, head_mock, mtime, mtime_date):
         head_mock.return_value.status_code = 200
+        head_mock.return_value.headers = {
+            'Content-Length': 100,
+            'Last-Modified': mtime
+        }
         result = self.service.show(self.href)
-        head_mock.assert_called_with(self.href)
-        self.assertEqual({'size': 1, 'properties': {}}, result)
+        head_mock.assert_called_once_with(self.href)
+        self.assertEqual({'size': 100, 'updated_at': mtime_date,
+                          'properties': {}}, result)
+
+    def test_show_rfc_822(self):
+        self._test_show(mtime='Tue, 15 Nov 2014 08:12:31 GMT',
+                        mtime_date=datetime.datetime(2014, 11, 15, 8, 12, 31))
+
+    def test_show_rfc_850(self):
+        self._test_show(mtime='Tuesday, 15-Nov-14 08:12:31 GMT',
+                        mtime_date=datetime.datetime(2014, 11, 15, 8, 12, 31))
+
+    def test_show_ansi_c(self):
+        self._test_show(mtime='Tue Nov 15 08:12:31 2014',
+                        mtime_date=datetime.datetime(2014, 11, 15, 8, 12, 31))
 
     @mock.patch.object(requests, 'head', autospec=True)
     def test_show_no_content_length(self, head_mock):
@@ -132,15 +150,21 @@ class FileImageServiceTestCase(base.TestCase):
                           self.service.validate_href, self.href)
         path_exists_mock.assert_called_once_with(self.href_path)
 
+    @mock.patch.object(os.path, 'getmtime', return_value=1431087909.1641912,
+                       autospec=True)
     @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True)
     @mock.patch.object(image_service.FileImageService, 'validate_href',
                        autospec=True)
-    def test_show(self, _validate_mock, getsize_mock):
+    def test_show(self, _validate_mock, getsize_mock, getmtime_mock):
         _validate_mock.return_value = self.href_path
         result = self.service.show(self.href)
         getsize_mock.assert_called_once_with(self.href_path)
+        getmtime_mock.assert_called_once_with(self.href_path)
         _validate_mock.assert_called_once_with(mock.ANY, self.href)
-        self.assertEqual({'size': 42, 'properties': {}}, result)
+        self.assertEqual({'size': 42,
+                          'updated_at': datetime.datetime(2015, 5, 8,
+                                                          12, 25, 9, 164191),
+                          'properties': {}}, result)
 
     @mock.patch.object(os, 'link', autospec=True)
     @mock.patch.object(os, 'remove', autospec=True)

ironic/tests/test_utils.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import datetime
 import errno
 import hashlib
 import os
@@ -434,6 +435,14 @@ class GenericUtilsTestCase(base.TestCase):
         # original value.
         self.assertEqual(value, utils.safe_rstrip(value))
 
+    @mock.patch.object(os.path, 'getmtime', return_value=1439465889.4964755,
+                       autospec=True)
+    def test_unix_file_modification_datetime(self, mtime_mock):
+        expected = datetime.datetime(2015, 8, 13, 11, 38, 9, 496475)
+        self.assertEqual(expected,
+                         utils.unix_file_modification_datetime('foo'))
+        mtime_mock.assert_called_once_with('foo')
+
 
 class MkfsTestCase(base.TestCase):
 

requirements.txt

@@ -16,6 +16,7 @@ python-neutronclient<3,>=2.6.0
 python-glanceclient>=0.18.0
 python-keystoneclient>=1.6.0
 python-swiftclient>=2.2.0
+pytz>=2013.6
 stevedore>=1.5.0 # Apache-2.0
 pysendfile>=2.0.0
 websockify>=0.6.0