From 23351390d87c3541e9df05164201024be0a3d42f Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Thu, 26 Mar 2015 06:17:59 +0400 Subject: [PATCH] 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 | 74 +++++++++--------- neutron/tests/unit/db/test_l3_hamode_db.py | 2 +- .../unit/scheduler/test_l3_agent_scheduler.py | 77 ++++++++++--------- 3 files changed, 78 insertions(+), 75 deletions(-) diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index bb873a73b69..e23501d4ff8 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/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, - router_ids=None, exclude_distributed=False): + 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 @@ -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'], - l3_agent.id): - self.create_ha_router_binding( + if not self._router_has_binding(context, router['id'], + l3_agent.id): + 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, - router_id, candidates) + chosen_agents = self._bind_ha_router(plugin, context, + 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, - agent): + def _create_ha_router_binding(self, plugin, context, router_id, tenant_id, + 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): - self.create_ha_router_binding(plugin, admin_ctx, - router_id, tenant_id, - agent) + if not self._router_has_binding(admin_ctx, router_id, + agent.id): + self._create_ha_router_binding(plugin, admin_ctx, + 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, - chosen_agents) + self._bind_ha_router_to_agents(plugin, context, router_id, + 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] diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 18a7392ad19..e13991aa0e0 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/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, diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index 267eceefa32..202139c2fe7 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/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_can_schedule')) as ( - gs, gr): + mock.patch.object(self.scheduler, '_get_routers_to_schedule'), + mock.patch.object(self.scheduler, '_get_routers_can_schedule') + ) 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_can_schedule')) as ( - mock_unscheduled_routers, mock_target_routers): + mock.patch.object(self.scheduler, '_get_routers_to_schedule'), + mock.patch.object(self.scheduler, '_get_routers_can_schedule') + ) 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): - self._test_filter_unscheduled_routers( + def test__filter_unscheduled_routers_already_scheduled(self): + self._test__filter_unscheduled_routers( [{'id': 'foo_router1'}, {'id': 'foo_router_2'}], [{'id': 'foo_agent_id'}], []) - def test_filter_unscheduled_routers_non_scheduled(self): - self._test_filter_unscheduled_routers( + def test__filter_unscheduled_routers_non_scheduled(self): + 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): - self._test_bind_routers_ha(has_binding=True) + def test__bind_routers_ha_has_binding(self): + self._test__bind_routers_ha(has_binding=True) - def test_bind_routers_ha_no_binding(self): - self._test_bind_routers_ha(has_binding=False) + def test__bind_routers_ha_no_binding(self): + self._test__bind_routers_ha(has_binding=False) class L3SchedulerBaseMixin(object):