Merge "Switch ip link command to pyroute2"

neutron/agent/linux/ip_lib.py

@@ -25,6 +25,7 @@ from neutron_lib import exceptions
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
+from pyroute2.netlink.rtnl import ifinfmsg
 from pyroute2 import netns
 import six
 
@@ -503,33 +504,41 @@ class IpLinkCommand(IpDeviceCommandBase):
     COMMAND = 'link'
 
     def set_address(self, mac_address):
-        self._as_root([], ('set', self.name, 'address', mac_address))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, address=mac_address)
 
     def set_allmulticast_on(self):
-        self._as_root([], ('set', self.name, 'allmulticast', 'on'))
+        privileged.set_link_flags(
+            self.name, self._parent.namespace, ifinfmsg.IFF_ALLMULTI)
 
     def set_mtu(self, mtu_size):
-        self._as_root([], ('set', self.name, 'mtu', mtu_size))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, mtu=mtu_size)
 
     def set_up(self):
-        return self._as_root([], ('set', self.name, 'up'))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, state='up')
 
     def set_down(self):
-        return self._as_root([], ('set', self.name, 'down'))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, state='down')
 
     def set_netns(self, namespace):
-        self._as_root([], ('set', self.name, 'netns', namespace))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, net_ns_fd=namespace)
         self._parent.namespace = namespace
 
     def set_name(self, name):
-        self._as_root([], ('set', self.name, 'name', name))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, ifname=name)
         self._parent.name = name
 
     def set_alias(self, alias_name):
-        self._as_root([], ('set', self.name, 'alias', alias_name))
+        privileged.set_link_attribute(
+            self.name, self._parent.namespace, ifalias=alias_name)
 
     def delete(self):
-        self._as_root([], ('delete', self.name))
+        privileged.delete_interface(self.name, self._parent.namespace)
 
     @property
     def address(self):
@@ -539,6 +548,10 @@ class IpLinkCommand(IpDeviceCommandBase):
     def state(self):
         return self.attributes.get('state')
 
+    @property
+    def allmulticast(self):
+        return self.attributes.get('allmulticast')
+
     @property
     def mtu(self):
         return self.attributes.get('mtu')
@@ -557,19 +570,8 @@ class IpLinkCommand(IpDeviceCommandBase):
 
     @property
     def attributes(self):
-        return self._parse_line(self._run(['o'], ('show', self.name)))
+        return privileged.get_link_attributes(self.name,
-
+                                              self._parent.namespace)
-    def _parse_line(self, value):
-        if not value:
-            return {}
-
-        device_name, settings = value.replace("\\", '').split('>', 1)
-        tokens = settings.split()
-        keys = tokens[::2]
-        values = [int(v) if v.isdigit() else v for v in tokens[1::2]]
-
-        retval = dict(zip(keys, values))
-        return retval
 
 
 class IpAddrCommand(IpDeviceCommandBase):
@@ -1103,7 +1105,6 @@ def network_namespace_exists(namespace, **kwargs):
 
 def ensure_device_is_ready(device_name, namespace=None):
     dev = IPDevice(device_name, namespace=namespace)
-    dev.set_log_fail_as_error(False)
     try:
         # Ensure the device has a MAC address and is up, even if it is already
         # up. If the device doesn't exist, a RuntimeError will be raised.

neutron/privileged/agent/linux/ip_lib.py

@@ -16,6 +16,7 @@ import socket
 from neutron_lib import constants
 import pyroute2
 from pyroute2.netlink import rtnl
+from pyroute2.netlink.rtnl import ifinfmsg
 from pyroute2.netlink.rtnl import ndmsg
 from pyroute2 import NetlinkError
 from pyroute2 import netns
@@ -115,7 +116,7 @@ def _get_link_id(device, namespace):
         raise NetworkInterfaceNotFound(msg)
 
 
-def _run_iproute_link(command, device, namespace, **kwargs):
+def _run_iproute_link(command, device, namespace=None, **kwargs):
     try:
         with _get_iproute(namespace) as ip:
             idx = _get_link_id(device, namespace)
@@ -234,6 +235,33 @@ def interface_exists(ifname, namespace):
         raise
 
 
+@privileged.default.entrypoint
+def set_link_flags(device, namespace, flags):
+    link = _run_iproute_link("get", device, namespace)[0]
+    new_flags = flags | link['flags']
+    return _run_iproute_link("set", device, namespace, flags=new_flags)
+
+
+@privileged.default.entrypoint
+def set_link_attribute(device, namespace, **attributes):
+    return _run_iproute_link("set", device, namespace, **attributes)
+
+
+@privileged.default.entrypoint
+def get_link_attributes(device, namespace):
+    link = _run_iproute_link("get", device, namespace)[0]
+    return {
+        'mtu': link.get_attr('IFLA_MTU'),
+        'qlen': link.get_attr('IFLA_TXQLEN'),
+        'state': link.get_attr('IFLA_OPERSTATE'),
+        'qdisc': link.get_attr('IFLA_QDISC'),
+        'brd': link.get_attr('IFLA_BROADCAST'),
+        'link/ether': link.get_attr('IFLA_ADDRESS'),
+        'alias': link.get_attr('IFLA_IFALIAS'),
+        'allmulticast': bool(link['flags'] & ifinfmsg.IFF_ALLMULTI)
+    }
+
+
 @privileged.default.entrypoint
 def add_neigh_entry(ip_version, ip_address, mac_address, device, namespace,
                     **kwargs):

neutron/tests/functional/agent/linux/test_ip_lib.py

@@ -480,6 +480,14 @@ class IpLibTestCase(IpLibTestFramework):
 
         self.assertEqual(1450, device.link.mtu)
 
+    def test_set_link_allmulticast_on(self):
+        attr = self.generate_device_details()
+        device = self.manage_device(attr)
+
+        self.assertFalse(device.link.allmulticast)
+        device.link.set_allmulticast_on()
+        self.assertTrue(device.link.allmulticast)
+
     def test_set_link_netns(self):
         attr = self.generate_device_details()
         device = self.manage_device(attr)

neutron/tests/unit/agent/l3/test_dvr_snat_ns.py

@@ -37,10 +37,11 @@ class TestDvrSnatNs(base.BaseTestCase):
                                                  self.driver,
                                                  use_ipv6=False)
 
+    @mock.patch('neutron.privileged.agent.linux.ip_lib.set_link_attribute')
     @mock.patch.object(utils, 'execute')
     @mock.patch.object(ip_lib, 'create_network_namespace')
     @mock.patch.object(ip_lib, 'network_namespace_exists')
-    def test_create(self, exists, create, execute):
+    def test_create(self, exists, create, execute, set_link_attr):
         exists.return_value = False
         self.snat_ns.create()
 

neutron/tests/unit/agent/linux/test_ip_lib.py

@@ -20,6 +20,7 @@ import mock
 import netaddr
 from neutron_lib import exceptions
 import pyroute2
+from pyroute2.netlink.rtnl import ifinfmsg
 from pyroute2.netlink.rtnl import ndmsg
 from pyroute2 import NetlinkError
 import testtools
@@ -36,53 +37,6 @@ NETNS_SAMPLE = [
     'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb',
     'cccccccc-cccc-cccc-cccc-cccccccccccc']
 
-LINK_SAMPLE = [
-    '1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN \\'
-    'link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0',
-    '2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP '
-    'qlen 1000\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff'
-    '\    alias openvswitch',
-    '3: br-int: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN '
-    '\    link/ether aa:bb:cc:dd:ee:ff brd ff:ff:ff:ff:ff:ff promiscuity 0',
-    '4: gw-ddc717df-49: <BROADCAST,MULTICAST> mtu 1500 qdisc noop '
-    'state DOWN \    link/ether fe:dc:ba:fe:dc:ba brd ff:ff:ff:ff:ff:ff '
-    'promiscuity 0',
-    '5: foo:foo: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state '
-    'UP qlen 1000\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff '
-    'promiscuity 0',
-    '6: foo@foo: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state '
-    'UP qlen 1000\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff '
-    'promiscuity 0',
-    '7: foo:foo@foo: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq '
-    'state UP qlen 1000'
-    '\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0',
-    '8: foo@foo:foo: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq '
-    'state UP qlen 1000'
-    '\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0',
-    '9: bar.9@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
-    ' noqueue master brq0b24798c-07 state UP mode DEFAULT'
-    '\    link/ether ab:04:49:b6:ab:a0 brd ff:ff:ff:ff:ff:ff promiscuity 0'
-    '\    vlan protocol 802.1q id 9 <REORDER_HDR>',
-    '10: bar@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
-    ' noqueue master brq0b24798c-07 state UP mode DEFAULT'
-    '\    link/ether ab:04:49:b6:ab:a0 brd ff:ff:ff:ff:ff:ff promiscuity 0'
-    '\    vlan protocol 802.1Q id 10 <REORDER_HDR>',
-    '11: bar:bar@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq '
-    'state UP qlen 1000'
-    '\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0'
-    '\    vlan id 11 <REORDER_HDR>',
-    '12: bar@bar@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq '
-    'state UP qlen 1000'
-    '\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0'
-    '\    vlan id 12 <REORDER_HDR>',
-    '13: bar:bar@bar@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 '
-    'qdisc mq state UP qlen 1000'
-    '\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0'
-    '\    vlan protocol 802.1q id 13 <REORDER_HDR>',
-    '14: bar@bar:bar@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 '
-    'qdisc mq state UP qlen 1000'
-    '\    link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0'
-    '\    vlan protocol 802.1Q id 14 <REORDER_HDR>']
 
 ADDR_SAMPLE = ("""
 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
@@ -784,85 +738,72 @@ class TestIpRuleCommand(TestIPCmdBase):
 class TestIpLinkCommand(TestIPCmdBase):
     def setUp(self):
         super(TestIpLinkCommand, self).setUp()
-        self.parent._run.return_value = LINK_SAMPLE[1]
         self.command = 'link'
         self.link_cmd = ip_lib.IpLinkCommand(self.parent)
 
-    def test_set_address(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
+    def test_set_address(self, set_link_attribute):
         self.link_cmd.set_address('aa:bb:cc:dd:ee:ff')
-        self._assert_sudo([], ('set', 'eth0', 'address', 'aa:bb:cc:dd:ee:ff'))
+        set_link_attribute.assert_called_once_with(
+            self.parent.name, self.parent.namespace,
+            address='aa:bb:cc:dd:ee:ff')
 
-    def test_set_allmulticast_on(self):
+    @mock.patch.object(priv_lib, 'set_link_flags')
+    def test_set_allmulticast_on(self, set_link_flags):
         self.link_cmd.set_allmulticast_on()
-        self._assert_sudo([], ('set', 'eth0', 'allmulticast', 'on'))
+        set_link_flags.assert_called_once_with(
+            self.parent.name, self.parent.namespace, ifinfmsg.IFF_ALLMULTI)
 
-    def test_set_mtu(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
+    def test_set_mtu(self, set_link_attribute):
         self.link_cmd.set_mtu(1500)
-        self._assert_sudo([], ('set', 'eth0', 'mtu', 1500))
+        set_link_attribute.assert_called_once_with(
+            self.parent.name, self.parent.namespace, mtu=1500)
 
-    def test_set_up(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
-        observed = self.link_cmd.set_up()
+    def test_set_up(self, set_link_attribute):
-        self.assertEqual(self.parent._as_root.return_value, observed)
+        self.link_cmd.set_up()
-        self._assert_sudo([], ('set', 'eth0', 'up'))
+        set_link_attribute.assert_called_once_with(
+            self.parent.name, self.parent.namespace, state='up')
 
-    def test_set_down(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
-        observed = self.link_cmd.set_down()
+    def test_set_down(self, set_link_attribute):
-        self.assertEqual(self.parent._as_root.return_value, observed)
+        self.link_cmd.set_down()
-        self._assert_sudo([], ('set', 'eth0', 'down'))
+        set_link_attribute.assert_called_once_with(
+            self.parent.name, self.parent.namespace, state='down')
 
-    def test_set_netns(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
+    def test_set_netns(self, set_link_attribute):
+        original_namespace = self.parent.namespace
         self.link_cmd.set_netns('foo')
-        self._assert_sudo([], ('set', 'eth0', 'netns', 'foo'))
+        set_link_attribute.assert_called_once_with(
+            'eth0', original_namespace, net_ns_fd='foo')
         self.assertEqual(self.parent.namespace, 'foo')
 
-    def test_set_name(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
+    def test_set_name(self, set_link_attribute):
+        original_name = self.parent.name
         self.link_cmd.set_name('tap1')
-        self._assert_sudo([], ('set', 'eth0', 'name', 'tap1'))
+        set_link_attribute.assert_called_once_with(
+            original_name, self.parent.namespace, ifname='tap1')
         self.assertEqual(self.parent.name, 'tap1')
 
-    def test_set_alias(self):
+    @mock.patch.object(priv_lib, 'set_link_attribute')
+    def test_set_alias(self, set_link_attribute):
         self.link_cmd.set_alias('openvswitch')
-        self._assert_sudo([], ('set', 'eth0', 'alias', 'openvswitch'))
+        set_link_attribute.assert_called_once_with(
+            self.parent.name, self.parent.namespace, ifalias='openvswitch')
 
-    def test_delete(self):
+    @mock.patch.object(priv_lib, 'delete_interface')
+    def test_delete(self, delete):
         self.link_cmd.delete()
-        self._assert_sudo([], ('delete', 'eth0'))
+        delete.assert_called_once_with(self.parent.name, self.parent.namespace)
 
-    def test_address_property(self):
+    @mock.patch.object(priv_lib, 'get_link_attributes')
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
+    def test_settings_property(self, get_link_attributes):
-        self.assertEqual(self.link_cmd.address, 'cc:dd:ee:ff:ab:cd')
+        self.link_cmd.attributes
-
+        get_link_attributes.assert_called_once_with(
-    def test_mtu_property(self):
+            self.parent.name, self.parent.namespace)
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
-        self.assertEqual(self.link_cmd.mtu, 1500)
-
-    def test_qdisc_property(self):
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
-        self.assertEqual(self.link_cmd.qdisc, 'mq')
-
-    def test_qlen_property(self):
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
-        self.assertEqual(self.link_cmd.qlen, 1000)
-
-    def test_alias_property(self):
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
-        self.assertEqual(self.link_cmd.alias, 'openvswitch')
-
-    def test_state_property(self):
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
-        self.assertEqual(self.link_cmd.state, 'UP')
-
-    def test_settings_property(self):
-        expected = {'mtu': 1500,
-                    'qlen': 1000,
-                    'state': 'UP',
-                    'qdisc': 'mq',
-                    'brd': 'ff:ff:ff:ff:ff:ff',
-                    'link/ether': 'cc:dd:ee:ff:ab:cd',
-                    'alias': 'openvswitch'}
-        self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1])
-        self.assertEqual(self.link_cmd.attributes, expected)
-        self._assert_call(['o'], ('show', 'eth0'))
 
 
 class TestIpAddrCommand(TestIPCmdBase):
@@ -1394,10 +1335,15 @@ class TestDeviceExists(base.BaseTestCase):
             self.assertFalse(ip_lib.ensure_device_is_ready("eth0"))
 
     def test_ensure_device_is_ready_no_link_address(self):
-        with mock.patch.object(ip_lib.IPDevice, '_execute') as _execute:
+        with mock.patch.object(
-            # Use lo, it has no MAC address
+            priv_lib, 'get_link_attributes'
-            _execute.return_value = LINK_SAMPLE[0]
+        ) as get_link_attributes, mock.patch.object(
+            priv_lib, 'set_link_attribute'
+        ) as set_link_attribute:
+            get_link_attributes.return_value = {}
             self.assertFalse(ip_lib.ensure_device_is_ready("lo"))
+            get_link_attributes.assert_called_once_with("lo", None)
+            set_link_attribute.assert_not_called()
 
 
 class TestGetRoutingTable(base.BaseTestCase):