Change to use iproute2 instead of brctl
brctl is being deprecated in some Linux distros, so change neutron to start using iproute2 commands or the pyroute2 library where possible. Added create() to IpLinkCommand class to allow usage of pyroute2 for bridge creation. Change-Id: If679e79fa3242ee1cd8610b5525deca35b41c87e Closes-bug: #1801919
This commit is contained in:
Brian Haley
parent
90dd08b156
commit
2572c158f5
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -271,9 +271,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)
|
||||
|
|
@ -515,6 +516,10 @@ class IpDeviceCommandBase(IpCommandBase):
|
|||
def name(self):
|
||||
return self._parent.name
|
||||
|
||||
@property
|
||||
def kind(self):
|
||||
return self._parent.kind
|
||||
|
||||
|
||||
class IpLinkCommand(IpDeviceCommandBase):
|
||||
COMMAND = 'link'
|
||||
|
|
@ -558,6 +563,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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -793,6 +793,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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue