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

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index 687de998eae3786e215ce566be5c1d25beb3766b..2fda9adb459fdc621d021d1eb763e81cfe295c0a 100644 (file)
--- 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 fe81630a73caa3c3b72c7b68b24e9a775ac15081..619670055e394f3cd675c6f48828ad631972026b 100755 (executable)
--- 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 2d4daaa7c7e46fed05d5148e515bac1f96d8e250..1c0ee4d3f3f4679f870a5c5f1732346e435333fc 100644 (file)
--- 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)