openstack
/
neutron
Code Issues Proposed changes

Do not use exceptions in get_link_id() to control the code flow 

Instead of using exceptions as control flow, check the Pyroute2 command
result and only raise the Neutron exception if needed. This will also
reduce the traceback log in case of raising NetworkInterfaceNotFound.

Although in Python the use of exception for this is common, this is
usually considered an antipattern.

Change-Id: I0e8bb3b0f6a46f2bac75e38c6ac6cdd094247f89
Closes-Bug: #1849449
This commit is contained in:
Rodolfo Alonso Hernandez 2019-10-23 08:31:59 +00:00
parent f5a827c2be
commit ecef65b9b5
2 changed files with 22 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
neutron/privileged/agent/linux/ip_lib.py
View File

@ -241,11 +241,11 @@ def _translate_ip_device_exception(e, device=None, namespace=None):
def get_link_id(device, namespace):
try:
with get_iproute(namespace) as ip:
return ip.link_lookup(ifname=device)[0]
except IndexError:
with get_iproute(namespace) as ip:
link_id = ip.link_lookup(ifname=device)
if not link_id or len(link_id) < 1:
raise NetworkInterfaceNotFound(device=device, namespace=namespace)
return link_id[0]
def _run_iproute_link(command, device, namespace=None, **kwargs):

23
neutron/tests/unit/privileged/agent/linux/test_ip_lib.py
View File

@ -47,11 +47,24 @@ class IpLibTestCase(base.BaseTestCase):
def test_run_iproute_link_interface_not_exists(self):
with mock.patch.object(pyroute2, "IPRoute") as iproute_mock:
ip_mock = iproute_mock()
ip_mock.__enter__().link_lookup.return_value = []
self.assertRaises(
priv_lib.NetworkInterfaceNotFound,
priv_lib._run_iproute_link,
"test_cmd", "eth0", None, test_param="test_value")
ret_values = [
[], # No interface found.
None, # Unexpected output but also handled.
]
for ret_val in ret_values:
ip_mock.__enter__().link_lookup.return_value = ret_val
self.assertRaises(
priv_lib.NetworkInterfaceNotFound,
priv_lib._run_iproute_link,
"test_cmd", "eth0", None, test_param="test_value")
@mock.patch.object(priv_lib, 'get_iproute')
def test_get_link_id(self, mock_iproute):
mock_ip = mock.Mock()
mock_ip.link_lookup.return_value = ['interface_id']
mock_iproute.return_value.__enter__.return_value = mock_ip
self.assertEqual('interface_id',
priv_lib.get_link_id('device', 'namespace'))
def test_run_iproute_link_interface_removed_during_call(self):
with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: