]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
netns commands should always run in the root ns
authorMark McClain <mark.mcclain@dreamhost.com>
Fri, 31 Aug 2012 19:48:27 +0000 (15:48 -0400)
committerMark McClain <mark.mcclain@dreamhost.com>
Sat, 1 Sep 2012 19:11:52 +0000 (15:11 -0400)
fixes bug 1044542

Previously, some netns commands were executed in the context of the
namespace defined in the wrapper. These commands should be be executed in
the root namespace for the most reliable results.

Change-Id: I4915aa6f75af3f65ce2adb40360bcd841f549239

quantum/agent/linux/ip_lib.py
quantum/tests/unit/test_linux_ip_lib.py

index 1ad2d166b0d203fedf217fbfe0158b82f946097b..e854d636ced5ffc7dd07e83e31d28dd44c4a0167 100644 (file)
@@ -29,14 +29,17 @@ class SubProcessBase(object):
         else:
             return self._execute(options, command, args)
 
-    def _as_root(self, options, command, args):
+    def _as_root(self, options, command, args, use_root_namespace=False):
         if not self.root_helper:
             raise exceptions.SudoRequired()
+
+        namespace = self.namespace if not use_root_namespace else None
+
         return self._execute(options,
                              command,
                              args,
                              self.root_helper,
-                             self.namespace)
+                             namespace)
 
     @classmethod
     def _execute(cls, options, command, args, root_helper=None,
@@ -132,7 +135,8 @@ class IpCommandBase(object):
     def _as_root(self, *args, **kwargs):
         return self._parent._as_root(kwargs.get('options', []),
                                      self.COMMAND,
-                                     args)
+                                     args,
+                                     kwargs.get('use_root_namespace', False))
 
 
 class IpDeviceCommandBase(IpCommandBase):
@@ -309,16 +313,11 @@ class IpNetnsCommand(IpCommandBase):
     COMMAND = 'netns'
 
     def add(self, name):
-        self._as_root('add', name)
+        self._as_root('add', name, use_root_namespace=True)
         return IPWrapper(self._parent.root_helper, name)
 
     def delete(self, name):
-        if not self._parent.root_helper:
-            raise exceptions.SudoRequired()
-        else:
-            return utils.execute(
-                ['ip', 'netns', 'delete', name],
-                root_helper=self._parent.root_helper)
+        self._as_root('delete', name, use_root_namespace=True)
 
     def execute(self, cmds, addl_env={}):
         if not self._parent.root_helper:
@@ -332,7 +331,7 @@ class IpNetnsCommand(IpCommandBase):
                 root_helper=self._parent.root_helper)
 
     def exists(self, name):
-        output = self._as_root('list', options='o')
+        output = self._as_root('list', options='o', use_root_namespace=True)
 
         for line in output.split('\n'):
             if name == line.strip():
index 813dff9dcdd948ebb063eeb3a0795049e9ebc796..73397561c122783f4256d37dd021fac82c53fe86 100644 (file)
@@ -264,11 +264,13 @@ class TestIPCommandBase(unittest.TestCase):
 
     def test_as_root(self):
         self.ip_cmd._as_root('link')
-        self.ip.assert_has_calls([mock.call._as_root([], 'foo', ('link', ))])
+        self.ip.assert_has_calls(
+            [mock.call._as_root([], 'foo', ('link', ), False)])
 
     def test_as_root_with_options(self):
         self.ip_cmd._as_root('link', options='o')
-        self.ip.assert_has_calls([mock.call._as_root('o', 'foo', ('link', ))])
+        self.ip.assert_has_calls(
+            [mock.call._as_root('o', 'foo', ('link', ), False)])
 
 
 class TestIPDeviceCommandBase(unittest.TestCase):
@@ -294,9 +296,10 @@ class TestIPCmdBase(unittest.TestCase):
         self.parent.assert_has_calls([
             mock.call._run(options, self.command, args)])
 
-    def _assert_sudo(self, options, args):
-        self.parent.assert_has_calls([
-            mock.call._as_root(options, self.command, args)])
+    def _assert_sudo(self, options, args, force_root_namespace=False):
+        self.parent.assert_has_calls(
+            [mock.call._as_root(options, self.command, args,
+                                force_root_namespace)])
 
 
 class TestIpLinkCommand(TestIPCmdBase):
@@ -483,28 +486,27 @@ class TestIpNetnsCommand(TestIPCmdBase):
 
     def test_add_namespace(self):
         ns = self.netns_cmd.add('ns')
-        self._assert_sudo([], ('add', 'ns'))
+        self._assert_sudo([], ('add', 'ns'), force_root_namespace=True)
         self.assertEqual(ns.namespace, 'ns')
 
     def test_delete_namespace(self):
         with mock.patch('quantum.agent.linux.utils.execute') as execute:
             self.netns_cmd.delete('ns')
-            execute.assert_called_once_with(['ip', 'netns', 'delete', 'ns'],
-                                            root_helper='sudo')
+            self._assert_sudo([], ('delete', 'ns'), force_root_namespace=True)
 
     def test_namespace_exists(self):
         retval = '\n'.join(NETNS_SAMPLE)
         self.parent._as_root.return_value = retval
         self.assertTrue(
             self.netns_cmd.exists('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'))
-        self._assert_sudo('o', ('list',))
+        self._assert_sudo('o', ('list',), force_root_namespace=True)
 
     def test_namespace_doest_not_exist(self):
         retval = '\n'.join(NETNS_SAMPLE)
         self.parent._as_root.return_value = retval
         self.assertFalse(
             self.netns_cmd.exists('bbbbbbbb-1111-2222-3333-bbbbbbbbbbbb'))
-        self._assert_sudo('o', ('list',))
+        self._assert_sudo('o', ('list',), force_root_namespace=True)
 
     def test_execute(self):
         self.parent.namespace = 'ns'