OOP cleanup: start protected method names with underscore

This slightly improves readability of l3_schedulers module.

Change-Id: I362143939b513bb3b2a02e7472efa26e8c83cb96
Closes-Bug: #1436922

neutron/scheduler/l3_agent_scheduler.py

@@ -51,7 +51,7 @@ class L3Scheduler(object):
         """
         pass
 
-    def router_has_binding(self, context, router_id, l3_agent_id):
+    def _router_has_binding(self, context, router_id, l3_agent_id):
         router_binding_model = l3_agentschedulers_db.RouterL3AgentBinding
 
         query = context.session.query(router_binding_model)
@@ -60,7 +60,7 @@ class L3Scheduler(object):
 
         return query.count() > 0
 
-    def filter_unscheduled_routers(self, context, plugin, routers):
+    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:
@@ -75,7 +75,7 @@ class L3Scheduler(object):
                 unscheduled_routers.append(router)
         return unscheduled_routers
 
-    def get_unscheduled_routers(self, context, plugin):
+    def _get_unscheduled_routers(self, context, plugin):
         """Get routers with no agent binding."""
         # TODO(gongysh) consider the disabled agent's router
         no_agent_binding = ~sql.exists().where(
@@ -88,8 +88,8 @@ class L3Scheduler(object):
                 context, filters={'id': unscheduled_router_ids})
         return []
 
-    def get_routers_to_schedule(self, context, plugin,
+    def _get_routers_to_schedule(self, context, plugin,
-                                router_ids=None, exclude_distributed=False):
+                                 router_ids=None, exclude_distributed=False):
         """Verify that the routers specified need to be scheduled.
 
         :param context: the context
@@ -100,10 +100,11 @@ class L3Scheduler(object):
         """
         if router_ids is not None:
             routers = plugin.get_routers(context, filters={'id': router_ids})
-            unscheduled_routers = self.filter_unscheduled_routers(
+            unscheduled_routers = self._filter_unscheduled_routers(
                 context, plugin, routers)
         else:
-            unscheduled_routers = self.get_unscheduled_routers(context, plugin)
+            unscheduled_routers = self._get_unscheduled_routers(context,
+                                                                plugin)
 
         if exclude_distributed:
             unscheduled_routers = [
@@ -111,7 +112,7 @@ class L3Scheduler(object):
             ]
         return unscheduled_routers
 
-    def get_routers_can_schedule(self, context, plugin, routers, l3_agent):
+    def _get_routers_can_schedule(self, context, plugin, routers, l3_agent):
         """Get the subset of routers that can be scheduled on the L3 agent."""
         ids_to_discard = set()
         for router in routers:
@@ -142,25 +143,25 @@ class L3Scheduler(object):
         # 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(
+        unscheduled_routers = self._get_routers_to_schedule(
             context, plugin, router_ids, exclude_distributed=True)
         if not unscheduled_routers:
             if utils.is_extension_supported(
                     plugin, constants.L3_HA_MODE_EXT_ALIAS):
-                return self.schedule_ha_routers_to_additional_agent(
+                return self._schedule_ha_routers_to_additional_agent(
                     plugin, context, l3_agent)
 
-        target_routers = self.get_routers_can_schedule(
+        target_routers = self._get_routers_can_schedule(
             context, plugin, unscheduled_routers, l3_agent)
         if not target_routers:
             LOG.warn(_LW('No routers compatible with L3 agent configuration'
                          ' on host %s'), host)
             return False
 
-        self.bind_routers(context, plugin, target_routers, l3_agent)
+        self._bind_routers(context, plugin, target_routers, l3_agent)
         return True
 
-    def get_candidates(self, plugin, context, sync_router):
+    def _get_candidates(self, plugin, context, sync_router):
         """Return L3 agents where a router could be scheduled."""
         with context.session.begin(subtransactions=True):
             # allow one router is hosted by just
@@ -195,12 +196,12 @@ class L3Scheduler(object):
 
             return candidates
 
-    def bind_routers(self, context, plugin, routers, l3_agent):
+    def _bind_routers(self, context, plugin, routers, l3_agent):
         for router in routers:
             if router.get('ha'):
-                if not self.router_has_binding(context, router['id'],
+                if not self._router_has_binding(context, router['id'],
-                                               l3_agent.id):
+                                                l3_agent.id):
-                    self.create_ha_router_binding(
+                    self._create_ha_router_binding(
                         plugin, context, router['id'],
                         router['tenant_id'], l3_agent)
             else:
@@ -249,7 +250,7 @@ class L3Scheduler(object):
                 # from agent
                 plugin.unbind_snat_servicenode(context, router_id)
                 return
-        candidates = candidates or self.get_candidates(
+        candidates = candidates or self._get_candidates(
             plugin, context, sync_router)
         if not candidates:
             return
@@ -257,8 +258,8 @@ class L3Scheduler(object):
             for chosen_agent in candidates:
                 self.bind_router(context, router_id, chosen_agent)
         elif sync_router.get('ha', False):
-            chosen_agents = self.bind_ha_router(plugin, context,
+            chosen_agents = self._bind_ha_router(plugin, context,
-                                                router_id, candidates)
+                                                 router_id, candidates)
             if not chosen_agents:
                 return
             chosen_agent = chosen_agents[-1]
@@ -278,19 +279,19 @@ class L3Scheduler(object):
         """Choose agents from candidates based on a specific policy."""
         pass
 
-    def get_num_of_agents_for_ha(self, candidates_count):
+    def _get_num_of_agents_for_ha(self, candidates_count):
         return (min(self.max_ha_agents, candidates_count) if self.max_ha_agents
                 else candidates_count)
 
-    def enough_candidates_for_ha(self, candidates):
+    def _enough_candidates_for_ha(self, candidates):
         if not candidates or len(candidates) < self.min_ha_agents:
             LOG.error(_LE("Not enough candidates, a HA router needs at least "
                           "%s agents"), self.min_ha_agents)
             return False
         return True
 
-    def create_ha_router_binding(self, plugin, context, router_id, tenant_id,
+    def _create_ha_router_binding(self, plugin, context, router_id, tenant_id,
-                                 agent):
+                                  agent):
         """Creates and binds a new HA port for this agent."""
         ha_network = plugin.get_ha_network(context, tenant_id)
         port_binding = plugin.add_ha_port(context.elevated(), router_id,
@@ -298,7 +299,7 @@ class L3Scheduler(object):
         port_binding.l3_agent_id = agent['id']
         self.bind_router(context, router_id, agent)
 
-    def schedule_ha_routers_to_additional_agent(self, plugin, context, agent):
+    def _schedule_ha_routers_to_additional_agent(self, plugin, context, agent):
         """Bind already scheduled routers to the agent.
 
         Retrieve the number of agents per router and check if the router has
