From: Assaf Muller <amuller@redhat.com>
Date: Fri, 13 Feb 2015 16:35:02 +0000 (-0500)
Subject: Delete qg device during DVR-SNAT router deletion
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a172e200685567a71844f65ae09d79635073a2f8;p=openstack-build%2Fneutron-build.git

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 687de998e..2fda9adb4 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 fe81630a7..619670055 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 2d4daaa7c..1c0ee4d3f 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)