Merge "Change to use iproute2 instead of brctl"

neutron/agent/linux/bridge_lib.py

@@ -52,16 +52,16 @@ def get_bridge_names():
 
 
 class BridgeDevice(ip_lib.IPDevice):
-    def _brctl(self, cmd):
+    def _ip_link(self, cmd):
-        cmd = ['brctl'] + cmd
+        cmd = ['ip', 'link'] + cmd
         ip_wrapper = ip_lib.IPWrapper(self.namespace)
         return ip_wrapper.netns.execute(cmd, run_as_root=True)
 
     @classmethod
     def addbr(cls, name, namespace=None):
-        bridge = cls(name, namespace)
+        bridge = cls(name, namespace, 'bridge')
         try:
-            bridge._brctl(['addbr', bridge.name])
+            bridge.link.create()
         except RuntimeError:
             with excutils.save_and_reraise_exception() as ectx:
                 ectx.reraise = not bridge.exists()
@@ -78,19 +78,21 @@ class BridgeDevice(ip_lib.IPDevice):
             return cls(name)
 
     def delbr(self):
-        return self._brctl(['delbr', self.name])
+        return self.link.delete()
 
     def addif(self, interface):
-        return self._brctl(['addif', self.name, interface])
+        return self._ip_link(['set', 'dev', interface, 'master', self.name])
 
     def delif(self, interface):
-        return self._brctl(['delif', self.name, interface])
+        return self._ip_link(['set', 'dev', interface, 'nomaster'])
 
     def setfd(self, fd):
-        return self._brctl(['setfd', self.name, str(fd)])
+        return self._ip_link(['set', 'dev', self.name, 'type', 'bridge',
+                              'forward_delay', str(fd)])
 
     def disable_stp(self):
-        return self._brctl(['stp', self.name, 'off'])
+        return self._ip_link(['set', 'dev', self.name, 'type', 'bridge',
+                              'stp_state', 0])
 
     def owns_interface(self, interface):
         return os.path.exists(

neutron/agent/linux/ip_lib.py

@@ -249,9 +249,10 @@ class IPWrapper(SubProcessBase):
 
 
 class IPDevice(SubProcessBase):
-    def __init__(self, name, namespace=None):
+    def __init__(self, name, namespace=None, kind='link'):
         super(IPDevice, self).__init__(namespace=namespace)
         self._name = name
+        self.kind = kind
         self.link = IpLinkCommand(self)
         self.addr = IpAddrCommand(self)
         self.route = IpRouteCommand(self)
@@ -493,6 +494,10 @@ class IpDeviceCommandBase(IpCommandBase):
     def name(self):
         return self._parent.name
 
+    @property
+    def kind(self):
+        return self._parent.kind
+
 
 class IpLinkCommand(IpDeviceCommandBase):
     COMMAND = 'link'
@@ -536,6 +541,10 @@ class IpLinkCommand(IpDeviceCommandBase):
         privileged.set_link_attribute(
             self.name, self._parent.namespace, ifalias=alias_name)
 
+    def create(self):
+        privileged.create_interface(self.name, self._parent.namespace,
+                                    self.kind)
+
     def delete(self):
         privileged.delete_interface(self.name, self._parent.namespace)
 

neutron/privileged/agent/linux/ip_lib.py

@@ -90,6 +90,19 @@ class IpAddressAlreadyExists(RuntimeError):
         super(IpAddressAlreadyExists, self).__init__(message)
 
 
+class InterfaceAlreadyExists(RuntimeError):
+    message = _("Interface %(device)s already exists.")
+
+    def __init__(self, message=None, device=None):
+        # NOTE(slaweq): 'message' can be passed as an optional argument
+        # because of how privsep daemon works. If exception is raised in
+        # function called by privsep daemon, it will then try to reraise it
+        # and will call it always with passing only message from originally
+        # raised exception.
+        message = message or self.message % {'device': device}
+        super(InterfaceAlreadyExists, self).__init__(message)
+
+
 def _make_route_dict(destination, nexthop, device, scope):
     return {'destination': destination,
             'nexthop': nexthop,
@@ -276,6 +289,10 @@ def create_interface(ifname, namespace, kind, **kwargs):
                 link_key = "vxlan_link" if kind == "vxlan" else "link"
                 kwargs[link_key] = _get_link_id(physical_interface, namespace)
             return ip.link("add", ifname=ifname, kind=kind, **kwargs)
+    except NetlinkError as e:
+        if e.code == errno.EEXIST:
+            raise InterfaceAlreadyExists(device=ifname)
+        raise
     except OSError as e:
         if e.errno == errno.ENOENT:
             raise NetworkNamespaceNotFound(netns_name=namespace)

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

@@ -18,6 +18,7 @@
 import mock
 
 from neutron.agent.linux import bridge_lib
+from neutron.privileged.agent.linux import ip_lib as priv_lib
 from neutron.tests import base
 
 
@@ -34,6 +35,10 @@ class BridgeLibTest(base.BaseTestCase):
             return_value=True).start()
         ip_wrapper = mock.patch('neutron.agent.linux.ip_lib.IPWrapper').start()
         self.execute = ip_wrapper.return_value.netns.execute
+        self.create_p = mock.patch.object(priv_lib, 'create_interface')
+        self.create = self.create_p.start()
+        self.delete_p = mock.patch.object(priv_lib, 'delete_interface')
+        self.delete = self.delete_p.start()
 
     def _verify_bridge_mock(self, cmd):
         self.execute.assert_called_once_with(cmd, run_as_root=True)
@@ -62,13 +67,16 @@ class BridgeLibTest(base.BaseTestCase):
     def _test_br(self, namespace=None):
         br = bridge_lib.BridgeDevice.addbr(self._BR_NAME, namespace)
         self.assertEqual(namespace, br.namespace)
-        self._verify_bridge_mock(['brctl', 'addbr', self._BR_NAME])
+        self.create.assert_called_once_with(self._BR_NAME, br.namespace,
+                                            'bridge')
 
         br.setfd(0)
-        self._verify_bridge_mock(['brctl', 'setfd', self._BR_NAME, '0'])
+        self._verify_bridge_mock(['ip', 'link', 'set', 'dev', self._BR_NAME,
+                                  'type', 'bridge', 'forward_delay', '0'])
 
         br.disable_stp()
-        self._verify_bridge_mock(['brctl', 'stp', self._BR_NAME, 'off'])
+        self._verify_bridge_mock(['ip', 'link', 'set', 'dev', self._BR_NAME,
+                                  'type', 'bridge', 'stp_state', 0])
 
         br.disable_ipv6()
         cmd = 'net.ipv6.conf.%s.disable_ipv6=1' % self._BR_NAME
@@ -76,14 +84,15 @@ class BridgeLibTest(base.BaseTestCase):
 
         br.addif(self._IF_NAME)
         self._verify_bridge_mock(
-            ['brctl', 'addif', self._BR_NAME, self._IF_NAME])
+            ['ip', 'link', 'set', 'dev', self._IF_NAME,
+             'master', self._BR_NAME])
 
         br.delif(self._IF_NAME)
         self._verify_bridge_mock(
-            ['brctl', 'delif', self._BR_NAME, self._IF_NAME])
+            ['ip', 'link', 'set', 'dev', self._IF_NAME, 'nomaster'])
 
         br.delbr()
-        self._verify_bridge_mock(['brctl', 'delbr', self._BR_NAME])
+        self.delete.assert_called_once_with(self._BR_NAME, br.namespace)
 
     def test_addbr_with_namespace(self):
         self._test_br(self._NAMESPACE)
@@ -92,7 +101,7 @@ class BridgeLibTest(base.BaseTestCase):
         self._test_br()
 
     def test_addbr_exists(self):
-        self.execute.side_effect = RuntimeError()
+        self.create.side_effect = RuntimeError()
         with mock.patch.object(bridge_lib.BridgeDevice, 'exists',
                                return_value=True):
             bridge_lib.BridgeDevice.addbr(self._BR_NAME)

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

@@ -771,6 +771,12 @@ class TestIpLinkCommand(TestIPCmdBase):
         set_link_attribute.assert_called_once_with(
             self.parent.name, self.parent.namespace, ifalias='openvswitch')
 
+    @mock.patch.object(priv_lib, 'create_interface')
+    def test_create(self, create):
+        self.link_cmd.create()
+        create.assert_called_once_with(self.parent.name, self.parent.namespace,
+                                       self.parent.kind)
+
     @mock.patch.object(priv_lib, 'delete_interface')
     def test_delete(self, delete):
         self.link_cmd.delete()