From ce712b2ab56f2aeaf9fe01929c0034ef8f4a766a Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Thu, 30 Jan 2014 15:27:02 +0100 Subject: [PATCH] Metadata agent caches networks for routers During cloud-init there are several calls that asks neutron API for the same data which will not be most likely changed. Specifically router's networks are cached. Closes-bug: #1276440 Conflicts: neutron/tests/unit/test_metadata_agent.py Change-Id: Ic5eedb8057c7f4934eed08869ebf55c91e6edfc9 (cherry picked from commit 3faea81c6029033c85cefd6e98d7a3e719e858f5) --- etc/metadata_agent.ini | 8 + neutron/agent/metadata/agent.py | 63 ++++++-- neutron/tests/unit/test_metadata_agent.py | 186 ++++++++++++++++++---- 3 files changed, 218 insertions(+), 39 deletions(-) diff --git a/etc/metadata_agent.ini b/etc/metadata_agent.ini index c2f59cd283f..94d4b03ef06 100644 --- a/etc/metadata_agent.ini +++ b/etc/metadata_agent.ini @@ -36,3 +36,11 @@ 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. +# Example of URL using memory caching backend +# with ttl set to 5 seconds: cache_url = memory://?default_ttl=5 +# 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 = diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py index 23f4f279372..19352158b75 100644 --- a/neutron/agent/metadata/agent.py +++ b/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 @@ -85,6 +86,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( @@ -119,6 +124,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() @@ -126,18 +174,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: - 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'] + ports = self._get_ports(remote_address, network_id, router_id) self.auth_info = qclient.get_auth_info() if len(ports) == 1: @@ -324,6 +361,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='') agent_conf.register_agent_state_opts_helper(cfg.CONF) cfg.CONF(project='neutron') config.setup_logging(cfg.CONF) diff --git a/neutron/tests/unit/test_metadata_agent.py b/neutron/tests/unit/test_metadata_agent.py index 6f7f926acd0..377da92a85c 100644 --- a/neutron/tests/unit/test_metadata_agent.py +++ b/neutron/tests/unit/test_metadata_agent.py @@ -16,6 +16,7 @@ # # @author: Mark McClain, DreamHost +import contextlib import socket import mock @@ -41,11 +42,18 @@ class FakeConf(object): nova_metadata_ip = '9.9.9.9' nova_metadata_port = 8775 metadata_proxy_shared_secret = 'secret' + 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.addCleanup(self.qclient_p.stop) @@ -54,7 +62,7 @@ class TestMetadataProxyHandler(base.BaseTestCase): self.log = self.log_p.start() self.addCleanup(self.log_p.stop) - self.handler = agent.MetadataProxyHandler(FakeConf) + self.handler = agent.MetadataProxyHandler(self.fake_conf) def test_call(self): req = mock.Mock() @@ -84,9 +92,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): @@ -94,34 +211,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 = [ - mock.call( - username=FakeConf.admin_user, - tenant_name=FakeConf.admin_tenant_name, - region_name=FakeConf.auth_region, - auth_url=FakeConf.auth_url, - password=FakeConf.admin_password, - auth_strategy=FakeConf.auth_strategy, - token=None, - insecure=FakeConf.auth_insecure, - ca_cert=FakeConf.auth_ca_cert, - endpoint_url=None, - endpoint_type=FakeConf.endpoint_type) - ] + new_qclient_call = mock.call( + username=FakeConf.admin_user, + tenant_name=FakeConf.admin_tenant_name, + region_name=FakeConf.auth_region, + auth_url=FakeConf.auth_url, + password=FakeConf.admin_password, + auth_strategy=FakeConf.auth_strategy, + token=None, + insecure=FakeConf.auth_insecure, + ca_cert=FakeConf.auth_ca_cert, + endpoint_url=None, + 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) @@ -133,7 +251,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'}] @@ -152,7 +270,7 @@ class TestMetadataProxyHandler(base.BaseTestCase): 'X-Neutron-Router-ID': router_id } - networks = ['net1', 'net2'] + networks = ('net1', 'net2') ports = [ [{'network_id': 'net1'}, {'network_id': 'net2'}], [] @@ -177,7 +295,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') ) @@ -191,7 +309,7 @@ class TestMetadataProxyHandler(base.BaseTestCase): self.assertEqual( self._get_instance_and_tenant_id_helper(headers, ports, - networks=['the_id']), + networks=('the_id',)), (None, None) ) @@ -264,6 +382,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())