diff --git a/neutron/agent/linux/bridge_lib.py b/neutron/agent/linux/bridge_lib.py index dfb1100ff2f..edf89fcbcfb 100644 --- a/neutron/agent/linux/bridge_lib.py +++ b/neutron/agent/linux/bridge_lib.py @@ -52,16 +52,16 @@ def get_bridge_names(): class BridgeDevice(ip_lib.IPDevice): - def _brctl(self, cmd): - cmd = ['brctl'] + cmd + def _ip_link(self, 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( diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 7e5ba7606d5..84490b265b4 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/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) diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 1637aa85322..4d9c0ffbd2c 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/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) diff --git a/neutron/tests/unit/agent/linux/test_bridge_lib.py b/neutron/tests/unit/agent/linux/test_bridge_lib.py index 1cbe3e643c6..6ea43e2397d 100644 --- a/neutron/tests/unit/agent/linux/test_bridge_lib.py +++ b/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) diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index d55faff60ab..4b62bebb23a 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/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()