openstack
/
neutron
Code Issues Proposed changes

Always use BridgeDevice to manage linuxbridges 

BridgeDevice[1] class provides methods to manage linuxbridges through
brctl. This change adds some methods to BridgeDevice in order to
always use BridgeDevice to manage linuxbridges and respect DRY
principle.

[1] neutron.agent.linux.bridge_lib

Change-Id: I4b9d755677bba0d487a261004d9ba9b11116101f
This commit is contained in:
Cedric Brandily 2015-05-08 23:28:37 +02:00
parent f4e0762b4a
commit 59a61c351d
7 changed files with 75 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
neutron/agent/linux/bridge_lib.py
View File

@ -39,3 +39,9 @@ class BridgeDevice(ip_lib.IPDevice):
def delif(self, interface):
return self._brctl(['delif', self.name, interface])
def setfd(self, fd):
return self._brctl(['setfd', self.name, str(fd)])
def disable_stp(self):
return self._brctl(['stp', self.name, 'off'])

4
neutron/agent/linux/ip_lib.py
View File

@ -348,10 +348,10 @@ class IpLinkCommand(IpDeviceCommandBase):
self._as_root([], ('set', self.name, 'mtu', mtu_size))
def set_up(self):
self._as_root([], ('set', self.name, 'up'))
return self._as_root([], ('set', self.name, 'up'))
def set_down(self):
self._as_root([], ('set', self.name, 'down'))
return self._as_root([], ('set', self.name, 'down'))
def set_netns(self, namespace):
self._as_root([], ('set', self.name, 'netns', namespace))

35
neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
View File

@ -33,6 +33,7 @@ from oslo_service import loopingcall
from oslo_service import service
from six import moves
from neutron.agent.linux import bridge_lib
from neutron.agent.linux import ip_lib
from neutron.agent.linux import utils
from neutron.agent import rpc as agent_rpc
@ -303,21 +304,18 @@ class LinuxBridgeManager(object):
LOG.debug("Starting bridge %(bridge_name)s for subinterface "
"%(interface)s",
{'bridge_name': bridge_name, 'interface': interface})
if utils.execute(['brctl', 'addbr', bridge_name],
run_as_root=True):
bridge_device = bridge_lib.BridgeDevice.addbr(bridge_name)
if bridge_device.setfd(0):
return
if utils.execute(['brctl', 'setfd', bridge_name,
str(0)], run_as_root=True):
if bridge_device.disable_stp():
return
if utils.execute(['brctl', 'stp', bridge_name,
'off'], run_as_root=True):
return
if utils.execute(['ip', 'link', 'set', bridge_name,
'up'], run_as_root=True):
if bridge_device.link.set_up():
return
LOG.debug("Done starting bridge %(bridge_name)s for "
"subinterface %(interface)s",
{'bridge_name': bridge_name, 'interface': interface})
else:
bridge_device = bridge_lib.BridgeDevice(bridge_name)
if not interface:
return bridge_name
@ -331,11 +329,9 @@ class LinuxBridgeManager(object):
# Check if the interface is not enslaved in another bridge
if self.is_device_on_bridge(interface):
bridge = self.get_bridge_for_tap_device(interface)
utils.execute(['brctl', 'delif', bridge, interface],
run_as_root=True)
bridge_lib.BridgeDevice(bridge).delif(interface)
utils.execute(['brctl', 'addif', bridge_name, interface],
run_as_root=True)
bridge_device.addif(interface)
except Exception as e:
LOG.error(_LE("Unable to add %(interface)s to %(bridge_name)s"
"! Exception: %(e)s"),
@ -401,8 +397,7 @@ class LinuxBridgeManager(object):
'bridge_name': bridge_name}
LOG.debug("Adding device %(tap_device_name)s to bridge "
"%(bridge_name)s", data)
if utils.execute(['brctl', 'addif', bridge_name, tap_device_name],
run_as_root=True):
if bridge_lib.BridgeDevice(bridge_name).addif(tap_device_name):
return False
else:
data = {'tap_device_name': tap_device_name,
@ -450,11 +445,10 @@ class LinuxBridgeManager(object):
self.delete_vlan(interface)
LOG.debug("Deleting bridge %s", bridge_name)
if utils.execute(['ip', 'link', 'set', bridge_name, 'down'],
run_as_root=True):
bridge_device = bridge_lib.BridgeDevice(bridge_name)
if bridge_device.link.set_down():
return
if utils.execute(['brctl', 'delbr', bridge_name],
run_as_root=True):
if bridge_device.delbr():
return
LOG.debug("Done deleting bridge %s", bridge_name)
@ -477,8 +471,7 @@ class LinuxBridgeManager(object):
"%(bridge_name)s",
{'interface_name': interface_name,
'bridge_name': bridge_name})
if utils.execute(['brctl', 'delif', bridge_name, interface_name],
run_as_root=True):
if bridge_lib.BridgeDevice(bridge_name).delif(interface_name):
return False
LOG.debug("Done removing device %(interface_name)s from bridge "
"%(bridge_name)s",

9
neutron/tests/functional/agent/linux/test_ebtables_driver.py
View File

@ -13,8 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.
from neutron.agent.linux import bridge_lib
from neutron.agent.linux import ebtables_driver
from neutron.agent.linux import ip_lib
from neutron.tests.common import machine_fixtures
from neutron.tests.common import net_helpers
from neutron.tests.functional import base
@ -85,14 +85,13 @@ class EbtablesLowLevelTestCase(base.BaseSudoTestCase):
# Pick one of the namespaces and setup a bridge for the local ethernet
# interface there, because ebtables only works on bridged interfaces.
self.source.execute(['brctl', 'addbr', 'mybridge'])
self.source.execute(
['brctl', 'addif', 'mybridge', self.source.port.name])
dev_mybridge = bridge_lib.BridgeDevice.addbr(
'mybridge', self.source.namespace)
dev_mybridge.addif(self.source.port.name)
# Take the IP addrss off one of the interfaces and apply it to the
# bridge interface instead.
self.source.port.addr.delete(self.source.ip_cidr)
dev_mybridge = ip_lib.IPDevice("mybridge", self.source.namespace)
dev_mybridge.link.set_up()
dev_mybridge.addr.add(self.source.ip_cidr)

6
neutron/tests/unit/agent/linux/test_bridge_lib.py
View File

@ -41,6 +41,12 @@ class BridgeLibTest(base.BaseTestCase):
self.assertEqual(namespace, br.namespace)
self._verify_bridge_mock(['brctl', 'addbr', self._BR_NAME])
br.setfd(0)
self._verify_bridge_mock(['brctl', 'setfd', self._BR_NAME, '0'])
br.disable_stp()
self._verify_bridge_mock(['brctl', 'stp', self._BR_NAME, 'off'])
br.addif(self._IF_NAME)
self._verify_bridge_mock(
['brctl', 'addif', self._BR_NAME, self._IF_NAME])

6
neutron/tests/unit/agent/linux/test_ip_lib.py
View File

@ -619,11 +619,13 @@ class TestIpLinkCommand(TestIPCmdBase):
self._assert_sudo([], ('set', 'eth0', 'mtu', 1500))
def test_set_up(self):
self.link_cmd.set_up()
observed = self.link_cmd.set_up()
self.assertEqual(self.parent._as_root.return_value, observed)
self._assert_sudo([], ('set', 'eth0', 'up'))
def test_set_down(self):
self.link_cmd.set_down()
observed = self.link_cmd.set_down()
self.assertEqual(self.parent._as_root.return_value, observed)
self._assert_sudo([], ('set', 'eth0', 'down'))
def test_set_netns(self):

66
neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
View File

@ -17,6 +17,7 @@ import os
import mock
from oslo_config import cfg
from neutron.agent.linux import bridge_lib
from neutron.agent.linux import ip_lib
from neutron.agent.linux import utils
from neutron.common import constants
@ -577,9 +578,12 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertFalse(self.lbm._bridge_exists_and_ensure_up("br0"))
def test_ensure_bridge(self):
bridge_device = mock.Mock()
bridge_device_old = mock.Mock()
with mock.patch.object(self.lbm,
'_bridge_exists_and_ensure_up') as de_fn,\
mock.patch.object(utils, 'execute') as exec_fn,\
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device_old) as br_fn, \
mock.patch.object(self.lbm,
'update_interface_ip_details') as upd_fn,\
mock.patch.object(self.lbm,
@ -588,7 +592,10 @@ class TestLinuxBridgeManager(base.BaseTestCase):
mock.patch.object(self.lbm,
'get_bridge_for_tap_device') as get_if_br_fn:
de_fn.return_value = False
exec_fn.return_value = False
br_fn.addbr.return_value = bridge_device
bridge_device.setfd.return_value = False
bridge_device.disable_stp.return_value = False
bridge_device.link.set_up.return_value = False
self.assertEqual(self.lbm.ensure_bridge("br0", None), "br0")
ie_fn.return_Value = False
self.lbm.ensure_bridge("br0", "eth0")
@ -599,24 +606,17 @@ class TestLinuxBridgeManager(base.BaseTestCase):
upd_fn.assert_called_with("br0", "eth0", "ips", "gateway")
ie_fn.assert_called_with("br0", "eth0")
exec_fn.side_effect = Exception()
de_fn.return_value = True
bridge_device.delif.side_effect = Exception()
self.lbm.ensure_bridge("br0", "eth0")
ie_fn.assert_called_with("br0", "eth0")
exec_fn.reset_mock()
exec_fn.side_effect = None
de_fn.return_value = True
ie_fn.return_value = False
get_if_br_fn.return_value = "br1"
self.lbm.ensure_bridge("br0", "eth0")
expected = [
mock.call(['brctl', 'delif', 'br1', 'eth0'],
run_as_root=True),
mock.call(['brctl', 'addif', 'br0', 'eth0'],
run_as_root=True),
]
exec_fn.assert_has_calls(expected)
bridge_device_old.delif.assert_called_once_with('eth0')
br_fn.return_value.addif.assert_called_once_with('eth0')
def test_ensure_physical_in_bridge(self):
self.assertFalse(
@ -653,11 +653,13 @@ class TestLinuxBridgeManager(base.BaseTestCase):
)
de_fn.return_value = True
bridge_device = mock.Mock()
with mock.patch.object(self.lbm, "ensure_local_bridge") as en_fn,\
mock.patch.object(utils, "execute") as exec_fn,\
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device), \
mock.patch.object(self.lbm,
"get_bridge_for_tap_device") as get_br:
exec_fn.return_value = False
bridge_device.addif.retun_value = False
get_br.return_value = True
self.assertTrue(self.lbm.add_tap_interface("123",
p_const.TYPE_LOCAL,
@ -666,7 +668,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
en_fn.assert_called_with("123")
get_br.return_value = False
exec_fn.return_value = True
bridge_device.addif.retun_value = True
self.assertFalse(self.lbm.add_tap_interface("123",
p_const.TYPE_LOCAL,
"physnet1", None,
@ -698,6 +700,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
"1", "tap234")
def test_delete_vlan_bridge(self):
bridge_device = mock.Mock()
with mock.patch.object(ip_lib, "device_exists") as de_fn,\
mock.patch.object(self.lbm,
"get_interfaces_on_bridge") as getif_fn,\
@ -707,7 +710,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
mock.patch.object(self.lbm,
"update_interface_ip_details") as updif_fn,\
mock.patch.object(self.lbm, "delete_vxlan") as del_vxlan,\
mock.patch.object(utils, "execute") as exec_fn:
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
de_fn.return_value = False
self.lbm.delete_vlan_bridge("br0")
self.assertFalse(getif_fn.called)
@ -715,12 +719,13 @@ class TestLinuxBridgeManager(base.BaseTestCase):
de_fn.return_value = True
getif_fn.return_value = ["eth0", "eth1", "vxlan-1002"]
if_det_fn.return_value = ("ips", "gateway")
exec_fn.return_value = False
bridge_device.link.set_down.return_value = False
self.lbm.delete_vlan_bridge("br0")
updif_fn.assert_called_with("eth1", "br0", "ips", "gateway")
del_vxlan.assert_called_with("vxlan-1002")
def test_delete_vlan_bridge_with_ip(self):
bridge_device = mock.Mock()
with mock.patch.object(ip_lib, "device_exists") as de_fn,\
mock.patch.object(self.lbm,
"get_interfaces_on_bridge") as getif_fn,\
@ -730,16 +735,18 @@ class TestLinuxBridgeManager(base.BaseTestCase):
mock.patch.object(self.lbm,
"update_interface_ip_details") as updif_fn,\
mock.patch.object(self.lbm, "delete_vlan") as del_vlan,\
mock.patch.object(utils, "execute") as exec_fn:
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
de_fn.return_value = True
getif_fn.return_value = ["eth0", "eth1.1"]
if_det_fn.return_value = ("ips", "gateway")
exec_fn.return_value = False
bridge_device.link.set_down.return_value = False
self.lbm.delete_vlan_bridge("br0")
updif_fn.assert_called_with("eth1.1", "br0", "ips", "gateway")
self.assertFalse(del_vlan.called)
def test_delete_vlan_bridge_no_ip(self):
bridge_device = mock.Mock()
with mock.patch.object(ip_lib, "device_exists") as de_fn,\
mock.patch.object(self.lbm,
"get_interfaces_on_bridge") as getif_fn,\
@ -749,10 +756,11 @@ class TestLinuxBridgeManager(base.BaseTestCase):
mock.patch.object(self.lbm,
"update_interface_ip_details") as updif_fn,\
mock.patch.object(self.lbm, "delete_vlan") as del_vlan,\
mock.patch.object(utils, "execute") as exec_fn:
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
de_fn.return_value = True
getif_fn.return_value = ["eth0", "eth1.1"]
exec_fn.return_value = False
bridge_device.link.set_down.return_value = False
if_det_fn.return_value = ([], None)
self.lbm.delete_vlan_bridge("br0")
del_vlan.assert_called_with("eth1.1")
@ -765,19 +773,21 @@ class TestLinuxBridgeManager(base.BaseTestCase):
lbm = linuxbridge_neutron_agent.LinuxBridgeManager(
interface_mappings)
bridge_device = mock.Mock()
with mock.patch.object(ip_lib, "device_exists") as de_fn,\
mock.patch.object(lbm,
"get_interfaces_on_bridge") as getif_fn,\
mock.patch.object(lbm, "remove_interface"),\
mock.patch.object(lbm, "delete_vxlan") as del_vxlan,\
mock.patch.object(utils, "execute") as exec_fn:
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
de_fn.return_value = False
lbm.delete_vlan_bridge("br0")
self.assertFalse(getif_fn.called)
de_fn.return_value = True
getif_fn.return_value = ["vxlan-1002"]
exec_fn.return_value = False
bridge_device.link.set_down.return_value = False
lbm.delete_vlan_bridge("br0")
del_vxlan.assert_called_with("vxlan-1002")
@ -795,10 +805,12 @@ class TestLinuxBridgeManager(base.BaseTestCase):
del_br_fn.assert_called_once_with('brqnet1')
def test_remove_interface(self):
bridge_device = mock.Mock()
with mock.patch.object(ip_lib, "device_exists") as de_fn,\
mock.patch.object(self.lbm,
"is_device_on_bridge") as isdev_fn,\
mock.patch.object(utils, "execute") as exec_fn:
mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
de_fn.return_value = False
self.assertFalse(self.lbm.remove_interface("br0", "eth0"))
self.assertFalse(isdev_fn.called)
@ -808,10 +820,10 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertTrue(self.lbm.remove_interface("br0", "eth0"))
isdev_fn.return_value = True
exec_fn.return_value = True
bridge_device.delif.return_value = True
self.assertFalse(self.lbm.remove_interface("br0", "eth0"))
exec_fn.return_value = False
bridge_device.delif.return_value = False
self.assertTrue(self.lbm.remove_interface("br0", "eth0"))
def test_delete_vlan(self):
@ -822,7 +834,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertFalse(exec_fn.called)
de_fn.return_value = True
exec_fn.return_value = False
exec_fn.return_value = True
self.lbm.delete_vlan("eth1.1")
self.assertTrue(exec_fn.called)