Merge "Avoid auto-scheduling for distributed routers"

neutron/scheduler/l3_agent_scheduler.py

@@ -41,23 +41,13 @@ class L3Scheduler(object):
         """
         pass
 
-    def dvr_has_binding(self, context, router_id, l3_agent_id):
-        router_binding_model = l3_agentschedulers_db.RouterL3AgentBinding
-
-        query = context.session.query(router_binding_model)
-        query = query.filter(router_binding_model.router_id == router_id,
-                             router_binding_model.l3_agent_id == l3_agent_id)
-
-        return query.count() > 0
-
     def filter_unscheduled_routers(self, context, plugin, routers):
         """Filter from list of routers the ones that are not scheduled."""
         unscheduled_routers = []
         for router in routers:
             l3_agents = plugin.get_l3_agents_hosting_routers(
                 context, [router['id']], admin_state_up=True)
-            # TODO(armando-migliaccio): remove dvr-related check
+            if l3_agents:
-            if l3_agents and not router.get('distributed', False):
                 LOG.debug(('Router %(router_id)s has already been '
                            'hosted by L3 agent %(agent_id)s'),
                           {'router_id': router['id'],
@@ -79,19 +69,28 @@ class L3Scheduler(object):
                 context, filters={'id': unscheduled_router_ids})
         return []
 
-    def get_routers_to_schedule(self, context, plugin, router_ids=None):
+    def get_routers_to_schedule(self, context, plugin,
+                                router_ids=None, exclude_distributed=False):
         """Verify that the routers specified need to be scheduled.
 
         :param context: the context
         :param plugin: the core plugin
         :param router_ids: the list of routers to be checked for scheduling
+        :param exclude_distributed: whether or not to consider dvr routers
         :returns: the list of routers to be scheduled
         """
         if router_ids is not None:
             routers = plugin.get_routers(context, filters={'id': router_ids})
-            return self.filter_unscheduled_routers(context, plugin, routers)
+            unscheduled_routers = self.filter_unscheduled_routers(
+                context, plugin, routers)
         else:
-            return self.get_unscheduled_routers(context, plugin)
+            unscheduled_routers = self.get_unscheduled_routers(context, plugin)
+
+        if exclude_distributed:
+            unscheduled_routers = [
+                r for r in unscheduled_routers if not r.get('distributed')
+            ]
+        return unscheduled_routers
 
     def get_routers_can_schedule(self, context, plugin, routers, l3_agent):
         """Get the subset of routers that can be scheduled on the L3 agent."""
@@ -121,8 +120,11 @@ class L3Scheduler(object):
         if not l3_agent:
             return False
 
+        # NOTE(armando-migliaccio): DVR routers should not be auto
+        # scheduled because auto-scheduling may interfere with the
+        # placement rules for IR and SNAT namespaces.
         unscheduled_routers = self.get_routers_to_schedule(
-            context, plugin, router_ids)
+            context, plugin, router_ids, exclude_distributed=True)
         if not unscheduled_routers:
             return False
 
@@ -173,9 +175,6 @@ class L3Scheduler(object):
 
     def bind_routers(self, context, routers, l3_agent):
         for router in routers:
-            if (router.get('distributed', False) and
-                self.dvr_has_binding(context, router['id'], l3_agent.id)):
-                    continue
             self.bind_router(context, router['id'], l3_agent)
 
     def bind_router(self, context, router_id, chosen_agent):

neutron/tests/unit/test_l3_schedulers.py

@@ -171,6 +171,20 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
         mock_get.assert_called_once_with(mock.ANY, self.plugin)
         self.assertEqual(expected_routers, unscheduled_routers)
 
+    def test_get_routers_to_schedule_exclude_distributed(self):
+        routers = [
+            {'id': 'foo_router1', 'distributed': True}, {'id': 'foo_router_2'}
+        ]
+        expected_routers = [{'id': 'foo_router_2'}]
+        with mock.patch.object(self.scheduler,
+                               'get_unscheduled_routers') as mock_get:
+            mock_get.return_value = routers
+            unscheduled_routers = self.scheduler.get_routers_to_schedule(
+                mock.ANY, self.plugin,
+                router_ids=None, exclude_distributed=True)
+        mock_get.assert_called_once_with(mock.ANY, self.plugin)
+        self.assertEqual(expected_routers, unscheduled_routers)
+
     def _test_get_routers_can_schedule(self, routers, agent, target_routers):
         self.plugin.get_l3_agent_candidates.return_value = agent
         result = self.scheduler.get_routers_can_schedule(
@@ -207,15 +221,6 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
             self.scheduler.bind_routers(mock.ANY, routers, mock.ANY)
         mock_bind.assert_called_once_with(mock.ANY, 'foo_router', mock.ANY)
 
-    def test_bind_routers_dvr(self):
-        routers = [{'id': 'foo_router', 'distributed': True}]
-        agent = agents_db.Agent(id='foo_agent')
-        with mock.patch.object(self.scheduler, 'dvr_has_binding') as mock_dvr:
-            with mock.patch.object(self.scheduler, 'bind_router') as mock_bind:
-                self.scheduler.bind_routers(mock.ANY, routers, agent)
-        mock_dvr.assert_called_once_with(mock.ANY, 'foo_router', 'foo_agent')
-        self.assertFalse(mock_bind.called)
-
 
 class L3SchedulerTestExtensionManager(object):