centralized router is incorrectly scheduled

When agent_mode=dvr only distributed routers should be
scheduled to that agent.  This change fixes this problem
by not allowing centralized routers to be scheduled
to an agent configured with agent_mode=dvr

Closes-bug: 1350667
Change-Id: I34e118535094520f2cee853f39d582a625acde20

neutron/db/l3_agentschedulers_db.py

@@ -330,9 +330,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
                  ex_net_id != gateway_external_network_id)):
                 continue
             is_router_distributed = sync_router.get('distributed', False)
-            if not is_router_distributed and agent_mode == 'legacy':
+            if agent_mode in ('legacy', 'dvr_snat') and (
+                not is_router_distributed):
                 candidates.append(l3_agent)
-            elif (agent_mode.startswith('dvr') and
+            elif is_router_distributed and agent_mode.startswith('dvr') and (
                 self.check_vmexists_on_l3agent(
                     context, l3_agent, sync_router['id'], subnet_id)):
                 candidates.append(l3_agent)

neutron/tests/unit/test_l3_schedulers.py

@@ -58,6 +58,26 @@ SECOND_L3_AGENT = {
     'start_flag': True
 }
 
+HOST_DVR = 'my_l3_host_dvr'
+DVR_L3_AGENT = {
+    'binary': 'neutron-l3-agent',
+    'host': HOST_DVR,
+    'topic': topics.L3_AGENT,
+    'configurations': {'agent_mode': 'dvr'},
+    'agent_type': constants.AGENT_TYPE_L3,
+    'start_flag': True
+}
+
+HOST_DVR_SNAT = 'my_l3_host_dvr_snat'
+DVR_SNAT_L3_AGENT = {
+    'binary': 'neutron-l3-agent',
+    'host': HOST_DVR_SNAT,
+    'topic': topics.L3_AGENT,
+    'configurations': {'agent_mode': 'dvr_snat'},
+    'agent_type': constants.AGENT_TYPE_L3,
+    'start_flag': True
+}
+
 DB_PLUGIN_KLASS = ('neutron.plugins.openvswitch.ovs_neutron_plugin.'
                    'OVSNeutronPluginV2')
 
@@ -106,6 +126,23 @@ class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin,
                                              filters={'host': [HOST]})
         self.agent_id2 = agent_db[0].id
 
+    def _register_l3_dvr_agents(self):
+        callback = agents_db.AgentExtRpcCallback()
+        callback.report_state(self.adminContext,
+                              agent_state={'agent_state': DVR_L3_AGENT},
+                              time=timeutils.strtime())
+        agent_db = self.plugin.get_agents_db(self.adminContext,
+                                             filters={'host': [HOST_DVR]})
+        self.l3_dvr_agent = agent_db[0]
+
+        callback.report_state(self.adminContext,
+                              agent_state={'agent_state': DVR_SNAT_L3_AGENT},
+                              time=timeutils.strtime())
+        agent_db = self.plugin.get_agents_db(self.adminContext,
+                                             filters={'host': [HOST_DVR_SNAT]})
+        self.l3_dvr_snat_id = agent_db[0].id
+        self.l3_dvr_snat_agent = agent_db[0]
+
     def _set_l3_agent_admin_state(self, context, agent_id, state=True):
         update = {'agent': {'admin_state_up': state}}
         self.plugin.update_agent(context, agent_id, update)
@@ -162,6 +199,40 @@ class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin,
             args, kwargs = flog.call_args
             self.assertIn('has already been scheduled', args[0])
 
+    def _check_get_l3_agent_candidates(self, router, agent_list, exp_host):
+        candidates = self.get_l3_agent_candidates(self.adminContext,
+                                                  router, agent_list,
+                                                  subnet_id=None)
+        self.assertEqual(len(candidates), 1)
+        self.assertEqual(candidates[0]['host'], exp_host)
+
+    def test_get_l3_agent_candidates(self):
+        self._register_l3_dvr_agents()
+        router = self._make_router(self.fmt,
+                                   tenant_id=str(uuid.uuid4()),
+                                   name='r2')
+        router['external_gateway_info'] = None
+        router['id'] = str(uuid.uuid4())
+        agent_list = [self.agent1, self.l3_dvr_agent]
+
+        # test legacy agent_mode case: only legacy agent should be candidate
+        router['distributed'] = False
+        exp_host = FIRST_L3_AGENT.get('host')
+        self._check_get_l3_agent_candidates(router, agent_list, exp_host)
+
+        # test dvr agent_mode case only dvr agent should be candidate
+        router['distributed'] = True
+        exp_host = DVR_L3_AGENT.get('host')
+        self._check_get_l3_agent_candidates(router, agent_list, exp_host)
+
+        # test dvr_snat agent_mode cases: dvr_snat agent can host
+        # centralized and distributed routers
+        agent_list = [self.l3_dvr_snat_agent]
+        exp_host = DVR_SNAT_L3_AGENT.get('host')
+        self._check_get_l3_agent_candidates(router, agent_list, exp_host)
+        router['distributed'] = False
+        self._check_get_l3_agent_candidates(router, agent_list, exp_host)
+
 
 class L3AgentChanceSchedulerTestCase(L3SchedulerTestCase):