@@ -314,15 +315,16 @@ class L3Scheduler(object):
             max_agents_not_reached = (
                 not self.max_ha_agents or agents < self.max_ha_agents)
             if max_agents_not_reached:
-                if not self.router_has_binding(admin_ctx, router_id, agent.id):
+                if not self._router_has_binding(admin_ctx, router_id,
-                    self.create_ha_router_binding(plugin, admin_ctx,
+                                                agent.id):
-                                                  router_id, tenant_id,
+                    self._create_ha_router_binding(plugin, admin_ctx,
-                                                  agent)
+                                                   router_id, tenant_id,
+                                                   agent)
                     scheduled = True
 
         return scheduled
 
-    def bind_ha_router_to_agents(self, plugin, context, router_id,
+    def _bind_ha_router_to_agents(self, plugin, context, router_id,
                                  chosen_agents):
         port_bindings = plugin.get_ha_router_port_bindings(context,
                                                            [router_id])
@@ -335,17 +337,17 @@ class L3Scheduler(object):
                       '%(agent_id)s)',
                       {'router_id': router_id, 'agent_id': agent.id})
 
-    def bind_ha_router(self, plugin, context, router_id, candidates):
+    def _bind_ha_router(self, plugin, context, router_id, candidates):
         """Bind a HA router to agents based on a specific policy."""
 
-        if not self.enough_candidates_for_ha(candidates):
+        if not self._enough_candidates_for_ha(candidates):
             return
 
         chosen_agents = self._choose_router_agents_for_ha(
             plugin, context, candidates)
 
-        self.bind_ha_router_to_agents(plugin, context, router_id,
+        self._bind_ha_router_to_agents(plugin, context, router_id,
-                                      chosen_agents)
+                                       chosen_agents)
 
         return chosen_agents
 
@@ -362,7 +364,7 @@ class ChanceScheduler(L3Scheduler):
         return random.choice(candidates)
 
     def _choose_router_agents_for_ha(self, plugin, context, candidates):
-        num_agents = self.get_num_of_agents_for_ha(len(candidates))
+        num_agents = self._get_num_of_agents_for_ha(len(candidates))
         return random.sample(candidates, num_agents)
 
 
@@ -381,7 +383,7 @@ class LeastRoutersScheduler(L3Scheduler):
         return chosen_agent
 
     def _choose_router_agents_for_ha(self, plugin, context, candidates):
-        num_agents = self.get_num_of_agents_for_ha(len(candidates))
+        num_agents = self._get_num_of_agents_for_ha(len(candidates))
         ordered_agents = plugin.get_l3_agents_ordered_by_num_routers(
             context, [candidate['id'] for candidate in candidates])
         return ordered_agents[:num_agents]

neutron/tests/unit/db/test_l3_hamode_db.py

@@ -94,7 +94,7 @@ class L3HATestFramework(testlib_api.SqlTestCase):
         with self.admin_ctx.session.begin(subtransactions=True):
             scheduler = l3_agent_scheduler.ChanceScheduler()
             agents_db = self.plugin.get_agents_db(self.admin_ctx)
-            scheduler.bind_ha_router_to_agents(
+            scheduler._bind_ha_router_to_agents(
                 self.plugin,
                 self.admin_ctx,
                 router_id,

neutron/tests/unit/scheduler/test_l3_agent_scheduler.py

@@ -96,9 +96,9 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
     def test_auto_schedule_routers(self):
         self.plugin.get_enabled_agent_on_host.return_value = [mock.ANY]
         with contextlib.nested(
-            mock.patch.object(self.scheduler, 'get_routers_to_schedule'),
+            mock.patch.object(self.scheduler, '_get_routers_to_schedule'),
-            mock.patch.object(self.scheduler, 'get_routers_can_schedule')) as (
+            mock.patch.object(self.scheduler, '_get_routers_can_schedule')
-            gs, gr):
+        ) as (gs, gr):
             result = self.scheduler.auto_schedule_routers(
                 self.plugin, mock.ANY, mock.ANY, mock.ANY)
         self.assertTrue(self.plugin.get_enabled_agent_on_host.called)
@@ -117,7 +117,7 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
         type(self.plugin).supported_extension_aliases = (
             mock.PropertyMock(return_value=[]))
         with mock.patch.object(self.scheduler,
-                               'get_routers_to_schedule') as mock_routers:
+                               '_get_routers_to_schedule') as mock_routers:
             mock_routers.return_value = []
             result = self.scheduler.auto_schedule_routers(
                 self.plugin, mock.ANY, mock.ANY, mock.ANY)
@@ -127,9 +127,9 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
     def test_auto_schedule_routers_no_target_routers(self):
         self.plugin.get_enabled_agent_on_host.return_value = [mock.ANY]
         with contextlib.nested(
-            mock.patch.object(self.scheduler, 'get_routers_to_schedule'),
+            mock.patch.object(self.scheduler, '_get_routers_to_schedule'),
-            mock.patch.object(self.scheduler, 'get_routers_can_schedule')) as (
+            mock.patch.object(self.scheduler, '_get_routers_can_schedule')
-            mock_unscheduled_routers, mock_target_routers):
+        ) as (mock_unscheduled_routers, mock_target_routers):
             mock_unscheduled_routers.return_value = mock.ANY
             mock_target_routers.return_value = None
             result = self.scheduler.auto_schedule_routers(
@@ -137,101 +137,102 @@ class L3SchedulerBaseTestCase(base.BaseTestCase):
         self.assertTrue(self.plugin.get_enabled_agent_on_host.called)
         self.assertFalse(result)
 
-    def test_get_routers_to_schedule_with_router_ids(self):
+    def test__get_routers_to_schedule_with_router_ids(self):
         router_ids = ['foo_router_1', 'foo_router_2']
         expected_routers = [
             {'id': 'foo_router1'}, {'id': 'foo_router_2'}
         ]
         self.plugin.get_routers.return_value = expected_routers
         with mock.patch.object(self.scheduler,
-                               'filter_unscheduled_routers') as mock_filter:
+                               '_filter_unscheduled_routers') as mock_filter:
             mock_filter.return_value = expected_routers
-            unscheduled_routers = self.scheduler.get_routers_to_schedule(
+            unscheduled_routers = self.scheduler._get_routers_to_schedule(
                 mock.ANY, self.plugin, router_ids)
         mock_filter.assert_called_once_with(
             mock.ANY, self.plugin, expected_routers)
         self.assertEqual(expected_routers, unscheduled_routers)
 
-    def test_get_routers_to_schedule_without_router_ids(self):
+    def test__get_routers_to_schedule_without_router_ids(self):
         expected_routers = [
             {'id': 'foo_router1'}, {'id': 'foo_router_2'}
         ]
         with mock.patch.object(self.scheduler,
-                               'get_unscheduled_routers') as mock_get:
+                               '_get_unscheduled_routers') as mock_get:
             mock_get.return_value = expected_routers
