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
Change-Id: Iae95ccb8eeb06d0fd5fc7d71e63408b3f843b371
(cherry picked from commit 31a7feea6b)

neutron/agent/l3/dvr_fip_ns.py

@@ -165,6 +165,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):
@@ -179,7 +184,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)

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):

neutron/agent/l3/namespaces.py

@@ -13,9 +13,12 @@
 #    under the License.
 #
 
-from oslo_log import log as logging
+import functools
 
-from neutron._i18n import _LE
+from oslo_log import log as logging
+from oslo_utils import excutils
+
+from neutron._i18n import _LE, _LW
 from neutron.agent.linux import ip_lib
 
 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):
@@ -82,6 +104,9 @@ class Namespace(object):
             msg = _LE('Failed trying to delete namespace: %s')
             LOG.exception(msg, self.name)
 
+    def exists(self):
+        return self.ip_wrapper_root.netns.exists(self.name)
+
 
 class RouterNamespace(Namespace):
 
@@ -95,6 +120,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):

neutron/agent/l3/router_info.py

@@ -959,9 +959,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):

neutron/tests/unit/agent/l3/test_dvr_fip_ns.py

@@ -204,9 +204,12 @@ class TestDvrFipNs(base.BaseTestCase):
         ip_wrapper.get_devices.return_value = [dev1, dev2]
 
         with mock.patch.object(self.fip_ns.ip_wrapper_root.netns,
-                               'delete') as delete:
+                               'delete') as delete,\
+                mock.patch.object(self.fip_ns.ip_wrapper_root.netns,
+                                  'exists', return_value=True) as exists:
             self.fip_ns.delete()
-            delete.assert_called_once_with(mock.ANY)
+            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()
@@ -216,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')
     def _test_create_rtr_2_fip_link(self, dev_exists, addr_exists,

neutron/tests/unit/agent/l3/test_router_info.py

@@ -166,6 +166,23 @@ class TestRouterInfo(base.BaseTestCase):
         self.assertEqual(new_mark_ids, new_ri.available_mark_ids)
         self.assertTrue(ri.available_mark_ids != new_ri.available_mark_ids)
 
+    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):