Fixed L3 agent manual scheduling for HA routers

When a router is manually scheduled we check if a router can be scheduled
to the given L3 agent. In this check we were missing handling for
HA router, instead we raised an exception as a last resort.

In addition, we create an HA port on the newly scheduled agent.
The HA port is created within the router's namespace on that agent.
Previously when manually scheduling this was not done at all.

This patch takes care of HA routers by allowing them to be
manually scheduled to a L3 agent if the router is not already
scheduled to that given L3 agent.

The patch also adds an HA port in the router's namespace on the newly
scheduled L3 agent.

In addition the patch also removes the 'max_l3_agents_per_router'
enforcement when manually schedulling a router on a agent. This is
done as the admin should be able to schedule a router on an agent
if he desires, moreover this is consistent with the
'min_l3_agents_per_router' that is not enforced when manually
descheduling a router from an agent.

Co-Authored-By: gong yong sheng <gong.yongsheng@99cloud.net>
Co-Authored-By: Nir Magnezi <nmagnezi@redhat.com>
Closes-Bug: #1401095

Change-Id: I1e2d281cfec50e7fff4ec68659882fc7c0cb4a29

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index 83d404d525301a839f3c3754237ec1ebff7745af..9c6413054fc4c484aabdfd43e9f54ca4115c19b5 100644 (file)
--- a/neutron/db/l3_agentschedulers_db.py
+++ b/neutron/db/l3_agentschedulers_db.py
@@ -35,6 +35,7 @@ from neutron.db import model_base
 from neutron.extensions import l3agentscheduler
 from neutron.i18n import _LE, _LI, _LW
 from neutron import manager
+from neutron.plugins.common import constants as service_constants
 
 
 LOG = logging.getLogger(__name__)
@@ -182,7 +183,9 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
                 return False
         if router.get('distributed'):
             return False
-        # non-dvr case: centralized router is already bound to some agent
+        if router.get('ha'):
+            return True
+        # legacy router case: router is already bound to some agent
         raise l3agentscheduler.RouterHostedByL3Agent(
             router_id=router_id,
             agent_id=bindings[0].l3_agent_id)
@@ -193,7 +196,15 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
         agent_id = agent['id']
         if self.router_scheduler:
             try:
-                self.router_scheduler.bind_router(context, router_id, agent)
+                if router.get('ha'):
+                    plugin = manager.NeutronManager.get_service_plugins().get(
+                        service_constants.L3_ROUTER_NAT)
+                    self.router_scheduler.create_ha_port_and_bind(
+                        plugin, context, router['id'],
+                        router['tenant_id'], agent)
+                else:
+                    self.router_scheduler.bind_router(
+                        context, router_id, agent)
             except db_exc.DBError:
                 raise l3agentscheduler.RouterSchedulingFailed(
                     router_id=router_id, agent_id=agent_id)
