From 5f749768676e6739db1e01a03ddb7f3cb43d48f8 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Wed, 16 Oct 2013 17:51:04 +0400 Subject: [PATCH] Reschedule router if new external gateway is on other network An L3 agent may be associated with just one external network. If router's new external gateway is on other network then the router needs to be rescheduled to the proper l3 agent Change-Id: Ia0ed924403137ac4578ca562b57988292c41c1fe Closes-Bug: #1234750 --- neutron/agent/l3_agent.py | 1 + neutron/db/l3_agentschedulers_db.py | 37 +++++- neutron/db/l3_db.py | 80 +++++++++++++ neutron/extensions/l3agentscheduler.py | 5 + neutron/scheduler/l3_agent_scheduler.py | 12 +- neutron/tests/unit/test_agent_ext_plugin.py | 19 +++ neutron/tests/unit/test_l3_plugin.py | 121 ++++++++++++++++++++ 7 files changed, 266 insertions(+), 9 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 34f6fbabd5d..8ddbd757569 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -905,6 +905,7 @@ class L3NATAgentWithStateReport(L3NATAgent): 'router_id': self.conf.router_id, 'handle_internal_only_routers': self.conf.handle_internal_only_routers, + 'external_network_bridge': self.conf.external_network_bridge, 'gateway_external_network_id': self.conf.gateway_external_network_id, 'interface_driver': self.conf.interface_driver}, diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index fa9d03b098c..698a786fa6f 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -97,6 +97,13 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, which leads to re-schedule or be added to another agent manually. """ agent = self._get_agent(context, agent_id) + self._unbind_router(context, router_id, agent_id) + l3_notifier = self.agent_notifiers.get(constants.AGENT_TYPE_L3) + if l3_notifier: + l3_notifier.router_removed_from_agent( + context, router_id, agent.host) + + def _unbind_router(self, context, router_id, agent_id): with context.session.begin(subtransactions=True): query = context.session.query(RouterL3AgentBinding) query = query.filter( @@ -108,10 +115,32 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, raise l3agentscheduler.RouterNotHostedByL3Agent( router_id=router_id, agent_id=agent_id) context.session.delete(binding) + + def reschedule_router(self, context, router_id, candidates=None): + """Reschedule router to a new l3 agent + + Remove the router from the agent(s) currently hosting it and + schedule it again + """ + cur_agents = self.list_l3_agents_hosting_router( + context, router_id)['agents'] + with context.session.begin(subtransactions=True): + for agent in cur_agents: + self._unbind_router(context, router_id, agent['id']) + + new_agent = self.schedule_router(context, router_id, + candidates=candidates) + if not new_agent: + raise l3agentscheduler.RouterReschedulingFailed( + router_id=router_id) + l3_notifier = self.agent_notifiers.get(constants.AGENT_TYPE_L3) if l3_notifier: - l3_notifier.router_removed_from_agent( - context, router_id, agent.host) + for agent in cur_agents: + l3_notifier.router_removed_from_agent( + context, router_id, agent['host']) + l3_notifier.router_added_to_agent( + context, [router_id], new_agent.host) def list_routers_on_l3_agent(self, context, agent_id): query = context.session.query(RouterL3AgentBinding.router_id) @@ -239,10 +268,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, return self.router_scheduler.auto_schedule_routers( self, context, host, router_ids) - def schedule_router(self, context, router): + def schedule_router(self, context, router, candidates=None): if self.router_scheduler: return self.router_scheduler.schedule( - self, context, router) + self, context, router, candidates) def schedule_routers(self, context, routers): """Schedule the routers to l3 agents.""" diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 6096b19d3c3..947bba0b55f 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -21,8 +21,10 @@ from neutron.api.rpc.agentnotifiers import l3_rpc_agent_api from neutron.api.v2 import attributes from neutron.common import constants as l3_constants from neutron.common import exceptions as n_exc +from neutron.common import utils from neutron.db import model_base from neutron.db import models_v2 +from neutron.extensions import external_net from neutron.extensions import l3 from neutron import manager from neutron.openstack.common import log as logging @@ -135,10 +137,19 @@ class L3_NAT_db_mixin(l3.RouterPluginBase): def update_router(self, context, id, router): r = router['router'] has_gw_info = False + gw_info = None if EXTERNAL_GW_INFO in r: has_gw_info = True gw_info = r[EXTERNAL_GW_INFO] del r[EXTERNAL_GW_INFO] + # check whether router needs and can be rescheduled to the proper + # l3 agent (associated with given external network); + # do check before update in DB as an exception will be raised + # in case no proper l3 agent found + candidates = None + if has_gw_info: + candidates = self._check_router_needs_rescheduling( + context, id, gw_info) with context.session.begin(subtransactions=True): if has_gw_info: self._update_router_gw_info(context, id, gw_info) @@ -146,10 +157,79 @@ class L3_NAT_db_mixin(l3.RouterPluginBase): # Ensure we actually have something to update if r.keys(): router_db.update(r) + if candidates: + l3_plugin = manager.NeutronManager.get_service_plugins().get( + constants.L3_ROUTER_NAT) + l3_plugin.reschedule_router(context, id, candidates) + self.l3_rpc_notifier.routers_updated( context, [router_db['id']]) return self._make_router_dict(router_db) + def _check_router_needs_rescheduling(self, context, router_id, gw_info): + """Checks whether router's l3 agent can handle the given network + + When external_network_bridge is set, each L3 agent can be associated + with at most one external network. If router's new external gateway + is on other network then the router needs to be rescheduled to the + proper l3 agent. + If external_network_bridge is not set then the agent + can support multiple external networks and rescheduling is not needed + + :return: list of candidate agents if rescheduling needed, + None otherwise; raises exception if there is no eligible l3 agent + associated with target external network + """ + # TODO(obondarev): rethink placement of this func as l3 db manager is + # not really a proper place for agent scheduling stuff + network_id = gw_info.get('network_id') if gw_info else None + if not network_id: + return + + nets = self._core_plugin.get_networks( + context, {external_net.EXTERNAL: [True]}) + # nothing to do if there is only one external network + if len(nets) <= 1: + return + + # first get plugin supporting l3 agent scheduling + # (either l3 service plugin or core_plugin) + l3_plugin = manager.NeutronManager.get_service_plugins().get( + constants.L3_ROUTER_NAT) + if (not utils.is_extension_supported( + l3_plugin, + l3_constants.L3_AGENT_SCHEDULER_EXT_ALIAS) or + l3_plugin.router_scheduler is None): + # that might mean that we are dealing with non-agent-based + # implementation of l3 services + return + + cur_agents = l3_plugin.list_l3_agents_hosting_router( + context, router_id)['agents'] + for agent in cur_agents: + ext_net_id = agent['configurations'].get( + 'gateway_external_network_id') + ext_bridge = agent['configurations'].get( + 'external_network_bridge', 'br-ex') + if (ext_net_id == network_id or + (not ext_net_id and not ext_bridge)): + return + + # otherwise find l3 agent with matching gateway_external_network_id + active_agents = l3_plugin.get_l3_agents(context, active=True) + router = { + 'id': router_id, + 'external_gateway_info': {'network_id': network_id} + } + candidates = l3_plugin.get_l3_agent_candidates( + router, active_agents) + if not candidates: + msg = (_('No eligible l3 agent associated with external network ' + '%s found') % network_id) + raise n_exc.BadRequest(resource='router', msg=msg) + + return candidates + def _create_router_gw_port(self, context, router, network_id): # Port has no 'tenant-id', as it is hidden from user gw_port = self._core_plugin.create_port(context.elevated(), { diff --git a/neutron/extensions/l3agentscheduler.py b/neutron/extensions/l3agentscheduler.py index da6bcb64e94..36f731d2cbe 100644 --- a/neutron/extensions/l3agentscheduler.py +++ b/neutron/extensions/l3agentscheduler.py @@ -161,6 +161,11 @@ class RouterSchedulingFailed(exceptions.Conflict): " the L3 Agent %(agent_id)s.") +class RouterReschedulingFailed(exceptions.Conflict): + message = _("Failed rescheduling router %(router_id)s: " + "no eligible l3 agent found.") + + class RouterNotHostedByL3Agent(exceptions.Conflict): message = _("The router %(router_id)s is not hosted" " by L3 agent %(agent_id)s.") diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index e0cccd1ecb3..5a0e2dcafa9 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -36,7 +36,7 @@ LOG = logging.getLogger(__name__) class L3Scheduler(object): @abc.abstractmethod - def schedule(self, plugin, context, router_id): + def schedule(self, plugin, context, router_id, candidates=None): """Schedule the router to an active L3 agent. Schedule the router only if it is not already scheduled. @@ -161,10 +161,11 @@ class L3Scheduler(object): class ChanceScheduler(L3Scheduler): """Randomly allocate an L3 agent for a router.""" - def schedule(self, plugin, context, router_id): + def schedule(self, plugin, context, router_id, candidates=None): with context.session.begin(subtransactions=True): sync_router = plugin.get_router(context, router_id) - candidates = self.get_candidates(plugin, context, sync_router) + candidates = candidates or self.get_candidates( + plugin, context, sync_router) if not candidates: return @@ -176,10 +177,11 @@ class ChanceScheduler(L3Scheduler): class LeastRoutersScheduler(L3Scheduler): """Allocate to an L3 agent with the least number of routers bound.""" - def schedule(self, plugin, context, router_id): + def schedule(self, plugin, context, router_id, candidates=None): with context.session.begin(subtransactions=True): sync_router = plugin.get_router(context, router_id) - candidates = self.get_candidates(plugin, context, sync_router) + candidates = candidates or self.get_candidates( + plugin, context, sync_router) if not candidates: return diff --git a/neutron/tests/unit/test_agent_ext_plugin.py b/neutron/tests/unit/test_agent_ext_plugin.py index a685e8a9b6d..a59cd334865 100644 --- a/neutron/tests/unit/test_agent_ext_plugin.py +++ b/neutron/tests/unit/test_agent_ext_plugin.py @@ -161,6 +161,25 @@ class AgentDBTestMixIn(object): time=timeutils.strtime()) return [dhcp_host] + def _register_one_l3_agent(self, host=L3_HOSTA, internal_only=True, + ext_net_id='', ext_bridge=''): + l3 = { + 'binary': 'neutron-l3-agent', + 'host': host, + 'topic': topics.L3_AGENT, + 'configurations': {'use_namespaces': True, + 'router_id': None, + 'handle_internal_only_routers': internal_only, + 'external_network_bridge': ext_bridge, + 'gateway_external_network_id': ext_net_id, + 'interface_driver': 'interface_driver', + }, + 'agent_type': constants.AGENT_TYPE_L3} + callback = agents_db.AgentExtRpcCallback() + callback.report_state(self.adminContext, + agent_state={'agent_state': l3}, + time=timeutils.strtime()) + class AgentDBTestCase(AgentDBTestMixIn, test_db_plugin.NeutronDbPluginV2TestCase): diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py index 05aa3220ec3..4c81aa80720 100644 --- a/neutron/tests/unit/test_l3_plugin.py +++ b/neutron/tests/unit/test_l3_plugin.py @@ -29,15 +29,19 @@ from neutron import context from neutron.db import api as qdbapi from neutron.db import db_base_plugin_v2 from neutron.db import external_net_db +from neutron.db import l3_agentschedulers_db from neutron.db import l3_db +from neutron.db import l3_rpc_base from neutron.db import model_base from neutron.extensions import external_net from neutron.extensions import l3 from neutron.manager import NeutronManager +from neutron.openstack.common import importutils from neutron.openstack.common import log as logging from neutron.openstack.common.notifier import test_notifier from neutron.openstack.common import uuidutils from neutron.plugins.common import constants as service_constants +from neutron.tests.unit import test_agent_ext_plugin from neutron.tests.unit import test_api_v2 from neutron.tests.unit import test_api_v2_extension from neutron.tests.unit import test_db_plugin @@ -251,6 +255,18 @@ class TestL3NatIntPlugin(TestL3NatBasePlugin, supported_extension_aliases = ["external-net", "router"] +# This plugin class is for tests with plugin that integrates L3 and L3 agent +# scheduling. +class TestL3NatIntAgentSchedulingPlugin(TestL3NatIntPlugin, + l3_agentschedulers_db. + L3AgentSchedulerDbMixin): + + supported_extension_aliases = ["external-net", "router", + "l3_agent_scheduler"] + router_scheduler = importutils.import_object( + cfg.CONF.router_scheduler_driver) + + # This plugin class is for tests with plugin not supporting L3. class TestNoL3NatPlugin(TestL3NatBasePlugin): @@ -1767,12 +1783,20 @@ class L3AgentDbTestCaseBase(L3NatTestCaseMixin): class L3BaseForIntTests(test_db_plugin.NeutronDbPluginV2TestCase): + mock_rescheduling = True + def setUp(self, plugin=None, ext_mgr=None, service_plugins=None): if not plugin: plugin = 'neutron.tests.unit.test_l3_plugin.TestL3NatIntPlugin' # for these tests we need to enable overlapping ips cfg.CONF.set_default('allow_overlapping_ips', True) ext_mgr = ext_mgr or L3TestExtensionManager() + + if self.mock_rescheduling: + rescheduling_patcher = mock.patch( + '%s._check_router_needs_rescheduling' % plugin) + rescheduling_patcher.start().return_value = False + super(L3BaseForIntTests, self).setUp(plugin=plugin, ext_mgr=ext_mgr, service_plugins=service_plugins) @@ -1800,6 +1824,103 @@ class L3BaseForSepTests(test_db_plugin.NeutronDbPluginV2TestCase): self.setup_notification_driver() +class L3NatDBIntAgentSchedulingTestCase(L3BaseForIntTests, + L3NatTestCaseMixin, + test_agent_ext_plugin. + AgentDBTestMixIn): + + """Unit tests for core plugin with L3 routing and scheduling integrated.""" + + def setUp(self, plugin='neutron.tests.unit.test_l3_plugin.' + 'TestL3NatIntAgentSchedulingPlugin', + ext_mgr=None, service_plugins=None): + self.mock_rescheduling = False + super(L3NatDBIntAgentSchedulingTestCase, self).setUp( + plugin, ext_mgr, service_plugins) + self.adminContext = context.get_admin_context() + + def _assert_router_on_agent(self, router_id, agent_host): + plugin = NeutronManager.get_service_plugins().get( + service_constants.L3_ROUTER_NAT) + agents = plugin.list_l3_agents_hosting_router( + self.adminContext, router_id)['agents'] + self.assertEqual(len(agents), 1) + self.assertEqual(agents[0]['host'], agent_host) + + def test_update_gateway_agent_exists_supporting_network(self): + with contextlib.nested(self.router(), + self.subnet(), + self.subnet()) as (r, s1, s2): + self._set_net_external(s1['subnet']['network_id']) + l3_rpc = l3_rpc_base.L3RpcCallbackMixin() + self._register_one_l3_agent( + host='host1', + ext_net_id=s1['subnet']['network_id']) + self._register_one_l3_agent( + host='host2', internal_only=False, + ext_net_id=s2['subnet']['network_id']) + l3_rpc.sync_routers(self.adminContext, + host='host1') + self._assert_router_on_agent(r['router']['id'], 'host1') + + self._add_external_gateway_to_router( + r['router']['id'], + s1['subnet']['network_id']) + self._assert_router_on_agent(r['router']['id'], 'host1') + + self._set_net_external(s2['subnet']['network_id']) + self._add_external_gateway_to_router( + r['router']['id'], + s2['subnet']['network_id']) + self._assert_router_on_agent(r['router']['id'], 'host2') + + self._remove_external_gateway_from_router( + r['router']['id'], + s2['subnet']['network_id']) + + def test_update_gateway_agent_exists_supporting_multiple_network(self): + with contextlib.nested(self.router(), + self.subnet(), + self.subnet()) as (r, s1, s2): + self._set_net_external(s1['subnet']['network_id']) + l3_rpc = l3_rpc_base.L3RpcCallbackMixin() + self._register_one_l3_agent( + host='host1', + ext_net_id=s1['subnet']['network_id']) + self._register_one_l3_agent( + host='host2', internal_only=False, + ext_net_id='', ext_bridge='') + l3_rpc.sync_routers(self.adminContext, + host='host1') + self._assert_router_on_agent(r['router']['id'], 'host1') + + self._add_external_gateway_to_router( + r['router']['id'], + s1['subnet']['network_id']) + self._assert_router_on_agent(r['router']['id'], 'host1') + + self._set_net_external(s2['subnet']['network_id']) + self._add_external_gateway_to_router( + r['router']['id'], + s2['subnet']['network_id']) + self._assert_router_on_agent(r['router']['id'], 'host2') + + self._remove_external_gateway_from_router( + r['router']['id'], + s2['subnet']['network_id']) + + def test_router_update_gateway_no_eligible_l3_agent(self): + with self.router() as r: + with self.subnet() as s1: + with self.subnet() as s2: + self._set_net_external(s1['subnet']['network_id']) + self._set_net_external(s2['subnet']['network_id']) + self._add_external_gateway_to_router( + r['router']['id'], + s1['subnet']['network_id'], + expected_code=exc.HTTPBadRequest.code) + + class L3AgentDbIntTestCase(L3BaseForIntTests, L3AgentDbTestCaseBase): """Unit tests for methods called by the L3 agent for