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

quantum/agent/l3_agent.py

@@ -29,6 +29,7 @@
 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 @@ def _destroy_all_router_namespaces(self):
     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 @@ def external_gateway_added(self, ri, ex_gw_port, internal_cidrs):
         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 @@ def external_gateway_removed(self, ri, ex_gw_port, internal_cidrs):
                                 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 @@ def internal_network_removed(self, ri, ex_gw_port, port_id, internal_cidr):
         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']

quantum/agent/linux/interface.py

@@ -154,10 +154,6 @@ def unplug(self, device_name, bridge=None, namespace=None):
         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 @@ def unplug(self, device_name, bridge=None, namespace=None):
             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."""

quantum/agent/linux/ip_lib.py

@@ -338,7 +338,7 @@ def add(self, name):
     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 @@ def execute(self, cmds, addl_env={}):
             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)

quantum/tests/unit/test_linux_dhcp.py

@@ -331,7 +331,8 @@ def test_disable_retain_port(self):
 
         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 @@ def test_disable(self):
 
         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 @@ def fake_argv(index):
                 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 @@ def test_reload_allocations(self):
 
         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):

quantum/tests/unit/test_linux_ip_lib.py

@@ -580,7 +580,8 @@ def test_execute(self):
             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 @@ def test_execute_env_var_prepend(self):
             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):