Merge "improve speed of metadata"

7 files changed, 171 insertions, 75 deletions

diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py
index 89ac27542..cfa7e91be 100644
--- a/nova/api/ec2/__init__.py
+++ b/nova/api/ec2/__init__.py
@@ -156,7 +156,7 @@ class Lockout(wsgi.Middleware):
         if FLAGS.memcached_servers:
             import memcache
         else:
-            from nova.testing.fake import memcache
+            from nova.common import memorycache as memcache
         self.mc = memcache.Client(FLAGS.memcached_servers,
                                   debug=0)
         super(Lockout, self).__init__(application)
diff --git a/nova/api/metadata/handler.py b/nova/api/metadata/handler.py
index 15bf9283e..acadab28f 100644
--- a/nova/api/metadata/handler.py
+++ b/nova/api/metadata/handler.py
@@ -19,6 +19,7 @@
 """Metadata request handler."""
 
 import base64
+import collections
 
 import webob.dec
 import webob.exc
@@ -32,6 +33,8 @@ from nova import exception
 from nova import flags
 from nova import log as logging
 from nova import network
+from nova.rpc import common as rpc_common
+from nova import utils
 from nova import volume
 from nova import wsgi
 
@@ -41,6 +44,11 @@ FLAGS = flags.FLAGS
 flags.DECLARE('use_forwarded_for', 'nova.api.auth')
 flags.DECLARE('dhcp_domain', 'nova.network.manager')
 
+if FLAGS.memcached_servers:
+    import memcache
+else:
+    from nova.common import memorycache as memcache
+
 _DEFAULT_MAPPINGS = {'ami': 'sda1',
                      'ephemeral0': 'sda2',
                      'root': block_device.DEFAULT_ROOT_DEV_NAME,
@@ -76,7 +84,35 @@ class MetadataRequestHandler(wsgi.Application):
                 network_api=self.network_api,
                 volume_api=volume.API())
 
-    def _format_instance_mapping(self, ctxt, instance_ref):
+        self.metadata_mapper = {
+            'user-data': self.user_data,
+            'meta-data': {
+                'instance-id': self.instance_id,
+                'instance-type': self.instance_type,
+                'ami-id': self.ami_id,
+                'kernel-id': self.kernel_id,
+                'ramdisk-id': self.ramdisk_id,
+                'block-device-mapping': self.block_device_mapping,
+                'hostname': self.hostname,
+                'public-hostname': self.hostname,
+                'local-hostname': self.hostname,
+                'local-ipv4': self.local_ipv4,
+                'public-ipv4': self.public_ipv4,
+                'security-groups': self.security_groups,
+                'public-keys': self.public_keys,
+                'ami-launch-index': self.ami_launch_index,
+                'reservation-id': self.reservation_id,
+                'placement': self.placement,
+                'instance-action': self.instance_action,
+                'ami-manifest-path': self.ami_manifest_path,
+            }
+        }
+
+        self._cache = memcache.Client(FLAGS.memcached_servers, debug=0)
+
+    def _format_instance_mapping(self, instance_ref):
+        ctxt = context.get_admin_context()
+
         root_device_name = instance_ref['root_device_name']
         if root_device_name is None:
             return _DEFAULT_MAPPINGS
@@ -122,72 +158,123 @@ class MetadataRequestHandler(wsgi.Application):
 
         return mappings
 
-    def get_metadata(self, address):
+    def get_instance(self, address):
+        """get instance_ref for a given fixed_ip, raising
+        exception.NotFound if unable to find instance
+
+        this will attempt to use memcache or fake memcache (an in-memory
+        cache) to remove the DB query + RPC query for batched calls (eg
+        cloud-init making dozens of queries on boot)
+        """
+
+        cache_key = 'metadata-%s' % address
+        instance_dict = self._cache.get(cache_key)
+        if instance_dict:
+            return instance_dict
+
         if not address:
             raise exception.FixedIpNotFoundForAddress(address=address)
 
         ctxt = context.get_admin_context()
