openstack
/
neutron
Code Issues Proposed changes

Use admin context when removing DVR router on vm port deletion 

In case non-admin tenant removes last VM on a shared network (owned
by admin) connected to a DVR router (also owned by admin) we need
to remove the router from the host where there are no more dvr
serviceable ports. Commit edbade4861
fixed logic that determines routers that should be removed from host.
However in order to actually remove the router we also need admin
context.

This was not caught by unit tests and one reason for that is so called
'mock everything' approach which is evil and generally useless.
This patch replaces unit tests with functional tests that we able
to catch the bug.

Closes-Bug: #1424096
Change-Id: Ia6cdf2294562c2a2727350c78eeab155097e0c33
(cherry picked from commit 96ba199d73)
This commit is contained in:
Oleg Bondarev 2015-12-15 17:58:51 +03:00 committed by Adolfo Duarte
parent 584b0eed17
commit 69a384a9af
3 changed files with 61 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
neutron/db/l3_dvrscheduler_db.py
View File

@ -487,8 +487,11 @@ def _notify_port_delete(event, resource, trigger, **kwargs):
service_constants.L3_ROUTER_NAT)
l3plugin.dvr_vmarp_table_update(context, port, "del")
for router in removed_routers:
# we need admin context in case a tenant removes the last dvr
# serviceable port on a shared network owned by admin, where router
# is also owned by admin
l3plugin.remove_router_from_l3_agent(
context, router['agent_id'], router['router_id'])
context.elevated(), router['agent_id'], router['router_id'])
def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):

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

@ -16,11 +16,15 @@ import mock
from neutron.api.v2 import attributes
from neutron.common import constants as l3_const
from neutron import context
from neutron.extensions import external_net
from neutron.tests.common import helpers
from neutron.tests.unit.plugins.ml2 import base as ml2_test_base
DEVICE_OWNER_COMPUTE = l3_const.DEVICE_OWNER_COMPUTE_PREFIX + 'fake'
class L3DvrTestCase(ml2_test_base.ML2TestFramework):
def setUp(self):
super(L3DvrTestCase, self).setUp()
@ -276,7 +280,7 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
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(
self.context, router['id'], HOST1)
mock.ANY, router['id'], HOST1)
def _test_create_floating_ip_agent_notification(self, dvr=True):
with self.subnet() as ext_subnet,\
@ -447,6 +451,55 @@ 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_remove_from_agent_on_vm_port_deletion(
self, non_admin_port=False):
# register l3 agent in dvr mode in addition to existing dvr_snat agent
HOST = 'host1'
non_admin_tenant = 'tenant1'
dvr_agent = helpers.register_l3_agent(
host=HOST, agent_mode=l3_const.L3_AGENT_MODE_DVR)
router = self._create_router()
with self.network(shared=True) as net, \
self.subnet(network=net) as subnet, \
self.port(subnet=subnet,
device_owner=DEVICE_OWNER_COMPUTE,
tenant_id=non_admin_tenant,
set_context=non_admin_port) as port:
self.core_plugin.update_port(
self.context, port['port']['id'],
{'port': {'binding:host_id': HOST}})
self.l3_plugin.add_router_interface(
self.context, router['id'],
{'subnet_id': subnet['subnet']['id']})
# router should be scheduled to agent on HOST
agents = self.l3_plugin.list_l3_agents_hosting_router(
self.context, router['id'])
self.assertEqual(1, len(agents['agents']))
self.assertEqual(dvr_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:
ctx = context.Context(
'', non_admin_tenant) if non_admin_port else self.context
self._delete('ports', port['port']['id'], neutron_context=ctx)
# now when port is deleted the router should be unscheduled
agents = self.l3_plugin.list_l3_agents_hosting_router(
self.context, router['id'])
self.assertEqual(0, len(agents['agents']))
remove_mock.assert_called_once_with(
mock.ANY, router['id'], HOST)
def test_router_remove_from_agent_on_vm_port_deletion(self):
self._test_router_remove_from_agent_on_vm_port_deletion()
def test_admin_router_remove_from_agent_on_vm_port_deletion(self):
self._test_router_remove_from_agent_on_vm_port_deletion(
non_admin_port=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

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

@ -1016,7 +1016,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
l3_dvrscheduler_db._notify_l3_agent_port_update(
'port', 'after_update', mock.ANY, **kwargs)
l3plugin.remove_router_from_l3_agent.assert_called_once_with(
self.adminContext, 'foo_agent', 'foo_id')
mock.ANY, 'foo_agent', 'foo_id')
self.assertEqual(2, l3plugin.dvr_vmarp_table_update.call_count)
l3plugin.dvr_update_router_addvm.assert_called_once_with(
self.adminContext, kwargs.get('port'))
@ -1061,7 +1061,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
self.assertFalse(l3plugin.dvr_update_router_addvm.called)
l3plugin.remove_router_from_l3_agent.assert_called_once_with(
self.adminContext, 'foo_agent', 'foo_id')
mock.ANY, 'foo_agent', 'foo_id')
def test__notify_port_delete(self):
plugin = manager.NeutronManager.get_plugin()
@ -1085,7 +1085,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase):
l3plugin.dvr_vmarp_table_update.assert_called_once_with(
self.adminContext, mock.ANY, 'del')
l3plugin.remove_router_from_l3_agent.assert_called_once_with(
self.adminContext, 'foo_agent', 'foo_id')
mock.ANY, 'foo_agent', 'foo_id')
def test_dvr_update_router_addvm(self):
port = {