Make metadata cache time configurable
The metadata cache expiration is now a config option.
New Option:
metadata_cache_expiration - Default 15
    Time in seconds to cache metadata objects.
This also adds basic unit tests for the MetadataRequestHandler, which
might be nice to have.
Co-Authored-By: Chet Burgess <cheburge@cisco.com>
Co-Authored-By: Nicolas Simonds <nisimond@cisco.com>
DocImpact
Closes-Bug: 1366139
Change-Id: I4e829ec52cf1445339f325b5895987c82dc7a8f4
			
			
This commit is contained in:
		@@ -35,8 +35,6 @@ from nova.openstack.common import memorycache
 | 
				
			|||||||
from nova import utils
 | 
					from nova import utils
 | 
				
			||||||
from nova import wsgi
 | 
					from nova import wsgi
 | 
				
			||||||
 | 
					
 | 
				
			||||||
CACHE_EXPIRATION = 15  # in seconds
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
CONF = cfg.CONF
 | 
					CONF = cfg.CONF
 | 
				
			||||||
CONF.import_opt('use_forwarded_for', 'nova.api.auth')
 | 
					CONF.import_opt('use_forwarded_for', 'nova.api.auth')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -52,7 +50,19 @@ metadata_proxy_opts = [
 | 
				
			|||||||
         help='Shared secret to validate proxies Neutron metadata requests'),
 | 
					         help='Shared secret to validate proxies Neutron metadata requests'),
 | 
				
			||||||
]
 | 
					]
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					metadata_opts = [
 | 
				
			||||||
 | 
					    cfg.IntOpt('metadata_cache_expiration',
 | 
				
			||||||
 | 
					               default=15,
 | 
				
			||||||
 | 
					               help='Time in seconds to cache metadata; 0 to disable '
 | 
				
			||||||
 | 
					                    'metadata caching entirely (not recommended). Increasing'
 | 
				
			||||||
 | 
					                    'this should improve response times of the metadata API '
 | 
				
			||||||
 | 
					                    'when under heavy load. Higher values may increase memory'
 | 
				
			||||||
 | 
					                    'usage and result in longer times for host metadata '
 | 
				
			||||||
 | 
					                    'changes to take effect.')
 | 
				
			||||||
 | 
					]
 | 
				
			||||||
 | 
					
 | 
				
			||||||
CONF.register_opts(metadata_proxy_opts, 'neutron')
 | 
					CONF.register_opts(metadata_proxy_opts, 'neutron')
 | 
				
			||||||
 | 
					CONF.register_opts(metadata_opts)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
LOG = logging.getLogger(__name__)
 | 
					LOG = logging.getLogger(__name__)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -71,6 +81,7 @@ class MetadataRequestHandler(wsgi.Application):
 | 
				
			|||||||
        cache_key = 'metadata-%s' % address
 | 
					        cache_key = 'metadata-%s' % address
 | 
				
			||||||
        data = self._cache.get(cache_key)
 | 
					        data = self._cache.get(cache_key)
 | 
				
			||||||
        if data:
 | 
					        if data:
 | 
				
			||||||
 | 
					            LOG.debug("Using cached metadata for %s", address)
 | 
				
			||||||
            return data
 | 
					            return data
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        try:
 | 
					        try:
 | 
				
			||||||
@@ -78,7 +89,8 @@ class MetadataRequestHandler(wsgi.Application):
 | 
				
			|||||||
        except exception.NotFound:
 | 
					        except exception.NotFound:
 | 
				
			||||||
            return None
 | 
					            return None
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        self._cache.set(cache_key, data, CACHE_EXPIRATION)
 | 
					        if CONF.metadata_cache_expiration > 0:
 | 
				
			||||||
 | 
					            self._cache.set(cache_key, data, CONF.metadata_cache_expiration)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        return data
 | 
					        return data
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -86,6 +98,7 @@ class MetadataRequestHandler(wsgi.Application):
 | 
				
			|||||||
        cache_key = 'metadata-%s' % instance_id
 | 
					        cache_key = 'metadata-%s' % instance_id
 | 
				
			||||||
        data = self._cache.get(cache_key)
 | 
					        data = self._cache.get(cache_key)
 | 
				
			||||||
        if data:
 | 
					        if data:
 | 
				
			||||||
 | 
					            LOG.debug("Using cached metadata for instance %s", instance_id)
 | 
				
			||||||
            return data
 | 
					            return data
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        try:
 | 
					        try:
 | 
				
			||||||
