Change check_ports_exist_on_l3agent to pass the subnet_ids
The get_subnet_ids_on_router is called for every
available l3agent in check_ports_exist_on_l3agent.
This introduces un-necessary call to the same
function multiple times which is expensive since it
calls get_ports internally.
In large scale the time taken to schedule a VM
on a given N-Node increases.
By passing the subnet_ids to check_ports_exist_on_l3agent
we would be only calling once get_subnet_ids_on_router in
the get_l3_agent_candidates.
This patch addresses the above problem by calling
get_subnet_ids_on_router just once.
Related-Bug: #1513678
Conflicts:
neutron/tests/unit/db/test_agentschedulers_db.py
Change-Id: I9d130f98e07bfe571eee32b827ff9af4010ff0fb
(cherry picked from commit 062ad0a0a6
)
This commit is contained in:
parent
85f72db223
commit
9246cffc7c
|
@ -453,15 +453,12 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
|
|||
if agentschedulers_db.AgentSchedulerDbMixin.is_eligible_agent(
|
||||
active, l3_agent)]
|
||||
|
||||
def check_ports_exist_on_l3agent(self, context, l3_agent, router_id):
|
||||
def check_ports_exist_on_l3agent(
|
||||
self, context, l3_agent, subnet_ids):
|
||||
"""
|
||||
This function checks for existence of dvr serviceable
|
||||
ports on the host, running the input l3agent.
|
||||
"""
|
||||
subnet_ids = self.get_subnet_ids_on_router(context, router_id)
|
||||
if not subnet_ids:
|
||||
return False
|
||||
|
||||
core_plugin = manager.NeutronManager.get_plugin()
|
||||
filter = {'fixed_ips': {'subnet_id': subnet_ids}}
|
||||
ports = core_plugin.get_ports(context, filters=filter)
|
||||
|
@ -476,6 +473,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
|
|||
ignore_admin_state=False):
|
||||
"""Get the valid l3 agents for the router from a list of l3_agents."""
|
||||
candidates = []
|
||||
is_router_distributed = sync_router.get('distributed', False)
|
||||
if is_router_distributed:
|
||||
subnet_ids = self.get_subnet_ids_on_router(
|
||||
context, sync_router['id'])
|
||||
for l3_agent in l3_agents:
|
||||
if not ignore_admin_state and not l3_agent.admin_state_up:
|
||||
# ignore_admin_state True comes from manual scheduling
|
||||
|
@ -498,16 +499,15 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
|
|||
(ex_net_id and gateway_external_network_id and
|
||||
ex_net_id != gateway_external_network_id)):
|
||||
continue
|
||||
is_router_distributed = sync_router.get('distributed', False)
|
||||
if agent_mode in (
|
||||
constants.L3_AGENT_MODE_LEGACY,
|
||||
constants.L3_AGENT_MODE_DVR_SNAT) and (
|
||||
not is_router_distributed):
|
||||
candidates.append(l3_agent)
|
||||
elif is_router_distributed and agent_mode.startswith(
|
||||
constants.L3_AGENT_MODE_DVR) and (
|
||||
self.check_ports_exist_on_l3agent(
|
||||
context, l3_agent, sync_router['id'])):
|
||||
elif (is_router_distributed and subnet_ids and
|
||||
agent_mode.startswith(constants.L3_AGENT_MODE_DVR) and (
|
||||
self.check_ports_exist_on_l3agent(
|
||||
context, l3_agent, subnet_ids))):
|
||||
candidates.append(l3_agent)
|
||||
return candidates
|
||||
|
||||
|
|
|
@ -306,11 +306,14 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
|||
constants.L3_ROUTER_NAT)
|
||||
l3_agents = plugin.get_l3_agents_hosting_routers(context,
|
||||
[router['id']])
|
||||
for l3_agent in l3_agents:
|
||||
if not plugin.check_ports_exist_on_l3agent(context, l3_agent,
|
||||
router['id']):
|
||||
plugin.remove_router_from_l3_agent(
|
||||
context, l3_agent['id'], router['id'])
|
||||
subnet_ids = plugin.get_subnet_ids_on_router(
|
||||
context, router['id'])
|
||||
if subnet_ids:
|
||||
for l3_agent in l3_agents:
|
||||
if not plugin.check_ports_exist_on_l3agent(
|
||||
context, l3_agent, subnet_ids):
|
||||
plugin.remove_router_from_l3_agent(
|
||||
context, l3_agent['id'], router['id'])
|
||||
router_interface_info = self._make_router_interface_info(
|
||||
router['id'], port['tenant_id'], port['id'], subnets[0]['id'],
|
||||
[subnet['id'] for subnet in subnets])
|
||||
|
|
|
@ -774,7 +774,11 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
|
|||
with self.subnet() as s, \
|
||||
mock.patch.object(
|
||||
self.l3plugin,
|
||||
'check_ports_exist_on_l3agent') as port_exists:
|
||||
'check_ports_exist_on_l3agent') as port_exists,\
|
||||
mock.patch.object(
|
||||
self.l3plugin,
|
||||
'get_subnet_ids_on_router') as rtr_subnets:
|
||||
rtr_subnets.return_value = [{'id': '1234'}]
|
||||
net_id = s['subnet']['network_id']
|
||||
self._set_net_external(net_id)
|
||||
router = {'name': 'router1',
|
||||
|
|
|
@ -467,6 +467,8 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
|||
plugin = mock.MagicMock()
|
||||
plugin.get_l3_agents_hosting_routers = mock.Mock(
|
||||
return_value=[mock.MagicMock()])
|
||||
plugin.get_subnet_ids_on_router = mock.Mock(
|
||||
return_value=interface_info)
|
||||
plugin.check_ports_exist_on_l3agent = mock.Mock(
|
||||
return_value=False)
|
||||
plugin.remove_router_from_l3_agent = mock.Mock(
|
||||
|
@ -503,6 +505,8 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
|||
router_dict = {'name': 'test_router', 'admin_state_up': True,
|
||||
'distributed': True}
|
||||
router = self._create_router(router_dict)
|
||||
plugin = mock.MagicMock()
|
||||
plugin.get_subnet_ids_on_router = mock.Mock()
|
||||
with self.network() as net_ext,\
|
||||
self.subnet() as subnet1,\
|
||||
self.subnet(cidr='20.0.0.0/24') as subnet2:
|
||||
|
@ -534,7 +538,8 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
|||
with mock.patch.object(manager.NeutronManager,
|
||||
'get_service_plugins') as get_svc_plugin:
|
||||
get_svc_plugin.return_value = {
|
||||
plugin_const.L3_ROUTER_NAT: self.mixin}
|
||||
plugin_const.L3_ROUTER_NAT: plugin}
|
||||
self.mixin.manager = manager
|
||||
self.mixin.remove_router_interface(
|
||||
self.ctx, router['id'], {'port_id': dvr_ports[0]['id']})
|
||||
|
||||
|
@ -547,6 +552,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
|||
dvr_ports = self.core_plugin.get_ports(
|
||||
self.ctx, filters=dvr_filters)
|
||||
self.assertEqual(1, len(dvr_ports))
|
||||
self.assertEqual(1, plugin.get_subnet_ids_on_router.call_count)
|
||||
|
||||
def test__validate_router_migration_notify_advanced_services(self):
|
||||
router = {'name': 'foo_router', 'admin_state_up': False}
|
||||
|
|
|
@ -640,6 +640,7 @@ class L3SchedulerTestBaseMixin(object):
|
|||
agent_list = [self.agent1, self.l3_dvr_agent]
|
||||
# test dvr agent_mode case only dvr agent should be candidate
|
||||
router['distributed'] = True
|
||||
self.get_subnet_ids_on_router = mock.Mock()
|
||||
self.check_ports_exist_on_l3agent = mock.Mock(return_value=True)
|
||||
self._check_get_l3_agent_candidates(router, agent_list, HOST_DVR)
|
||||
|
||||
|
@ -653,6 +654,7 @@ class L3SchedulerTestBaseMixin(object):
|
|||
agent_list = [self.agent1, self.l3_dvr_agent]
|
||||
router['distributed'] = True
|
||||
# Test no VMs present case
|
||||
self.get_subnet_ids_on_router = mock.Mock()
|
||||
self.check_ports_exist_on_l3agent = mock.Mock(return_value=False)
|
||||
self._check_get_l3_agent_candidates(
|
||||
router, agent_list, HOST_DVR, count=0)
|
||||
|
@ -667,6 +669,7 @@ class L3SchedulerTestBaseMixin(object):
|
|||
router['distributed'] = True
|
||||
|
||||
agent_list = [self.l3_dvr_snat_agent]
|
||||
self.get_subnet_ids_on_router = mock.Mock()
|
||||
self.check_ports_exist_on_l3agent = mock.Mock(return_value=True)
|
||||
self._check_get_l3_agent_candidates(router, agent_list, HOST_DVR_SNAT)
|
||||
|
||||
|
@ -682,6 +685,7 @@ class L3SchedulerTestBaseMixin(object):
|
|||
agent_list = [self.l3_dvr_snat_agent]
|
||||
self.check_ports_exist_on_l3agent = mock.Mock(return_value=False)
|
||||
# Test no VMs present case
|
||||
self.get_subnet_ids_on_router = mock.Mock()
|
||||
self.check_ports_exist_on_l3agent.return_value = False
|
||||
self._check_get_l3_agent_candidates(
|
||||
router, agent_list, HOST_DVR_SNAT, count=0)
|
||||
|
|
Loading…
Reference in New Issue