]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Remove root_helper arg from linux interface
authorTerry Wilson <twilson@redhat.com>
Wed, 11 Feb 2015 03:36:58 +0000 (21:36 -0600)
committerTerry Wilson <twilson@redhat.com>
Fri, 13 Feb 2015 10:08:36 +0000 (04:08 -0600)
Change-Id: Id691f152b84bcf1eab3689693e27e372042f43a6
Partially-Implements: blueprint rootwrap-daemon-mode

neutron/agent/linux/interface.py
neutron/tests/unit/test_linux_interface.py

index dbdbb9a4e3113f4f24e1e3d529eb364c4bbfe0f4..d8308a70450f20bb04d67d9a0b0a1f44a694bb02 100644 (file)
@@ -20,7 +20,6 @@ from oslo_config import cfg
 from oslo_utils import importutils
 import six
 
-from neutron.agent.common import config
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import ovs_lib
 from neutron.agent.linux import utils
@@ -77,7 +76,6 @@ class LinuxInterfaceDriver(object):
 
     def __init__(self, conf):
         self.conf = conf
-        self.root_helper = config.get_root_helper(conf)
 
     def init_l3(self, device_name, ip_cidrs, namespace=None,
                 preserve_ips=[], gateway=None, extra_subnets=[]):
@@ -86,9 +84,7 @@ class LinuxInterfaceDriver(object):
         ip_cidrs: list of 'X.X.X.X/YY' strings
         preserve_ips: list of ip cidrs that should not be removed from device
         """
-        device = ip_lib.IPDevice(device_name,
-                                 self.root_helper,
-                                 namespace=namespace)
+        device = ip_lib.IPDevice(device_name, namespace=namespace)
 
         previous = {}
         for address in device.addr.list(scope='global', filters=['permanent']):
@@ -225,13 +221,11 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
         if not bridge:
             bridge = self.conf.ovs_integration_bridge
 
-        if not ip_lib.device_exists(device_name,
-                                    self.root_helper,
-                                    namespace=namespace):
+        if not ip_lib.device_exists(device_name, namespace=namespace):
 
             self.check_bridge_exists(bridge)
 
-            ip = ip_lib.IPWrapper(self.root_helper)
+            ip = ip_lib.IPWrapper()
             tap_name = self._get_tap_name(device_name, prefix)
 
             if self.conf.ovs_use_veth:
@@ -276,9 +270,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
         try:
             ovs.delete_port(tap_name)
             if self.conf.ovs_use_veth:
-                device = ip_lib.IPDevice(device_name,
-                                         self.root_helper,
-                                         namespace)
+                device = ip_lib.IPDevice(device_name, namespace=namespace)
                 device.link.delete()
                 LOG.debug("Unplugged interface '%s'", device_name)
         except RuntimeError:
@@ -293,10 +285,8 @@ class MidonetInterfaceDriver(LinuxInterfaceDriver):
         """This method is called by the Dhcp agent or by the L3 agent
         when a new network is created
         """
-        if not ip_lib.device_exists(device_name,
-                                    self.root_helper,
-                                    namespace=namespace):
-            ip = ip_lib.IPWrapper(self.root_helper)
+        if not ip_lib.device_exists(device_name, namespace=namespace):
+            ip = ip_lib.IPWrapper()
             tap_name = device_name.replace(prefix or n_const.TAP_DEVICE_PREFIX,
                                            n_const.TAP_DEVICE_PREFIX)
 
@@ -314,24 +304,21 @@ class MidonetInterfaceDriver(LinuxInterfaceDriver):
             root_dev.link.set_up()
 
             cmd = ['mm-ctl', '--bind-port', port_id, device_name]
-            utils.execute(cmd, self.root_helper)
+            utils.execute(cmd, run_as_root=True)
 
         else:
             LOG.info(_LI("Device %s already exists"), device_name)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         # the port will be deleted by the dhcp agent that will call the plugin
-        device = ip_lib.IPDevice(device_name,
-                                 self.root_helper,
-                                 namespace)
+        device = ip_lib.IPDevice(device_name, namespace=namespace)
         try:
             device.link.delete()
         except RuntimeError:
             LOG.error(_LE("Failed unplugging interface '%s'"), device_name)
         LOG.debug("Unplugged interface '%s'", device_name)
 
-        ip_lib.IPWrapper(
-            self.root_helper, namespace).garbage_collect_namespace()
+        ip_lib.IPWrapper(namespace=namespace).garbage_collect_namespace()
 
 
 class IVSInterfaceDriver(LinuxInterfaceDriver):
@@ -350,16 +337,14 @@ class IVSInterfaceDriver(LinuxInterfaceDriver):
 
     def _ivs_add_port(self, device_name, port_id, mac_address):
         cmd = ['ivs-ctl', 'add-port', device_name]
-        utils.execute(cmd, self.root_helper)
+        utils.execute(cmd, run_as_root=True)
 
     def plug(self, network_id, port_id, device_name, mac_address,
              bridge=None, namespace=None, prefix=None):
         """Plug in the interface."""
-        if not ip_lib.device_exists(device_name,
-                                    self.root_helper,
-                                    namespace=namespace):
+        if not ip_lib.device_exists(device_name, namespace=namespace):
 
-            ip = ip_lib.IPWrapper(self.root_helper)
+            ip = ip_lib.IPWrapper()
             tap_name = self._get_tap_name(device_name, prefix)
 
             root_dev, ns_dev = ip.add_veth(tap_name, device_name)
@@ -387,10 +372,8 @@ class IVSInterfaceDriver(LinuxInterfaceDriver):
         tap_name = self._get_tap_name(device_name, prefix)
         try:
             cmd = ['ivs-ctl', 'del-port', tap_name]
-            utils.execute(cmd, self.root_helper)
-            device = ip_lib.IPDevice(device_name,
-                                     self.root_helper,
-                                     namespace)
+            utils.execute(cmd, run_as_root=True)
+            device = ip_lib.IPDevice(device_name, namespace=namespace)
             device.link.delete()
             LOG.debug("Unplugged interface '%s'", device_name)
         except RuntimeError:
@@ -406,10 +389,8 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
     def plug(self, network_id, port_id, device_name, mac_address,
              bridge=None, namespace=None, prefix=None):
         """Plugin the interface."""
-        if not ip_lib.device_exists(device_name,
-                                    self.root_helper,
-                                    namespace=namespace):
-            ip = ip_lib.IPWrapper(self.root_helper)
+        if not ip_lib.device_exists(device_name, namespace=namespace):
+            ip = ip_lib.IPWrapper()
 
             # Enable agent to define the prefix
             tap_name = device_name.replace(prefix or self.DEV_NAME_PREFIX,
@@ -431,7 +412,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
-        device = ip_lib.IPDevice(device_name, self.root_helper, namespace)
+        device = ip_lib.IPDevice(device_name, namespace=namespace)
         try:
             device.link.delete()
             LOG.debug("Unplugged interface '%s'", device_name)
@@ -470,13 +451,11 @@ class MetaInterfaceDriver(LinuxInterfaceDriver):
 
     def _set_device_plugin_tag(self, network_id, device_name, namespace=None):
         plugin_tag = self._get_flavor_by_network_id(network_id)
-        device = ip_lib.IPDevice(device_name, self.conf.AGENT.root_helper,
-                                 namespace)
+        device = ip_lib.IPDevice(device_name, namespace=namespace)
         device.link.set_alias(plugin_tag)
 
     def _get_device_plugin_tag(self, device_name, namespace=None):
-        device = ip_lib.IPDevice(device_name, self.conf.AGENT.root_helper,
-                                 namespace)
+        device = ip_lib.IPDevice(device_name, namespace=namespace)
         return device.link.alias
 
     def get_device_name(self, port):
index 827f5033e7b69272626fa9724575e4a8d630e8b8..e5e21b6fe7fb6b27839e0c8d5652307933ef30e4 100644 (file)
@@ -60,7 +60,6 @@ class TestBase(base.BaseTestCase):
         super(TestBase, self).setUp()
         self.conf = config.setup_conf()
         self.conf.register_opts(interface.OPTS)
-        config.register_root_helper(self.conf)
         self.ip_dev_p = mock.patch.object(ip_lib, 'IPDevice')
         self.ip_dev = self.ip_dev_p.start()
         self.ip_p = mock.patch.object(ip_lib, 'IPWrapper')
@@ -86,7 +85,7 @@ class TestABCDriver(TestBase):
         bc.init_l3('tap0', ['192.168.1.2/24'], namespace=ns,
                    extra_subnets=[{'cidr': '172.20.0.0/24'}])
         self.ip_dev.assert_has_calls(
-            [mock.call('tap0', 'sudo', namespace=ns),
+            [mock.call('tap0', namespace=ns),
              mock.call().addr.list(scope='global', filters=['permanent']),
              mock.call().addr.add(4, '192.168.1.2/24', '192.168.1.255'),
              mock.call().addr.delete(4, '172.16.77.240/24'),
@@ -116,7 +115,7 @@ class TestABCDriver(TestBase):
         bc.init_l3('tap0', ['192.168.1.2/24'], namespace=ns,
                    preserve_ips=['192.168.1.3/32'])
         self.ip_dev.assert_has_calls(
-            [mock.call('tap0', 'sudo', namespace=ns),
+            [mock.call('tap0', namespace=ns),
              mock.call().addr.list(scope='global', filters=['permanent']),
              mock.call().addr.add(4, '192.168.1.2/24', '192.168.1.255')])
         self.assertFalse(self.ip_dev().addr.delete.called)
@@ -134,7 +133,7 @@ class TestABCDriver(TestBase):
         bc.init_l3('tap0', ['2001:db8:a::124/64'], namespace=ns,
                    extra_subnets=[{'cidr': '2001:db8:b::/64'}])
         self.ip_dev.assert_has_calls(
-            [mock.call('tap0', 'sudo', namespace=ns),
+            [mock.call('tap0', namespace=ns),
              mock.call().addr.list(scope='global', filters=['permanent']),
              mock.call().addr.add(6, '2001:db8:a::124/64',
                                   '2001:db8:a:0:ffff:ffff:ffff:ffff'),
@@ -189,7 +188,7 @@ class TestOVSInterfaceDriver(TestBase):
         self.conf.set_override('ovs_integration_bridge', br)
         self.assertEqual(self.conf.ovs_integration_bridge, br)
 
-        def device_exists(dev, root_helper=None, namespace=None):
+        def device_exists(dev, namespace=None):
             return dev == br
 
         ovs = interface.OVSInterfaceDriver(self.conf)
@@ -214,7 +213,7 @@ class TestOVSInterfaceDriver(TestBase):
         if not bridge:
             bridge = 'br-int'
 
-        def device_exists(dev, root_helper=None, namespace=None):
+        def device_exists(dev, namespace=None):
             return dev == bridge
 
         with mock.patch.object(ovs_lib.OVSBridge, 'replace_port') as replace:
@@ -234,7 +233,7 @@ class TestOVSInterfaceDriver(TestBase):
                     'iface-status': 'active',
                     'attached-mac': 'aa:bb:cc:dd:ee:ff'}))
 
-        expected = [mock.call('sudo'),
+        expected = [mock.call(),
                     mock.call().device('tap0'),
                     mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff')]
         expected.extend(additional_expectation)
@@ -288,7 +287,7 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
         if not bridge:
             bridge = 'br-int'
 
-        def device_exists(dev, root_helper=None, namespace=None):
+        def device_exists(dev, namespace=None):
             return dev == bridge
 
         ovs = interface.OVSInterfaceDriver(self.conf)
@@ -297,7 +296,7 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
         root_dev = mock.Mock()
         ns_dev = mock.Mock()
         self.ip().add_veth = mock.Mock(return_value=(root_dev, ns_dev))
-        expected = [mock.call('sudo'),
+        expected = [mock.call(),
                     mock.call().add_veth('tap0', devname,
                                          namespace2=namespace)]
 
@@ -338,7 +337,7 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
             ovs.unplug('ns-0', bridge=bridge)
             ovs_br.assert_has_calls([mock.call(bridge),
                                      mock.call().delete_port('tap0')])
-        self.ip_dev.assert_has_calls([mock.call('ns-0', 'sudo', None),
+        self.ip_dev.assert_has_calls([mock.call('ns-0', namespace=None),
                                       mock.call().link.delete()])
 
 
@@ -355,7 +354,7 @@ class TestBridgeInterfaceDriver(TestBase):
         self._test_plug(namespace='01234567-1234-1234-99')
 
     def _test_plug(self, namespace=None, mtu=None):
-        def device_exists(device, root_helper=None, namespace=None):
+        def device_exists(device, namespace=None):
             return device.startswith('brq')
 
         root_veth = mock.Mock()
@@ -372,7 +371,7 @@ class TestBridgeInterfaceDriver(TestBase):
                 mac_address,
                 namespace=namespace)
 
-        ip_calls = [mock.call('sudo'),
+        ip_calls = [mock.call(),
                     mock.call().add_veth('tap0', 'ns-0', namespace2=namespace)]
         ns_veth.assert_has_calls([mock.call.link.set_address(mac_address)])
         if mtu:
@@ -406,7 +405,7 @@ class TestBridgeInterfaceDriver(TestBase):
         with mock.patch('neutron.agent.linux.interface.LOG') as log:
             br = interface.BridgeInterfaceDriver(self.conf)
             br.unplug('tap0')
-            [mock.call(), mock.call('tap0', 'sudo'), mock.call().link.delete()]
+            [mock.call(), mock.call('tap0'), mock.call().link.delete()]
             self.assertEqual(log.error.call_count, 1)
 
     def test_unplug(self):
@@ -416,7 +415,7 @@ class TestBridgeInterfaceDriver(TestBase):
             br.unplug('tap0')
             self.assertEqual(log.call_count, 1)
 
-        self.ip_dev.assert_has_calls([mock.call('tap0', 'sudo', None),
+        self.ip_dev.assert_has_calls([mock.call('tap0', namespace=None),
                                       mock.call().link.delete()])
 
 
@@ -459,14 +458,14 @@ class TestMetaInterfaceDriver(TestBase):
         meta_interface._set_device_plugin_tag(driver,
                                               'tap0',
                                               namespace=None)
-        expected = [mock.call('tap0', 'sudo', None),
+        expected = [mock.call('tap0', namespace=None),
                     mock.call().link.set_alias('fake1')]
         self.ip_dev.assert_has_calls(expected)
         namespace = '01234567-1234-1234-99'
         meta_interface._set_device_plugin_tag(driver,
                                               'tap1',
                                               namespace=namespace)
-        expected = [mock.call('tap1', 'sudo', '01234567-1234-1234-99'),
+        expected = [mock.call('tap1', namespace='01234567-1234-1234-99'),
                     mock.call().link.set_alias('fake1')]
         self.ip_dev.assert_has_calls(expected)
 
@@ -475,11 +474,11 @@ class TestMetaInterfaceDriver(TestBase):
         self.ip_dev().link.alias = 'fake1'
         plugin_tag0 = meta_interface._get_device_plugin_tag('tap0',
                                                             namespace=None)
-        expected = [mock.call('tap0', 'sudo', None)]
+        expected = [mock.call('tap0', namespace=None)]
         self.ip_dev.assert_has_calls(expected)
         self.assertEqual('fake1', plugin_tag0)
         namespace = '01234567-1234-1234-99'
-        expected = [mock.call('tap1', 'sudo', '01234567-1234-1234-99')]
+        expected = [mock.call('tap1', namespace='01234567-1234-1234-99')]
         plugin_tag1 = meta_interface._get_device_plugin_tag(
             'tap1',
             namespace=namespace)
@@ -506,7 +505,7 @@ class TestIVSInterfaceDriver(TestBase):
         if not devname:
             devname = 'ns-0'
 
-        def device_exists(dev, root_helper=None, namespace=None):
+        def device_exists(dev, namespace=None):
             return dev == 'indigo'
 
         ivs = interface.IVSInterfaceDriver(self.conf)
@@ -517,7 +516,7 @@ class TestIVSInterfaceDriver(TestBase):
         ns_dev = mock.Mock()
         self.ip().add_veth = mock.Mock(return_value=(root_dev, _ns_dev))
         self.ip().device = mock.Mock(return_value=(ns_dev))
-        expected = [mock.call('sudo'), mock.call().add_veth('tap0', devname),
+        expected = [mock.call(), mock.call().add_veth('tap0', devname),
                     mock.call().device(devname)]
 
         ivsctl_cmd = ['ivs-ctl', 'add-port', 'tap0']
@@ -529,7 +528,7 @@ class TestIVSInterfaceDriver(TestBase):
                      'aa:bb:cc:dd:ee:ff',
                      namespace=namespace,
                      prefix=prefix)
-            execute.assert_called_once_with(ivsctl_cmd, 'sudo')
+            execute.assert_called_once_with(ivsctl_cmd, run_as_root=True)
 
         ns_dev.assert_has_calls(
             [mock.call.link.set_address('aa:bb:cc:dd:ee:ff')])
@@ -558,8 +557,8 @@ class TestIVSInterfaceDriver(TestBase):
         ivsctl_cmd = ['ivs-ctl', 'del-port', 'tap0']
         with mock.patch.object(utils, 'execute') as execute:
             ivs.unplug('ns-0')
-            execute.assert_called_once_with(ivsctl_cmd, 'sudo')
-            self.ip_dev.assert_has_calls([mock.call('ns-0', 'sudo', None),
+            execute.assert_called_once_with(ivsctl_cmd, run_as_root=True)
+            self.ip_dev.assert_has_calls([mock.call('ns-0', namespace=None),
                                           mock.call().link.delete()])
 
 
@@ -567,7 +566,6 @@ class TestMidonetInterfaceDriver(TestBase):
     def setUp(self):
         self.conf = config.setup_conf()
         self.conf.register_opts(interface.OPTS)
-        config.register_root_helper(self.conf)
         self.driver = interface.MidonetInterfaceDriver(self.conf)
         self.network_id = uuidutils.generate_uuid()
         self.port_id = uuidutils.generate_uuid()
@@ -589,9 +587,9 @@ class TestMidonetInterfaceDriver(TestBase):
                 self.network_id, self.port_id,
                 self.device_name, self.mac_address,
                 self.bridge, self.namespace)
-            execute.assert_called_once_with(cmd, 'sudo')
+            execute.assert_called_once_with(cmd, run_as_root=True)
 
-        expected = [mock.call(), mock.call('sudo'),
+        expected = [mock.call(), mock.call(),
                     mock.call().add_veth(self.device_name,
                                          self.device_name,
                                          namespace2=self.namespace),
@@ -610,7 +608,6 @@ class TestMidonetInterfaceDriver(TestBase):
         self.driver.unplug(self.device_name, self.bridge, self.namespace)
 
         self.ip_dev.assert_has_calls([
-            mock.call(self.device_name, self.driver.root_helper,
-                      self.namespace),
+            mock.call(self.device_name, namespace=self.namespace),
             mock.call().link.delete()])
         self.ip.assert_has_calls(mock.call().garbage_collect_namespace())