diff --git a/doc/source/deploy/install-guide.rst b/doc/source/deploy/install-guide.rst index 1ddc541f84..7a7bf7625e 100644 --- a/doc/source/deploy/install-guide.rst +++ b/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 diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index 5b8ab57870..c82a90df7b 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/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') diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index e0e2c05c68..7a7f54b96a 100644 --- a/ironic/common/image_service.py +++ b/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': {} } diff --git a/ironic/common/utils.py b/ironic/common/utils.py index 756702461a..2bc340cdf1 100644 --- a/ironic/common/utils.py +++ b/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 + ) + ) diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py index 1387eb705d..c1b134fdb4 100644 --- a/ironic/drivers/modules/image_cache.py +++ b/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) + # 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 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 diff --git a/ironic/tests/drivers/test_image_cache.py b/ironic/tests/drivers/test_image_cache.py index 7e72506390..3d666cd6c2 100644 --- a/ironic/tests/drivers/test_image_cache.py +++ b/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): diff --git a/ironic/tests/test_glance_service.py b/ironic/tests/test_glance_service.py index 5ec6251417..ce255283a6 100644 --- a/ironic/tests/test_glance_service.py +++ b/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() diff --git a/ironic/tests/test_image_service.py b/ironic/tests/test_image_service.py index 312b56a16c..e35439a095 100644 --- a/ironic/tests/test_image_service.py +++ b/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) diff --git a/ironic/tests/test_utils.py b/ironic/tests/test_utils.py index 7076fed903..30f51456e6 100644 --- a/ironic/tests/test_utils.py +++ b/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): diff --git a/requirements.txt b/requirements.txt index e6436ea9b5..0f5fb91c91 100644 --- a/requirements.txt +++ b/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