From a172e200685567a71844f65ae09d79635073a2f8 Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Fri, 13 Feb 2015 11:35:02 -0500 Subject: [PATCH] Delete qg device during DVR-SNAT router deletion In the DVR SNAT case, the 'qg' device was not deleted because of patch: https://review.openstack.org/#/c/151882/ During functional testing, the device is deleted during the external bridge deletion. Because that happens after the SNAT namespace is already deleted, it can cause a kernel panic or ovs-vswitchd crash for certain OVS versions. Also added assertions that all router interfaces were properly cleaned up during functional testing, and enabled the unit tests to catch this type of error. Change-Id: Ica86a030860aa967d5b685b5bfb5546a85b2a809 Closes-Bug: #1418097 --- neutron/agent/l3/agent.py | 5 +++-- neutron/tests/functional/agent/test_l3_agent.py | 11 +++++++++++ neutron/tests/unit/test_l3_agent.py | 6 +++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 687de998eae..2fda9adb459 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -891,9 +891,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, def external_gateway_removed(self, ri, ex_gw_port, interface_name): if ri.router['distributed']: self.process_router_floating_ip_nat_rules(ri) - interface_name = self._get_external_device_interface_name( + to_fip_interface_name = self._get_external_device_interface_name( ri, ex_gw_port) - self.process_router_floating_ip_addresses(ri, interface_name) + self.process_router_floating_ip_addresses( + ri, to_fip_interface_name) for p in ri.internal_ports: internal_interface = self.get_internal_device_name(p['id']) self._snat_redirect_remove(ri, p, internal_interface) diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py index fe81630a73c..619670055e3 100755 --- a/neutron/tests/functional/agent/test_l3_agent.py +++ b/neutron/tests/functional/agent/test_l3_agent.py @@ -30,6 +30,7 @@ from neutron.agent import l3_agent as l3_agent_main from neutron.agent.linux import dhcp from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib +from neutron.agent.linux import ovs_lib from neutron.agent.metadata import agent as metadata_agent from neutron.common import config as common_config from neutron.common import constants as l3_constants @@ -255,6 +256,14 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase): for extra_route in router.router['routes']: self.assertIn(extra_route, routes) + def _assert_interfaces_deleted_from_ovs(self): + def assert_ovs_bridge_empty(bridge_name): + bridge = ovs_lib.OVSBridge(bridge_name, self.root_helper) + self.assertFalse(bridge.get_port_name_list()) + + assert_ovs_bridge_empty(self.agent.conf.ovs_integration_bridge) + assert_ovs_bridge_empty(self.agent.conf.external_network_bridge) + class L3AgentTestCase(L3AgentTestFramework): def test_observer_notifications_legacy_router(self): @@ -412,6 +421,7 @@ class L3AgentTestCase(L3AgentTestFramework): self._delete_router(self.agent, router.router_id) + self._assert_interfaces_deleted_from_ovs() self._assert_router_does_not_exist(router) if enable_ha: self.assertFalse(router.keepalived_manager.process.active) @@ -611,6 +621,7 @@ class TestDvrRouter(L3AgentTestFramework): self._assert_extra_routes(router) self._delete_router(self.agent, router.router_id) + self._assert_interfaces_deleted_from_ovs() self._assert_router_does_not_exist(router) def generate_dvr_router_info(self, enable_ha=False, enable_snat=False): diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 2d4daaa7c7e..1c0ee4d3f3f 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -475,7 +475,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): elif action == 'remove': self.device_exists.return_value = True agent.external_gateway_removed(ri, ex_gw_port, interface_name) - self.assertEqual(self.mock_driver.unplug.call_count, 1) + self.mock_driver.unplug.assert_called_once_with( + interface_name, + bridge=agent.conf.external_network_bridge, + namespace=mock.ANY, + prefix=mock.ANY) else: raise Exception("Invalid action %s" % action)