Resolve issue where router can't be removed from L3-agent in dvr mode

Fixes the problem where the L3 DVR Scheduler is unable
to remove a DVR router from a L3 agent running in
'dvr' mode.

Closes-bug: 1489091
Change-Id: Id128a81d2cf7108936715ee305012fbff23ffdbf

diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py
index 9b8ad89a458ee3d6c6fc872bd69ef423f3376950..6caba149441434e6156703ccbcb55ff4fb56f119 100644 (file)
--- a/neutron/db/l3_dvrscheduler_db.py
+++ b/neutron/db/l3_dvrscheduler_db.py
@@ -458,16 +458,20 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
                   context, agent, router)
 
     def remove_router_from_l3_agent(self, context, agent_id, router_id):
+        binding = None
         router = self.get_router(context, router_id)
         if router['external_gateway_info'] and router.get('distributed'):
             binding = self.unbind_snat(context, router_id, agent_id=agent_id)
+            # binding only exists when agent mode is dvr_snat
             if binding:
                 notification_not_sent = self.unbind_router_servicenode(context,
                                              router_id, binding)
                 if notification_not_sent:
                     self.l3_rpc_notifier.routers_updated(
                         context, [router_id], schedule_routers=False)
-        else:
+
+        # Below Needs to be done when agent mode is legacy or dvr.
+        if not binding:
             super(L3_DVRsch_db_mixin,
                   self).remove_router_from_l3_agent(
                     context, agent_id, router_id)

diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
index 8b4c546a952e2f199e551735277bd386f00b2e8a..e4421e4aae496efb9d05b551a6d1de47e03063ac 100644 (file)
--- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
+++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
@@ -1415,6 +1415,101 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
         core_plugin.assert_called_once_with()
         l3_notifier.assert_called_once_with()
 
+    def _test_remove_router_from_l3_agent_dvr_snat(self, ursn_return):
+        agent_id = 'dvr_snat_l3_agent_id'
+        router_id = 'dvr-router-1'
+        router = {
+            'id': router_id,
+            'distributed': True,
+            'external_gateway_info': {'network_id': str(uuid.uuid4()),
+                                      'enable_snat': True}
+        }
+
+        binding = l3_dvrscheduler_db.CentralizedSnatL3AgentBinding(
+            router_id=router_id, l3_agent_id=agent_id,
+            l3_agent=agents_db.Agent())
+
+        self.dut.l3_rpc_notifier = mock.Mock()
+        with mock.patch.object(self.dut, 'get_router') as mock_gr,\
+                mock.patch.object(self.dut, 'unbind_snat') as mock_us,\
+                mock.patch.object(
+                    self.dut,
+                    'unbind_router_servicenode') as mock_ursn,\
+                mock.patch('neutron.db.l3_agentschedulers_db.'
+                           'L3AgentSchedulerDbMixin.'
+                           'remove_router_from_l3_agent') as mock_super_rrl3a:
+            mock_gr.return_value = router
+            mock_us.return_value = binding
+            mock_ursn.return_value = ursn_return
+
+            self.dut.remove_router_from_l3_agent(self.adminContext,
+                                                 agent_id,
+                                                 router_id)
+            mock_gr.assert_called_once_with(self.adminContext, router_id)
+
+            us_params = {'agent_id': agent_id}
+            mock_us.assert_called_once_with(self.adminContext,
+                                            router_id,
+                                            **us_params)
+            mock_ursn.assert_called_once_with(self.adminContext,
+                                              router_id,
+                                              binding)
+            self.assertFalse(mock_super_rrl3a.called)
+
+            if ursn_return:
+                routers_updated_params = {'schedule_routers': False}
+                (self.dut.l3_rpc_notifier.routers_updated.
+                 assert_called_once_with(self.adminContext,
+                                         [router_id],
+                                         **routers_updated_params))
+            else:
+                self.assertFalse(self.dut.l3_rpc_notifier.
+                                 routers_updated.called)
+
+    def test_remove_router_from_l3_agent_dvr_snat_mode(self):
+        self._test_remove_router_from_l3_agent_dvr_snat(True)
+        self._test_remove_router_from_l3_agent_dvr_snat(False)
+
+    def test_remove_router_from_l3_agent_dvr_mode(self):
+        agent_id = 'dvr_l3_agent_id'
+        router_id = 'dvr-router-1'
+        router = {
+            'id': router_id,
+            'distributed': True,
+            'external_gateway_info': {'network_id': str(uuid.uuid4()),
+                                      'enable_snat': True}
+        }
+
+        self.dut.l3_rpc_notifier = mock.Mock()
+        with mock.patch.object(self.dut, 'get_router') as mock_gr,\
+                mock.patch.object(self.dut, 'unbind_snat') as mock_us,\
+                mock.patch.object(
+                    self.dut,
+                    'unbind_router_servicenode') as mock_ursn,\
+                mock.patch('neutron.db.l3_agentschedulers_db.'
+                           'L3AgentSchedulerDbMixin.'
+                           'remove_router_from_l3_agent') as mock_super_rrl3a:
+            mock_gr.return_value = router
+            mock_us.return_value = None
+            mock_ursn.return_value = True
+
+            self.dut.remove_router_from_l3_agent(self.adminContext,
+                                                 agent_id,
+                                                 router_id)
+
+            mock_gr.assert_called_once_with(self.adminContext, router_id)
+
+            us_params = {'agent_id': agent_id}
+            mock_us.assert_called_once_with(self.adminContext,
+                                            router_id,
+                                            **us_params)
+
+            self.assertFalse(self.dut.l3_rpc_notifier.routers_updated.called)
+            self.assertFalse(mock_ursn.called)
+            mock_super_rrl3a.assert_called_with(self.adminContext,
+                                                agent_id,
+                                                router_id)
+
 
 class L3HAPlugin(db_v2.NeutronDbPluginV2,
                  l3_hamode_db.L3_HA_NAT_db_mixin,