diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index e37a1fbc98..829f7dd532 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -35,6 +35,7 @@ from neutron.agent.linux import ra from neutron.agent import rpc as agent_rpc from neutron.common import config as common_config from neutron.common import constants as l3_constants +from neutron.common import exceptions as n_exc from neutron.common import ipv6_utils from neutron.common import rpc as n_rpc from neutron.common import topics @@ -42,7 +43,7 @@ from neutron.common import utils as common_utils from neutron import context from neutron import manager from neutron.openstack.common import excutils -from neutron.openstack.common.gettextutils import _LW +from neutron.openstack.common.gettextutils import _LE, _LW from neutron.openstack.common import importutils from neutron.openstack.common import log as logging from neutron.openstack.common import loopingcall @@ -1779,48 +1780,38 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, LOG.debug(_('Got router added to agent :%r'), payload) self.routers_updated(context, payload) - def _process_routers(self, routers): - pool = eventlet.GreenPool() + def _process_router_if_compatible(self, router): if (self.conf.external_network_bridge and not ip_lib.device_exists(self.conf.external_network_bridge)): LOG.error(_("The external network bridge '%s' does not exist"), self.conf.external_network_bridge) return + # If namespaces are disabled, only process the router associated + # with the configured agent id. + if (not self.conf.use_namespaces and + router['id'] != self.conf.router_id): + raise n_exc.RouterNotCompatibleWithAgent(router_id=router['id']) + + # Either ex_net_id or handle_internal_only_routers must be set + ex_net_id = (router['external_gateway_info'] or {}).get('network_id') + if not ex_net_id and not self.conf.handle_internal_only_routers: + raise n_exc.RouterNotCompatibleWithAgent(router_id=router['id']) + + # If target_ex_net_id and ex_net_id are set they must be equal target_ex_net_id = self._fetch_external_net_id() - # if routers are all the routers we have (They are from router sync on - # starting or when error occurs during running), we seek the - # routers which should be removed. - # If routers are from server side notification, we seek them - # from subset of incoming routers and ones we have now. - prev_router_ids = set(self.router_info) & set( - [router['id'] for router in routers]) - cur_router_ids = set() - for r in routers: - # If namespaces are disabled, only process the router associated - # with the configured agent id. - if (not self.conf.use_namespaces and - r['id'] != self.conf.router_id): - continue - ex_net_id = (r['external_gateway_info'] or {}).get('network_id') - if not ex_net_id and not self.conf.handle_internal_only_routers: - continue - if (target_ex_net_id and ex_net_id and - ex_net_id != target_ex_net_id): - # Double check that our single external_net_id has not changed - # by forcing a check by RPC. - if (ex_net_id != self._fetch_external_net_id(force=True)): - continue - cur_router_ids.add(r['id']) - if r['id'] not in self.router_info: - self._router_added(r['id'], r) - ri = self.router_info[r['id']] - ri.router = r - pool.spawn_n(self.process_router, ri) - # identify and remove routers that no longer exist - for router_id in prev_router_ids - cur_router_ids: - pool.spawn_n(self._router_removed, router_id) - pool.waitall() + if (target_ex_net_id and ex_net_id and ex_net_id != target_ex_net_id): + # Double check that our single external_net_id has not changed + # by forcing a check by RPC. + if ex_net_id != self._fetch_external_net_id(force=True): + raise n_exc.RouterNotCompatibleWithAgent( + router_id=router['id']) + + if router['id'] not in self.router_info: + self._router_added(router['id'], router) + ri = self.router_info[router['id']] + ri.router = router + self.process_router(ri) def _process_router_update(self): for rp, update in self._queue.each_update_to_next_router(): @@ -1844,7 +1835,15 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self._router_removed(update.id) continue - self._process_routers([router]) + try: + self._process_router_if_compatible(router) + except n_exc.RouterNotCompatibleWithAgent as e: + LOG.exception(e.msg) + # Was the router previously handled by this agent? + if router['id'] in self.router_info: + LOG.error(_LE("Removing incompatible router '%s'"), + router['id']) + self._router_removed(router['id']) LOG.debug("Finished a router update for %s", update.id) rp.fetched_and_processed(update.timestamp) diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index be62388aa6..1fd83e46ee 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -335,3 +335,7 @@ class DeviceIDNotOwnedByTenant(Conflict): class InvalidCIDR(BadRequest): message = _("Invalid CIDR %(input)s given as IP prefix") + + +class RouterNotCompatibleWithAgent(NeutronException): + message = _("Router '%(router_id)s' is not compatible with this agent") diff --git a/neutron/services/vpn/agent.py b/neutron/services/vpn/agent.py index 12e969df9b..6f67ccf1f0 100644 --- a/neutron/services/vpn/agent.py +++ b/neutron/services/vpn/agent.py @@ -130,15 +130,15 @@ class VPNAgent(l3_agent.L3NATAgentWithStateReport): for device in self.devices: device.destroy_router(router_id) - def _process_routers(self, routers): + def _process_router_if_compatible(self, router): """Router sync event. This method overwrites parent class method. - :param routers: list of routers + :param router: a router """ - super(VPNAgent, self)._process_routers(routers) + super(VPNAgent, self)._process_router_if_compatible(router) for device in self.devices: - device.sync(self.context, routers) + device.sync(self.context, [router]) def main(): diff --git a/neutron/tests/unit/services/vpn/test_vpn_agent.py b/neutron/tests/unit/services/vpn/test_vpn_agent.py index 4cde577d23..2a2a9a14f8 100644 --- a/neutron/tests/unit/services/vpn/test_vpn_agent.py +++ b/neutron/tests/unit/services/vpn/test_vpn_agent.py @@ -183,15 +183,14 @@ class TestVPNAgent(base.BaseTestCase): self.agent._router_removed(router_id) device.destroy_router.assert_called_once_with(router_id) - def test_process_routers(self): + def test_process_router_if_compatible(self): self.plugin_api.get_external_network_id.return_value = None - routers = [ - {'id': _uuid(), - 'admin_state_up': True, - 'routes': [], - 'external_gateway_info': {}}] + router = {'id': _uuid(), + 'admin_state_up': True, + 'routes': [], + 'external_gateway_info': {}} device = mock.Mock() self.agent.devices = [device] - self.agent._process_routers(routers) - device.sync.assert_called_once_with(mock.ANY, routers) + self.agent._process_router_if_compatible(router) + device.sync.assert_called_once_with(mock.ANY, [router]) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 8de7922d21..264804a596 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -1870,108 +1870,94 @@ vrrp_instance VR_1 { self.assertEqual(['1234'], agent._router_ids()) self.assertFalse(agent._clean_stale_namespaces) - def test_process_routers_with_no_ext_net_in_conf(self): + def test_process_router_if_compatible_with_no_ext_net_in_conf(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_external_network_id.return_value = 'aaa' - routers = [ - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'aaa'}}] + router = {'id': _uuid(), + 'routes': [], + 'admin_state_up': True, + 'external_gateway_info': {'network_id': 'aaa'}} - agent._process_routers(routers) - self.assertIn(routers[0]['id'], agent.router_info) + agent._process_router_if_compatible(router) + self.assertIn(router['id'], agent.router_info) self.plugin_api.get_external_network_id.assert_called_with( agent.context) - def test_process_routers_with_cached_ext_net(self): + def test_process_router_if_compatible_with_cached_ext_net(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_external_network_id.return_value = 'aaa' agent.target_ex_net_id = 'aaa' - routers = [ - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'aaa'}}] + router = {'id': _uuid(), + 'routes': [], + 'admin_state_up': True, + 'external_gateway_info': {'network_id': 'aaa'}} - agent._process_routers(routers) - self.assertIn(routers[0]['id'], agent.router_info) + agent._process_router_if_compatible(router) + self.assertIn(router['id'], agent.router_info) self.assertFalse(self.plugin_api.get_external_network_id.called) - def test_process_routers_with_stale_cached_ext_net(self): + def test_process_router_if_compatible_with_stale_cached_ext_net(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_external_network_id.return_value = 'aaa' agent.target_ex_net_id = 'bbb' - routers = [ - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'aaa'}}] + router = {'id': _uuid(), + 'routes': [], + 'admin_state_up': True, + 'external_gateway_info': {'network_id': 'aaa'}} - agent._process_routers(routers) - self.assertIn(routers[0]['id'], agent.router_info) + agent._process_router_if_compatible(router) + self.assertIn(router['id'], agent.router_info) self.plugin_api.get_external_network_id.assert_called_with( agent.context) - def test_process_routers_with_no_ext_net_in_conf_and_two_net_plugin(self): + def test_process_router_if_compatible_w_no_ext_net_and_2_net_plugin(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - routers = [ - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'aaa'}}] + router = {'id': _uuid(), + 'routes': [], + 'admin_state_up': True, + 'external_gateway_info': {'network_id': 'aaa'}} agent.router_info = {} self.plugin_api.get_external_network_id.side_effect = ( n_exc.TooManyExternalNetworks()) self.assertRaises(n_exc.TooManyExternalNetworks, - agent._process_routers, - routers) - self.assertNotIn(routers[0]['id'], agent.router_info) + agent._process_router_if_compatible, + router) + self.assertNotIn(router['id'], agent.router_info) - def test_process_routers_with_ext_net_in_conf(self): + def test_process_router_if_compatible_with_ext_net_in_conf(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_external_network_id.return_value = 'aaa' - routers = [ - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'aaa'}}, - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'bbb'}}] + router = {'id': _uuid(), + 'routes': [], + 'admin_state_up': True, + 'external_gateway_info': {'network_id': 'bbb'}} agent.router_info = {} self.conf.set_override('gateway_external_network_id', 'aaa') - agent._process_routers(routers) - self.assertIn(routers[0]['id'], agent.router_info) - self.assertNotIn(routers[1]['id'], agent.router_info) + self.assertRaises(n_exc.RouterNotCompatibleWithAgent, + agent._process_router_if_compatible, + router) + self.assertNotIn(router['id'], agent.router_info) - def test_process_routers_with_no_bridge_no_ext_net_in_conf(self): + def test_process_router_if_compatible_with_no_bridge_no_ext_net(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_external_network_id.return_value = 'aaa' - routers = [ - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'aaa'}}, - {'id': _uuid(), - 'routes': [], - 'admin_state_up': True, - 'external_gateway_info': {'network_id': 'bbb'}}] + router = {'id': _uuid(), + 'routes': [], + 'admin_state_up': True, + 'external_gateway_info': {'network_id': 'aaa'}} agent.router_info = {} self.conf.set_override('external_network_bridge', '') - agent._process_routers(routers) - self.assertIn(routers[0]['id'], agent.router_info) - self.assertIn(routers[1]['id'], agent.router_info) + agent._process_router_if_compatible(router) + self.assertIn(router['id'], agent.router_info) def test_nonexistent_interface_driver(self): self.conf.set_override('interface_driver', None)