+
         try:
             fixed_ip = self.network_api.get_fixed_ip_by_address(ctxt, address)
-            instance_ref = db.instance_get(ctxt, fixed_ip['instance_id'])
-        except exception.NotFound:
-            return None
+        except rpc_common.RemoteError:
+            raise exception.FixedIpNotFoundForAddress(address=address)
+        instance_ref = db.instance_get(ctxt, fixed_ip['instance_id'])
+        instance_dict = utils.to_primitive(instance_ref)
 
-        hostname = "%s.%s" % (instance_ref['hostname'], FLAGS.dhcp_domain)
-        host = instance_ref['host']
-        services = db.service_get_all_by_host(ctxt.elevated(), host)
-        availability_zone = ec2utils.get_availability_zone_by_host(services,
-                                                                   host)
+        self._cache.set(cache_key, instance_dict, 15)
+
+        return instance_dict
+
+    def user_data(self, address):
+        instance_ref = self.get_instance(address)
+        return base64.b64decode(instance_ref['user_data'])
+
+    def instance_id(self, address):
+        instance_ref = self.get_instance(address)
+        return ec2utils.id_to_ec2_id(instance_ref['id'])
+
+    def instance_type(self, address):
+        instance_ref = self.get_instance(address)
+        return instance_ref['instance_type']['name']
+
+    def ami_id(self, address):
+        instance_ref = self.get_instance(address)
+        return ec2utils.image_ec2_id(instance_ref['image_ref'])
+
+    def kernel_id(self, address):
+        instance_ref = self.get_instance(address)
+        kernel_id = instance_ref.get('kernel_id')
+        if kernel_id:
+            return ec2utils.image_ec2_id(kernel_id,
+                                         ec2utils.image_type('kernel'))
+
+    def ramdisk_id(self, address):
+        instance_ref = self.get_instance(address)
+        ramdisk_id = instance_ref.get('ramdisk_id')
+        if ramdisk_id:
+            return ec2utils.image_ec2_id(ramdisk_id,
+                                         ec2utils.image_type('ramdisk'))
+
+    def ami_launch_index(self, address):
+        instance_ref = self.get_instance(address)
+        return instance_ref['launch_index']
 
+    def block_device_mapping(self, address):
+        instance_ref = self.get_instance(address)
+        return self._format_instance_mapping(instance_ref)
+
+    def hostname(self, address):
+        instance_ref = self.get_instance(address)
+        return "%s.%s" % (instance_ref['hostname'], FLAGS.dhcp_domain)
+
+    def local_ipv4(self, address):
+        return address
+
+    def public_ipv4(self, address):
+        instance_ref = self.get_instance(address)
+        ctxt = context.get_admin_context()
         ip_info = ec2utils.get_ip_info_for_instance(ctxt, instance_ref)
         floating_ips = ip_info['floating_ips']
         floating_ip = floating_ips and floating_ips[0] or ''
+        return floating_ip
 
