Skip to content

Commit 7f7acc1

Browse files
committed
fix: correctly retry device read when device is unstable
Cause: The retry loop for reading from a device would bail out of the loop before retrying because the loop should have used a greater-than-or-equal to test the retry condition rather than a less-than. Consequence: If the device is not stable, the loop would bail and issue an exception rather than trying to retry the read. Fix: Use the correct loop condition. Result: The network role can correctly retry the read when the device is unstable. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
1 parent fd99c0b commit 7f7acc1

2 files changed

Lines changed: 61 additions & 2 deletions

File tree

library/network_connections.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def link_infos(cls, refresh=False):
235235
"cannot read stable link-infos. They keep changing"
236236
)
237237
except Exception:
238-
if try_count < 50:
238+
if try_count >= 50:
239239
raise
240240
continue
241241
break

tests/unit/test_network_connections.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5591,12 +5591,71 @@ def test_invalid_pkey_values(self):
55915591
)
55925592

55935593

5594-
class TestSysUtils(unittest.TestCase):
5594+
class TestSysUtils(Python26CompatTestCase):
5595+
def setUp(self):
5596+
if hasattr(SysUtil, "_link_infos"):
5597+
del SysUtil._link_infos
5598+
55955599
def test_link_read_permaddress(self):
55965600
self.assertEqual(SysUtil._link_read_permaddress("lo"), "00:00:00:00:00:00")
55975601
self.assertEqual(SysUtil._link_read_permaddress("fakeiface"), None)
55985602
self.assertEqual(SysUtil._link_read_permaddress("morethansixteenchars"), None)
55995603

5604+
def test_link_infos_retries_when_fetch_results_differ(self):
5605+
"""Simulate unstable /sys reads; link_infos must retry, not fail once."""
5606+
stable = {
5607+
"eth0": {
5608+
"ifindex": 2,
5609+
"ifname": "eth0",
5610+
"address": "52:54:00:00:00:01",
5611+
"perm-address": None,
5612+
"bond-port-perm-hwaddr": None,
5613+
}
5614+
}
5615+
unstable = {
5616+
"eth1": {
5617+
"ifindex": 3,
5618+
"ifname": "eth1",
5619+
"address": "52:54:00:00:00:02",
5620+
"perm-address": None,
5621+
"bond-port-perm-hwaddr": None,
5622+
}
5623+
}
5624+
fetch_mock = mock.Mock(side_effect=[unstable, stable, stable])
5625+
with mock.patch.object(SysUtil, "_link_infos_fetch", fetch_mock):
5626+
result = SysUtil.link_infos(refresh=True)
5627+
self.assertEqual(result, stable)
5628+
self.assertEqual(fetch_mock.call_count, 3)
5629+
5630+
def test_link_infos_raises_after_max_retries(self):
5631+
"""Unstable reads must eventually fail after 50 attempts."""
5632+
counter = itertools.count()
5633+
5634+
def unstable_fetch():
5635+
n = next(counter)
5636+
return {
5637+
"eth{n}".format(n=n): {
5638+
"ifindex": n,
5639+
"ifname": "eth{n}".format(n=n),
5640+
"address": "52:54:00:00:00:{0:02x}".format(n % 256),
5641+
"perm-address": None,
5642+
"bond-port-perm-hwaddr": None,
5643+
}
5644+
}
5645+
5646+
# Use Mock(side_effect=...) so Py2.7 does not treat the replacement as an
5647+
# unbound method (plain functions patched onto @staticmethod fail there).
5648+
fetch_mock = mock.Mock(side_effect=unstable_fetch)
5649+
with mock.patch.object(SysUtil, "_link_infos_fetch", fetch_mock):
5650+
self.assertRaisesRegex(
5651+
Exception,
5652+
"stable link-infos",
5653+
SysUtil.link_infos,
5654+
refresh=True,
5655+
)
5656+
# 50 attempts: 2 fetches on first try, then 1 per retry (2 + 49 = 51).
5657+
self.assertEqual(fetch_mock.call_count, 51)
5658+
56005659

56015660
if __name__ == "__main__":
56025661
unittest.main()

0 commit comments

Comments
 (0)