From afd6b2f5aed86da2cc4b2e4b41a69d1197600089 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 2 Dec 2019 11:48:28 +0000 Subject: [PATCH] Avoid raising NetworkInterfaceNotFound exception in DHCP agent logs In "ip_lib.ensure_device_is_ready", before retrieving the interface attributes, a check is done to know if the interface exists. In case it does not exist, the exception "NetworkInterfaceNotFound" will not be raised and written in the logs. Conflicts: neutron/privileged/agent/linux/ip_lib.py Change-Id: I4b9fd0885d850601717274a5058e042871211bbb Closes-Bug: #1854723 (cherry picked from commit 8cc2765b5fdd229c9f6f19b83ad9e1ba030040ae) --- neutron/agent/linux/ip_lib.py | 8 +++++-- neutron/privileged/agent/linux/ip_lib.py | 24 ++++++++++++------- neutron/tests/unit/agent/linux/test_ip_lib.py | 14 +++++++---- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 4f929c365f1..362ece1838e 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -509,6 +509,10 @@ class IpLinkCommand(IpDeviceCommandBase): return privileged.get_link_attributes(self.name, self._parent.namespace) + @property + def exists(self): + return privileged.interface_exists(self.name, self._parent.namespace) + class IpAddrCommand(IpDeviceCommandBase): COMMAND = 'addr' @@ -946,8 +950,8 @@ def ensure_device_is_ready(device_name, namespace=None): dev = IPDevice(device_name, namespace=namespace) try: # Ensure the device has a MAC address and is up, even if it is already - # up. If the device doesn't exist, a RuntimeError will be raised. - if not dev.link.address: + # up. + if not dev.link.exists or not dev.link.address: LOG.error("Device %s cannot be used as it has no MAC " "address", device_name) return False diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 3403bc08dc4..3bb04297e45 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -17,6 +17,7 @@ import socket from neutron_lib import constants from oslo_concurrency import lockutils +from oslo_log import log as logging import pyroute2 from pyroute2 import netlink from pyroute2.netlink import exceptions as netlink_exceptions @@ -31,6 +32,8 @@ from neutron._i18n import _ from neutron import privileged +LOG = logging.getLogger(__name__) + _IP_VERSION_FAMILY_MAP = {4: socket.AF_INET, 6: socket.AF_INET6} NETNS_RUN_DIR = '/var/run/netns' @@ -240,12 +243,17 @@ def _translate_ip_device_exception(e, device=None, namespace=None): namespace=namespace) -def get_link_id(device, namespace): - try: - with get_iproute(namespace) as ip: - return ip.link_lookup(ifname=device)[0] - except IndexError: - raise NetworkInterfaceNotFound(device=device, namespace=namespace) +def get_link_id(device, namespace, raise_exception=True): + with get_iproute(namespace) as ip: + link_id = ip.link_lookup(ifname=device) + if not link_id or len(link_id) < 1: + if raise_exception: + raise NetworkInterfaceNotFound(device=device, namespace=namespace) + else: + LOG.debug('Interface %(dev)s not found in namespace %(namespace)s', + {'dev': device, 'namespace': namespace}) + return None + return link_id[0] def _run_iproute_link(command, device, namespace=None, **kwargs): @@ -376,10 +384,8 @@ def delete_interface(ifname, namespace, **kwargs): @privileged.default.entrypoint def interface_exists(ifname, namespace): try: - idx = get_link_id(ifname, namespace) + idx = get_link_id(ifname, namespace, raise_exception=False) return bool(idx) - except NetworkInterfaceNotFound: - return False except OSError as e: if e.errno == errno.ENOENT: return False diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 63b97962812..905f83f5a0f 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -936,15 +936,21 @@ class TestDeviceExists(base.BaseTestCase): def test_ensure_device_is_ready_no_link_address(self): with mock.patch.object( - priv_lib, 'get_link_attributes' - ) as get_link_attributes, mock.patch.object( - priv_lib, 'set_link_attribute' - ) as set_link_attribute: + priv_lib, 'get_link_attributes') as get_link_attributes, \ + mock.patch.object(priv_lib, 'set_link_attribute') as \ + set_link_attribute, \ + mock.patch.object(priv_lib, 'interface_exists', + return_value=True): get_link_attributes.return_value = {} self.assertFalse(ip_lib.ensure_device_is_ready("lo")) get_link_attributes.assert_called_once_with("lo", None) set_link_attribute.assert_not_called() + def test_ensure_device_is_ready_no_device(self): + with mock.patch.object(priv_lib, 'interface_exists', + return_value=False): + self.assertFalse(ip_lib.ensure_device_is_ready("lo")) + class TestGetRoutingTable(base.BaseTestCase): ip_db_interfaces = {