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

@@ -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},

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."""

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(), {

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.")

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
 

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):

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