openstack/neutron-vpnaas
Code Issues Proposed changes

Fix argument name mismatch in L3-RPC sync_routers

Browse Source 
In sync_routers L3-RPC method l3-agent sends router_ids but the
server side expected router_id. This commit fixes the server side
to accept router_ids, and drops "fullsync" arg from the agent side
(fullsync is not used anywhere and it does not affect RPC signature).
This change allows l3-agent to sync only the specified routers
instead of all routers.

Fixes bug #1201553

As a result of the above change, auto_schedule_routers() and
list_active_sync_routers_on_active_l3_agent() in L3 scheduler
needs to handle a list of router IDs. This commit changes L3 scheduler
to accept a list of router IDs in the above two methods.

Also fixes the argument order of fullsync and router_ids in get_routers
in L3PluginApi. L3-agent main code expects router_ids as the second arg.

Change-Id: I22e8d11b9676cbcfe9e72449031bb63071be8314
This commit is contained in:
Akihiro MOTOKI 2013-07-16 17:05:19 +09:00 committed by Mark McClain
parent 7750c38e12
commit de1967452e
5 changed files with 117 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
neutron/agent/l3_agent.py
View File

@ -67,11 +67,10 @@ class L3PluginApi(proxy.RpcProxy):
topic=topic, default_version=self.BASE_RPC_API_VERSION)
self.host = host
def get_routers(self, context, fullsync=True, router_ids=None):
def get_routers(self, context, router_ids=None):
"""Make a remote process call to retrieve the sync data for routers."""
return self.call(context,
self.make_msg('sync_routers', host=self.host,
fullsync=fullsync,
router_ids=router_ids),
topic=self.topic)

15
neutron/db/agentschedulers_db.py
View File

@ -123,7 +123,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
result = self.auto_schedule_routers(context,
agent_db.host,
router_id)
[router_id])
if not result:
raise l3agentscheduler.RouterSchedulingFailed(
router_id=router_id, agent_id=id)
@ -166,7 +166,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
return {'routers': []}
def list_active_sync_routers_on_active_l3_agent(
self, context, host, router_id):
self, context, host, router_ids):
agent = self._get_agent_by_type_and_host(
context, constants.AGENT_TYPE_L3, host)
if not agent.admin_state_up:
@ -174,9 +174,12 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
query = context.session.query(RouterL3AgentBinding.router_id)
query = query.filter(
RouterL3AgentBinding.l3_agent_id == agent.id)
if router_id:
query = query.filter(RouterL3AgentBinding.router_id == router_id)
if not router_ids:
pass
else:
query = query.filter(
RouterL3AgentBinding.router_id.in_(router_ids))
router_ids = [item[0] for item in query]
if router_ids:
return self.get_sync_data(context, router_ids=router_ids,
@ -272,10 +275,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
candidates.append(l3_agent)
return candidates
def auto_schedule_routers(self, context, host, router_id):
def auto_schedule_routers(self, context, host, router_ids):
if self.router_scheduler:
return self.router_scheduler.auto_schedule_routers(
self, context, host, router_id)
self, context, host, router_ids)
def schedule_router(self, context, router):
if self.router_scheduler:

10
neutron/db/l3_rpc_base.py
View File

@ -33,22 +33,22 @@ class L3RpcCallbackMixin(object):
"""Sync routers according to filters to a specific agent.
@param context: contain user information
@param kwargs: host, or router_id
@param kwargs: host, router_ids
@return: a list of routers
with their interfaces and floating_ips
"""
router_id = kwargs.get('router_id')
router_ids = kwargs.get('router_ids')
host = kwargs.get('host')
context = neutron_context.get_admin_context()
plugin = manager.NeutronManager.get_plugin()
if utils.is_extension_supported(
plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):
if cfg.CONF.router_auto_schedule:
plugin.auto_schedule_routers(context, host, router_id)
plugin.auto_schedule_routers(context, host, router_ids)
routers = plugin.list_active_sync_routers_on_active_l3_agent(
context, host, router_id)
context, host, router_ids)
else:
routers = plugin.get_sync_data(context, router_id)
routers = plugin.get_sync_data(context, router_ids)
LOG.debug(_("Routers returned to l3 agent:\n %s"),
jsonutils.dumps(routers, indent=5))
return routers

38
neutron/scheduler/l3_agent_scheduler.py
View File

@ -36,10 +36,11 @@ class ChanceScheduler(object):
can be introduced later.
"""
def auto_schedule_routers(self, plugin, context, host, router_id):
def auto_schedule_routers(self, plugin, context, host, router_ids):
"""Schedule non-hosted routers to L3 Agent running on host.
If router_id is given, only this router is scheduled
if it is not hosted yet.
If router_ids is given, each router in router_ids is scheduled
if it is not scheduled yet. Otherwise all unscheduled routers
are scheduled.
Don't schedule the routers which are hosted already
by active l3 agents.
"""
@ -59,42 +60,45 @@ class ChanceScheduler(object):
if agents_db.AgentDbMixin.is_agent_down(
l3_agent.heartbeat_timestamp):
LOG.warn(_('L3 agent %s is not active'), l3_agent.id)
# check if the specified router is hosted
if router_id:
# check if each of the specified routers is hosted
if router_ids:
unscheduled_router_ids = []
for router_id in router_ids:
l3_agents = plugin.get_l3_agents_hosting_routers(
context, [router_id], admin_state_up=True)
if l3_agents:
LOG.debug(_('Router %(router_id)s has already been hosted'
' by L3 agent %(agent_id)s'),
LOG.debug(_('Router %(router_id)s has already been'
' hosted by L3 agent %(agent_id)s'),
{'router_id': router_id,
'agent_id': l3_agents[0]['id']})
else:
unscheduled_router_ids.append(router_id)
if not unscheduled_router_ids:
# all (specified) routers are already scheduled
return False
# get the router ids
if router_id:
router_ids = [(router_id,)]
else:
# get all routers that are not hosted
#TODO(gongysh) consider the disabled agent's router
stmt = ~exists().where(
l3_db.Router.id ==
agentschedulers_db.RouterL3AgentBinding.router_id)
router_ids = context.session.query(
l3_db.Router.id).filter(stmt).all()
if not router_ids:
unscheduled_router_ids = [router_id_[0] for router_id_ in
context.session.query(
l3_db.Router.id).filter(stmt)]
if not unscheduled_router_ids:
LOG.debug(_('No non-hosted routers'))
return False
# check if the configuration of l3 agent is compatible
# with the router
router_ids = [router_id_[0] for router_id_ in router_ids]
routers = plugin.get_routers(context, filters={'id': router_ids})
routers = plugin.get_routers(
context, filters={'id': unscheduled_router_ids})
to_removed_ids = []
for router in routers:
candidates = plugin.get_l3_agent_candidates(router, [l3_agent])
if not candidates:
to_removed_ids.append(router['id'])
router_ids = list(set(router_ids) - set(to_removed_ids))
router_ids = set(unscheduled_router_ids) - set(to_removed_ids)
if not router_ids:
LOG.warn(_('No routers compatible with L3 agent configuration'
' on host %s'), host)

76
neutron/tests/unit/openvswitch/test_agent_scheduler.py
View File

@ -568,10 +568,13 @@ class OvsAgentSchedulerTestCase(test_l3_plugin.L3NatTestCaseMixin,
with self.router() as router:
l3_rpc = l3_rpc_base.L3RpcCallbackMixin()
self._register_agent_states()
l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
l3_rpc.sync_routers(self.adminContext, host=L3_HOSTB)
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
ret_b = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTB)
l3_agents = self._list_l3_agents_hosting_router(
router['router']['id'])
self.assertEqual(1, len(ret_a))
self.assertIn(router['router']['id'], [r['id'] for r in ret_a])
self.assertFalse(len(ret_b))
self.assertEqual(1, len(l3_agents['agents']))
self.assertEqual(L3_HOSTA, l3_agents['agents'][0]['host'])
@ -682,6 +685,75 @@ class OvsAgentSchedulerTestCase(test_l3_plugin.L3NatTestCaseMixin,
self.assertEqual(1, len(l3_agents_1['agents']))
self.assertEqual(0, len(l3_agents_2['agents']))
def test_rpc_sync_routers(self):
l3_rpc = l3_rpc_base.L3RpcCallbackMixin()
self._register_agent_states()
# No routers
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
self.assertEqual(0, len(ret_a))
with contextlib.nested(self.router(),
self.router(),
self.router()) as routers:
router_ids = [r['router']['id'] for r in routers]
# Get all routers
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
self.assertEqual(3, len(ret_a))
self.assertEqual(set(router_ids), set([r['id'] for r in ret_a]))
# Get all routers (router_ids=None)
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
router_ids=None)
self.assertEqual(3, len(ret_a))
self.assertEqual(set(router_ids), set([r['id'] for r in ret_a]))
# Get router2 only
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
router_ids=[router_ids[1]])
self.assertEqual(1, len(ret_a))
self.assertIn(router_ids[1], [r['id'] for r in ret_a])
# Get router1 and router3
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
router_ids=[router_ids[0],
router_ids[2]])
self.assertEqual(2, len(ret_a))
self.assertIn(router_ids[0], [r['id'] for r in ret_a])
self.assertIn(router_ids[2], [r['id'] for r in ret_a])
def test_router_auto_schedule_for_specified_routers(self):
def _sync_router_with_ids(router_ids, exp_synced, exp_hosted, host_id):
ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
router_ids=router_ids)
self.assertEqual(exp_synced, len(ret_a))
for r in router_ids:
self.assertIn(r, [r['id'] for r in ret_a])
host_routers = self._list_routers_hosted_by_l3_agent(host_id)
num_host_routers = len(host_routers['routers'])
self.assertEqual(exp_hosted, num_host_routers)
l3_rpc = l3_rpc_base.L3RpcCallbackMixin()
self._register_agent_states()
hosta_id = self._get_agent_id(constants.AGENT_TYPE_L3, L3_HOSTA)
with contextlib.nested(self.router(), self.router(),
self.router(), self.router()) as routers:
router_ids = [r['router']['id'] for r in routers]
# Sync router1 (router1 is scheduled)
_sync_router_with_ids([router_ids[0]], 1, 1, hosta_id)
# Sync router1 only (no router is scheduled)
_sync_router_with_ids([router_ids[0]], 1, 1, hosta_id)
# Schedule router2
_sync_router_with_ids([router_ids[1]], 1, 2, hosta_id)
# Sync router2 and router4 (router4 is scheduled)
_sync_router_with_ids([router_ids[1], router_ids[3]],
2, 3, hosta_id)
# Sync all routers (router3 is scheduled)
_sync_router_with_ids(router_ids, 4, 4, hosta_id)
def test_router_schedule_with_candidates(self):
l3_hosta = {
'binary': 'neutron-l3-agent',