From f2bf1f95b3a4f0d8e44675eb3915fed0d62ba38f Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Sun, 16 Feb 2025 09:51:22 +0100 Subject: [PATCH 1/5] tests: Remove bgpd marker in `srv6_encap_src_addr` topotest The `srv6_encap_src_addr` does not use bgp. As such, it should not have bgpd marker. Signed-off-by: Carmine Scarpitta --- tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py index 854bc1cdad..c46274ca01 100755 --- a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py +++ b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py @@ -27,7 +27,7 @@ from lib import topotest from lib.topogen import Topogen, TopoRouter, get_topogen from lib.topolog import logger -pytestmark = [pytest.mark.bgpd, pytest.mark.sharpd] +pytestmark = [pytest.mark.sharpd] def open_json_file(filename): From cb38a6f9624f362cad65af23f8fe81ff86738402 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Sun, 16 Feb 2025 10:13:15 +0100 Subject: [PATCH 2/5] tests: Do not load sharpd config in `srv6_encap_src_addr` topotest The `srv6_encap_src_addr` topotest tries to load sharpd.conf file that does not exist, which produces the following warning: ``` 2025-02-16 09:23:35,151 WARNING: topo: missing config 'r1' for '/media/frr/tests/topotests/srv6_encap_src_addr/r1/sharpd.conf' creating empty file '/etc/frr/sharpd.conf' ``` Since this topotest doesn't actually use sharpd, there's no point in loading the config file. Signed-off-by: Carmine Scarpitta --- .../topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py index c46274ca01..6aeda3f57a 100755 --- a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py +++ b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py @@ -49,9 +49,6 @@ def setup_module(mod): router.load_config( TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) ) - router.load_config( - TopoRouter.RD_SHARP, os.path.join(CWD, "{}/sharpd.conf".format(rname)) - ) tgen.start_router() From d8483f410eb6896e57ba625dd534320dc29e14e4 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Sun, 16 Feb 2025 10:14:16 +0100 Subject: [PATCH 3/5] tests: Do not load bgpd config in `srv6_encap_src_addr` topotest The `srv6_encap_src_addr` topotest tries to load bgpd.conf file that does not exist, which produces the following warning: ``` 2025-02-16 09:23:35,151 WARNING: topo: missing config 'r1' for '/media/frr/tests/topotests/srv6_encap_src_addr/r1/bgpd.conf' creating empty file '/etc/frr/bgpd.conf' ``` Since this topotest doesn't actually use bgpd, there's no point in loading the config file. Signed-off-by: Carmine Scarpitta --- .../topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py index 6aeda3f57a..271f54885a 100755 --- a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py +++ b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py @@ -46,9 +46,6 @@ def setup_module(mod): router.load_config( TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) ) - router.load_config( - TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) - ) tgen.start_router() From 206a647ea64b4810386b718efe83ffb734f18bd9 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Sun, 16 Feb 2025 10:23:07 +0100 Subject: [PATCH 4/5] tests: Increase retry timeout in `srv6_encap_src_addr` topotest The `srv6_encap_src_addr` topotest uses a waiting time that is too small. For this reason during startup it prints a warning: ``` 2025-02-16 09:23:47,704 WARNING: topo: Waiting time is too small (count=10, wait=1), using default values (count=20, wait=3) ``` This commit increases the waiting time to fix the warning. Signed-off-by: Carmine Scarpitta --- .../srv6_encap_src_addr/test_srv6_encap_src_addr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py index 271f54885a..da26a739a2 100755 --- a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py +++ b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py @@ -64,7 +64,7 @@ def test_zebra_srv6_encap_src_addr(tgen): expected = json.loads(open(json_file).read()) ok = topotest.router_json_cmp_retry( - r1, "show segment-routing srv6 manager json", expected + r1, "show segment-routing srv6 manager json", expected, retry_timeout=15 ) assert ok, '"r1" JSON output mismatches' @@ -93,7 +93,7 @@ def test_zebra_srv6_encap_src_addr_unset(tgen): expected = json.loads(open(json_file).read()) ok = topotest.router_json_cmp_retry( - r1, "show segment-routing srv6 manager json", expected + r1, "show segment-routing srv6 manager json", expected, retry_timeout=15 ) assert ok, '"r1" JSON output mismatches' @@ -122,7 +122,7 @@ def test_zebra_srv6_encap_src_addr_set(tgen): expected = json.loads(open(json_file).read()) ok = topotest.router_json_cmp_retry( - r1, "show segment-routing srv6 manager json", expected + r1, "show segment-routing srv6 manager json", expected, retry_timeout=15 ) assert ok, '"r1" JSON output mismatches' From c621b5e759ecbbb1ac7b43e8d70428fd6b0a50a1 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Sun, 16 Feb 2025 10:59:05 +0100 Subject: [PATCH 5/5] tests: Fix intermittent failures in `srv6_encap_src_addr` topotest The `srv6_encap_src_addr` runs a vtysh command to configure the SRv6 encapsulation source address and then immediately invokes an iproute2 command to verify that zebra has set this address in the kernel. There is no wait between the two operations and the verification is attempted only once. If the topotest does not find the expected address it fails immediately. The problem is that when topotest is run on a heavyily loaded system, it can take some time for zebra to set the address in the kernel. In this case, when the topotest checks the kernel address right after running the vtysh command, it doesn't find the expected address because zebra hasn't set it yet. This commit gives zebra some time to configure the address. It keeps to check that the address is the expected one for about 1 minute. If after 1 minute the address is not the expected one then the test fails. Signed-off-by: Carmine Scarpitta --- .../r1/ip_tunsrc_show.json | 5 +++ .../r1/ip_tunsrc_show_set.json | 5 +++ .../r1/ip_tunsrc_show_unset.json | 5 +++ .../test_srv6_encap_src_addr.py | 36 +++++++++++++++---- 4 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show.json create mode 100644 tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_set.json create mode 100644 tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_unset.json diff --git a/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show.json b/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show.json new file mode 100644 index 0000000000..868e7180f0 --- /dev/null +++ b/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show.json @@ -0,0 +1,5 @@ +[ + { + "tunsrc": "fc00:0:1::1" + } +] \ No newline at end of file diff --git a/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_set.json b/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_set.json new file mode 100644 index 0000000000..868e7180f0 --- /dev/null +++ b/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_set.json @@ -0,0 +1,5 @@ +[ + { + "tunsrc": "fc00:0:1::1" + } +] \ No newline at end of file diff --git a/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_unset.json b/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_unset.json new file mode 100644 index 0000000000..309050f5ce --- /dev/null +++ b/tests/topotests/srv6_encap_src_addr/r1/ip_tunsrc_show_unset.json @@ -0,0 +1,5 @@ +[ + { + "tunsrc": "::" + } +] \ No newline at end of file diff --git a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py index da26a739a2..2db5e70695 100755 --- a/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py +++ b/tests/topotests/srv6_encap_src_addr/test_srv6_encap_src_addr.py @@ -18,6 +18,7 @@ import os import sys import json import pytest +from functools import partial CWD = os.path.dirname(os.path.realpath(__file__)) sys.path.append(os.path.join(CWD, "../")) @@ -54,6 +55,28 @@ def teardown_module(): tgen.stop_topology() +def router_compare_json_output(rname, command, reference, count=120, wait=0.5): + "Compare router JSON output" + + def _router_json_cmp(router, cmd, data, exact=False): + """ + Runs `cmd` that returns JSON data and compare with `data` contents. + """ + return topotest.json_cmp(json.loads(router.cmd(cmd)), data, exact) + + logger.info('Comparing router "%s" "%s" output', rname, command) + + tgen = get_topogen() + filename = "{}/{}/{}".format(CWD, rname, reference) + expected = json.loads(open(filename).read()) + + # Run test function until we get an result. Wait at most 60 seconds. + test_func = partial(_router_json_cmp, tgen.gears[rname], command, expected) + _, diff = topotest.run_and_expect(test_func, None, count=count, wait=wait) + assertmsg = '"{}" JSON output mismatches the expected result'.format(rname) + assert diff is None, assertmsg + + def test_zebra_srv6_encap_src_addr(tgen): "Test SRv6 encapsulation source address." logger.info("Test SRv6 encapsulation source address.") @@ -68,8 +91,7 @@ def test_zebra_srv6_encap_src_addr(tgen): ) assert ok, '"r1" JSON output mismatches' - output = r1.cmd("ip sr tunsrc show") - assert output == "tunsrc addr fc00:0:1::1\n" + router_compare_json_output("r1", "ip --json sr tunsrc show", "ip_tunsrc_show.json") def test_zebra_srv6_encap_src_addr_unset(tgen): @@ -97,8 +119,9 @@ def test_zebra_srv6_encap_src_addr_unset(tgen): ) assert ok, '"r1" JSON output mismatches' - output = r1.cmd("ip sr tunsrc show") - assert output == "tunsrc addr ::\n" + router_compare_json_output( + "r1", "ip --json sr tunsrc show", "ip_tunsrc_show_unset.json" + ) def test_zebra_srv6_encap_src_addr_set(tgen): @@ -126,8 +149,9 @@ def test_zebra_srv6_encap_src_addr_set(tgen): ) assert ok, '"r1" JSON output mismatches' - output = r1.cmd("ip sr tunsrc show") - assert output == "tunsrc addr fc00:0:1::1\n" + router_compare_json_output( + "r1", "ip --json sr tunsrc show", "ip_tunsrc_show_set.json" + ) if __name__ == "__main__":