L3 agent: check router namespace existence before delete

Router namespace absence may lead to infinite loop in l3 agent trying
to delete the router.
This patch adds checks before going into namespace to prevent RuntimeError
and following infinite loop.

Closes-Bug: #1606844
(cherry picked from commit 31a7feea6b60dac138b00652d2f16982a3b25f78)

Conflicts:
	neutron/agent/l3/namespaces.py
	neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
	neutron/tests/unit/agent/l3/test_router_info.py

Change-Id: Iae95ccb8eeb06d0fd5fc7d71e63408b3f843b371

6 files changed, 70 insertions, 8 deletions

diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py
index 6557cc267b..0ce50baa12 100644
--- a/neutron/agent/l3/dvr_fip_ns.py
+++ b/neutron/agent/l3/dvr_fip_ns.py
@@ -164,6 +164,11 @@ class FipNamespace(namespaces.Namespace):
 
     def delete(self):
         self.destroyed = True
+        self._delete()
+        self.agent_gateway_port = None
+
+    @namespaces.check_ns_existence
+    def _delete(self):
         ip_wrapper = ip_lib.IPWrapper(namespace=self.name)
         for d in ip_wrapper.get_devices(exclude_loopback=True):
             if d.name.startswith(FIP_2_ROUTER_DEV_PREFIX):
@@ -178,7 +183,6 @@ class FipNamespace(namespaces.Namespace):
                                    bridge=ext_net_bridge,
                                    namespace=self.name,
                                    prefix=FIP_EXT_DEV_PREFIX)
-        self.agent_gateway_port = None
 
         # TODO(mrsmith): add LOG warn if fip count != 0
         LOG.debug('DVR: destroy fip namespace: %s', self.name)
diff --git a/neutron/agent/l3/dvr_snat_ns.py b/neutron/agent/l3/dvr_snat_ns.py
index 2e360cca3a..d3a911dda7 100644
--- a/neutron/agent/l3/dvr_snat_ns.py
+++ b/neutron/agent/l3/dvr_snat_ns.py
@@ -33,6 +33,7 @@ class SnatNamespace(namespaces.Namespace):
     def get_snat_ns_name(cls, router_id):
         return namespaces.build_ns_name(SNAT_NS_PREFIX, router_id)
 
+    @namespaces.check_ns_existence
     def delete(self):
         ns_ip = ip_lib.IPWrapper(namespace=self.name)
         for d in ns_ip.get_devices(exclude_loopback=True):
diff --git a/neutron/agent/l3/namespaces.py b/neutron/agent/l3/namespaces.py
index e2f2f3a5cf..05aef26dae 100644
--- a/neutron/agent/l3/namespaces.py
+++ b/neutron/agent/l3/namespaces.py
@@ -13,10 +13,13 @@
 #    under the License.
 #
 
+import functools
+
 from oslo_log import log as logging
+from oslo_utils import excutils
 
 from neutron.agent.linux import ip_lib
-from neutron.i18n import _LE
+from neutron.i18n import _LE, _LW
 
 LOG = logging.getLogger(__name__)
 
@@ -58,6 +61,25 @@ def get_id_from_ns_name(ns_name):
         return ns_name[dash_index + 1:]
 
 
+def check_ns_existence(f):
+    @functools.wraps(f)
+    def wrapped(self, *args, **kwargs):
+        if not self.exists():
+            LOG.warning(_LW('Namespace %(name)s does not exists. Skipping '
+                            '%(func)s'),
+                        {'name': self.name, 'func': f.__name__})
+            return
+        try:
+            return f(self, *args, **kwargs)
+        except RuntimeError:
+            with excutils.save_and_reraise_exception() as ctx:
+                if not self.exists():
+                    LOG.debug('Namespace %(name)s was concurrently deleted',
+                              self.name)
+                    ctx.reraise = False
+    return wrapped
+
+
 class Namespace(object):
 
     def __init__(self, name, agent_conf, driver, use_ipv6):
@@ -99,6 +121,7 @@ class RouterNamespace(Namespace):
     def _get_ns_name(cls, router_id):
         return build_ns_name(NS_PREFIX, router_id)
 
