Remove check on dhcp enabled subnets while scheduling dvr

In check_ports_exist_on_l3agent we have an optimization fix
that checks for the subnets associated with the router and if
the subnets have dhcp enabled we go ahead and create the
router if it is a dvr_snat agent.

This was introduced in liberty since we saw some race condition
in the gate with single node failures.
It may not be completely right, since the dhcp agents can
run on non dvr_snat nodes as well.

Based on recommendation from the reviews, and a recent upstream
patch that sends notification on port create, we would want to
remove this and monitor the situation.

This would reduce the load on check_ports_exist_on_l3agent for
non dvr_snat nodes.

Related-Bug: #1513678

Conflicts:

	neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py
	neutron/tests/unit/scheduler/test_l3_agent_scheduler.py

Change-Id: I0f50dc1101b2013caf03a64a4f48e2d03ea87b26
(cherry picked from commit c2483b73c2)
This commit is contained in:
Swaminathan Vasudevan 2015-11-25 15:15:17 -08:00 committed by Oleg Bondarev
parent ba9485d2cc
commit 96d4ab3de5
4 changed files with 24 additions and 126 deletions

View File

@ -463,23 +463,6 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
return False
core_plugin = manager.NeutronManager.get_plugin()
# NOTE(swami):Before checking for existence of dvr
# serviceable ports on the host managed by the l3
# agent, let's verify if at least one subnet has
# dhcp enabled. If so, then the host will have a
# dvr serviceable port, which is in fact the DHCP
# port.
# This optimization is valid assuming that the L3
# DVR_SNAT node will be the one hosting the DHCP
# Agent.
agent_mode = self._get_agent_mode(l3_agent)
for subnet_id in subnet_ids:
subnet_dict = core_plugin.get_subnet(context, subnet_id)
if (subnet_dict['enable_dhcp'] and (
agent_mode == constants.L3_AGENT_MODE_DVR_SNAT)):
return True
filter = {'fixed_ips': {'subnet_id': subnet_ids}}
ports = core_plugin.get_ports(context, filters=filter)
for port in ports:

View File

