From c6d0e45c9fb07fab4edf9be3a11ca00c8ffd0b84 Mon Sep 17 00:00:00 2001 From: Yoni Shafrir Date: Wed, 17 Dec 2014 09:25:55 +0200 Subject: [PATCH] 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 Co-Authored-By: Nir Magnezi Closes-Bug: #1401095 Change-Id: I1e2d281cfec50e7fff4ec68659882fc7c0cb4a29 --- neutron/db/l3_agentschedulers_db.py | 22 +++++- neutron/db/l3_hamode_db.py | 9 +++ neutron/scheduler/l3_agent_scheduler.py | 12 +-- .../unit/scheduler/test_l3_agent_scheduler.py | 77 +++++++++++++++++-- 4 files changed, 107 insertions(+), 13 deletions(-) diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index 83d404d5253..9c6413054fc 100644 --- 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 96647d8b1c9..ef8c0b2fbd3 100644 --- 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 4d20ba7b888..223a5487442 100644 --- 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 a0d22f7c826..a8f5c15dd44 100644 --- 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):