openstack
/
neutron
Code Issues Proposed changes

Change the logic of "catch_exceptions" 

Changed the logic of decorator "catch_exceptions" to provide a
Pythonic output: if succeeds, returns True; if fails, returns False.

Change-Id: Idbf2ffb4ef403f343973bde3b3599f9cd3aaddf2
Story: #2007686
Task: #39975
This commit is contained in:
Rodolfo Alonso Hernandez 2020-12-22 17:09:00 +00:00
parent 607f15c1ac
commit b9e725ed72
3 changed files with 34 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -34,15 +34,17 @@ BRIDGE_PATH_FOR_DEVICE = BRIDGE_PORT_FS_FOR_DEVICE + '/bridge'
def catch_exceptions(function):
"""Catch bridge command exceptions and mimic $? output"""
"""Catch bridge command exceptions
Returns True if succeeds and False if fails
"""
@functools.wraps(function)
def decorated_function(self, *args, **kwargs):
try:
function(self, *args, **kwargs)
return 0
return True
except (RuntimeError, OSError, netlink_exceptions.NetlinkError):
return 1
return False
return decorated_function

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

@ -33,6 +33,7 @@ import oslo_messaging
from oslo_service import service
from oslo_utils import excutils
from neutron._i18n import _
from neutron.agent.linux import bridge_lib
from neutron.agent.linux import ip_lib
from neutron.api.rpc.handlers import securitygroups_rpc as sg_rpc
@ -436,9 +437,9 @@ class LinuxBridgeManager(amb.CommonAgentManagerBase):
"%(interface)s",
{'bridge_name': bridge_name, 'interface': interface})
bridge_device = bridge_lib.BridgeDevice.addbr(bridge_name)
if bridge_device.setfd(0):
if not bridge_device.setfd(0):
return
if bridge_device.disable_stp():
if not bridge_device.disable_stp():
return
if bridge_device.link.set_up():
return
@ -457,19 +458,14 @@ class LinuxBridgeManager(amb.CommonAgentManagerBase):
# Check if the interface is part of the bridge
if not bridge_device.owns_interface(interface):
try:
# Check if the interface is attached to another bridge
bridge = bridge_lib.BridgeDevice.get_interface_bridge(
interface)
if bridge:
bridge.delif(interface)
# Check if the interface is attached to another bridge
bridge = bridge_lib.BridgeDevice.get_interface_bridge(interface)
if bridge:
bridge.delif(interface)
bridge_device.addif(interface)
except Exception as e:
LOG.error("Unable to add %(interface)s to %(bridge_name)s"
"! Exception: %(e)s",
{'interface': interface, 'bridge_name': bridge_name,
'e': e})
if not bridge_device.addif(interface):
LOG.error("Unable to add %(interface)s to %(bridge_name)s",
{'interface': interface, 'bridge_name': bridge_name})
return
return bridge_name
@ -562,7 +558,8 @@ class LinuxBridgeManager(amb.CommonAgentManagerBase):
'bridge_name': bridge_name}
LOG.debug("Adding device %(tap_device_name)s to bridge "
"%(bridge_name)s", data)
if bridge_lib.BridgeDevice(bridge_name).addif(tap_device_name):
if not bridge_lib.BridgeDevice(bridge_name).addif(
tap_device_name):
return False
else:
data = {'tap_device_name': tap_device_name,
@ -636,24 +633,24 @@ class LinuxBridgeManager(amb.CommonAgentManagerBase):
"%(bridge_name)s",
{'interface_name': interface_name,
'bridge_name': bridge_name})
try:
bridge_device.delif(interface_name)
if bridge_device.delif(interface_name):
LOG.debug("Done removing device %(interface_name)s from "
"bridge %(bridge_name)s",
{'interface_name': interface_name,
'bridge_name': bridge_name})
return True
except RuntimeError:
with excutils.save_and_reraise_exception() as ctxt:
if not bridge_device.owns_interface(interface_name):
# the exception was likely a side effect of the tap
# being deleted by some other agent during handling
ctxt.reraise = False
LOG.debug("Cannot remove %(interface_name)s from "
"%(bridge_name)s. It is not on the bridge.",
{'interface_name': interface_name,
'bridge_name': bridge_name})
return False
else:
if not bridge_device.owns_interface(interface_name):
LOG.debug("Cannot remove %(interface_name)s from "
"%(bridge_name)s. It is not on the bridge.",
{'interface_name': interface_name,
'bridge_name': bridge_name})
return False
msg = _("Error deleting %(interface_name)s from bridge "
"%(bridge_name)s") % {'interface_name': interface_name,
'bridge_name': bridge_name}
raise RuntimeError(msg)
else:
LOG.debug("Cannot remove device %(interface_name)s bridge "
"%(bridge_name)s does not exist",

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

@ -521,8 +521,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
'get_interface_bridge') as get_if_br_fn:
de_fn.return_value = False
br_fn.addbr.return_value = bridge_device
bridge_device.setfd.return_value = 0
bridge_device.disable_stp.return_value = 0
bridge_device.setfd.return_value = True
bridge_device.disable_stp.return_value = True
bridge_device.disable_ipv6.return_value = False
bridge_device.link.set_up.return_value = 0
self.assertEqual("br0", self.lbm.ensure_bridge("br0", None))
@ -631,7 +631,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
en_fn.assert_called_with("123", "brq999")
get_br.return_value = False
bridge_device.addif.retun_value = 1
bridge_device.addif.return_value = False
self.assertFalse(self.lbm.add_tap_interface(
"123", constants.TYPE_LOCAL, "physnet1",
None, "tap1", dev_owner_prefix, None))
@ -790,7 +790,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
with mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
bridge_device.exists.return_value = True
bridge_device.delif.side_effect = RuntimeError
bridge_device.delif.return_value = False
bridge_device.owns_interface.side_effect = [True, False]
self.lbm.remove_interface("br0", 'tap0')