Merge "Metadata agent caches networks for routers"

etc/metadata_agent.ini

@@ -50,3 +50,9 @@ admin_password = %SERVICE_PASSWORD%
 
 # Number of backlog requests to configure the metadata server socket with
 # metadata_backlog = 128
+
+# URL to connect to the cache backend.
+# default_ttl=0 parameter will cause cache entries to never expire.
+# Otherwise default_ttl specifies time in seconds a cache entry is valid for.
+# No cache is used in case no value is passed.
+# cache_url = memory://?default_ttl=5

neutron/agent/metadata/agent.py

@@ -35,6 +35,7 @@ from neutron.common import constants as n_const
 from neutron.common import topics
 from neutron.common import utils
 from neutron import context
+from neutron.openstack.common.cache import cache
 from neutron.openstack.common import excutils
 from neutron.openstack.common import log as logging
 from neutron.openstack.common import loopingcall
@@ -98,6 +99,10 @@ class MetadataProxyHandler(object):
     def __init__(self, conf):
         self.conf = conf
         self.auth_info = {}
+        if self.conf.cache_url:
+            self._cache = cache.get_cache(self.conf.cache_url)
+        else:
+            self._cache = False
 
     def _get_neutron_client(self):
         qclient = client.Client(
@@ -132,6 +137,49 @@ class MetadataProxyHandler(object):
                     'Please try your request again.')
             return webob.exc.HTTPInternalServerError(explanation=unicode(msg))
 
+    @utils.cache_method_results
+    def _get_router_networks(self, router_id):
+        """Find all networks connected to given router."""
+        qclient = self._get_neutron_client()
+
+        internal_ports = qclient.list_ports(
+            device_id=router_id,
+            device_owner=n_const.DEVICE_OWNER_ROUTER_INTF)['ports']
+        return tuple(p['network_id'] for p in internal_ports)
+
+    @utils.cache_method_results
+    def _get_ports_for_remote_address(self, remote_address, networks):
+        """Get list of ports that has given ip address and are part of
+        given networks.
+
+        :param networks: list of networks in which the ip address will be
+                         searched for
+
+        """
+        qclient = self._get_neutron_client()
+
+        return qclient.list_ports(
+            network_id=networks,
+            fixed_ips=['ip_address=%s' % remote_address])['ports']
+
+    def _get_ports(self, remote_address, network_id=None, router_id=None):
+        """Search for all ports that contain passed ip address and belongs to
+        given network.
+
+        If no network is passed ports are searched on all networks connected to
+        given router. Either one of network_id or router_id must be passed.
+
+        """
+        if network_id:
+            networks = (network_id,)
+        elif router_id:
+            networks = self._get_router_networks(router_id)
+        else:
+            raise TypeError(_("Either one of parameter network_id or router_id"
+                              " must be passed to _get_ports method."))
+
+        return self._get_ports_for_remote_address(remote_address, networks)
+
     def _get_instance_and_tenant_id(self, req):
         qclient = self._get_neutron_client()
 
@@ -139,18 +187,7 @@ class MetadataProxyHandler(object):
         network_id = req.headers.get('X-Neutron-Network-ID')
         router_id = req.headers.get('X-Neutron-Router-ID')
 
-        if network_id:
+        ports = self._get_ports(remote_address, network_id, router_id)
-            networks = [network_id]
-        else:
-            internal_ports = qclient.list_ports(
-                device_id=router_id,
-                device_owner=n_const.DEVICE_OWNER_ROUTER_INTF)['ports']
-
-            networks = [p['network_id'] for p in internal_ports]
-
-        ports = qclient.list_ports(
-            network_id=networks,
-            fixed_ips=['ip_address=%s' % remote_address])['ports']
 
         self.auth_info = qclient.get_auth_info()
         if len(ports) == 1:
@@ -344,6 +381,8 @@ def main():
     eventlet.monkey_patch()
     cfg.CONF.register_opts(UnixDomainMetadataProxy.OPTS)
     cfg.CONF.register_opts(MetadataProxyHandler.OPTS)
+    cache.register_oslo_configs(cfg.CONF)
+    cfg.CONF.set_default(name='cache_url', default='memory://?default_ttl=5')
     agent_conf.register_agent_state_opts_helper(cfg.CONF)
     cfg.CONF(project='neutron')
     config.setup_logging(cfg.CONF)

neutron/tests/unit/test_metadata_agent.py

@@ -16,6 +16,7 @@
 #
 # @author: Mark McClain, DreamHost
 
+import contextlib
 import socket
 
 import mock
@@ -45,18 +46,25 @@ class FakeConf(object):
     nova_metadata_insecure = True
     nova_client_cert = 'nova_cert'
     nova_client_priv_key = 'nova_priv_key'
+    cache_url = ''
 
 
-class TestMetadataProxyHandler(base.BaseTestCase):
+class FakeConfCache(FakeConf):
+    cache_url = 'memory://?default_ttl=5'
+
+
+class TestMetadataProxyHandlerCache(base.BaseTestCase):
+    fake_conf = FakeConfCache
+
     def setUp(self):
-        super(TestMetadataProxyHandler, self).setUp()
+        super(TestMetadataProxyHandlerCache, self).setUp()
         self.qclient_p = mock.patch('neutronclient.v2_0.client.Client')
         self.qclient = self.qclient_p.start()
 
         self.log_p = mock.patch.object(agent, 'LOG')
         self.log = self.log_p.start()
 
-        self.handler = agent.MetadataProxyHandler(FakeConf)
+        self.handler = agent.MetadataProxyHandler(self.fake_conf)
 
     def test_call(self):
         req = mock.Mock()
@@ -86,9 +94,118 @@ class TestMetadataProxyHandler(base.BaseTestCase):
             self.assertIsInstance(retval, webob.exc.HTTPInternalServerError)
             self.assertEqual(len(self.log.mock_calls), 2)
 
+    def test_get_router_networks(self):
+        router_id = 'router-id'
+        expected = ('network_id1', 'network_id2')
+        ports = {'ports': [{'network_id': 'network_id1', 'something': 42},
+                           {'network_id': 'network_id2',
+                            'something_else': 32}],
+                 'not_used': [1, 2, 3]}
+        mock_list_ports = self.qclient.return_value.list_ports
+        mock_list_ports.return_value = ports
+        networks = self.handler._get_router_networks(router_id)
+        mock_list_ports.assert_called_once_with(
+            device_id=router_id,
+            device_owner=constants.DEVICE_OWNER_ROUTER_INTF)
+        self.assertEqual(expected, networks)
+
+    def _test_get_router_networks_twice_helper(self):
+        router_id = 'router-id'
+        ports = {'ports': [{'network_id': 'network_id1', 'something': 42}],
+                 'not_used': [1, 2, 3]}
+        expected_networks = ('network_id1',)
+        with mock.patch(
+            'neutron.openstack.common.timeutils.utcnow_ts', return_value=0):
+            mock_list_ports = self.qclient.return_value.list_ports
+            mock_list_ports.return_value = ports
+            networks = self.handler._get_router_networks(router_id)
+            mock_list_ports.assert_called_once_with(
+                device_id=router_id,
+                device_owner=constants.DEVICE_OWNER_ROUTER_INTF)
+            self.assertEqual(expected_networks, networks)
+            networks = self.handler._get_router_networks(router_id)
+
+    def test_get_router_networks_twice(self):
+        self._test_get_router_networks_twice_helper()
+        self.assertEqual(
+            1, self.qclient.return_value.list_ports.call_count)
+
+    def test_get_ports_for_remote_address(self):
+        remote_address = 'remote_address'
+        networks = 'networks'
+        fixed_ips = ["ip_address=%s" % remote_address]
+        ports = self.handler._get_ports_for_remote_address(remote_address,
+                                                           networks)
+        mock_list_ports = self.qclient.return_value.list_ports
+        mock_list_ports.assert_called_once_with(
+            network_id=networks, fixed_ips=fixed_ips)
+        self.assertEqual(mock_list_ports.return_value.__getitem__('ports'),
+                         ports)
+
+    def _get_ports_for_remote_address_cache_hit_helper(self):
+        remote_address = 'remote_address'
+        networks = ('net1', 'net2')
+        fixed_ips = ["ip_address=%s" % remote_address]
+        ports = self.handler._get_ports_for_remote_address(remote_address,
+                                                           networks)
+        mock_list_ports = self.qclient.return_value.list_ports
+        mock_list_ports.assert_called_once_with(
+            network_id=networks, fixed_ips=fixed_ips)
+        self.assertEqual(
+            mock_list_ports.return_value.__getitem__('ports'), ports)
+        self.assertEqual(1, mock_list_ports.call_count)
+        self.handler._get_ports_for_remote_address(remote_address,
+                                                   networks)
+
+    def test_get_ports_for_remote_address_cache_hit(self):
+        self._get_ports_for_remote_address_cache_hit_helper()
+        self.assertEqual(
+            1, self.qclient.return_value.list_ports.call_count)
+
+    def test_get_ports_network_id(self):
+        network_id = 'network-id'
+        router_id = 'router-id'
+        remote_address = 'remote-address'
+        expected = ['port1']
+        networks = (network_id,)
+        with contextlib.nested(
+            mock.patch.object(self.handler, '_get_ports_for_remote_address'),
+            mock.patch.object(self.handler, '_get_router_networks')
+        ) as (mock_get_ip_addr, mock_get_router_networks):
+            mock_get_ip_addr.return_value = expected
+            ports = self.handler._get_ports(remote_address, network_id,
+                                            router_id)
+            mock_get_ip_addr.assert_called_once_with(remote_address,
+                                                     networks)
+            self.assertFalse(mock_get_router_networks.called)
+        self.assertEqual(expected, ports)
+
+    def test_get_ports_router_id(self):
+        router_id = 'router-id'
+        remote_address = 'remote-address'
+        expected = ['port1']
+        networks = ('network1', 'network2')
+        with contextlib.nested(
+            mock.patch.object(self.handler,
+                              '_get_ports_for_remote_address',
+                              return_value=expected),
+            mock.patch.object(self.handler,
+                              '_get_router_networks',
+                              return_value=networks)
+        ) as (mock_get_ip_addr, mock_get_router_networks):
+            ports = self.handler._get_ports(remote_address,
+                                            router_id=router_id)
+            mock_get_router_networks.called_once_with(router_id)
+        mock_get_ip_addr.assert_called_once_with(remote_address, networks)
+        self.assertEqual(expected, ports)
+
+    def test_get_ports_no_id(self):
+        self.assertRaises(TypeError, self.handler._get_ports, 'remote_address')
+
     def _get_instance_and_tenant_id_helper(self, headers, list_ports_retval,
                                            networks=None, router_id=None):
-        headers['X-Forwarded-For'] = '192.168.1.1'
+        remote_address = '192.168.1.1'
+        headers['X-Forwarded-For'] = remote_address
         req = mock.Mock(headers=headers)
 
         def mock_list_ports(*args, **kwargs):
@@ -96,34 +213,35 @@ class TestMetadataProxyHandler(base.BaseTestCase):
 
         self.qclient.return_value.list_ports.side_effect = mock_list_ports
         instance_id, tenant_id = self.handler._get_instance_and_tenant_id(req)
-        expected = [
+        new_qclient_call = mock.call(
-            mock.call(
+            username=FakeConf.admin_user,
-                username=FakeConf.admin_user,
+            tenant_name=FakeConf.admin_tenant_name,
-                tenant_name=FakeConf.admin_tenant_name,
+            region_name=FakeConf.auth_region,
-                region_name=FakeConf.auth_region,
+            auth_url=FakeConf.auth_url,
-                auth_url=FakeConf.auth_url,
+            password=FakeConf.admin_password,
-                password=FakeConf.admin_password,
+            auth_strategy=FakeConf.auth_strategy,
-                auth_strategy=FakeConf.auth_strategy,
+            token=None,
-                token=None,
+            insecure=FakeConf.auth_insecure,
-                insecure=FakeConf.auth_insecure,
+            ca_cert=FakeConf.auth_ca_cert,
-                ca_cert=FakeConf.auth_ca_cert,
+            endpoint_url=None,
-                endpoint_url=None,
+            endpoint_type=FakeConf.endpoint_type)
-                endpoint_type=FakeConf.endpoint_type)
+        expected = [new_qclient_call]
-        ]
 
         if router_id:
-            expected.append(
+            expected.extend([
+                new_qclient_call,
                 mock.call().list_ports(
                     device_id=router_id,
                     device_owner=constants.DEVICE_OWNER_ROUTER_INTF
                 )
-            )
+            ])
 
-        expected.append(
+        expected.extend([
+            new_qclient_call,
             mock.call().list_ports(
-                network_id=networks or [],
+                network_id=networks or tuple(),
                 fixed_ips=['ip_address=192.168.1.1'])
-        )
+        ])
 
         self.qclient.assert_has_calls(expected)
 
@@ -135,7 +253,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
             'X-Neutron-Router-ID': router_id
         }
 
-        networks = ['net1', 'net2']
+        networks = ('net1', 'net2')
         ports = [
             [{'network_id': 'net1'}, {'network_id': 'net2'}],
             [{'device_id': 'device_id', 'tenant_id': 'tenant_id'}]
@@ -154,7 +272,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
             'X-Neutron-Router-ID': router_id
         }
 
-        networks = ['net1', 'net2']
+        networks = ('net1', 'net2')
         ports = [
             [{'network_id': 'net1'}, {'network_id': 'net2'}],
             []
@@ -179,7 +297,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
 
         self.assertEqual(
             self._get_instance_and_tenant_id_helper(headers, ports,
-                                                    networks=['the_id']),
+                                                    networks=('the_id',)),
             ('device_id', 'tenant_id')
         )
 
@@ -193,7 +311,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
 
         self.assertEqual(
             self._get_instance_and_tenant_id_helper(headers, ports,
-                                                    networks=['the_id']),
+                                                    networks=('the_id',)),
             (None, None)
         )
 
@@ -274,6 +392,20 @@ class TestMetadataProxyHandler(base.BaseTestCase):
         )
 
 
+class TestMetadataProxyHandlerNoCache(TestMetadataProxyHandlerCache):
+    fake_conf = FakeConf
+
+    def test_get_router_networks_twice(self):
+        self._test_get_router_networks_twice_helper()
+        self.assertEqual(
+            2, self.qclient.return_value.list_ports.call_count)
+
+    def test_get_ports_for_remote_address_cache_hit(self):
+        self._get_ports_for_remote_address_cache_hit_helper()
+        self.assertEqual(
+            2, self.qclient.return_value.list_ports.call_count)
+
+
 class TestUnixDomainHttpProtocol(base.BaseTestCase):
     def test_init_empty_client(self):
         u = agent.UnixDomainHttpProtocol(mock.Mock(), '', mock.Mock())