Merge "L3 agent: check router namespace existence before delete"

neutron/agent/l3/dvr_fip_ns.py

@@ -166,6 +166,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):
@@ -180,7 +185,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):
@@ -98,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

@@ -1016,9 +1016,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

@@ -205,9 +205,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()
@@ -217,6 +220,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):