Do not remove router from dvr_snat agents on dvr port deletion
When dvr serviceable port is deleted/migrated, dvr callback checks if there are any more dvr serviceable ports on the host and if there are no - removes the router from the agent on that host. To prevent snat namespace removal we need to check SNAT bindings and not remove router from l3 agent hosting SNAT. Closes-Bug: #1524908 Change-Id: I8c76bccdf495702e4c550df2eadec93c63e32120
This commit is contained in:
parent
d6d43b32ca
commit
521e1c9fac
|
@ -368,11 +368,19 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
|||
subnet_ids = plugin.get_subnet_ids_on_router(
|
||||
context, router['id'])
|
||||
if subnet_ids:
|
||||
binding_table = l3_dvrsched_db.CentralizedSnatL3AgentBinding
|
||||
snat_binding = context.session.query(binding_table).filter_by(
|
||||
router_id=router['id']).first()
|
||||
for l3_agent in l3_agents:
|
||||
if not plugin.check_dvr_serviceable_ports_on_host(
|
||||
context, l3_agent['host'], subnet_ids):
|
||||
plugin.remove_router_from_l3_agent(
|
||||
context, l3_agent['id'], router['id'])
|
||||
is_this_snat_agent = (
|
||||
snat_binding and
|
||||
snat_binding.l3_agent_id == l3_agent['id'])
|
||||
if (is_this_snat_agent or
|
||||
plugin.check_dvr_serviceable_ports_on_host(
|
||||
context, l3_agent['host'], subnet_ids)):
|
||||
continue
|
||||
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'], subnet['id'],
|
||||
[subnet['id']])
|
||||
|
|
|
@ -178,8 +178,17 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
|
|||
'on host %(host)s', {'port': port_id,
|
||||
'host': port_host})
|
||||
return []
|
||||
agent = self._get_agent_by_type_and_host(
|
||||
context, n_const.AGENT_TYPE_L3, port_host)
|
||||
removed_router_info = []
|
||||
for router_id in router_ids:
|
||||
snat_binding = context.session.query(
|
||||
CentralizedSnatL3AgentBinding).filter_by(
|
||||
router_id=router_id).filter_by(
|
||||
l3_agent_id=agent.id).first()
|
||||
if snat_binding:
|
||||
# not removing from the agent hosting SNAT for the router
|
||||
continue
|
||||
subnet_ids = self.get_subnet_ids_on_router(admin_context,
|
||||
router_id)
|
||||
if self.check_dvr_serviceable_ports_on_host(
|
||||
|
@ -199,9 +208,7 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
|
|||
# unbind this port from router
|
||||
dvr_binding['router_id'] = None
|
||||
dvr_binding.update(dvr_binding)
|
||||
agent = self._get_agent_by_type_and_host(context,
|
||||
n_const.AGENT_TYPE_L3,
|
||||
port_host)
|
||||
|
||||
info = {'router_id': router_id, 'host': port_host,
|
||||
'agent_id': str(agent.id)}
|
||||
removed_router_info.append(info)
|
||||
|
|
|
@ -654,3 +654,82 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
|
|||
topic=topics.L3_AGENT,
|
||||
version='1.1')]
|
||||
mock_prepare.assert_has_calls(expected, any_order=True)
|
||||
|
||||
def test_router_is_not_removed_from_snat_agent_on_interface_removal(self):
|
||||
"""Check that dvr router is not removed from l3 agent hosting
|
||||
SNAT for it on router interface removal
|
||||
"""
|
||||
router = self._create_router()
|
||||
kwargs = {'arg_list': (external_net.EXTERNAL,),
|
||||
external_net.EXTERNAL: True}
|
||||
host = self.l3_agent['host']
|
||||
with self.subnet() as subnet,\
|
||||
self.network(**kwargs) as ext_net,\
|
||||
self.subnet(network=ext_net, cidr='20.0.0.0/24'):
|
||||
self.l3_plugin._update_router_gw_info(
|
||||
self.context, router['id'],
|
||||
{'network_id': ext_net['network']['id']})
|
||||
self.l3_plugin.add_router_interface(
|
||||
self.context, router['id'],
|
||||
{'subnet_id': subnet['subnet']['id']})
|
||||
|
||||
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||
self.context, router['id'])
|
||||
self.assertEqual(1, len(agents['agents']))
|
||||
csnat_agent_host = self.l3_plugin.get_snat_bindings(
|
||||
self.context, [router['id']])[0]['l3_agent']['host']
|
||||
self.assertEqual(host, csnat_agent_host)
|
||||
with mock.patch.object(self.l3_plugin,
|
||||
'_l3_rpc_notifier') as l3_notifier:
|
||||
self.l3_plugin.remove_router_interface(
|
||||
self.context, router['id'],
|
||||
{'subnet_id': subnet['subnet']['id']})
|
||||
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||
self.context, router['id'])
|
||||
self.assertEqual(1, len(agents['agents']))
|
||||
self.assertFalse(l3_notifier.router_removed_from_agent.called)
|
||||
|
||||
def test_router_is_not_removed_from_snat_agent_on_dhcp_port_deletion(self):
|
||||
"""Check that dvr router is not removed from l3 agent hosting
|
||||
SNAT for it on DHCP port removal
|
||||
"""
|
||||
router = self._create_router()
|
||||
kwargs = {'arg_list': (external_net.EXTERNAL,),
|
||||
external_net.EXTERNAL: True}
|
||||
with self.network(**kwargs) as ext_net,\
|
||||
self.subnet(network=ext_net),\
|
||||
self.subnet(cidr='20.0.0.0/24') as subnet,\
|
||||
self.port(subnet=subnet,
|
||||
device_owner=constants.DEVICE_OWNER_DHCP) as port:
|
||||
self.core_plugin.update_port(
|
||||
self.context, port['port']['id'],
|
||||
{'port': {'binding:host_id': self.l3_agent['host']}})
|
||||
self.l3_plugin._update_router_gw_info(
|
||||
self.context, router['id'],
|
||||
{'network_id': ext_net['network']['id']})
|
||||
self.l3_plugin.add_router_interface(
|
||||
self.context, router['id'],
|
||||
{'subnet_id': subnet['subnet']['id']})
|
||||
|
||||
# router should be scheduled to the dvr_snat l3 agent
|
||||
csnat_agent_host = self.l3_plugin.get_snat_bindings(
|
||||
self.context, [router['id']])[0]['l3_agent']['host']
|
||||
self.assertEqual(self.l3_agent['host'], csnat_agent_host)
|
||||
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||
self.context, router['id'])
|
||||
self.assertEqual(1, len(agents['agents']))
|
||||
self.assertEqual(self.l3_agent['id'], agents['agents'][0]['id'])
|
||||
|
||||
notifier = self.l3_plugin.agent_notifiers[
|
||||
constants.AGENT_TYPE_L3]
|
||||
with mock.patch.object(
|
||||
notifier, 'router_removed_from_agent') as remove_mock:
|
||||
self._delete('ports', port['port']['id'])
|
||||
# now when port is deleted the router still has external
|
||||
# gateway and should still be scheduled to the snat agent
|
||||
agents = self.l3_plugin.list_l3_agents_hosting_router(
|
||||
self.context, router['id'])
|
||||
self.assertEqual(1, len(agents['agents']))
|
||||
self.assertEqual(self.l3_agent['id'],
|
||||
agents['agents'][0]['id'])
|
||||
self.assertFalse(remove_mock.called)
|
||||
|
|
|
@ -479,47 +479,6 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
|||
fip, floatingip, router))
|
||||
self.assertFalse(create_fip.called)
|
||||
|
||||
def test_remove_router_interface_delete_router_l3agent_binding(self):
|
||||
interface_info = {'subnet_id': '123'}
|
||||
router = mock.MagicMock()
|
||||
router.extra_attributes.distributed = True
|
||||
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_dvr_serviceable_ports_on_host = mock.Mock(
|
||||
return_value=False)
|
||||
plugin.remove_router_from_l3_agent = mock.Mock(
|
||||
return_value=None)
|
||||
with mock.patch.object(self.mixin, '_get_router') as grtr,\
|
||||
mock.patch.object(self.mixin, '_get_device_owner') as gdev,\
|
||||
mock.patch.object(self.mixin,
|
||||
'_remove_interface_by_subnet') as rmintf,\
|
||||
mock.patch.object(
|
||||
self.mixin,
|
||||
'delete_csnat_router_interface_ports') as delintf,\
|
||||
mock.patch.object(manager.NeutronManager,
|
||||
'get_service_plugins') as gplugin,\
|
||||
mock.patch.object(self.mixin,
|
||||
'_make_router_interface_info') as mkintf,\
|
||||
mock.patch.object(self.mixin,
|
||||
'notify_router_interface_action') as notify:
|
||||
grtr.return_value = router
|
||||
gdev.return_value = mock.Mock()
|
||||
rmintf.return_value = (mock.MagicMock(), mock.MagicMock())
|
||||
mkintf.return_value = mock.Mock()
|
||||
gplugin.return_value = {plugin_const.L3_ROUTER_NAT: plugin}
|
||||
delintf.return_value = None
|
||||
notify.return_value = None
|
||||
|
||||
self.mixin.manager = manager
|
||||
self.mixin.remove_router_interface(
|
||||
self.ctx, mock.Mock(), interface_info)
|
||||
self.assertTrue(plugin.get_l3_agents_hosting_routers.called)
|
||||
self.assertTrue(plugin.check_dvr_serviceable_ports_on_host.called)
|
||||
self.assertTrue(plugin.remove_router_from_l3_agent.called)
|
||||
|
||||
def test_remove_router_interface_csnat_ports_removal(self):
|
||||
router_dict = {'name': 'test_router', 'admin_state_up': True,
|
||||
'distributed': True}
|
||||
|
|
Loading…
Reference in New Issue