+    @check_ns_existence
     def delete(self):
         ns_ip = ip_lib.IPWrapper(namespace=self.name)
         for d in ns_ip.get_devices(exclude_loopback=True):
diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py
index eaadada665..e22bc93d74 100644
--- a/neutron/agent/l3/router_info.py
+++ b/neutron/agent/l3/router_info.py
@@ -744,9 +744,13 @@ class RouterInfo(object):
         :param agent: Passes the agent in order to send RPC messages.
         """
         LOG.debug("process router delete")
-        self._process_internal_ports(agent.pd)
-        agent.pd.sync_router(self.router['id'])
-        self._process_external_on_delete(agent)
+        if self.router_namespace.exists():
+            self._process_internal_ports(agent.pd)
+            agent.pd.sync_router(self.router['id'])
+            self._process_external_on_delete(agent)
+        else:
+            LOG.warning(_LW("Can't gracefully delete the router %s: "
+                            "no router namespace found."), self.router['id'])
 
     @common_utils.exception_logger()
     def process(self, agent):
diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
index f57b34a34e..b379b2258c 100644
--- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
+++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
@@ -203,9 +203,13 @@ class TestDvrFipNs(base.BaseTestCase):
         dev2.name = 'fg-aaaa'
         ip_wrapper.get_devices.return_value = [dev1, dev2]
 
-        self.conf.router_delete_namespaces = False
-
-        self.fip_ns.delete()
+        with mock.patch.object(self.fip_ns.ip_wrapper_root.netns,
+                               'delete') as delete,\
+                mock.patch.object(self.fip_ns.ip_wrapper_root.netns,
+                                  'exists', return_value=True) as exists:
+            self.fip_ns.delete()
+            exists.assert_called_once_with(self.fip_ns.name)
+            delete.assert_called_once_with(self.fip_ns.name)
 
         ext_net_bridge = self.conf.external_network_bridge
         ns_name = self.fip_ns.get_name()
@@ -215,6 +219,15 @@ class TestDvrFipNs(base.BaseTestCase):
                                                    namespace=ns_name)
         ip_wrapper.del_veth.assert_called_once_with('fpr-aaaa')
 
+    def test_destroy_no_namespace(self):
+        with mock.patch.object(self.fip_ns.ip_wrapper_root.netns,
+                               'delete') as delete,\
+                mock.patch.object(self.fip_ns.ip_wrapper_root.netns,
+                                  'exists', return_value=False) as exists:
+            self.fip_ns.delete()
+            exists.assert_called_once_with(self.fip_ns.name)
+            self.assertFalse(delete.called)
+
     @mock.patch.object(ip_lib, 'IPWrapper')
     @mock.patch.object(ip_lib, 'IPDevice')
     @mock.patch.object(ip_lib, 'device_exists')
diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py
index 63f1847dd8..9be9cac768 100644
--- a/neutron/tests/unit/agent/l3/test_router_info.py
+++ b/neutron/tests/unit/agent/l3/test_router_info.py
@@ -123,6 +123,23 @@ class TestRouterInfo(base.BaseTestCase):
                     'via', '10.100.10.30']]
         self._check_agent_method_called(expected)
 
+    def test_process_delete(self):
+        ri = router_info.RouterInfo(_uuid(), {}, **self.ri_kwargs)
+        ri.router = {'id': _uuid()}
+        with mock.patch.object(ri, '_process_internal_ports') as p_i_p,\
+            mock.patch.object(ri, '_process_external_on_delete') as p_e_o_d:
+            self.mock_ip.netns.exists.return_value = False
+            ri.process_delete(mock.Mock())
+            self.assertFalse(p_i_p.called)
+            self.assertFalse(p_e_o_d.called)
+
+            p_i_p.reset_mock()
+            p_e_o_d.reset_mock()
+            self.mock_ip.netns.exists.return_value = True
+            ri.process_delete(mock.Mock())
+            p_i_p.assert_called_once_with(mock.ANY)
+            p_e_o_d.assert_called_once_with(mock.ANY)
+
 
 class BasicRouterTestCaseFramework(base.BaseTestCase):
     def _create_router(self, router=None, **kwargs):