Avoid auto-scheduling for distributed routers

The reason for this is twofold:

- It may relieve contention on DB access while
  both servers and l3 agents are busy setting up
  and syncing routers down respectively.

- It prevents accidental placement of namespaces
  during the L3 sync_routers process, as auto
  scheduling without taking into account the state
  of the L3 agents, as well as the state of the
  routers being processed, may overrule the placement
  decision made during router operations.

Partial-bug: #1356121
Partial-bug: #1359326

Change-Id: Ia677ce212145d6cee65adeb1d8ae594e6ac5e34d

diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py
index 5e3823fb56d046bd67529f7be0156acee67ef22c..6854a01a02ddc410a88daf8924c77ffba2edb11b 100644 (file)
--- a/neutron/scheduler/l3_agent_scheduler.py
+++ b/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 and not router.get('distributed', False):
+            if l3_agents:
                 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
 
@@ -174,9 +176,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):

diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py
index 14ef8327d14025ba5af9079a80ee9e9ee3b37e66..d13a78dbc1d2f243a66c562377d112494871d572 100644 (file)
--- a/neutron/tests/unit/test_l3_schedulers.py
+++ b/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):