From: sadasu <sadasu@cisco.com>
Date: Thu, 25 Apr 2013 20:05:22 +0000 (-0400)
Subject: Improve checking of return values for functions in linuxbridge agent plugin
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fa69228606ff42233a3f241a455157e675d398df;p=openstack-build%2Fneutron-build.git

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
---

diff --git a/quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py b/quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py
index a7229d5d9..ef5cec669 100755
--- a/quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py
+++ b/quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py
@@ -139,8 +139,8 @@ class LinuxBridgeManager:
         """Create a vlan and bridge unless they already exist."""
         interface = self.ensure_vlan(physical_interface, vlan_id)
         bridge_name = self.get_bridge_name(network_id)
-        self.ensure_bridge(bridge_name, interface)
-        return interface
+        if self.ensure_bridge(bridge_name, interface):
+            return interface
 
     def get_interface_details(self, interface):
         device = self.ip.device(interface)
@@ -154,13 +154,13 @@ class LinuxBridgeManager:
         """Create a non-vlan bridge unless it already exists."""
         bridge_name = self.get_bridge_name(network_id)
         ips, gateway = self.get_interface_details(physical_interface)
-        self.ensure_bridge(bridge_name, physical_interface, ips, gateway)
-        return physical_interface
+        if self.ensure_bridge(bridge_name, physical_interface, ips, gateway):
+            return physical_interface
 
     def ensure_local_bridge(self, network_id):
         """Create a local bridge unless it already exists."""
         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):
         """Create a vlan unless it already exists."""
@@ -234,7 +234,7 @@ class LinuxBridgeManager:
                       {'bridge_name': bridge_name, 'interface': interface})
 
         if not interface:
-            return
+            return bridge_name
 
         # Update IP info if necessary
         self.update_interface_ip_details(bridge_name, interface, ips, gateway)
@@ -250,6 +250,7 @@ class LinuxBridgeManager:
                           {'interface': interface, 'bridge_name': bridge_name,
                            'e': e})
                 return
+        return bridge_name
 
     def ensure_physical_in_bridge(self, network_id,
                                   physical_network,
@@ -258,14 +259,12 @@ class LinuxBridgeManager:
         if not physical_interface:
             LOG.error(_("No mapping for physical network %s"),
                       physical_network)
-            return False
-
+            return
         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:
-            self.ensure_vlan_bridge(network_id, physical_interface,
-                                    vlan_id)
-        return True
+            return self.ensure_vlan_bridge(network_id, physical_interface,
+                                           vlan_id)
 
     def add_tap_interface(self, network_id, physical_network, vlan_id,
                           tap_device_name):
@@ -282,12 +281,10 @@ class LinuxBridgeManager:
         bridge_name = self.get_bridge_name(network_id)
         if int(vlan_id) == lconst.LOCAL_VLAN_ID:
             self.ensure_local_bridge(network_id)
-        else:
-            result = self.ensure_physical_in_bridge(network_id,
-                                                    physical_network,
-                                                    vlan_id)
-            if not result:
-                return False
+        elif not self.ensure_physical_in_bridge(network_id,
+                                                physical_network,
+                                                vlan_id):
+            return False
 
         # Check if device needs to be added to bridge
         tap_device_in_bridge = self.get_bridge_for_tap_device(tap_device_name)
diff --git a/quantum/tests/unit/linuxbridge/test_lb_quantum_agent.py b/quantum/tests/unit/linuxbridge/test_lb_quantum_agent.py
index f2a6ebfb0..f399fee25 100644
--- a/quantum/tests/unit/linuxbridge/test_lb_quantum_agent.py
+++ b/quantum/tests/unit/linuxbridge/test_lb_quantum_agent.py
@@ -325,7 +325,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
         ) as (de_fn, exec_fn, upd_fn, ie_fn):
             de_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
             self.lbm.ensure_bridge("br0", "eth0")
             self.assertTrue(upd_fn.called)