Skip to content

Commit bcf2d69

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 bcf2d69

2 files changed

Lines changed: 54 additions & 1 deletion

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: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5592,11 +5592,64 @@ def test_invalid_pkey_values(self):
55925592

55935593

55945594
class TestSysUtils(unittest.TestCase):
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+
with mock.patch.object(SysUtil, "_link_infos_fetch", unstable_fetch):
5647+
with self.assertRaises(Exception) as ctx:
5648+
SysUtil.link_infos(refresh=True)
5649+
self.assertIn("stable link-infos", str(ctx.exception))
5650+
# 50 attempts: 2 fetches on first try, then 1 per retry (2 + 49 = 51).
5651+
self.assertEqual(next(counter), 51)
5652+
56005653

56015654
if __name__ == "__main__":
56025655
unittest.main()

0 commit comments

Comments
 (0)