From: Hong Hui Xiao <xiaohhui@cn.ibm.com>
Date: Tue, 8 Dec 2015 06:17:54 +0000 (-0500)
Subject: Separate the command for replace_port to delete and add
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=8b7e5997dae54a03ad2850c43b7070bc00c90273;p=openstack-build%2Fneutron-build.git

Separate the command for replace_port to delete and add

When a port has been added to router namespace, trying to replace the
port by adding del-port and add-port in one command, will not bring
the new port to kernel. Even if the port is updated in ovs db and can
be found on br-int, system can't see the new port. This will break
the following actions, which will manipulate the new port by ip
commands. A mail list has been filed to discuss this issue at [1].

The problem here will break the scenario that namespace is deleted
unexpectedly, and l3-agent tries to rebuild the namespace at restart.

Separating replace_port to two commands: del-port and add-port,
matches the original logic and has been verified that it can resolve
the problem here.

[1] http://openvswitch.org/pipermail/discuss/2015-December/019667.html

Change-Id: If36bcf5a0cccb667f3087aea1e9ea9f20eb3a563
Closes-Bug: #1519926
---

diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py
index dee22132d..a542bb1c0 100644
--- a/neutron/agent/common/ovs_lib.py
+++ b/neutron/agent/common/ovs_lib.py
@@ -230,8 +230,12 @@ class OVSBridge(BaseOVS):
 
     def replace_port(self, port_name, *interface_attr_tuples):
         """Replace existing port or create it, and configure port interface."""
+
+        # NOTE(xiaohhui): If del_port is inside the transaction, there will
+        # only be one command for replace_port. This will cause the new port
+        # not be found by system, which will lead to Bug #1519926.
+        self.ovsdb.del_port(port_name).execute()
         with self.ovsdb.transaction() as txn:
-            txn.add(self.ovsdb.del_port(port_name))
             txn.add(self.ovsdb.add_port(self.br_name, port_name,
                                         may_exist=False))
             if interface_attr_tuples:
diff --git a/neutron/tests/functional/agent/l3/test_legacy_router.py b/neutron/tests/functional/agent/l3/test_legacy_router.py
index 2f075970d..389237fb8 100644
--- a/neutron/tests/functional/agent/l3/test_legacy_router.py
+++ b/neutron/tests/functional/agent/l3/test_legacy_router.py
@@ -18,6 +18,7 @@ import mock
 
 from neutron.agent.l3 import namespace_manager
 from neutron.agent.l3 import namespaces
+from neutron.agent.linux import ip_lib
 from neutron.callbacks import events
 from neutron.callbacks import registry
 from neutron.callbacks import resources
@@ -73,6 +74,27 @@ class L3AgentTestCase(framework.L3AgentTestFramework):
         self._router_lifecycle(enable_ha=False, dual_stack=True,
                                v6_ext_gw_with_sub=False)
 
+    def test_legacy_router_ns_rebuild(self):
+        router_info = self.generate_router_info(False)
+        router = self.manage_router(self.agent, router_info)
+        gw_port = router.router['gw_port']
+        gw_inf_name = router.get_external_device_name(gw_port['id'])
+        gw_device = ip_lib.IPDevice(gw_inf_name, namespace=router.ns_name)
+        router_ports = [gw_device]
+        for i_port in router_info.get(l3_constants.INTERFACE_KEY, []):
+            interface_name = router.get_internal_device_name(i_port['id'])
+            router_ports.append(
+                ip_lib.IPDevice(interface_name, namespace=router.ns_name))
+
+        namespaces.Namespace.delete(router.router_namespace)
+
+        # l3 agent should be able to rebuild the ns when it is deleted
+        self.manage_router(self.agent, router_info)
+        # Assert the router ports are there in namespace
+        self.assertTrue(all([port.exists() for port in router_ports]))
+
+        self._delete_router(self.agent, router.router_id)
+
     def test_conntrack_disassociate_fip_legacy_router(self):
         self._test_conntrack_disassociate_fip(ha=False)