@@ -223,6 +234,13 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
         """
         agent = self._get_agent(context, agent_id)
         self._unbind_router(context, router_id, agent_id)
+
+        router = self.get_router(context, router_id)
+        if router.get('ha'):
+            plugin = manager.NeutronManager.get_service_plugins().get(
+                service_constants.L3_ROUTER_NAT)
+            plugin.delete_ha_interfaces_on_host(context, router_id, agent.host)
+
         l3_notifier = self.agent_notifiers.get(constants.AGENT_TYPE_L3)
         if l3_notifier:
             l3_notifier.router_removed_from_agent(

diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py
index 96647d8b1c957de92ace3df4912c3bb6ab2c0af3..ef8c0b2fbd3d4cfa3c80e180b3adacb2c28d6eda 100644 (file)
--- a/neutron/db/l3_hamode_db.py
+++ b/neutron/db/l3_hamode_db.py
@@ -336,6 +336,15 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin):
             self._core_plugin.delete_port(admin_ctx, port['id'],
                                           l3_port_check=False)
 
+    def delete_ha_interfaces_on_host(self, context, router_id, host):
+        admin_ctx = context.elevated()
+        port_ids = (binding.port_id for binding
+                    in self.get_ha_router_port_bindings(admin_ctx,
+                                                        [router_id], host))
+        for port_id in port_ids:
+            self._core_plugin.delete_port(admin_ctx, port_id,
+                                          l3_port_check=False)
+
     def _notify_ha_interfaces_updated(self, context, router_id):
         self.l3_rpc_notifier.routers_updated(
             context, [router_id], shuffle_agents=True)

diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py
index 4d20ba7b888ab294c857a7e2a765ba3dadc5a96b..223a54874425febd8a94dc0e1f89bb6aa63e2684 100644 (file)
--- a/neutron/scheduler/l3_agent_scheduler.py
+++ b/neutron/scheduler/l3_agent_scheduler.py
@@ -200,7 +200,7 @@ class L3Scheduler(object):
             if router.get('ha'):
                 if not self._router_has_binding(context, router['id'],
                                                 l3_agent.id):
-                    self._create_ha_router_binding(
+                    self.create_ha_port_and_bind(
                         plugin, context, router['id'],
                         router['tenant_id'], l3_agent)
             else:
@@ -289,8 +289,8 @@ class L3Scheduler(object):
             return False
         return True
 
-    def _create_ha_router_binding(self, plugin, context, router_id, tenant_id,
-                                  agent):
+    def create_ha_port_and_bind(self, plugin, context, router_id,
+                                tenant_id, agent):
         """Creates and binds a new HA port for this agent."""
         ha_network = plugin.get_ha_network(context, tenant_id)
         port_binding = plugin.add_ha_port(context.elevated(), router_id,
@@ -316,9 +316,9 @@ class L3Scheduler(object):
             if max_agents_not_reached:
                 if not self._router_has_binding(admin_ctx, router_id,
                                                 agent.id):
-                    self._create_ha_router_binding(plugin, admin_ctx,
-                                                   router_id, tenant_id,
-                                                   agent)
+                    self.create_ha_port_and_bind(plugin, admin_ctx,
+                                                 router_id, tenant_id,
+                                                 agent)
                     scheduled = True
 
         return scheduled

diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
index a0d22f7c82639faace7e03268cc60c6b8330ff7f..a8f5c15dd449a3ac50845c869e7a44eb74b5818b 100644 (file)
--- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
+++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
@@ -256,7 +256,7 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
                                '_router_has_binding',
                                return_value=has_binding) as mock_has_binding,\
                 mock.patch.object(self.scheduler,
-                                  '_create_ha_router_binding') as mock_bind:
+                                  'create_ha_port_and_bind') as mock_bind:
             self.scheduler._bind_routers(mock.ANY, mock.ANY, routers, agent)
             mock_has_binding.assert_called_once_with(mock.ANY, 'foo_router',
                                                      'foo_agent')
@@ -1421,6 +1421,9 @@ class L3HATestCaseMixin(testlib_api.SqlTestCase,
         self.plugin = L3HAPlugin()
 
         self.setup_coreplugin('neutron.plugins.ml2.plugin.Ml2Plugin')
+        cfg.CONF.set_override('service_plugins',
+                              ['neutron.services.l3_router.'
+                              'l3_router_plugin.L3RouterPlugin'])
         mock.patch.object(l3_hamode_db.L3_HA_NAT_db_mixin,
                           '_notify_ha_interfaces_updated').start()
 
@@ -1495,12 +1498,14 @@ class L3_HA_scheduler_db_mixinTestCase(L3HATestCaseMixin):
 
 class L3AgentSchedulerDbMixinTestCase(L3HATestCaseMixin):
 
-    def test_reschedule_ha_routers_from_down_agents(self):
+    def _setup_ha_router(self):
         router = self._create_ha_router()
         self.plugin.schedule_router(self.adminContext, router['id'])
-        agents = self.plugin.get_l3_agents_hosting_routers(
-            self.adminContext, [router['id']],
-            admin_state_up=True)
+        agents = self._get_agents_scheduled_for_router(router)
+        return router, agents
+
+    def test_reschedule_ha_routers_from_down_agents(self):
+        agents = self._setup_ha_router()[1]
         self.assertEqual(2, len(agents))
         self._set_l3_agent_dead(self.agent_id1)
         with mock.patch.object(self.plugin, 'reschedule_router') as reschedule:
@@ -1538,6 +1543,68 @@ class L3AgentSchedulerDbMixinTestCase(L3HATestCaseMixin):
         self.assertEqual({'agents': []},
                          self.plugin._get_agents_dict_for_router([]))
 
+    def test_manual_add_ha_router_to_agent(self):
+        cfg.CONF.set_override('max_l3_agents_per_router', 2)
+        router, agents = self._setup_ha_router()
+        self.assertEqual(2, len(agents))
+        agent = helpers.register_l3_agent(host='myhost_3')
+        # We allow to exceed max l3 agents per router via manual scheduling
+        self.plugin.add_router_to_l3_agent(
+            self.adminContext, agent.id, router['id'])
+        agents = self._get_agents_scheduled_for_router(router)
+        self.assertIn(agent.id, [_agent.id for _agent in agents])
+        self.assertEqual(3, len(agents))
+
+    def test_manual_remove_ha_router_from_agent(self):
+        router, agents = self._setup_ha_router()
+        self.assertEqual(2, len(agents))
+        agent = agents.pop()
+        # Remove router from agent and make sure it is removed
+        self.plugin.remove_router_from_l3_agent(
+            self.adminContext, agent.id, router['id'])
+        agents = self._get_agents_scheduled_for_router(router)
+        self.assertEqual(1, len(agents))
+        self.assertNotIn(agent.id, [_agent.id for _agent in agents])
+
+    def test_manual_remove_ha_router_from_all_agents(self):
+        router, agents = self._setup_ha_router()
+        self.assertEqual(2, len(agents))
+        agent = agents.pop()
+        self.plugin.remove_router_from_l3_agent(
+            self.adminContext, agent.id, router['id'])
+        agent = agents.pop()
+        self.plugin.remove_router_from_l3_agent(
+            self.adminContext, agent.id, router['id'])
+        agents = self._get_agents_scheduled_for_router(router)
+        self.assertEqual(0, len(agents))
+
+    def _get_agents_scheduled_for_router(self, router):
+        return self.plugin.get_l3_agents_hosting_routers(
+            self.adminContext, [router['id']],
+            admin_state_up=True)
+
+    def test_delete_ha_interfaces_from_agent(self):
+        router, agents = self._setup_ha_router()
+        agent = agents.pop()
+        self.plugin.remove_router_from_l3_agent(
+            self.adminContext, agent.id, router['id'])
+        session = self.adminContext.session
+        db = l3_hamode_db.L3HARouterAgentPortBinding
+        results = session.query(db).filter_by(
+            router_id=router['id'])
+        results = [binding.l3_agent_id for binding in results.all()]
+        self.assertNotIn(agent.id, results)
+
+    def test_add_ha_interface_to_l3_agent(self):
+        agent = self.plugin.get_agents_db(self.adminContext)[0]
+        router = self._create_ha_router()
+        self.plugin.add_router_to_l3_agent(self.adminContext, agent.id,
+                                           router['id'])
+        # Verify agent has HA interface
+        ha_ports = self.plugin.get_ha_router_port_bindings(self.adminContext,
+                                                           [router['id']])
+        self.assertIn(agent.id, [ha_port.l3_agent_id for ha_port in ha_ports])
+
 
 class L3HAChanceSchedulerTestCase(L3HATestCaseMixin):