DVR: make sure snat portion is always scheduled when needed

commit 236e408272bcb9b8e957524864e571b5afdc4623 introduced
a regression where if router without external gateway was already
scheduled to all dvr_snat agents, then when adding external gateway
to the router, snat portion scheduling was skipped.
The patch fixes regression and adds corresponding unit (functional
in fact) test.

Closes-Bug: #1486627
Change-Id: Iad7e53bd57836f257d7110bc054d58029484ab99

diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py
index ae7adbf38ead189550178b594fd59cf0e98b2b93..e41a34968d77e024ae161044b81b5f9546ede193 100644 (file)
--- a/neutron/scheduler/l3_agent_scheduler.py
+++ b/neutron/scheduler/l3_agent_scheduler.py
@@ -234,11 +234,8 @@ class L3Scheduler(object):
         sync_router = plugin.get_router(context, router_id)
         candidates = candidates or self._get_candidates(
             plugin, context, sync_router)
-        if not candidates:
-            return
-
-        router_distributed = sync_router.get('distributed', False)
-        if router_distributed:
+        chosen_agent = None
+        if sync_router.get('distributed', False):
             for chosen_agent in candidates:
                 self.bind_router(context, router_id, chosen_agent)
 
@@ -249,13 +246,15 @@ class L3Scheduler(object):
             if not snat_bindings and router_gw_exists:
                 # If GW exists for DVR routers and no SNAT binding
                 # call the schedule_snat_router
-                plugin.schedule_snat_router(
+                chosen_agent = plugin.schedule_snat_router(
                     context, router_id, sync_router)
             elif not router_gw_exists and snat_bindings:
                 # If DVR router and no Gateway but SNAT Binding exists then
                 # call the unbind_snat_servicenode to unbind the snat service
                 # from agent
                 plugin.unbind_snat_servicenode(context, router_id)
+        elif not candidates:
+            return
         elif sync_router.get('ha', False):
             chosen_agents = self._bind_ha_router(plugin, context,
                                                  router_id, candidates)

diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py
index a250f402841995da763c52da1a31f308adf33cd2..1633afd39b614252b90e66049b0333069f8ee9f6 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py
@@ -1021,6 +1021,48 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
                          set([a['configurations']['agent_mode'] for a in
                               l3agents['agents']]))
 
+    def test_dvr_router_snat_scheduling_late_ext_gw_add(self):
+        """Test snat scheduling for the case when dvr router is already
+        scheduled to all dvr_snat agents and then external gateway is added.
+        """
+        helpers.register_l3_agent(
+            host=L3_HOSTA, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT)
+        helpers.register_l3_agent(
+            host=L3_HOSTB, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT)
+        with self.subnet() as s_int,\
+                self.subnet(cidr='20.0.0.0/24') as s_ext:
+            net_id = s_ext['subnet']['network_id']
+            self._set_net_external(net_id)
+
+            router = {'name': 'router1',
+                      'admin_state_up': True,
+                      'distributed': True}
+            r = self.l3plugin.create_router(self.adminContext,
+                                            {'router': router})
+            # add router interface first
+            self.l3plugin.add_router_interface(self.adminContext, r['id'],
+                {'subnet_id': s_int['subnet']['id']})
+            # check that router is scheduled to both dvr_snat agents
+            l3agents = self._list_l3_agents_hosting_router(r['id'])
+            self.assertEqual(2, len(l3agents['agents']))
+            # check that snat is not scheduled as router is not connected to
+            # external network
+            snat_agents = self.l3plugin.get_snat_bindings(
+                self.adminContext, [r['id']])
+            self.assertEqual(0, len(snat_agents))
+
+            # connect router to external network
+            self.l3plugin.update_router(self.adminContext, r['id'],
+                {'router': {'external_gateway_info': {'network_id': net_id}}})
+            # router should still be scheduled to both dvr_snat agents
+            l3agents = self._list_l3_agents_hosting_router(r['id'])
+            self.assertEqual(2, len(l3agents['agents']))
+            # now snat portion should be scheduled as router is connected
+            # to external network
+            snat_agents = self.l3plugin.get_snat_bindings(
+                self.adminContext, [r['id']])
+            self.assertEqual(1, len(snat_agents))
+
     def test_dvr_router_csnat_rescheduling(self):
         helpers.register_l3_agent(
             host=L3_HOSTA, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT)