From: Christian Hopps Date: Fri, 20 Aug 2021 12:24:23 +0000 (-0400) Subject: tests: fix broken bgp GR test (non-deterministic) X-Git-Tag: base_8.1~175^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=8d2e57fe2807db2fdcf3a572829962747810053b;p=matthieu%2Ffrr.git 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 --- diff --git a/tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2.py b/tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2.py index e7ce216042..83bf4fcc18 100644 --- a/tests/topotests/bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2.py +++ b/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 diff --git a/tests/topotests/lib/bgp.py b/tests/topotests/lib/bgp.py index 2f1f67439f..922dee1291 100644 --- a/tests/topotests/lib/bgp.py +++ b/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,80 +3777,86 @@ 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 + if not check_address_types(addr_type): + logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) + return - bgp_addr_type = topo["routers"][router]["bgp"]["address_family"] + routers = tgen.routers() + if dut not in routers: + return "{} not in routers".format(dut) - if addr_type in bgp_addr_type: - if not check_address_types(addr_type): - continue + rnode = routers[dut] + bgp_addr_type = topo["routers"][dut]["bgp"]["address_family"] - bgp_neighbors = bgp_addr_type[addr_type]["unicast"]["neighbor"] + if addr_type not in bgp_addr_type: + return "{} not in bgp_addr_types".format(addr_type) - 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"] + if peer not in bgp_addr_type[addr_type]["unicast"]["neighbor"]: + return "{} not a peer of {} over {}".format(peer, dut, addr_type) - if dest_link in data: - neighbor_ip = data[dest_link][addr_type].split("/")[0] + 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) - logger.info( - "[DUT: {}]: Checking bgp graceful-restart" - " show o/p {}".format(dut, neighbor_ip) - ) + neighbor_ip = nbr_links[dut][addr_type].split("/")[0] - show_bgp_graceful_json = run_frr_cmd( - rnode, - "show bgp {} neighbor {} graceful-restart json".format( - addr_type, neighbor_ip - ), - isjson=True, - ) + logger.info( + "[DUT: {}]: Checking bgp graceful-restart show o/p {} for {}".format( + dut, neighbor_ip, addr_family + ) + ) - show_bgp_graceful_json_out = show_bgp_graceful_json[neighbor_ip] + show_bgp_graceful_json = run_frr_cmd( + rnode, + "show bgp {} neighbor {} graceful-restart json".format( + addr_type, neighbor_ip + ), + isjson=True, + ) - if show_bgp_graceful_json_out["neighborAddr"] == neighbor_ip: - logger.info("Neighbor ip matched {}".format(neighbor_ip)) - else: - errormsg = "Neighbor ip NOT a match {}".format(neighbor_ip) - return errormsg + show_bgp_graceful_json_out = show_bgp_graceful_json[neighbor_ip] - if addr_family == "ipv4Unicast": - if "ipv4Unicast" in show_bgp_graceful_json_out: - logger.info("ipv4Unicast present for {} ".format(neighbor_ip)) - return True - else: - errormsg = "ipv4Unicast NOT present for {} ".format(neighbor_ip) - return errormsg + if show_bgp_graceful_json_out["neighborAddr"] == neighbor_ip: + logger.info("Neighbor ip matched {}".format(neighbor_ip)) + else: + errormsg = "Neighbor ip NOT a match {}".format(neighbor_ip) + return errormsg - elif addr_family == "ipv6Unicast": - if "ipv6Unicast" in show_bgp_graceful_json_out: - logger.info("ipv6Unicast present for {} ".format(neighbor_ip)) - return True - else: - errormsg = "ipv6Unicast NOT present for {} ".format(neighbor_ip) - return errormsg - else: - errormsg = "Aaddress family: {} present for {} ".format( - addr_family, neighbor_ip - ) - return errormsg + if addr_family == "ipv4Unicast": + if "ipv4Unicast" in show_bgp_graceful_json_out: + logger.info("ipv4Unicast present for {} ".format(neighbor_ip)) + return True + else: + errormsg = "ipv4Unicast NOT present for {} ".format(neighbor_ip) + return errormsg + + elif addr_family == "ipv6Unicast": + if "ipv6Unicast" in show_bgp_graceful_json_out: + logger.info("ipv6Unicast present for {} ".format(neighbor_ip)) + return True + else: + errormsg = "ipv6Unicast NOT present for {} ".format(neighbor_ip) + return errormsg + else: + errormsg = "Aaddress family: {} present for {} ".format( + addr_family, neighbor_ip + ) + return errormsg logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name))