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. Depends-On: I40b8684f6ec9ddd31753f7bbbdb364d1c0ec838a Related-Bug: #1513678 Change-Id: I0f50dc1101b2013caf03a64a4f48e2d03ea87b26
This commit is contained in:
parent
a5b22858b8
commit
c2483b73c2
|
@ -462,23 +462,6 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
|
||||||
ports on the host, running the input l3agent.
|
ports on the host, running the input l3agent.
|
||||||
"""
|
"""
|
||||||
core_plugin = manager.NeutronManager.get_plugin()
|
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}}
|
filter = {'fixed_ips': {'subnet_id': subnet_ids}}
|
||||||
ports = core_plugin.get_ports(context, filters=filter)
|
ports = core_plugin.get_ports(context, filters=filter)
|
||||||
for port in ports:
|
for port in ports:
|
||||||
|
|
|
@ -470,12 +470,12 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
|
||||||
self.context, router['id'],
|
self.context, router['id'],
|
||||||
{'subnet_id': subnet['subnet']['id']})
|
{'subnet_id': subnet['subnet']['id']})
|
||||||
|
|
||||||
# since there are no vm ports on HOST, at this point the router
|
# since there are no vm ports on HOST, and the router
|
||||||
# should be scheduled to only dvr_snat agent
|
# 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(
|
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||||
self.context, router['id'])['agents']
|
self.context, router['id'])['agents']
|
||||||
self.assertEqual(1, len(agents))
|
self.assertEqual(0, len(agents))
|
||||||
self.assertEqual(self.l3_agent['id'], agents[0]['id'])
|
|
||||||
with mock.patch.object(self.l3_plugin,
|
with mock.patch.object(self.l3_plugin,
|
||||||
'_l3_rpc_notifier') as l3_notifier,\
|
'_l3_rpc_notifier') as l3_notifier,\
|
||||||
self.port(subnet=subnet,
|
self.port(subnet=subnet,
|
||||||
|
@ -486,16 +486,12 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
|
||||||
self.context, port['port']['id'],
|
self.context, port['port']['id'],
|
||||||
{'port': {portbindings.HOST_ID: HOST1}})
|
{'port': {portbindings.HOST_ID: HOST1}})
|
||||||
|
|
||||||
# now router should be scheduled to dvr_snat agent and
|
# now router should be scheduled to agent on HOST1
|
||||||
# dvr agent on host1
|
|
||||||
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||||
self.context, router['id'])['agents']
|
self.context, router['id'])['agents']
|
||||||
self.assertEqual(2, len(agents))
|
self.assertEqual(1, len(agents))
|
||||||
self.assertIn(dvr_agent1['id'],
|
self.assertEqual(dvr_agent1['id'], agents[0]['id'])
|
||||||
[agent['id'] for agent in agents])
|
# and notification should only be sent to the agent on HOST1
|
||||||
self.assertNotIn(dvr_agent2['id'],
|
|
||||||
[agent['id'] for agent in agents])
|
|
||||||
# and notification should only be sent to the agent on host1
|
|
||||||
l3_notifier.routers_updated_on_host.assert_called_once_with(
|
l3_notifier.routers_updated_on_host.assert_called_once_with(
|
||||||
self.context, {router['id']}, HOST1)
|
self.context, {router['id']}, HOST1)
|
||||||
self.assertFalse(l3_notifier.routers_updated.called)
|
self.assertFalse(l3_notifier.routers_updated.called)
|
||||||
|
@ -505,15 +501,11 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
|
||||||
self.core_plugin.update_port(
|
self.core_plugin.update_port(
|
||||||
self.context, port['port']['id'],
|
self.context, port['port']['id'],
|
||||||
{'port': {portbindings.HOST_ID: HOST2}})
|
{'port': {portbindings.HOST_ID: HOST2}})
|
||||||
# now router should be scheduled to dvr_snat agent and
|
# now router should only be scheduled to dvr agent on host2
|
||||||
# dvr agent on host2
|
|
||||||
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||||
self.context, router['id'])['agents']
|
self.context, router['id'])['agents']
|
||||||
self.assertEqual(2, len(agents))
|
self.assertEqual(1, len(agents))
|
||||||
self.assertIn(dvr_agent2['id'],
|
self.assertEqual(dvr_agent2['id'], agents[0]['id'])
|
||||||
[agent['id'] for agent in agents])
|
|
||||||
self.assertNotIn(dvr_agent1['id'],
|
|
||||||
[agent['id'] for agent in agents])
|
|
||||||
l3_notifier.routers_updated_on_host.assert_called_once_with(
|
l3_notifier.routers_updated_on_host.assert_called_once_with(
|
||||||
self.context, {router['id']}, HOST2)
|
self.context, {router['id']}, HOST2)
|
||||||
l3_notifier.router_removed_from_agent.assert_called_once_with(
|
l3_notifier.router_removed_from_agent.assert_called_once_with(
|
||||||
|
|
|
@ -1126,9 +1126,9 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
|
||||||
# add router interface first
|
# add router interface first
|
||||||
self.l3plugin.add_router_interface(self.adminContext, r['id'],
|
self.l3plugin.add_router_interface(self.adminContext, r['id'],
|
||||||
{'subnet_id': s_int['subnet']['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'])
|
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
|
# check that snat is not scheduled as router is not connected to
|
||||||
# external network
|
# external network
|
||||||
snat_agents = self.l3plugin.get_snat_bindings(
|
snat_agents = self.l3plugin.get_snat_bindings(
|
||||||
|
@ -1138,9 +1138,9 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
|
||||||
# connect router to external network
|
# connect router to external network
|
||||||
self.l3plugin.update_router(self.adminContext, r['id'],
|
self.l3plugin.update_router(self.adminContext, r['id'],
|
||||||
{'router': {'external_gateway_info': {'network_id': net_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'])
|
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
|
# now snat portion should be scheduled as router is connected
|
||||||
# to external network
|
# to external network
|
||||||
snat_agents = self.l3plugin.get_snat_bindings(
|
snat_agents = self.l3plugin.get_snat_bindings(
|
||||||
|
@ -1165,7 +1165,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
|
||||||
self.l3plugin.schedule_router(
|
self.l3plugin.schedule_router(
|
||||||
self.adminContext, r['id'])
|
self.adminContext, r['id'])
|
||||||
l3agents = self._list_l3_agents_hosting_router(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(
|
csnat_agent_host = self.l3plugin.get_snat_bindings(
|
||||||
self.adminContext, [r['id']])[0]['l3_agent']['host']
|
self.adminContext, [r['id']])[0]['l3_agent']['host']
|
||||||
self._take_down_agent_and_run_reschedule(csnat_agent_host)
|
self._take_down_agent_and_run_reschedule(csnat_agent_host)
|
||||||
|
@ -1194,24 +1194,27 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
|
||||||
self.adminContext, r['id'])
|
self.adminContext, r['id'])
|
||||||
l3agents = self.l3plugin.list_l3_agents_hosting_router(
|
l3agents = self.l3plugin.list_l3_agents_hosting_router(
|
||||||
self.adminContext, r['id'])
|
self.adminContext, r['id'])
|
||||||
self.assertEqual(2, len(l3agents['agents']))
|
self.assertEqual(1, len(l3agents['agents']))
|
||||||
csnat_agent = self.l3plugin.get_snat_bindings(
|
csnat_agent = self.l3plugin.get_snat_bindings(
|
||||||
self.adminContext, [r['id']])[0]['l3_agent']
|
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.l3plugin.remove_router_from_l3_agent(
|
||||||
self.adminContext, csnat_agent['id'], r['id'])
|
self.adminContext, csnat_agent['id'], r['id'])
|
||||||
|
|
||||||
l3agents = self.l3plugin.list_l3_agents_hosting_router(
|
l3agents = self.l3plugin.list_l3_agents_hosting_router(
|
||||||
self.adminContext, r['id'])
|
self.adminContext, r['id'])
|
||||||
self.assertEqual(1, len(l3agents['agents']))
|
self.assertEqual(0, len(l3agents['agents']))
|
||||||
self.assertFalse(self.l3plugin.get_snat_bindings(
|
self.assertFalse(self.l3plugin.get_snat_bindings(
|
||||||
self.adminContext, [r['id']]))
|
self.adminContext, [r['id']]))
|
||||||
|
|
||||||
self.l3plugin.add_router_to_l3_agent(
|
self.l3plugin.add_router_to_l3_agent(
|
||||||
self.adminContext, csnat_agent['id'], r['id'])
|
self.adminContext, csnat_agent['id'], r['id'])
|
||||||
|
|
||||||
l3agents = self._list_l3_agents_hosting_router(r['id'])
|
l3agents = self.l3plugin.list_l3_agents_hosting_router(
|
||||||
self.assertEqual(2, len(l3agents['agents']))
|
self.adminContext, r['id'])
|
||||||
|
self.assertEqual(1, len(l3agents['agents']))
|
||||||
new_csnat_agent = self.l3plugin.get_snat_bindings(
|
new_csnat_agent = self.l3plugin.get_snat_bindings(
|
||||||
self.adminContext, [r['id']])[0]['l3_agent']
|
self.adminContext, [r['id']])[0]['l3_agent']
|
||||||
self.assertEqual(csnat_agent['id'], new_csnat_agent['id'])
|
self.assertEqual(csnat_agent['id'], new_csnat_agent['id'])
|
||||||
|
|
|
@ -707,69 +707,6 @@ class L3SchedulerTestBaseMixin(object):
|
||||||
agent_list = [self.l3_dvr_snat_agent]
|
agent_list = [self.l3_dvr_snat_agent]
|
||||||
self._check_get_l3_agent_candidates(router, agent_list, HOST_DVR_SNAT)
|
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=[])
|
|
||||||
return l3_agent
|
|
||||||
|
|
||||||
def test_check_ports_exist_on_l3agent_no_subnets(self):
|
|
||||||
l3_agent = self._prepare_check_ports_exist_tests()
|
|
||||||
# no subnets
|
|
||||||
val = self.check_ports_exist_on_l3agent(
|
|
||||||
self.adminContext, l3_agent, [])
|
|
||||||
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.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], [subnet['id']])
|
|
||||||
self.assertTrue(val)
|
|
||||||
self.assertFalse(self.plugin.get_ports.called)
|
|
||||||
|
|
||||||
def test_check_ports_exist_on_l3agent_no_subnet_match(self):
|
|
||||||
l3_agent = 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, [])
|
|
||||||
self.assertFalse(val)
|
|
||||||
|
|
||||||
def test_check_ports_exist_on_l3agent_subnet_match(self):
|
|
||||||
l3_agent = self._prepare_check_ports_exist_tests()
|
|
||||||
# matching subnet
|
|
||||||
port = {'subnet_id': str(uuid.uuid4()),
|
|
||||||
portbindings.HOST_ID: 'host_1',
|
|
||||||
'device_owner': constants.DEVICE_OWNER_COMPUTE_PREFIX,
|
|
||||||
'id': 1234}
|
|
||||||
subnet = {'id': str(uuid.uuid4()),
|
|
||||||
'enable_dhcp': False}
|
|
||||||
self.plugin.get_ports.return_value = [port]
|
|
||||||
self.plugin.get_subnet = mock.Mock(return_value=subnet)
|
|
||||||
val = self.check_ports_exist_on_l3agent(self.adminContext,
|
|
||||||
l3_agent, [port['subnet_id']])
|
|
||||||
self.assertTrue(val)
|
|
||||||
|
|
||||||
def test_get_l3_agents_hosting_routers(self):
|
def test_get_l3_agents_hosting_routers(self):
|
||||||
agent = helpers.register_l3_agent('host_6')
|
agent = helpers.register_l3_agent('host_6')
|
||||||
router = self._make_router(self.fmt,
|
router = self._make_router(self.fmt,
|
||||||
|
|
Loading…
Reference in New Issue