From 0b4e42fe11bf918e18ea8f240d9055b3967b60bb Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Thu, 31 Jul 2014 19:20:00 -0700 Subject: [PATCH] Fix DB Duplicate error when scheduling distributed routers The error was caused by binding the router to an agent candidate that was already selected during the scheduling process. A DB lookup was also saved by passing the router object around; this led to a minor style cleanup. Closes-bug: #1351123 Change-Id: Ib71a0140c8a7fbd5b230609d33487f8adba252e7 --- neutron/db/l3_dvrscheduler_db.py | 22 +++++------ neutron/scheduler/l3_agent_scheduler.py | 7 ++-- neutron/tests/unit/test_l3_schedulers.py | 49 ++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 15 deletions(-) diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index 276faeda2..686be112f 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -248,27 +248,25 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin): LOG.debug('Removed binding for router %(router_id)s and ' 'agent %(id)s', {'router_id': router_id, 'id': agent_id}) - def schedule_snat_router(self, context, router_id, gw_exists): + def schedule_snat_router(self, context, router_id, sync_router, gw_exists): """Schedule the snat router on l3 service agent.""" if gw_exists: - query = (context.session. - query(CentralizedSnatL3AgentBinding). - filter_by(router_id=router_id)) - for bind in query: - agt_id = bind.l3_agent_id + binding = (context.session. + query(CentralizedSnatL3AgentBinding). + filter_by(router_id=router_id).first()) + if binding: + l3_agent_id = binding.l3_agent_id + l3_agent = binding.l3_agent LOG.debug('SNAT Router %(router_id)s has already been ' 'hosted by L3 agent ' - '%(agent_id)s', {'router_id': router_id, - 'agent_id': agt_id}) - self.bind_dvr_router_servicenode(context, - router_id, - bind.l3_agent) + '%(l3_agent_id)s', {'router_id': router_id, + 'l3_agent_id': l3_agent_id}) + self.bind_dvr_router_servicenode(context, router_id, l3_agent) return active_l3_agents = self.get_l3_agents(context, active=True) if not active_l3_agents: LOG.warn(_('No active L3 agents')) return - sync_router = self.get_router(context, router_id) snat_candidates = self.get_snat_candidates(sync_router, active_l3_agents) if snat_candidates: diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index a85015d2a..265d2011a 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -196,11 +196,12 @@ class L3Scheduler(object): candidates=None, hints=None): sync_router = plugin.get_router(context, router_id) subnet_id = hints.get('subnet_id') if hints else None - candidates = candidates or self.get_candidates( - plugin, context, sync_router, subnet_id) if (hints and 'gw_exists' in hints and sync_router.get('distributed', False)): - plugin.schedule_snat_router(context, router_id, sync_router) + plugin.schedule_snat_router( + context, router_id, sync_router, hints['gw_exists']) + candidates = candidates or self.get_candidates( + plugin, context, sync_router, subnet_id) if not candidates: return if sync_router.get('distributed', False): diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py index b794163b6..4936017bd 100644 --- a/neutron/tests/unit/test_l3_schedulers.py +++ b/neutron/tests/unit/test_l3_schedulers.py @@ -21,6 +21,7 @@ import uuid import mock from oslo.config import cfg +from sqlalchemy.orm import query from neutron.api.v2 import attributes as attr from neutron.common import constants @@ -129,6 +130,36 @@ class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin, router['router']['id'], subnet['subnet']['network_id']) self._delete('routers', router['router']['id']) + def test_schedule_router_distributed(self): + scheduler = l3_agent_scheduler.ChanceScheduler() + agent = agents_db.Agent() + agent.admin_state_up = True + agent.heartbeat_timestamp = timeutils.utcnow() + sync_router = { + 'id': 'foo_router_id', + 'distributed': True + } + plugin = mock.Mock() + plugin.get_router.return_value = sync_router + plugin.get_l3_agents_hosting_routers.return_value = [] + plugin.get_l3_agents.return_value = [agent] + plugin.get_l3_agent_candidates.return_value = [agent] + with mock.patch.object(scheduler, 'bind_router'): + scheduler._schedule_router( + plugin, self.adminContext, + 'foo_router_id', None, {'gw_exists': True}) + expected_calls = [ + mock.call.get_router(mock.ANY, 'foo_router_id'), + mock.call.schedule_snat_router( + mock.ANY, 'foo_router_id', sync_router, True), + mock.call.get_l3_agents_hosting_routers( + mock.ANY, ['foo_router_id'], admin_state_up=True), + mock.call.get_l3_agents(mock.ANY, active=True), + mock.call.get_l3_agent_candidates( + mock.ANY, sync_router, [agent], None), + ] + plugin.assert_has_calls(expected_calls) + def _test_schedule_bind_router(self, agent, router): ctx = self.adminContext session = ctx.session @@ -381,3 +412,21 @@ class L3DvrSchedulerTestCase(base.BaseTestCase): 'thisHost', 'dvr_port1', sub_ids) self.assertFalse(result) + + def test_schedule_snat_router_with_snat_candidates(self): + agent = agents_db.Agent() + agent.admin_state_up = True + agent.heartbeat_timestamp = timeutils.utcnow() + with contextlib.nested( + mock.patch.object(query.Query, 'first'), + mock.patch.object(self.dut, 'get_l3_agents'), + mock.patch.object(self.dut, 'get_snat_candidates'), + mock.patch.object(self.dut, 'bind_snat_servicenode')) as ( + mock_query, mock_agents, mock_candidates, mock_bind): + mock_query.return_value = [] + mock_agents.return_value = [agent] + mock_candidates.return_value = [agent] + self.dut.schedule_snat_router( + self.adminContext, 'foo_router_id', mock.ANY, True) + mock_bind.assert_called_once_with( + self.adminContext, 'foo_router_id', [agent]) -- 2.45.2