diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index 497546725a8..ada1c799aae 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -460,8 +460,11 @@ def _notify_port_delete(event, resource, trigger, **kwargs): service_constants.L3_ROUTER_NAT) l3plugin.update_arp_entry_for_dvr_service_port(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): diff --git a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py index dd2c70435f1..c15c42deab7 100644 --- a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py +++ b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py @@ -16,6 +16,7 @@ import mock from neutron.api.v2 import attributes from neutron.common import constants +from neutron import context from neutron.extensions import external_net from neutron.extensions import portbindings from neutron.tests.common import helpers @@ -509,4 +510,52 @@ 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_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=constants.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': {portbindings.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[ + constants.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) diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index 80a5db52b25..db991108ad5 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py @@ -1029,7 +1029,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.update_arp_entry_for_dvr_service_port.call_count) l3plugin.dvr_handle_new_service_port.assert_called_once_with( @@ -1078,7 +1078,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): self.assertFalse( l3plugin.dvr_handle_new_service_port.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() @@ -1103,7 +1103,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): 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_handle_new_service_port(self): port = { @@ -1231,197 +1231,6 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase): r1['id']) self.assertEqual(len(sub_ids), 0) - def _create_port(self, port_name, tenant_id, host, subnet_id, ip_address, - status='ACTIVE', - device_owner=DEVICE_OWNER_COMPUTE_NOVA): - return { - 'id': port_name + '-port-id', - 'tenant_id': tenant_id, - 'device_id': port_name, - 'device_owner': device_owner, - 'status': status, - portbindings.HOST_ID: host, - 'fixed_ips': [ - { - 'subnet_id': subnet_id, - 'ip_address': ip_address - } - ] - } - - def test_dvr_deletens_if_no_port_no_routers(self): - # Delete a vm port, the port subnet has no router interface. - vm_tenant_id = 'tenant-1' - my_context = n_context.Context('user-1', vm_tenant_id, is_admin=False) - vm_port_host = 'compute-node-1' - - vm_port = self._create_port( - 'deleted-vm', vm_tenant_id, vm_port_host, - 'shared-subnet', '10.10.10.3', - status='INACTIVE') - - vm_port_id = vm_port['id'] - fakePortDB = FakePortDB([vm_port]) - - 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_ports', side_effect=fakePortDB.get_ports),\ - mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.' - 'get_port', return_value=vm_port): - - routers = self.dut.dvr_deletens_if_no_port(my_context, vm_port_id) - self.assertEqual([], routers) - 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'] - - fakePortDB = FakePortDB([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.db.db_base_plugin_v2.NeutronDbPluginV2.' - '_get_ports_query'),\ - 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,\ - mock.patch.object(self.dut, - 'check_dvr_serviceable_ports_on_host', - return_value=True): - - 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): - class FakeAgent(object): - def __init__(self, id, host, agent_type): - self.id = id - self.host = host - self.agent_type = agent_type - - my_context = n_context.Context('user-1', vm_tenant, is_admin=False) - shared_subnet_id = '80947d4a-fbc8-484b-9f92-623a6bfcf3e0', - vm_port_host = 'compute-node-1' - - router_id = 'dvr-router' - dvr_port = self._create_port( - router_id, router_tenant, vm_port_host, - shared_subnet_id, '10.10.10.1', - device_owner=constants.DEVICE_OWNER_DVR_INTERFACE) - dvr_port_id = dvr_port['id'] - - deleted_vm_port = self._create_port( - 'deleted-vm', vm_tenant, vm_port_host, - shared_subnet_id, '10.10.10.3', - status='INACTIVE') - deleted_vm_port_id = deleted_vm_port['id'] - - fakePortDB = FakePortDB([dvr_port, deleted_vm_port]) - - dvr_port_binding = { - 'port_id': dvr_port_id, 'host': vm_port_host - } - - agent_id = 'l3-agent-on-compute-node-1' - l3_agent_on_vm_host = FakeAgent(agent_id, - vm_port_host, - constants.AGENT_TYPE_L3) - - 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=dvr_port_binding) as\ - mock_get_dvr_port_binding_by_host,\ - mock.patch('neutron.db.agents_db.AgentDbMixin.' - '_get_agent_by_type_and_host', - return_value=l3_agent_on_vm_host),\ - mock.patch.object(self.dut, - 'check_dvr_serviceable_ports_on_host', - return_value=False): - - routers = self.dut.dvr_deletens_if_no_port( - my_context, deleted_vm_port_id) - - expected_router = { - 'router_id': router_id, - 'host': vm_port_host, - 'agent_id': agent_id - } - self.assertEqual([expected_router], routers) - - mock_get_port_binding_host.assert_called_once_with( - self.adminContext.session, deleted_vm_port_id) - self.assertTrue(mock_get_ports.called) - mock_get_dvr_port_binding_by_host.assert_called_once_with( - my_context.session, dvr_port_id, vm_port_host) - - def test_dvr_deletens_if_no_ports_delete_admin_routers(self): - # test to see whether the last VM using a router created - # by the admin will be unscheduled on the compute node - self._test_dvr_deletens_if_no_ports_delete_routers( - 'tenant-1', 'admin-tenant') - - def test_dvr_deletens_if_no_ports_delete_tenant_routers(self): - # test to see whether the last VM using a tenant's private - # router will be unscheduled on the compute node - self._test_dvr_deletens_if_no_ports_delete_routers( - 'tenant-1', 'tenant-1') - def _prepare_schedule_snat_tests(self): agent = agents_db.Agent() agent.admin_state_up = True