-        ec2_id = ec2utils.id_to_ec2_id(instance_ref['id'])
-        image_ec2_id = ec2utils.image_ec2_id(instance_ref['image_ref'])
-        security_groups = db.security_group_get_by_instance(ctxt,
-                                                            instance_ref['id'])
-        security_groups = [x['name'] for x in security_groups]
-        mappings = self._format_instance_mapping(ctxt, instance_ref)
-        data = {
-            'user-data': base64.b64decode(instance_ref['user_data']),
-            'meta-data': {
-                'ami-id': image_ec2_id,
-                'ami-launch-index': instance_ref['launch_index'],
-                'ami-manifest-path': 'FIXME',
-                'block-device-mapping': mappings,
-                'hostname': hostname,
-                'instance-action': 'none',
-                'instance-id': ec2_id,
-                'instance-type': instance_ref['instance_type']['name'],
-                'local-hostname': hostname,
-                'local-ipv4': address,
-                'placement': {'availability-zone': availability_zone},
-                'public-hostname': hostname,
-                'public-ipv4': floating_ip,
-                'reservation-id': instance_ref['reservation_id'],
-                'security-groups': security_groups}}
+    def reservation_id(self, address):
+        instance_ref = self.get_instance(address)
+        return instance_ref['reservation_id']
+
+    def placement(self, address):
+        instance_ref = self.get_instance(address)
+        host = instance_ref['host']
+        ctxt = context.get_admin_context()
+        # note(ja): original code had ctx.elevated?
+        services = db.service_get_all_by_host(ctxt, host)
+        zone = ec2utils.get_availability_zone_by_host(services, host)
+        return {'availability-zone': zone}
+
+    def security_groups(self, address):
+        instance_ref = self.get_instance(address)
+        ctxt = context.get_admin_context()
+        groups = db.security_group_get_by_instance(ctxt,
+                                                   instance_ref['id'])
+        return [g['name'] for g in groups]
 
+    def public_keys(self, address):
+        instance_ref = self.get_instance(address)
         # public-keys should be in meta-data only if user specified one
         if instance_ref['key_name']:
-            data['meta-data']['public-keys'] = {
-                '0': {'_name': instance_ref['key_name'],
-                      'openssh-key': instance_ref['key_data']}}
-
-        for image_type in ['kernel', 'ramdisk']:
-            if instance_ref.get('%s_id' % image_type):
-                ec2_id = ec2utils.image_ec2_id(
-                        instance_ref['%s_id' % image_type],
-                        ec2utils.image_type(image_type))
-                data['meta-data']['%s-id' % image_type] = ec2_id
-
-        if False:  # TODO(vish): store ancestor ids
-            data['ancestor-ami-ids'] = []
-        if False:  # TODO(vish): store product codes
-            data['product-codes'] = []
-        return data
+            return {'0': {'_name': instance_ref['key_name'],
+                          'openssh-key': instance_ref['key_data']}}
+
+    def ami_manifest_path(self, address):
+        return 'Not Implemented'
 
-    def print_data(self, data):
+    def instance_action(self, address):
+        return 'none'
+
+    def format_data(self, data):
         if isinstance(data, dict):
             output = ''
             for key in data:
@@ -207,15 +294,21 @@ class MetadataRequestHandler(wsgi.Application):
         else:
             return str(data)
 
-    def lookup(self, path, data):
+    def lookup(self, path, address):
         items = path.split('/')
+        data = self.metadata_mapper
         for item in items:
             if item:
                 if not isinstance(data, dict):
+                    # FIXME(ja): should we check that we are at the end
+                    # of the path as well before we just return?
                     return data
                 if not item in data:
                     return None
                 data = data[item]
+                if isinstance(data, collections.Callable):
+                    # lazy evaluation
+                    data = data(address)
         return data
 
     @webob.dec.wsgify(RequestClass=wsgi.Request)
@@ -223,19 +316,20 @@ class MetadataRequestHandler(wsgi.Application):
         remote_address = req.remote_addr
         if FLAGS.use_forwarded_for:
             remote_address = req.headers.get('X-Forwarded-For', remote_address)
+
         try:
-            meta_data = self.get_metadata(remote_address)
-        except Exception:
+            data = self.lookup(req.path_info,
+                               remote_address)
+        except (exception.NotFound, exception.FixedIpNotFoundForAddress):
+            LOG.error(_('Failed to get metadata for ip: %s'), remote_address)
+            return webob.exc.HTTPNotFound()
+        except:
             LOG.exception(_('Failed to get metadata for ip: %s'),
                           remote_address)
             msg = _('An unknown error has occurred. '
                     'Please try your request again.')
-            exc = webob.exc.HTTPInternalServerError(explanation=unicode(msg))
-            return exc
-        if meta_data is None:
-            LOG.error(_('Failed to get metadata for ip: %s'), remote_address)
-            raise webob.exc.HTTPNotFound()
-        data = self.lookup(req.path_info, meta_data)
+            return webob.exc.HTTPInternalServerError(explanation=unicode(msg))
+
         if data is None:
             raise webob.exc.HTTPNotFound()
-        return self.print_data(data)
+        return self.format_data(data)
diff --git a/nova/auth/ldapdriver.py b/nova/auth/ldapdriver.py
index 4c9431b8e..91135f1c7 100644
--- a/nova/auth/ldapdriver.py
+++ b/nova/auth/ldapdriver.py
@@ -96,7 +96,7 @@ LOG = logging.getLogger(__name__)
 if FLAGS.memcached_servers:
     import memcache
 else:
-    from nova.testing.fake import memcache
+    from nova.common import memorycache as memcache
 
 
 # TODO(vish): make an abstract base class with the same public methods
diff --git a/nova/auth/manager.py b/nova/auth/manager.py
index ca2a3add5..f03e453b8 100644
--- a/nova/auth/manager.py
+++ b/nova/auth/manager.py
@@ -97,7 +97,7 @@ LOG = logging.getLogger(__name__)
 if FLAGS.memcached_servers:
     import memcache
 else:
-    from nova.testing.fake import memcache
+    from nova.common import memorycache as memcache
 
 
 class AuthBase(object):
diff --git a/nova/testing/fake/memcache.py b/nova/common/memorycache.py
index e4f238aa9..f6a61a1c3 100644
--- a/nova/testing/fake/memcache.py
+++ b/nova/common/memorycache.py
@@ -29,11 +29,16 @@ class Client(object):
         self.cache = {}
 
     def get(self, key):
-        """Retrieves the value for a key or None."""
-        (timeout, value) = self.cache.get(key, (0, None))
-        if timeout == 0 or utils.utcnow_ts() < timeout:
-            return value
-        return None
+        """Retrieves the value for a key or None.
+
+        this expunges expired keys during each get"""
+
+        for k in self.cache.keys():
+            (timeout, _value) = self.cache[k]
+            if timeout and utils.utcnow_ts() >= timeout:
+                del self.cache[k]
+
+        return self.cache.get(key, (0, None))[1]
 
     def set(self, key, value, time=0, min_compress_len=0):
         """Sets the value for a key."""
diff --git a/nova/testing/fake/__init__.py b/nova/testing/fake/__init__.py
index d9eebbd23..5cdad4717 100644
--- a/nova/testing/fake/__init__.py
+++ b/nova/testing/fake/__init__.py
@@ -1,2 +1 @@
-import memcache
 import rabbit
diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py
index 7bd3ebcdd..a8fe448bb 100644
--- a/nova/tests/test_metadata.py
+++ b/nova/tests/test_metadata.py
@@ -119,7 +119,7 @@ class MetadataTestCase(test.TestCase):
         request = webob.Request.blank('/user-data')
         request.remote_addr = None
         response = request.get_response(self.app)
-        self.assertEqual(response.status_int, 500)
+        self.assertEqual(response.status_int, 404)
 
     def test_user_data_invalid_url(self):
         request = webob.Request.blank('/user-data-invalid')
@@ -177,9 +177,7 @@ class MetadataTestCase(test.TestCase):
                     'swap': '/dev/sdc',
                     'ebs0': '/dev/sdh'}
 
-        self.assertEqual(self.app._format_instance_mapping(ctxt,
-                                                           instance_ref0),
+        self.assertEqual(self.app._format_instance_mapping(instance_ref0),
                          handler._DEFAULT_MAPPINGS)
-        self.assertEqual(self.app._format_instance_mapping(ctxt,
-                                                           instance_ref1),
+        self.assertEqual(self.app._format_instance_mapping(instance_ref1),
                          expected)