Change router unbinding logic to be consistent with data model

Model allows router to be bound to different agents
Code should not make assumptions that the correspondence is 1-to-1

Closes-Bug: #1497980
Change-Id: Ieda9fc6e2d5a85194f2d022ea092cefb55183750

diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py
index 7d12209a1fa1c786e92647a05a94430ce7220cf6..425f47ec5c4d92be8243419b5d30464cc943990f 100644 (file)
--- a/neutron/db/l3_dvrscheduler_db.py
+++ b/neutron/db/l3_dvrscheduler_db.py
@@ -19,7 +19,6 @@ from oslo_db import exception as db_exc
 from oslo_log import log as logging
 import sqlalchemy as sa
 from sqlalchemy import orm
-from sqlalchemy.orm import exc
 from sqlalchemy.orm import joinedload
 
 from neutron.callbacks import events
@@ -256,7 +255,7 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
     def unbind_snat(self, context, router_id, agent_id=None):
         """Unbind snat from the chosen l3 service agent.
 
-        Unbinds from any L3 agent hosting SNAT if passed agent_id is None
+        Unbinds from all L3 agents hosting SNAT if passed agent_id is None
         """
         with context.session.begin(subtransactions=True):
             query = (context.session.
@@ -264,19 +263,15 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
                      filter_by(router_id=router_id))
             if agent_id:
                 query = query.filter_by(l3_agent_id=agent_id)
-            try:
-                binding = query.one()
-            except exc.NoResultFound:
-                LOG.debug('no snat router binding found for router: %('
-                          'router)s, agent: %(agent)s',
+            binding = query.first()
+            if not binding:
+                LOG.debug('no SNAT router binding found for router: '
+                          '%(router)s, agent: %(agent)s',
                           {'router': router_id, 'agent': agent_id or 'any'})
                 return
 
-            agent_id = binding.l3_agent_id
-            LOG.debug('Delete binding of the SNAT router %(router_id)s '
-                      'from agent %(id)s', {'router_id': router_id,
-                                            'id': agent_id})
-            context.session.delete(binding)
+            query.delete()
+        LOG.debug('Deleted binding of the SNAT router %s', router_id)
 
         return binding
 

diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
index 1624bb1db5d75abe7f4701372f72ddef3cdd34d7..db899db3a6dbce5975bd7360266a77bc3d2221a0 100644 (file)
--- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
+++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
@@ -1523,18 +1523,15 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
         binding = l3_dvrscheduler_db.CentralizedSnatL3AgentBinding(
             router_id=router_id, l3_agent_id='foo_l3_agent_id',
             l3_agent=agents_db.Agent())
-        with mock.patch.object(query.Query, 'one') as mock_query,\
-                mock.patch.object(self.adminContext.session,
-                                  'delete') as mock_session,\
+        with mock.patch.object(query.Query, 'first') as mock_first,\
                 mock.patch.object(query.Query, 'delete') as mock_delete,\
                 mock.patch.object(
                     self.dut,
                     'get_subnet_ids_on_router') as mock_get_subnets:
-            mock_query.return_value = binding
+            mock_first.return_value = binding
             mock_get_subnets.return_value = ['foo_subnet_id']
             self.dut.unbind_snat_servicenode(self.adminContext, router_id)
             mock_get_subnets.assert_called_with(self.adminContext, router_id)
-            self.assertTrue(mock_session.call_count)
             self.assertTrue(mock_delete.call_count)
         core_plugin.assert_called_once_with()
         l3_notifier.assert_called_once_with()