@@ -94,7 +107,8 @@ class MetadataRequestHandler(wsgi.Application):
 | 
				
			|||||||
        except exception.NotFound:
 | 
					        except exception.NotFound:
 | 
				
			||||||
            return None
 | 
					            return None
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        self._cache.set(cache_key, data, CACHE_EXPIRATION)
 | 
					        if CONF.metadata_cache_expiration > 0:
 | 
				
			||||||
 | 
					            self._cache.set(cache_key, data, CONF.metadata_cache_expiration)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        return data
 | 
					        return data
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -111,12 +111,13 @@ def fake_InstanceMetadata(stubs, inst_data, address=None,
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
def fake_request(stubs, mdinst, relpath, address="127.0.0.1",
 | 
					def fake_request(stubs, mdinst, relpath, address="127.0.0.1",
 | 
				
			||||||
                 fake_get_metadata=None, headers=None,
 | 
					                 fake_get_metadata=None, headers=None,
 | 
				
			||||||
                 fake_get_metadata_by_instance_id=None):
 | 
					                 fake_get_metadata_by_instance_id=None, app=None):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def get_metadata_by_remote_address(address):
 | 
					    def get_metadata_by_remote_address(address):
 | 
				
			||||||
        return mdinst
 | 
					        return mdinst
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    app = handler.MetadataRequestHandler()
 | 
					    if app is None:
 | 
				
			||||||
 | 
					        app = handler.MetadataRequestHandler()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if fake_get_metadata is None:
 | 
					    if fake_get_metadata is None:
 | 
				
			||||||
        fake_get_metadata = get_metadata_by_remote_address
 | 
					        fake_get_metadata = get_metadata_by_remote_address
 | 
				
			||||||
@@ -800,6 +801,81 @@ class MetadataHandlerTestCase(test.TestCase):
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
        _test_metadata_path('/2009-04-04/meta-data')
 | 
					        _test_metadata_path('/2009-04-04/meta-data')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def _metadata_handler_with_instance_id(self, hnd):
 | 
				
			||||||
 | 
					        expected_instance_id = 'a-b-c-d'
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        signed = hmac.new(
 | 
				
			||||||
 | 
					            CONF.neutron.metadata_proxy_shared_secret,
 | 
				
			||||||
 | 
					            expected_instance_id,
 | 
				
			||||||
 | 
					            hashlib.sha256).hexdigest()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        self.flags(service_metadata_proxy=True, group='neutron')
 | 
				
			||||||
 | 
					        response = fake_request(
 | 
				
			||||||
 | 
					            None, self.mdinst,
 | 
				
			||||||
 | 
					            relpath="/2009-04-04/user-data",
 | 
				
			||||||
 | 
					            address="192.192.192.2",
 | 
				
			||||||
 | 
					            fake_get_metadata=False,
 | 
				
			||||||
 | 
					            app=hnd,
 | 
				
			||||||
 | 
					            headers={'X-Forwarded-For': '192.192.192.2',
 | 
				
			||||||
 | 
					                     'X-Instance-ID': 'a-b-c-d',
 | 
				
			||||||
 | 
					                     'X-Tenant-ID': 'test',
 | 
				
			||||||
 | 
					                     'X-Instance-ID-Signature': signed})
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        self.assertEqual(200, response.status_int)
 | 
				
			||||||
 | 
					        self.assertEqual(base64.b64decode(self.instance['user_data']),
 | 
				
			||||||
 | 
					                         response.body)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @mock.patch.object(base, 'get_metadata_by_instance_id')
 | 
				
			||||||
 | 
					    def test_metadata_handler_with_instance_id(self, get_by_uuid):
 | 
				
			||||||
 | 
					        # test twice to ensure that the cache works
 | 
				
			||||||
 | 
					        get_by_uuid.return_value = self.mdinst
 | 
				
			||||||
 | 
					        self.flags(metadata_cache_expiration=15)
 | 
				
			||||||
 | 
					        hnd = handler.MetadataRequestHandler()
 | 
				
			||||||
 | 
					        self._metadata_handler_with_instance_id(hnd)
 | 
				
			||||||
 | 
					        self._metadata_handler_with_instance_id(hnd)
 | 
				
			||||||
 | 
					        self.assertEqual(1, get_by_uuid.call_count)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @mock.patch.object(base, 'get_metadata_by_instance_id')
 | 
				
			||||||
 | 
					    def test_metadata_handler_with_instance_id_no_cache(self, get_by_uuid):
 | 
				
			||||||
 | 
					        # test twice to ensure that disabling the cache works
 | 
				
			||||||
 | 
					        get_by_uuid.return_value = self.mdinst
 | 
				
			||||||
 | 
					        self.flags(metadata_cache_expiration=0)
 | 
				
			||||||
 | 
					        hnd = handler.MetadataRequestHandler()
 | 
				
			||||||
 | 
					        self._metadata_handler_with_instance_id(hnd)
 | 
				
			||||||
 | 
					        self._metadata_handler_with_instance_id(hnd)
 | 
				
			||||||
 | 
					        self.assertEqual(2, get_by_uuid.call_count)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def _metadata_handler_with_remote_address(self, hnd):
 | 
				
			||||||
 | 
					        response = fake_request(
 | 
				
			||||||
 | 
					            None, self.mdinst,
 | 
				
			||||||
 | 
					            fake_get_metadata=False,
 | 
				
			||||||
 | 
					            app=hnd,
 | 
				
			||||||
 | 
					            relpath="/2009-04-04/user-data",
 | 
				
			||||||
 | 
					            address="192.192.192.2")
 | 
				
			||||||
 | 
					        self.assertEqual(200, response.status_int)
 | 
				
			||||||
 | 
					        self.assertEqual(base64.b64decode(self.instance.user_data),
 | 
				
			||||||
 | 
					                         response.body)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @mock.patch.object(base, 'get_metadata_by_address')
 | 
				
			||||||
 | 
					    def test_metadata_handler_with_remote_address(self, get_by_uuid):
 | 
				
			||||||
 | 
					        # test twice to ensure that the cache works
 | 
				
			||||||
 | 
					        get_by_uuid.return_value = self.mdinst
 | 
				
			||||||
 | 
					        self.flags(metadata_cache_expiration=15)
 | 
				
			||||||
 | 
					        hnd = handler.MetadataRequestHandler()
 | 
				
			||||||
 | 
					        self._metadata_handler_with_remote_address(hnd)
 | 
				
			||||||
 | 
					        self._metadata_handler_with_remote_address(hnd)
 | 
				
			||||||
 | 
					        self.assertEqual(1, get_by_uuid.call_count)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @mock.patch.object(base, 'get_metadata_by_address')
 | 
				
			||||||
 | 
					    def test_metadata_handler_with_remote_address_no_cache(self, get_by_uuid):
 | 
				
			||||||
 | 
					        # test twice to ensure that disabling the cache works
 | 
				
			||||||
 | 
					        get_by_uuid.return_value = self.mdinst
 | 
				
			||||||
 | 
					        self.flags(metadata_cache_expiration=0)
 | 
				
			||||||
 | 
					        hnd = handler.MetadataRequestHandler()
 | 
				
			||||||
 | 
					        self._metadata_handler_with_remote_address(hnd)
 | 
				
			||||||
 | 
					        self._metadata_handler_with_remote_address(hnd)
 | 
				
			||||||
 | 
					        self.assertEqual(2, get_by_uuid.call_count)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class MetadataPasswordTestCase(test.TestCase):
 | 
					class MetadataPasswordTestCase(test.TestCase):
 | 
				
			||||||
    def setUp(self):
 | 
					    def setUp(self):
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user