From 74a9e832d790f659eb10b22ba0428b60733d384e Mon Sep 17 00:00:00 2001 From: elajkat Date: Tue, 8 Mar 2022 17:44:11 +0100 Subject: [PATCH] Add retry for privsep get_link_devices pyroute 0.6.6 introduced a new exception NetlinkDumpInterrupted which is raised when NLM_F_DUMP_INTR is set in the flags during dump of devices. The suggestion from pyroute developers is to retry in case of this exception (see [1]). [1]: https://github.com/svinota/pyroute2/issues/874#issuecomment-1063139555 Closes-Bug: #1962608 Change-Id: Ie195ad596fd148708fc30946bde964d52444afee --- lower-constraints.txt | 2 +- neutron/privileged/agent/linux/ip_lib.py | 49 +++++++++++++++++++ .../privileged/agent/linux/test_ip_lib.py | 9 +++- requirements.txt | 2 +- 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index 2566cf41783..cf60cc2db7e 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -98,7 +98,7 @@ PyMySQL==0.7.6 pyOpenSSL==17.1.0 pyparsing==2.1.0 pyperclip==1.5.27 -pyroute2==0.6.4 +pyroute2==0.6.6 python-dateutil==2.7.0 python-designateclient==2.7.0 python-editor==1.0.3 diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 91a9faed8f1..c31fed1cadb 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -24,6 +24,7 @@ from pyroute2.netlink import rtnl # pylint: disable=no-name-in-module from pyroute2.netlink.rtnl import ifinfmsg # pylint: disable=no-name-in-module from pyroute2.netlink.rtnl import ndmsg # pylint: disable=no-name-in-module from pyroute2 import netns # pylint: disable=no-name-in-module +import tenacity from neutron._i18n import _ from neutron import privileged @@ -376,6 +377,12 @@ def set_link_bridge_master(device, bridge, namespace=None): master=bridge_idx) +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.link_cmd.entrypoint def get_link_attributes(device, namespace): link = _run_iproute_link("get", device, namespace)[0] @@ -392,6 +399,12 @@ def get_link_attributes(device, namespace): } +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.link_cmd.entrypoint def get_link_vfs(device, namespace): link = _run_iproute_link('get', device, namespace=namespace, ext_mask=1)[0] @@ -463,6 +476,12 @@ def delete_neigh_entry(ip_version, ip_address, mac_address, device, namespace, raise +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.default.entrypoint def dump_neigh_entries(ip_version, device, namespace, **kwargs): """Dump all neighbour entries. @@ -560,6 +579,12 @@ def make_serializable(value): return _ensure_string(value) +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.default.entrypoint def get_link_devices(namespace, **kwargs): """List interfaces in a namespace @@ -590,6 +615,12 @@ def get_device_names(namespace, **kwargs): return device_names +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.default.entrypoint def get_ip_addresses(namespace, **kwargs): """List of IP addresses in a namespace @@ -605,6 +636,12 @@ def get_ip_addresses(namespace, **kwargs): raise +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.default.entrypoint def list_ip_rules(namespace, ip_version, match=None, **kwargs): """List all IP rules""" @@ -719,6 +756,12 @@ def add_ip_route(namespace, cidr, ip_version, device=None, via=None, raise +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.default.entrypoint def list_ip_routes(namespace, ip_version, device=None, table=None, **kwargs): """List IP routes""" @@ -748,6 +791,12 @@ def delete_ip_route(namespace, cidr, ip_version, device=None, via=None, raise +@tenacity.retry( + retry=tenacity.retry_if_exception_type( + netlink_exceptions.NetlinkDumpInterrupted), + wait=tenacity.wait_exponential(multiplier=0.02, max=1), + stop=tenacity.stop_after_delay(8), + reraise=True) @privileged.default.entrypoint def list_bridge_fdb(namespace=None, **kwargs): """List bridge fdb table""" diff --git a/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py b/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py index 8afa0b7a318..9a2177777e3 100644 --- a/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py @@ -17,6 +17,7 @@ from unittest import mock import pyroute2 from pyroute2 import netlink +from pyroute2.netlink import exceptions as netlink_exceptions from pyroute2.netlink.rtnl import ifinfmsg from neutron.privileged.agent.linux import ip_lib as priv_lib @@ -254,7 +255,8 @@ class IpLibTestCase(base.BaseTestCase): priv_lib.privileged.link_cmd.client_mode = False self.addCleanup(self._clean, client_mode) with mock.patch.object(priv_lib, '_run_iproute_link') as mock_iplink: - mock_iplink.return_value = [value] + mock_iplink.side_effect = [ + netlink_exceptions.NetlinkDumpInterrupted(), value] result = priv_lib.get_link_vfs('device', 'namespace') exp = {0: {'mac': 'mac_0', 'link_state': 0, 'max_tx_rate': 0, 'min_tx_rate': 0}, @@ -263,6 +265,11 @@ class IpLibTestCase(base.BaseTestCase): 2: {'mac': 'mac_2', 'link_state': 2, 'max_tx_rate': 2000, 'min_tx_rate': 1000}} self.assertEqual(exp, result) + # Check that _run_iproute_link was called twice + mock_iplink.assert_has_calls( + [mock.call('get', 'device', namespace='namespace', ext_mask=1), + mock.call('get', 'device', namespace='namespace', ext_mask=1)] + ) class MakeSerializableTestCase(base.BaseTestCase): diff --git a/requirements.txt b/requirements.txt index 24886b6c301..51c6ad2e02b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -49,7 +49,7 @@ ovs>=2.10.0 # Apache-2.0 ovsdbapp>=1.15.0 # Apache-2.0 packaging>=20.4 # Apache-2.0 psutil>=5.3.0 # BSD -pyroute2>=0.6.4;sys_platform!='win32' # Apache-2.0 (+ dual licensed GPL2) +pyroute2>=0.6.6;sys_platform!='win32' # Apache-2.0 (+ dual licensed GPL2) pyOpenSSL>=17.1.0 # Apache-2.0 python-novaclient>=9.1.0 # Apache-2.0