Ensure that l3 agent does not crash on restart

Fixes bug 1048108

The changeset also removed the namespace garbage collection from the port unplug. This
attempts to delete the namespace. This is problematic as there may be additional attributes
to the namespace that need to be dealt with.

Change-Id: I418a1ec4e9b65e5bea67ae84414019c3d6b54214

diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py
index bff54f834a9bc2d03b2d05f04de15b3abbc86eff..6a302b888f484ba16e40e646488d07a0a778bcfe 100644 (file)
--- a/quantum/agent/l3_agent.py
+++ b/quantum/agent/l3_agent.py
@@ -29,6 +29,7 @@ from quantum.agent.common import config
 from quantum.agent.linux import interface
 from quantum.agent.linux import ip_lib
 from quantum.agent.linux import iptables_manager
+from quantum.agent.linux import utils
 from quantum.db import l3_db
 from quantum.openstack.common import cfg
 from quantum.openstack.common import importutils
@@ -147,22 +148,19 @@ class L3NATAgent(object):
     def _destroy_router_namespace(self, namespace):
         ns_ip = ip_lib.IPWrapper(self.conf.root_helper,
                                  namespace=namespace)
-        for d in ns_ip.get_devices():
+        for d in ns_ip.get_devices(exclude_loopback=True):
             if d.name.startswith(INTERNAL_DEV_PREFIX):
                 # device is on default bridge
-                self.driver.unplug(d.name)
+                self.driver.unplug(d.name, namespace=namespace)
             elif d.name.startswith(EXTERNAL_DEV_PREFIX):
                 self.driver.unplug(d.name,
-                                   bridge=self.conf.external_network_bridge)
-        if self.conf.use_namespaces:
-            ns_ip.netns.delete(namespace)
+                                   bridge=self.conf.external_network_bridge,
+                                   namespace=namespace)
+        #(TODO) Address the failure for the deletion of the namespace
 
     def _create_router_namespace(self, ri):
             ip_wrapper_root = ip_lib.IPWrapper(self.conf.root_helper)
-            ip_wrapper_root.netns.add(ri.ns_name())
-
-            ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper,
-                                          namespace=ri.ns_name())
+            ip_wrapper = ip_wrapper_root.ensure_namespace(ri.ns_name())
             ip_wrapper.netns.execute(['sysctl', '-w', 'net.ipv4.ip_forward=1'])
 
     def daemon_loop(self):
@@ -364,10 +362,13 @@ class L3NATAgent(object):
         gw_ip = ex_gw_port['subnet']['gateway_ip']
         if ex_gw_port['subnet']['gateway_ip']:
             cmd = ['route', 'add', 'default', 'gw', gw_ip]
-            ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper,
-                                          namespace=ri.ns_name())
             if self.conf.use_namespaces:
-                ip_wrapper.netns.execute(cmd)
+                ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper,
+                                              namespace=ri.ns_name())
+                ip_wrapper.netns.execute(cmd, check_exit_code=False)
+            else:
+                utils.execute(cmd, check_exit_code=False,
+                              root_helper=self.conf.root_helper)
 
         for (c, r) in self.external_gateway_filter_rules():
             ri.iptables_manager.ipv4['filter'].add_rule(c, r)
@@ -384,7 +385,8 @@ class L3NATAgent(object):
                                 root_helper=self.conf.root_helper,
                                 namespace=ri.ns_name()):
             self.driver.unplug(interface_name,
-                               bridge=self.conf.external_network_bridge)
+                               bridge=self.conf.external_network_bridge,
+                               namespace=ri.ns_name())
 
         ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address']
         for c, r in self.external_gateway_filter_rules():
@@ -442,7 +444,7 @@ class L3NATAgent(object):
         if ip_lib.device_exists(interface_name,
                                 root_helper=self.conf.root_helper,
                                 namespace=ri.ns_name()):
-            self.driver.unplug(interface_name)
+            self.driver.unplug(interface_name, namespace=ri.ns_name())
 
         if ex_gw_port:
             ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address']

diff --git a/quantum/agent/linux/interface.py b/quantum/agent/linux/interface.py
index 8cf9a4dd9d903c948e4312b0a41a0139e9fa99e8..36017c05ebd6d2d2c63b0f05642a1dd62003c7cd 100644 (file)
--- a/quantum/agent/linux/interface.py
+++ b/quantum/agent/linux/interface.py
@@ -154,10 +154,6 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
         bridge = ovs_lib.OVSBridge(bridge, self.conf.root_helper)
         bridge.delete_port(device_name)
 
-        if namespace:
-            ip = ip_lib.IPWrapper(self.conf.root_helper, namespace)
-            ip.garbage_collect_namespace()
-
 
 class BridgeInterfaceDriver(LinuxInterfaceDriver):
     """Driver for creating bridge interfaces."""
@@ -200,10 +196,6 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
             LOG.error(_("Failed unplugging interface '%s'") %
                       device_name)
 
-        if namespace:
-            ip = ip_lib.IPWrapper(self.conf.root_helper, namespace)
-            ip.garbage_collect_namespace()
-
 
 class RyuInterfaceDriver(OVSInterfaceDriver):
     """Driver for creating a Ryu OVS interface."""

diff --git a/quantum/agent/linux/ip_lib.py b/quantum/agent/linux/ip_lib.py
index cdde30a9c3d72c0ca116cdea717d2766cdbb6d48..510dcec610f369c9650d0462abf0a162bc6fa181 100644 (file)
--- a/quantum/agent/linux/ip_lib.py
+++ b/quantum/agent/linux/ip_lib.py
@@ -338,7 +338,7 @@ class IpNetnsCommand(IpCommandBase):
     def delete(self, name):
         self._as_root('delete', name, use_root_namespace=True)
 
-    def execute(self, cmds, addl_env={}):
+    def execute(self, cmds, addl_env={}, check_exit_code=True):
         if not self._parent.root_helper:
             raise exceptions.SudoRequired()
         elif not self._parent.namespace:
@@ -347,7 +347,8 @@ class IpNetnsCommand(IpCommandBase):
             return utils.execute(
                 ['%s=%s' % pair for pair in addl_env.items()] +
                 ['ip', 'netns', 'exec', self._parent.namespace] + list(cmds),
-                root_helper=self._parent.root_helper)
+                root_helper=self._parent.root_helper,
+                check_exit_code=check_exit_code)
 
     def exists(self, name):
         output = self._as_root('list', options='o', use_root_namespace=True)

diff --git a/quantum/tests/unit/test_linux_dhcp.py b/quantum/tests/unit/test_linux_dhcp.py
index 8a32f81c777c4423c7fcdc26f7e5d173f848bdfd..9d239506f7bee781317f0c696d0c610ad3d2f3d7 100644 (file)
--- a/quantum/tests/unit/test_linux_dhcp.py
+++ b/quantum/tests/unit/test_linux_dhcp.py
@@ -331,7 +331,8 @@ class TestDhcpLocalProcess(TestBase):
 
         self.assertFalse(delegate.called)
         exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5]
-        self.execute.assert_called_once_with(exp_args, root_helper='sudo')
+        self.execute.assert_called_once_with(exp_args, root_helper='sudo',
+                                             check_exit_code=True)
 
     def test_disable(self):
         attrs_to_mock = dict([(a, mock.DEFAULT) for a in
@@ -348,7 +349,8 @@ class TestDhcpLocalProcess(TestBase):
 
         delegate.assert_has_calls([mock.call.destroy(network, 'tap0')])
         exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5]
-        self.execute.assert_called_once_with(exp_args, root_helper='sudo')
+        self.execute.assert_called_once_with(exp_args, root_helper='sudo',
+                                             check_exit_code=True)
 
     def test_pid(self):
         with mock.patch('__builtin__.open') as mock_open:
@@ -452,7 +454,8 @@ class TestDnsmasq(TestBase):
                 dm.spawn_process()
                 self.assertTrue(mocks['_output_opts_file'].called)
                 self.execute.assert_called_once_with(expected,
-                                                     root_helper='sudo')
+                                                     root_helper='sudo',
+                                                     check_exit_code=True)
 
     def test_spawn(self):
         self._test_spawn([])
@@ -539,7 +542,8 @@ tag:tag1,option:classless-static-route,%s,%s""".lstrip() % (fake_v6,
 
         self.safe.assert_has_calls([mock.call(exp_host_name, exp_host_data),
                                     mock.call(exp_opt_name, exp_opt_data)])
-        self.execute.assert_called_once_with(exp_args, root_helper='sudo')
+        self.execute.assert_called_once_with(exp_args, root_helper='sudo',
+                                             check_exit_code=True)
 
     def _test_lease_relay_script_helper(self, action, lease_remaining,
                                         path_exists=True):

diff --git a/quantum/tests/unit/test_linux_ip_lib.py b/quantum/tests/unit/test_linux_ip_lib.py
index 18958b6fa63ee3b062ad81ebf5cde676ed1076de..c7f96918afb65882ac4912d973a24f47960a8f1e 100644 (file)
--- a/quantum/tests/unit/test_linux_ip_lib.py
+++ b/quantum/tests/unit/test_linux_ip_lib.py
@@ -580,7 +580,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
             self.netns_cmd.execute(['ip', 'link', 'list'])
             execute.assert_called_once_with(['ip', 'netns', 'exec', 'ns', 'ip',
                                              'link', 'list'],
-                                            root_helper='sudo')
+                                            root_helper='sudo',
+                                            check_exit_code=True)
 
     def test_execute_env_var_prepend(self):
         self.parent.namespace = 'ns'
@@ -590,7 +591,7 @@ class TestIpNetnsCommand(TestIPCmdBase):
             execute.assert_called_once_with(
                 ['FOO=1', 'BAR=2', 'ip', 'netns', 'exec', 'ns', 'ip', 'link',
                  'list'],
-                root_helper='sudo')
+                root_helper='sudo', check_exit_code=True)
 
 
 class TestDeviceExists(unittest.TestCase):