Merge "Reschedule router if new external gateway is on other network"

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(
+            for agent in cur_agents:
-                context, router_id, agent.host)
+                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