Improve checking of return values for functions in linuxbridge agent plugin

Fixes bug 1167780

Changed return values for some ensure_* functions to make them more consistent.
Now all ensure_* functions return None on failure and return the entity that was
created within the function when successful. Made changes to the unit tests to
reflect the changed return values.

Change-Id: Ib015ee7cee50bae5d91a4e109e7381519c1e14f7
This commit is contained in:
sadasu 2013-04-25 16:05:22 -04:00
parent f94126739a
commit fa69228606
2 changed files with 16 additions and 19 deletions

View File

@ -139,8 +139,8 @@ class LinuxBridgeManager:
"""Create a vlan and bridge unless they already exist.""" """Create a vlan and bridge unless they already exist."""
interface = self.ensure_vlan(physical_interface, vlan_id) interface = self.ensure_vlan(physical_interface, vlan_id)
bridge_name = self.get_bridge_name(network_id) bridge_name = self.get_bridge_name(network_id)
self.ensure_bridge(bridge_name, interface) if self.ensure_bridge(bridge_name, interface):
return interface return interface
def get_interface_details(self, interface): def get_interface_details(self, interface):
device = self.ip.device(interface) device = self.ip.device(interface)
@ -154,13 +154,13 @@ class LinuxBridgeManager:
"""Create a non-vlan bridge unless it already exists.""" """Create a non-vlan bridge unless it already exists."""
bridge_name = self.get_bridge_name(network_id) bridge_name = self.get_bridge_name(network_id)
ips, gateway = self.get_interface_details(physical_interface) ips, gateway = self.get_interface_details(physical_interface)
self.ensure_bridge(bridge_name, physical_interface, ips, gateway) if self.ensure_bridge(bridge_name, physical_interface, ips, gateway):
return physical_interface return physical_interface
def ensure_local_bridge(self, network_id): def ensure_local_bridge(self, network_id):
"""Create a local bridge unless it already exists.""" """Create a local bridge unless it already exists."""
bridge_name = self.get_bridge_name(network_id) bridge_name = self.get_bridge_name(network_id)
self.ensure_bridge(bridge_name) return self.ensure_bridge(bridge_name)
def ensure_vlan(self, physical_interface, vlan_id): def ensure_vlan(self, physical_interface, vlan_id):
"""Create a vlan unless it already exists.""" """Create a vlan unless it already exists."""
@ -234,7 +234,7 @@ class LinuxBridgeManager:
{'bridge_name': bridge_name, 'interface': interface}) {'bridge_name': bridge_name, 'interface': interface})
if not interface: if not interface:
return return bridge_name
# Update IP info if necessary # Update IP info if necessary
self.update_interface_ip_details(bridge_name, interface, ips, gateway) self.update_interface_ip_details(bridge_name, interface, ips, gateway)
@ -250,6 +250,7 @@ class LinuxBridgeManager:
{'interface': interface, 'bridge_name': bridge_name, {'interface': interface, 'bridge_name': bridge_name,
'e': e}) 'e': e})
return return
return bridge_name
def ensure_physical_in_bridge(self, network_id, def ensure_physical_in_bridge(self, network_id,
physical_network, physical_network,
@ -258,14 +259,12 @@ class LinuxBridgeManager:
if not physical_interface: if not physical_interface:
LOG.error(_("No mapping for physical network %s"), LOG.error(_("No mapping for physical network %s"),
physical_network) physical_network)
return False return
if int(vlan_id) == lconst.FLAT_VLAN_ID: if int(vlan_id) == lconst.FLAT_VLAN_ID:
self.ensure_flat_bridge(network_id, physical_interface) return self.ensure_flat_bridge(network_id, physical_interface)
else: else:
self.ensure_vlan_bridge(network_id, physical_interface, return self.ensure_vlan_bridge(network_id, physical_interface,
vlan_id) vlan_id)
return True
def add_tap_interface(self, network_id, physical_network, vlan_id, def add_tap_interface(self, network_id, physical_network, vlan_id,
tap_device_name): tap_device_name):
@ -282,12 +281,10 @@ class LinuxBridgeManager:
bridge_name = self.get_bridge_name(network_id) bridge_name = self.get_bridge_name(network_id)
if int(vlan_id) == lconst.LOCAL_VLAN_ID: if int(vlan_id) == lconst.LOCAL_VLAN_ID:
self.ensure_local_bridge(network_id) self.ensure_local_bridge(network_id)
else: elif not self.ensure_physical_in_bridge(network_id,
result = self.ensure_physical_in_bridge(network_id, physical_network,
physical_network, vlan_id):
vlan_id) return False
if not result:
return False
# Check if device needs to be added to bridge # Check if device needs to be added to bridge
tap_device_in_bridge = self.get_bridge_for_tap_device(tap_device_name) tap_device_in_bridge = self.get_bridge_for_tap_device(tap_device_name)

View File

@ -325,7 +325,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
) as (de_fn, exec_fn, upd_fn, ie_fn): ) as (de_fn, exec_fn, upd_fn, ie_fn):
de_fn.return_value = False de_fn.return_value = False
exec_fn.return_value = False exec_fn.return_value = False
self.assertIsNone(self.lbm.ensure_bridge("br0", None)) self.assertEqual(self.lbm.ensure_bridge("br0", None), "br0")
ie_fn.return_Value = False ie_fn.return_Value = False
self.lbm.ensure_bridge("br0", "eth0") self.lbm.ensure_bridge("br0", "eth0")
self.assertTrue(upd_fn.called) self.assertTrue(upd_fn.called)