Add ability to cache swift temporary URLs

Caching of swift temporary is only useful for ironic python agent in
case of using proxies to download instance image.

This change adds the following:
- caching of temp URLs, so that proxy server does not invalidate the
  cached image for every deployment.
- [glance]swift_temp_url_expected_download_start_delay option, so
  that it is not possible to set swift_temp_url_duration less than
  the value specified. This option is also used to determine if
  cached temp URL will be still valid when agent ramdisk starts up.
  To preserve current behavior, this value is set to 0.

Partial-bug: #1526222

Change-Id: If79bc0067db92c8917a15a0989143a1133ad4519

etc/ironic/ironic.conf.sample

@@ -489,13 +489,12 @@
 # value)
 #protocol=http
 
-# Time interval (in seconds) for successive awake call
-# to AMT interface, this depends on the IdleTimeout
-# setting on AMT interface. AMT Interface will go to
-# sleep after 60 seconds of inactivity by default.
-# IdleTimeout=0 means AMT will not go to sleep at all.
-# Setting awake_interval=0 will disable awake call. (integer
-# value)
+# Time interval (in seconds) for successive awake call to AMT
+# interface, this depends on the IdleTimeout setting on AMT
+# interface. AMT Interface will go to sleep after 60 seconds
+# of inactivity by default. IdleTimeout=0 means AMT will not
+# go to sleep at all. Setting awake_interval=0 will disable
+# awake call. (integer value)
 #awake_interval=60
 
 
@@ -1003,9 +1002,28 @@
 # The length of time in seconds that the temporary URL will be
 # valid for. Defaults to 20 minutes. If some deploys get a 401
 # response code when trying to download from the temporary
-# URL, try raising this duration. (integer value)
+# URL, try raising this duration. This value must be greater
+# than or equal to the value for
+# swift_temp_url_expected_download_start_delay (integer value)
 #swift_temp_url_duration=1200
 
+# Whether to cache generated Swift temporary URLs. Setting it
+# to true is only useful when an image caching proxy is used.
+# Defaults to False. (boolean value)
+#swift_temp_url_cache_enabled=false
+
+# This is the delay (in seconds) from the time of the deploy
+# request (when the Swift temporary URL is generated) to when
+# the IPA ramdisk starts up and URL is used for the image
+# download. This value is used to check if the Swift temporary
+# URL duration is large enough to let the image download
+# begin. Also if temporary URL caching is enabled this will
+# determine if a cached entry will still be valid when the
+# download starts. swift_temp_url_duration value must be
+# greater than or equal to this option's value. Defaults to 0.
+# (integer value)
+#swift_temp_url_expected_download_start_delay=0
+
 # The "endpoint" (scheme, hostname, optional port) for the
 # Swift URL of the form
 # "endpoint_url/api_version/[account/]container/object_id". Do

ironic/common/glance_service/v2/image_service.py

@@ -13,8 +13,12 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import collections
+import time
+
 from oslo_config import cfg
 from oslo_utils import uuidutils
+import six
 from six.moves.urllib import parse as urlparse
 from swiftclient import utils as swift_utils
 
@@ -46,7 +50,27 @@ glance_opts = [
                       'will be valid for. Defaults to 20 minutes. If some '
                       'deploys get a 401 response code when trying to '
                       'download from the temporary URL, try raising this '
-                      'duration.')),
+                      'duration. This value must be greater than or equal to '
+                      'the value for '
+                      'swift_temp_url_expected_download_start_delay')),
+    cfg.BoolOpt('swift_temp_url_cache_enabled',
+                default=False,
+                help=_('Whether to cache generated Swift temporary URLs. '
+                       'Setting it to true is only useful when an image '
+                       'caching proxy is used. Defaults to False.')),
+    cfg.IntOpt('swift_temp_url_expected_download_start_delay',
+               default=0, min=0,
+               help=_('This is the delay (in seconds) from the time of the '
+                      'deploy request (when the Swift temporary URL is '
+                      'generated) to when the IPA ramdisk starts up and URL '
+                      'is used for the image download. This value is used to '
+                      'check if the Swift temporary URL duration is large '
+                      'enough to let the image download begin. Also if '
+                      'temporary URL caching is enabled this will determine '
+                      'if a cached entry will still be valid when the '
+                      'download starts. swift_temp_url_duration value must be '
+                      'greater than or equal to this option\'s value. '
+                      'Defaults to 0.')),
     cfg.StrOpt(
         'swift_endpoint_url',
         help=_('The "endpoint" (scheme, hostname, optional port) for '
@@ -93,16 +117,29 @@ glance_opts = [
                choices=['swift', 'radosgw'],
                help=_('Type of endpoint to use for temporary URLs. If the '
                       'Glance backend is Swift, use "swift"; if it is CEPH '
-                      'with RADOS gateway, use "radosgw".'))
+                      'with RADOS gateway, use "radosgw".')),
 ]
 
 CONF = cfg.CONF
 CONF.register_opts(glance_opts, group='glance')
 
+TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement',
+                                             ['url', 'url_expires_at'])
+
 
 class GlanceImageService(base_image_service.BaseImageService,
                          service.ImageService):
 
+    # A dictionary containing cached temp URLs in namedtuples
+    # in format:
+    # {
+    #     <image_id> : (
+    #          url=<temp_url>,
+    #          url_expires_at=<expiration_time>
+    #     )
+    # }
+    _cache = {}
+
     def detail(self, **kwargs):
         return self._detail(method='list', **kwargs)
 
@@ -124,10 +161,50 @@ class GlanceImageService(base_image_service.BaseImageService,
     def delete(self, image_id):
         return self._delete(image_id, method='delete')
 
+    def _generate_temp_url(self, path, seconds, key, method, endpoint,
+                           image_id):
+        """Get Swift temporary URL.
+
+        Generates (or returns the cached one if caching is enabled) a
+        temporary URL that gives unauthenticated access to the Swift object.
+
+        :param path: The full path to the Swift object. Example:
+            /v1/AUTH_account/c/o.
+        :param seconds: The amount of time in seconds the temporary URL will
+            be valid for.
+        :param key: The secret temporary URL key set on the Swift cluster.
+        :param method: A HTTP method, typically either GET or PUT, to allow for
+            this temporary URL.
+        :param endpoint: Endpoint URL of Swift service.
+        :param image_id: UUID of a Glance image.
+        :returns: temporary URL
+        """
+
+        if CONF.glance.swift_temp_url_cache_enabled:
+            self._remove_expired_items_from_cache()
+            if image_id in self._cache:
+                return self._cache[image_id].url
+
+        path = swift_utils.generate_temp_url(
+            path=path, seconds=seconds, key=key, method=method)
+
+        temp_url = '{endpoint_url}{url_path}'.format(
+            endpoint_url=endpoint, url_path=path)
+
+        if CONF.glance.swift_temp_url_cache_enabled:
+            query = urlparse.urlparse(temp_url).query
+            exp_time_str = dict(urlparse.parse_qsl(query))['temp_url_expires']
+            self._cache[image_id] = TempUrlCacheElement(
+                url=temp_url, url_expires_at=int(exp_time_str)
+            )
+
+        return temp_url
+
     def swift_temp_url(self, image_info):
         """Generate a no-auth Swift temporary URL.
 
-        This function will generate the temporary Swift URL using the image
+        This function will generate (or return the cached one if temp URL
+        cache is enabled) the temporary Swift URL using the image
         id from Glance and the config options: 'swift_endpoint_url',
         'swift_api_version', 'swift_account' and 'swift_container'.
         The temporary URL will be valid for 'swift_temp_url_duration' seconds.
@@ -156,11 +233,13 @@ class GlanceImageService(base_image_service.BaseImageService,
                 'The given image info does not have a valid image id: %s')
                 % image_info)
 
+        image_id = image_info['id']
+
         url_fragments = {
             'api_version': CONF.glance.swift_api_version,
             'account': CONF.glance.swift_account,
-            'container': self._get_swift_container(image_info['id']),
-            'object_id': image_info['id']
+            'container': self._get_swift_container(image_id),
+            'object_id': image_id
         }
 
         endpoint_url = CONF.glance.swift_endpoint_url
@@ -180,14 +259,15 @@ class GlanceImageService(base_image_service.BaseImageService,
             template = '/{api_version}/{account}/{container}/{object_id}'
 
         url_path = template.format(**url_fragments)
-        path = swift_utils.generate_temp_url(
+
+        return self._generate_temp_url(
             path=url_path,
             seconds=CONF.glance.swift_temp_url_duration,
             key=CONF.glance.swift_temp_url_key,
-            method='GET')
-
-        return '{endpoint_url}{url_path}'.format(
-            endpoint_url=endpoint_url, url_path=path)
+            method='GET',
+            endpoint=endpoint_url,
+            image_id=image_id
+        )
 
     def _validate_temp_url_config(self):
         """Validate the required settings for a temporary URL."""
@@ -204,9 +284,13 @@ class GlanceImageService(base_image_service.BaseImageService,
             raise exc.MissingParameterValue(_(
                 'Swift temporary URLs require a Swift account string. '
                 'You must provide "swift_account" as a config option.'))
-        if CONF.glance.swift_temp_url_duration < 0:
+        if (CONF.glance.swift_temp_url_duration <
+                CONF.glance.swift_temp_url_expected_download_start_delay):
             raise exc.InvalidParameterValue(_(
-                '"swift_temp_url_duration" must be a positive integer.'))
+                '"swift_temp_url_duration" must be greater than or equal to '
+                '"[glance]swift_temp_url_expected_download_start_delay" '
+                'option, otherwise the Swift temporary URL may expire before '
+                'the download starts.'))
         seed_num_chars = CONF.glance.swift_store_multiple_containers_seed
         if (seed_num_chars is None or seed_num_chars < 0
                 or seed_num_chars > 32):
@@ -260,3 +344,18 @@ class GlanceImageService(base_image_service.BaseImageService,
             raise exc.ImageNotFound(image_id=image_id)
 
         return getattr(image_meta, 'direct_url', None)
+
+    def _remove_expired_items_from_cache(self):
+        """Remove expired items from temporary URL cache
+
+        This function removes entries that will expire before the expected
+        usage time.
+        """
+        max_valid_time = (
+            int(time.time()) +
+            CONF.glance.swift_temp_url_expected_download_start_delay)
+        keys_to_remove = [
+            k for k, v in six.iteritems(self._cache)
+            if (v.url_expires_at < max_valid_time)]
+        for k in keys_to_remove:
+            del self._cache[k]

ironic/tests/unit/common/test_glance_service.py

@@ -22,12 +22,14 @@ import mock
 from oslo_config import cfg
 from oslo_context import context
 from oslo_serialization import jsonutils
+from oslo_utils import uuidutils
 from six.moves.urllib import parse as urlparse
 import testtools
 
 from ironic.common import exception
 from ironic.common.glance_service import base_image_service
 from ironic.common.glance_service import service_utils
+from ironic.common.glance_service.v2 import image_service as glance_v2
 from ironic.common import image_service as service
 from ironic.tests import base
 from ironic.tests.unit import stubs
@@ -688,6 +690,17 @@ class TestGlanceSwiftTempURL(base.TestCase):
             key=CONF.glance.swift_temp_url_key,
             method='GET')
 
+    @mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
+    def test_swift_temp_url_invalid_image_info(self, tempurl_mock):
+        self.service._validate_temp_url_config = mock.Mock()
+        image_info = {}
+        self.assertRaises(exception.ImageUnacceptable,
+                          self.service.swift_temp_url, image_info)
+        image_info = {'id': 'not an id'}
+        self.assertRaises(exception.ImageUnacceptable,
+                          self.service.swift_temp_url, image_info)
+        self.assertFalse(tempurl_mock.called)
+
     @mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
     def test_swift_temp_url_radosgw(self, tempurl_mock):
         self.config(temp_url_endpoint_type='radosgw', group='glance')
@@ -800,8 +813,10 @@ class TestGlanceSwiftTempURL(base.TestCase):
         self.config(temp_url_endpoint_type='radosgw', group='glance')
         self.service._validate_temp_url_config()
 
-    def test__validate_temp_url_endpoint_negative_duration(self):
-        self.config(swift_temp_url_duration=-1,
+    def test__validate_temp_url_endpoint_less_than_download_delay(self):
+        self.config(swift_temp_url_expected_download_start_delay=1000,
+                    group='glance')
+        self.config(swift_temp_url_duration=15,
                     group='glance')
         self.assertRaises(exception.InvalidParameterValue,
                           self.service._validate_temp_url_config)
@@ -821,6 +836,215 @@ class TestGlanceSwiftTempURL(base.TestCase):
                           self.service._validate_temp_url_config)
 
 
+class TestSwiftTempUrlCache(base.TestCase):
+
+    def setUp(self):
+        super(TestSwiftTempUrlCache, self).setUp()
+        client = stubs.StubGlanceClient()
+        self.context = context.RequestContext()
+        self.context.auth_token = 'fake'
+        self.config(swift_temp_url_expected_download_start_delay=100,
+                    group='glance')
+        self.config(swift_temp_url_key='correcthorsebatterystaple',
+                    group='glance')
+        self.config(swift_endpoint_url='https://swift.example.com',
+                    group='glance')
+        self.config(swift_account='AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30',
+                    group='glance')
+        self.config(swift_api_version='v1',
+                    group='glance')
+        self.config(swift_container='glance',
+                    group='glance')
+        self.config(swift_temp_url_duration=1200,
+                    group='glance')
+        self.config(swift_temp_url_cache_enabled=True,
+                    group='glance')
+        self.config(swift_store_multiple_containers_seed=0,
+                    group='glance')
+        self.glance_service = service.GlanceImageService(client, version=2,
+                                                         context=self.context)
+
+    @mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
+    def test_add_items_to_cache(self, tempurl_mock):
+        fake_image = {
+            'id': uuidutils.generate_uuid()
+        }
+
+        path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
+                '/glance'
+                '/%s' % fake_image['id'])
+        exp_time = int(time.time()) + 1200
+        tempurl_mock.return_value = (
+            path + '?temp_url_sig=hmacsig&temp_url_expires=%s' % exp_time)
+
+        cleanup_mock = mock.Mock()
+        self.glance_service._remove_expired_items_from_cache = cleanup_mock
+        self.glance_service._validate_temp_url_config = mock.Mock()
+
+        temp_url = self.glance_service.swift_temp_url(
+            image_info=fake_image)
+
+        self.assertEqual(CONF.glance.swift_endpoint_url +
+                         tempurl_mock.return_value,
+                         temp_url)
+        cleanup_mock.assert_called_once_with()
+        tempurl_mock.assert_called_with(
+            path=path,
+            seconds=CONF.glance.swift_temp_url_duration,
+            key=CONF.glance.swift_temp_url_key,
+            method='GET')
+        self.assertEqual((temp_url, exp_time),
+                         self.glance_service._cache[fake_image['id']])
+
+    @mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
+    def test_return_cached_tempurl(self, tempurl_mock):
+        fake_image = {
+            'id': uuidutils.generate_uuid()
+        }
+
+        exp_time = int(time.time()) + 1200
+        temp_url = CONF.glance.swift_endpoint_url + (
+            '/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
+            '/glance'
+            '/%(uuid)s'
+            '?temp_url_sig=hmacsig&temp_url_expires=%(exp_time)s' %
+            {'uuid': fake_image['id'], 'exp_time': exp_time}
+        )
+        self.glance_service._cache[fake_image['id']] = (
+            glance_v2.TempUrlCacheElement(url=temp_url,
+                                          url_expires_at=exp_time)
+        )
+
+        cleanup_mock = mock.Mock()
+        self.glance_service._remove_expired_items_from_cache = cleanup_mock
+        self.glance_service._validate_temp_url_config = mock.Mock()
+
+        self.assertEqual(
+            temp_url, self.glance_service.swift_temp_url(image_info=fake_image)
+        )
+
+        cleanup_mock.assert_called_once_with()
+        self.assertFalse(tempurl_mock.called)
+
+    @mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
+    def test_do_not_return_expired_tempurls(self, tempurl_mock):
+        fake_image = {
+            'id': uuidutils.generate_uuid()
+        }
+        old_exp_time = int(time.time()) + 99
+        path = (
+            '/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
+            '/glance'
+            '/%s' % fake_image['id']
+        )
+        query = '?temp_url_sig=hmacsig&temp_url_expires=%s'
+        self.glance_service._cache[fake_image['id']] = (
+            glance_v2.TempUrlCacheElement(
+                url=(CONF.glance.swift_endpoint_url + path +
+                     query % old_exp_time),
+                url_expires_at=old_exp_time)
+        )
+
+        new_exp_time = int(time.time()) + 1200
+        tempurl_mock.return_value = (
+            path + query % new_exp_time)
+
+        self.glance_service._validate_temp_url_config = mock.Mock()
+
+        fresh_temp_url = self.glance_service.swift_temp_url(
+            image_info=fake_image)
+
+        self.assertEqual(CONF.glance.swift_endpoint_url +
+                         tempurl_mock.return_value,
+                         fresh_temp_url)
+        tempurl_mock.assert_called_with(
+            path=path,
+            seconds=CONF.glance.swift_temp_url_duration,
+            key=CONF.glance.swift_temp_url_key,
+            method='GET')
+        self.assertEqual(
+            (fresh_temp_url, new_exp_time),
+            self.glance_service._cache[fake_image['id']])
+
+    def test_remove_expired_items_from_cache(self):
+        expired_items = {
+            uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement(
+                'fake-url-1',
+                int(time.time()) - 10
+            ),
+            uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement(
+                'fake-url-2',
+                int(time.time()) + 90  # Agent won't be able to start in time
+            )
+        }
+        valid_items = {
+            uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement(
+                'fake-url-3',
+                int(time.time()) + 1000
+            ),
+            uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement(
+                'fake-url-4',
+                int(time.time()) + 2000
+            )
+        }
+        self.glance_service._cache.update(expired_items)
+        self.glance_service._cache.update(valid_items)
+        self.glance_service._remove_expired_items_from_cache()
+        for uuid in valid_items:
+            self.assertEqual(valid_items[uuid],
+                             self.glance_service._cache[uuid])
+        for uuid in expired_items:
+            self.assertNotIn(uuid, self.glance_service._cache)
+
+    @mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
+    def _test__generate_temp_url(self, fake_image, tempurl_mock):
+        path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
+                '/glance'
+                '/%s' % fake_image['id'])
+        tempurl_mock.return_value = (
+            path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
+
+        self.glance_service._validate_temp_url_config = mock.Mock()
+
+        temp_url = self.glance_service._generate_temp_url(
+            path, seconds=CONF.glance.swift_temp_url_duration,
+            key=CONF.glance.swift_temp_url_key, method='GET',
+            endpoint=CONF.glance.swift_endpoint_url,
+            image_id=fake_image['id']
+        )
+
+        self.assertEqual(CONF.glance.swift_endpoint_url +
+                         tempurl_mock.return_value,
+                         temp_url)
+        tempurl_mock.assert_called_with(
+            path=path,
+            seconds=CONF.glance.swift_temp_url_duration,
+            key=CONF.glance.swift_temp_url_key,
+            method='GET')
+
+    def test_swift_temp_url_cache_enabled(self):
+        fake_image = {
+            'id': uuidutils.generate_uuid()
+        }
+        rm_expired = mock.Mock()
+        self.glance_service._remove_expired_items_from_cache = rm_expired
+        self._test__generate_temp_url(fake_image)
+        rm_expired.assert_called_once_with()
+        self.assertIn(fake_image['id'], self.glance_service._cache)
+
+    def test_swift_temp_url_cache_disabled(self):
+        self.config(swift_temp_url_cache_enabled=False,
+                    group='glance')
+        fake_image = {
+            'id': uuidutils.generate_uuid()
+        }
+        rm_expired = mock.Mock()
+        self.glance_service._remove_expired_items_from_cache = rm_expired
+        self._test__generate_temp_url(fake_image)
+        self.assertFalse(rm_expired.called)
+        self.assertNotIn(fake_image['id'], self.glance_service._cache)
+
+
 class TestGlanceUrl(base.TestCase):
 
     def test_generate_glance_http_url(self):