@ -237,12 +237,12 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
self.context, router['id'],
{'subnet_id': subnet['subnet']['id']})
# since there are no vm ports on HOST, at this point the router
# should be scheduled to only dvr_snat agent
# since there are no vm ports on HOST, and the router
# has no external gateway at this point the router
# should neither be scheduled to dvr nor to dvr_snat agents
agents = self.l3_plugin.list_l3_agents_hosting_router(
self.context, router['id'])['agents']
self.assertEqual(1, len(agents))
self.assertEqual(self.l3_agent['id'], agents[0]['id'])
self.assertEqual(0, len(agents))
with mock.patch.object(self.l3_plugin,
'_l3_rpc_notifier') as l3_notifier,\
self.port(subnet=subnet,
@ -253,16 +253,12 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
self.context, port['port']['id'],
{'port': {'binding:host_id': HOST1}})
# now router should be scheduled to dvr_snat agent and
# dvr agent on host1
# now router should be scheduled to agent on HOST1
agents = self.l3_plugin.list_l3_agents_hosting_router(
self.context, router['id'])['agents']
self.assertEqual(2, len(agents))
self.assertIn(dvr_agent1['id'],
[agent['id'] for agent in agents])
self.assertNotIn(dvr_agent2['id'],
[agent['id'] for agent in agents])
# and notification should only be sent to the agent on host1
self.assertEqual(1, len(agents))
self.assertEqual(dvr_agent1['id'], agents[0]['id'])
# and notification should only be sent to the agent on HOST1
l3_notifier.routers_updated_on_host.assert_called_once_with(
self.context, {router['id']}, HOST1)
self.assertFalse(l3_notifier.routers_updated.called)
@ -272,15 +268,11 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
self.core_plugin.update_port(
self.context, port['port']['id'],
{'port': {'binding:host_id': HOST2}})
# now router should be scheduled to dvr_snat agent and
# dvr agent on host2
# now router should only be scheduled to dvr agent on host2
agents = self.l3_plugin.list_l3_agents_hosting_router(
self.context, router['id'])['agents']
self.assertEqual(2, len(agents))
self.assertIn(dvr_agent2['id'],
[agent['id'] for agent in agents])
self.assertNotIn(dvr_agent1['id'],
[agent['id'] for agent in agents])
self.assertEqual(1, len(agents))
self.assertEqual(dvr_agent2['id'], agents[0]['id'])
l3_notifier.routers_updated_on_host.assert_called_once_with(
self.context, {router['id']}, HOST2)
l3_notifier.router_removed_from_agent.assert_called_once_with(

View File

@ -1122,9 +1122,9 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
# add router interface first
self.l3plugin.add_router_interface(self.adminContext, r['id'],
{'subnet_id': s_int['subnet']['id']})
# check that router is scheduled to both dvr_snat agents
# Check if the router is not scheduled to any of the agents
l3agents = self._list_l3_agents_hosting_router(r['id'])
self.assertEqual(2, len(l3agents['agents']))
self.assertEqual(0, len(l3agents['agents']))
# check that snat is not scheduled as router is not connected to
# external network
snat_agents = self.l3plugin.get_snat_bindings(
@ -1134,9 +1134,9 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
# connect router to external network
self.l3plugin.update_router(self.adminContext, r['id'],
{'router': {'external_gateway_info': {'network_id': net_id}}})
# router should still be scheduled to both dvr_snat agents
# router should still be scheduled to one of the dvr_snat agents
l3agents = self._list_l3_agents_hosting_router(r['id'])
self.assertEqual(2, len(l3agents['agents']))
self.assertEqual(1, len(l3agents['agents']))
# now snat portion should be scheduled as router is connected
# to external network
snat_agents = self.l3plugin.get_snat_bindings(
@ -1161,7 +1161,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
self.l3plugin.schedule_router(
self.adminContext, r['id'])
l3agents = self._list_l3_agents_hosting_router(r['id'])
self.assertEqual(2, len(l3agents['agents']))
self.assertEqual(1, len(l3agents['agents']))
csnat_agent_host = self.l3plugin.get_snat_bindings(
self.adminContext, [r['id']])[0]['l3_agent']['host']
self._take_down_agent_and_run_reschedule(csnat_agent_host)
@ -1190,24 +1190,27 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
self.adminContext, r['id'])
l3agents = self.l3plugin.list_l3_agents_hosting_router(
self.adminContext, r['id'])
self.assertEqual(2, len(l3agents['agents']))
self.assertEqual(1, len(l3agents['agents']))
csnat_agent = self.l3plugin.get_snat_bindings(
self.adminContext, [r['id']])[0]['l3_agent']
# NOTE: Removing the router from the l3_agent will
# remove all the namespace since there is no other
# serviceable ports in the node that requires it.
self.l3plugin.remove_router_from_l3_agent(
self.adminContext, csnat_agent['id'], r['id'])
l3agents = self.l3plugin.list_l3_agents_hosting_router(
self.adminContext, r['id'])
self.assertEqual(1, len(l3agents['agents']))
self.assertEqual(0, len(l3agents['agents']))
self.assertFalse(self.l3plugin.get_snat_bindings(
self.adminContext, [r['id']]))
self.l3plugin.add_router_to_l3_agent(
self.adminContext, csnat_agent['id'], r['id'])
l3agents = self._list_l3_agents_hosting_router(r['id'])
self.assertEqual(2, len(l3agents['agents']))
l3agents = self.l3plugin.list_l3_agents_hosting_router(
self.adminContext, r['id'])
self.assertEqual(1, len(l3agents['agents']))
new_csnat_agent = self.l3plugin.get_snat_bindings(
self.adminContext, [r['id']])[0]['l3_agent']
self.assertEqual(csnat_agent['id'], new_csnat_agent['id'])

View File

@ -698,86 +698,6 @@ class L3SchedulerTestBaseMixin(object):
agent_list = [self.l3_dvr_snat_agent]
self._check_get_l3_agent_candidates(router, agent_list, HOST_DVR_SNAT)
def _prepare_check_ports_exist_tests(self):
l3_agent = agents_db.Agent()
l3_agent.admin_state_up = True
l3_agent.host = 'host_1'
router = self._make_router(self.fmt,
tenant_id=str(uuid.uuid4()),
name='r2')
router['external_gateway_info'] = None
router['id'] = str(uuid.uuid4())
self.plugin.get_ports = mock.Mock(return_value=[])
self.get_subnet_ids_on_router = mock.Mock(return_value=[])
return l3_agent, router
def test_check_ports_exist_on_l3agent_no_subnets(self):
l3_agent, router = self._prepare_check_ports_exist_tests()
# no subnets
val = self.check_ports_exist_on_l3agent(self.adminContext,
l3_agent, router['id'])
self.assertFalse(val)
def test_check_ports_exist_on_l3agent_with_dhcp_enabled_subnets(self):
self._register_l3_dvr_agents()
router = self._make_router(self.fmt,
tenant_id=str(uuid.uuid4()),
name='r2')
router['external_gateway_info'] = None
router['id'] = str(uuid.uuid4())
router['distributed'] = True
agent_list = [self.l3_dvr_snat_agent]
subnet = {'id': str(uuid.uuid4()),
'enable_dhcp': True}
self.get_subnet_ids_on_router = mock.Mock(
return_value=[subnet['id']])
self.plugin.get_subnet = mock.Mock(return_value=subnet)
self.plugin.get_ports = mock.Mock()
val = self.check_ports_exist_on_l3agent(
self.adminContext, agent_list[0], router['id'])
self.assertTrue(val)
self.assertFalse(self.plugin.get_ports.called)
def test_check_ports_exist_on_l3agent_if_no_subnets_then_return(self):
l3_agent, router = self._prepare_check_ports_exist_tests()
with mock.patch.object(manager.NeutronManager,
'get_plugin') as getp:
getp.return_value = self.plugin
# no subnets and operation is remove_router_interface,
# so return immediately without calling get_ports
self.check_ports_exist_on_l3agent(self.adminContext,
l3_agent, router['id'])
self.assertFalse(self.plugin.get_ports.called)
def test_check_ports_exist_on_l3agent_no_subnet_match(self):
l3_agent, router = self._prepare_check_ports_exist_tests()
# no matching subnet
self.plugin.get_subnet_ids_on_router = mock.Mock(
return_value=[str(uuid.uuid4())])
val = self.check_ports_exist_on_l3agent(self.adminContext,
l3_agent, router['id'])
self.assertFalse(val)
def test_check_ports_exist_on_l3agent_subnet_match(self):
l3_agent, router = self._prepare_check_ports_exist_tests()
# matching subnet
port = {'subnet_id': str(uuid.uuid4()),
'binding:host_id': 'host_1',
'device_owner': 'compute:',
'id': 1234}
subnet = {'id': str(uuid.uuid4()),
'enable_dhcp': False}
self.plugin.get_ports.return_value = [port]
self.get_subnet_ids_on_router = mock.Mock(
return_value=[port['subnet_id']])
self.plugin.get_subnet = mock.Mock(return_value=subnet)
val = self.check_ports_exist_on_l3agent(self.adminContext,
l3_agent, router['id'])
self.assertTrue(val)
def test_get_l3_agents_hosting_routers(self):
agent = helpers.register_l3_agent('host_6')
router = self._make_router(self.fmt,