openstack
/
neutron
Code Issues Proposed changes

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.

testcase test_dvr_deletens_if_no_ports_no_removeable_routers
was removed because this change renders it invalid.

See patch https://review.openstack.org/#/c/296851/

Closes-Bug: #1524908
Change-Id: I8c76bccdf495702e4c550df2eadec93c63e32120
(cherry picked from commit 521e1c9fac)
This commit is contained in:
Oleg Bondarev 2015-12-10 19:49:25 +03:00 committed by Adolfo Duarte
parent 4b1d8dca9b
commit 3a22335834
5 changed files with 101 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
neutron/db/l3_dvr_db.py
View File

@ -309,11 +309,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_ports_exist_on_l3agent(
context, l3_agent, 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_ports_exist_on_l3agent(
context, l3_agent, 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'], subnets[0]['id'],
[subnet['id'] for subnet in subnets])

13
neutron/db/l3_dvrscheduler_db.py
View File

@ -183,8 +183,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)
port_exists_on_subnet = False
@ -212,9 +221,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)

79
neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py
View File

@ -446,3 +446,82 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
def test_delete_floating_ip_agent_notification_non_dvr(self):
self._test_delete_floating_ip_agent_notification(dvr=False)
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=l3_const.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[
l3_const.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)

41
neutron/tests/unit/db/test_l3_dvr_db.py
View File

@ -460,47 +460,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_ports_exist_on_l3agent = 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_ports_exist_on_l3agent.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}

55
neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
View File

@ -1341,61 +1341,6 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
mock_get_port_binding_host.assert_called_once_with(
self.adminContext.session, vm_port_id)
def test_dvr_deletens_if_no_ports_no_removeable_routers(self):
# A VM port is deleted, but the router can't be unscheduled from the
# compute node because there is another VM port present.
vm_tenant_id = 'tenant-1'
my_context = n_context.Context('user-1', vm_tenant_id, is_admin=False)
shared_subnet_id = '80947d4a-fbc8-484b-9f92-623a6bfcf3e0',
vm_port_host = 'compute-node-1'
dvr_port = self._create_port(
'dvr-router', 'admin-tenant', vm_port_host,
shared_subnet_id, '10.10.10.1',
device_owner=constants.DEVICE_OWNER_DVR_INTERFACE)
deleted_vm_port = self._create_port(
'deleted-vm', vm_tenant_id, vm_port_host,
shared_subnet_id, '10.10.10.3',
status='INACTIVE')
deleted_vm_port_id = deleted_vm_port['id']
running_vm_port = self._create_port(
'running-vn', 'tenant-2', vm_port_host,
shared_subnet_id, '10.10.10.33')
fakePortDB = FakePortDB([running_vm_port, deleted_vm_port, dvr_port])
vm_port_binding = {
'port_id': deleted_vm_port_id,
'host': vm_port_host
}
with mock.patch.object(my_context,
'elevated',
return_value=self.adminContext),\
mock.patch(
'neutron.plugins.ml2.db.get_port_binding_host',
return_value=vm_port_host) as mock_get_port_binding_host,\
mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.'
'get_port', side_effect=fakePortDB.get_port),\
mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.'
'get_ports', side_effect=fakePortDB.get_ports) as\
mock_get_ports,\
mock.patch('neutron.plugins.ml2.db.'
'get_dvr_port_binding_by_host',
return_value=vm_port_binding) as\
mock_get_dvr_port_binding_by_host:
routers = self.dut.dvr_deletens_if_no_port(
my_context, deleted_vm_port_id)
self.assertEqual([], routers)
mock_get_port_binding_host.assert_called_once_with(
self.adminContext.session, deleted_vm_port_id)
self.assertTrue(mock_get_ports.called)
self.assertFalse(mock_get_dvr_port_binding_by_host.called)
def _test_dvr_deletens_if_no_ports_delete_routers(self,
vm_tenant,
router_tenant):