Merge "Add ability to cache swift temporary URLs"

etc/ironic/ironic.conf.sample

@@ -1026,9 +1026,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']),
+            'container': self._get_swift_container(image_id),
-            'object_id': image_info['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')
+            method='GET',
-
+            endpoint=endpoint_url,
-        return '{endpoint_url}{url_path}'.format(
+            image_id=image_id
-            endpoint_url=endpoint_url, url_path=path)
+        )
 
     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):
+    def test__validate_temp_url_endpoint_less_than_download_delay(self):
-        self.config(swift_temp_url_duration=-1,
+        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):