tests: fix broken bgp GR test (non-deterministic)

- bugs in the support library function `verify_gr_address_family`
allowed this test to pass depending on ordering of python dictinoary
keys. Fix the bugs, fix the test.

Signed-off-by: Christian Hopps <chopps@labn.net>

tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2.py

@@ -249,6 +249,8 @@ def configure_gr_followed_by_clear(tgen, topo, input_dict, tc_name, dut, peer):
     This function groups the repetitive function calls into one function.
     """
 
+    logger.info("configure_gr_followed_by_clear: dut %s peer %s", dut, peer)
+
     result = create_router_bgp(tgen, topo, input_dict)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
@@ -766,9 +768,7 @@ def test_BGP_GR_10_p2(request):
     # Creating configuration from JSON
     reset_config_on_routers(tgen)
 
-    logger.info(
-        "[Step 1] : Test Setup " "[Helper Mode]R3-----R1[Restart Mode] initialized"
-    )
+    step("Test Setup: [Helper Mode]R3-----R1[Restart Mode] initialized")
 
     # Configure graceful-restart
     input_dict = {
@@ -847,6 +847,8 @@ def test_BGP_GR_10_p2(request):
     configure_gr_followed_by_clear(tgen, topo, input_dict, tc_name, dut="r1", peer="r3")
 
     for addr_type in ADDR_TYPES:
+        step("Verifying GR config and operational state for addr_type {}".format(addr_type))
+
         result = verify_graceful_restart(
             tgen, topo, addr_type, input_dict, dut="r1", peer="r3"
         )
@@ -870,7 +872,7 @@ def test_BGP_GR_10_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv4Unicast", dut="r1"
+            tgen, topo, addr_type, "ipv4Unicast", dut="r1", peer="r3",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
@@ -878,7 +880,7 @@ def test_BGP_GR_10_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv6Unicast", dut="r1"
+            tgen, topo, addr_type, "ipv6Unicast", dut="r1", peer="r3",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
@@ -886,7 +888,7 @@ def test_BGP_GR_10_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv4Unicast", dut="r3"
+            tgen, topo, addr_type, "ipv4Unicast", dut="r3", peer="r1",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
@@ -894,12 +896,14 @@ def test_BGP_GR_10_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv6Unicast", dut="r3"
+            tgen, topo, addr_type, "ipv6Unicast", dut="r3", peer="r1",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
         )
 
+    step("Killing bgpd on r1")
+
     # Kill BGPd daemon on R1
     kill_router_daemons(tgen, "r1", ["bgpd"])
 
@@ -917,6 +921,8 @@ def test_BGP_GR_10_p2(request):
             tc_name, result
         )
 
+    step("Starting bgpd on r1")
+
     # Start BGPd daemon on R1
     start_router_daemons(tgen, "r1", ["bgpd"])
 
@@ -1671,7 +1677,7 @@ def test_BGP_GR_26_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv4Unicast", dut="r1"
+            tgen, topo, addr_type, "ipv4Unicast", dut="r1", peer="r3",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
@@ -1679,7 +1685,7 @@ def test_BGP_GR_26_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv6Unicast", dut="r1"
+            tgen, topo, addr_type, "ipv6Unicast", dut="r1", peer="r3",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
@@ -1687,7 +1693,7 @@ def test_BGP_GR_26_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv4Unicast", dut="r3"
+            tgen, topo, addr_type, "ipv4Unicast", dut="r3", peer="r1",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result
@@ -1695,7 +1701,7 @@ def test_BGP_GR_26_p2(request):
 
         # verify multi address family
         result = verify_gr_address_family(
-            tgen, topo, addr_type, "ipv6Unicast", dut="r3"
+            tgen, topo, addr_type, "ipv6Unicast", dut="r3", peer="r1",
         )
         assert result is True, "Testcase {} : Failed \n Error {}".format(
             tc_name, result

tests/topotests/lib/bgp.py

@@ -3765,7 +3765,7 @@ def verify_graceful_restart_timers(tgen, topo, addr_type, input_dict, dut, peer)
 
 
 @retry(retry_timeout=8)
-def verify_gr_address_family(tgen, topo, addr_type, addr_family, dut, expected=True):
+def verify_gr_address_family(tgen, topo, addr_type, addr_family, dut, peer, expected=True):
     """
     This API is to verify gr_address_family in the BGP gr capability advertised
     by the neighbor router
@@ -3777,42 +3777,48 @@ def verify_gr_address_family(tgen, topo, addr_type, addr_family, dut, expected=T
     * `addr_type` : ip type ipv4/ipv6
     * `addr_type` : ip type IPV4 Unicast/IPV6 Unicast
     * `dut`: input dut router name
+    * `peer`: input peer router to check
     * `expected` : expected results from API, by-default True
 
     Usage
     -----
 
-    result = verify_gr_address_family(tgen, topo, "ipv4", "ipv4Unicast", "r1")
+    result = verify_gr_address_family(tgen, topo, "ipv4", "ipv4Unicast", "r1", "r3")
 
     Returns
     -------
-    errormsg(str) or True
+    errormsg(str) or None
     """
 
     logger.debug("Entering lib API: {}".format(sys._getframe().f_code.co_name))
 
-    for router, rnode in tgen.routers().items():
-        if router != dut:
-            continue
-
-        bgp_addr_type = topo["routers"][router]["bgp"]["address_family"]
-
-        if addr_type in bgp_addr_type:
     if not check_address_types(addr_type):
-                continue
+        logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name))
+        return
 
-            bgp_neighbors = bgp_addr_type[addr_type]["unicast"]["neighbor"]
+    routers = tgen.routers()
+    if dut not in routers:
+        return "{} not in routers".format(dut)
 
-            for bgp_neighbor, peer_data in bgp_neighbors.items():
-                for dest_link, peer_dict in peer_data["dest_link"].items():
-                    data = topo["routers"][bgp_neighbor]["links"]
+    rnode = routers[dut]
+    bgp_addr_type = topo["routers"][dut]["bgp"]["address_family"]
 
-                    if dest_link in data:
-                        neighbor_ip = data[dest_link][addr_type].split("/")[0]
+    if addr_type not in bgp_addr_type:
+        return "{} not in bgp_addr_types".format(addr_type)
+
+    if peer not in bgp_addr_type[addr_type]["unicast"]["neighbor"]:
+        return "{} not a peer of {} over {}".format(peer, dut, addr_type)
+
+    nbr_links = topo["routers"][peer]["links"]
+    if dut not in nbr_links or addr_type not in nbr_links[dut]:
+        return "peer {} missing back link to {} over {}".format(peer, dut, addr_type)
+
+    neighbor_ip = nbr_links[dut][addr_type].split("/")[0]
 
     logger.info(
-                "[DUT: {}]: Checking bgp graceful-restart"
-                " show o/p  {}".format(dut, neighbor_ip)
+        "[DUT: {}]: Checking bgp graceful-restart show o/p {} for {}".format(
+            dut, neighbor_ip, addr_family
+        )
     )
 
     show_bgp_graceful_json = run_frr_cmd(