x/vmware-nsx
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 55b1922355
commit b90167ca6c
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) topic=topic, default_version=self.BASE_RPC_API_VERSION)
self.host = host 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.""" """Make a remote process call to retrieve the sync data for routers."""
return self.call(context, return self.call(context,
self.make_msg('sync_routers', host=self.host, self.make_msg('sync_routers', host=self.host,
fullsync=fullsync,
router_ids=router_ids), router_ids=router_ids),
topic=self.topic) 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, result = self.auto_schedule_routers(context,
agent_db.host, agent_db.host,
router_id) [router_id])
if not result: if not result:
raise l3agentscheduler.RouterSchedulingFailed( raise l3agentscheduler.RouterSchedulingFailed(
router_id=router_id, agent_id=id) router_id=router_id, agent_id=id)
@ -166,7 +166,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
return {'routers': []} return {'routers': []}
def list_active_sync_routers_on_active_l3_agent( 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( agent = self._get_agent_by_type_and_host(
context, constants.AGENT_TYPE_L3, host) context, constants.AGENT_TYPE_L3, host)
if not agent.admin_state_up: if not agent.admin_state_up:
@ -174,9 +174,12 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
query = context.session.query(RouterL3AgentBinding.router_id) query = context.session.query(RouterL3AgentBinding.router_id)
query = query.filter( query = query.filter(
RouterL3AgentBinding.l3_agent_id == agent.id) 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] router_ids = [item[0] for item in query]
if router_ids: if router_ids:
return self.get_sync_data(context, router_ids=router_ids, return self.get_sync_data(context, router_ids=router_ids,
@ -272,10 +275,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
candidates.append(l3_agent) candidates.append(l3_agent)
return candidates return candidates
def auto_schedule_routers(self, context, host, router_id): def auto_schedule_routers(self, context, host, router_ids):
if self.router_scheduler: if self.router_scheduler:
return self.router_scheduler.auto_schedule_routers( return self.router_scheduler.auto_schedule_routers(
self, context, host, router_id) self, context, host, router_ids)
def schedule_router(self, context, router): def schedule_router(self, context, router):
if self.router_scheduler: 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. """Sync routers according to filters to a specific agent.
@param context: contain user information @param context: contain user information
@param kwargs: host, or router_id @param kwargs: host, router_ids
@return: a list of routers @return: a list of routers
with their interfaces and floating_ips with their interfaces and floating_ips
""" """
router_id = kwargs.get('router_id') router_ids = kwargs.get('router_ids')
host = kwargs.get('host') host = kwargs.get('host')
context = neutron_context.get_admin_context() context = neutron_context.get_admin_context()
plugin = manager.NeutronManager.get_plugin() plugin = manager.NeutronManager.get_plugin()
if utils.is_extension_supported( if utils.is_extension_supported(
plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS): plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):
if cfg.CONF.router_auto_schedule: 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( routers = plugin.list_active_sync_routers_on_active_l3_agent(
context, host, router_id) context, host, router_ids)
else: 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"), LOG.debug(_("Routers returned to l3 agent:\n %s"),
jsonutils.dumps(routers, indent=5)) jsonutils.dumps(routers, indent=5))
return routers return routers

52
neutron/scheduler/l3_agent_scheduler.py
View File

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