-            unscheduled_routers = self.scheduler.get_routers_to_schedule(
+            unscheduled_routers = self.scheduler._get_routers_to_schedule(
                 mock.ANY, self.plugin)
         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):
+    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:
+                               '_get_unscheduled_routers') as mock_get:
             mock_get.return_value = routers
-            unscheduled_routers = self.scheduler.get_routers_to_schedule(
+            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):
+    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(
+        result = self.scheduler._get_routers_can_schedule(
             mock.ANY, self.plugin, routers, mock.ANY)
         self.assertEqual(target_routers, result)
 
-    def _test_filter_unscheduled_routers(self, routers, agents, expected):
+    def _test__filter_unscheduled_routers(self, routers, agents, expected):
         self.plugin.get_l3_agents_hosting_routers.return_value = agents
-        unscheduled_routers = self.scheduler.filter_unscheduled_routers(
+        unscheduled_routers = self.scheduler._filter_unscheduled_routers(
             mock.ANY, self.plugin, routers)
         self.assertEqual(expected, unscheduled_routers)
 
-    def test_filter_unscheduled_routers_already_scheduled(self):
+    def test__filter_unscheduled_routers_already_scheduled(self):
-        self._test_filter_unscheduled_routers(
+        self._test__filter_unscheduled_routers(
             [{'id': 'foo_router1'}, {'id': 'foo_router_2'}],
             [{'id': 'foo_agent_id'}], [])
 
-    def test_filter_unscheduled_routers_non_scheduled(self):
+    def test__filter_unscheduled_routers_non_scheduled(self):
-        self._test_filter_unscheduled_routers(
+        self._test__filter_unscheduled_routers(
             [{'id': 'foo_router1'}, {'id': 'foo_router_2'}],
             None, [{'id': 'foo_router1'}, {'id': 'foo_router_2'}])
 
-    def test_get_routers_can_schedule_with_compat_agent(self):
+    def test__get_routers_can_schedule_with_compat_agent(self):
         routers = [{'id': 'foo_router'}]
-        self._test_get_routers_can_schedule(routers, mock.ANY, routers)
+        self._test__get_routers_can_schedule(routers, mock.ANY, routers)
 
-    def test_get_routers_can_schedule_with_no_compat_agent(self):
+    def test__get_routers_can_schedule_with_no_compat_agent(self):
         routers = [{'id': 'foo_router'}]
-        self._test_get_routers_can_schedule(routers, None, [])
+        self._test__get_routers_can_schedule(routers, None, [])
 
-    def test_bind_routers_centralized(self):
+    def test__bind_routers_centralized(self):
         routers = [{'id': 'foo_router'}]
         with mock.patch.object(self.scheduler, 'bind_router') as mock_bind:
-            self.scheduler.bind_routers(mock.ANY, mock.ANY, routers, mock.ANY)
+            self.scheduler._bind_routers(mock.ANY, mock.ANY, routers, mock.ANY)
         mock_bind.assert_called_once_with(mock.ANY, 'foo_router', mock.ANY)
 
-    def _test_bind_routers_ha(self, has_binding):
+    def _test__bind_routers_ha(self, has_binding):
         routers = [{'id': 'foo_router', 'ha': True, 'tenant_id': '42'}]
         agent = agents_db.Agent(id='foo_agent')
         with contextlib.nested(
-            mock.patch.object(self.scheduler, 'router_has_binding',
+            mock.patch.object(self.scheduler, '_router_has_binding',
                               return_value=has_binding),
-            mock.patch.object(self.scheduler, 'create_ha_router_binding')) as (
+            mock.patch.object(self.scheduler, '_create_ha_router_binding')
+        ) as (
                 mock_has_binding, mock_bind):
-            self.scheduler.bind_routers(mock.ANY, mock.ANY, routers, agent)
+            self.scheduler._bind_routers(mock.ANY, mock.ANY, routers, agent)
         mock_has_binding.assert_called_once_with(mock.ANY, 'foo_router',
                                                  'foo_agent')
         self.assertEqual(not has_binding, mock_bind.called)
 
-    def test_bind_routers_ha_has_binding(self):
+    def test__bind_routers_ha_has_binding(self):
-        self._test_bind_routers_ha(has_binding=True)
+        self._test__bind_routers_ha(has_binding=True)
 
-    def test_bind_routers_ha_no_binding(self):
+    def test__bind_routers_ha_no_binding(self):
-        self._test_bind_routers_ha(has_binding=False)
+        self._test__bind_routers_ha(has_binding=False)
 
 
 class L3SchedulerBaseMixin(object):