From af39fbe7df83ae3c84c08bbbb29d0f1fa196a51c Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Tue, 25 Aug 2020 10:50:12 -0400 Subject: [PATCH 1/3] tests: include all daemons in all-proto error tests Some daemons run in all-protocol-startup weren't included in error-output testing. Signed-off-by: Mark Stapp --- .../test_all_protocol_startup.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/topotests/all-protocol-startup/test_all_protocol_startup.py b/tests/topotests/all-protocol-startup/test_all_protocol_startup.py index 4602b039a6..8525838d96 100755 --- a/tests/topotests/all-protocol-startup/test_all_protocol_startup.py +++ b/tests/topotests/all-protocol-startup/test_all_protocol_startup.py @@ -263,6 +263,22 @@ def test_error_messages_daemons(): if log: error_logs += "r%s LDPd StdErr Output:\n" % i error_logs += log + + log = net['r1'].getStdErr('nhrpd') + if log: + error_logs += "r%s NHRPd StdErr Output:\n" % i + error_logs += log + + log = net['r1'].getStdErr('babeld') + if log: + error_logs += "r%s BABELd StdErr Output:\n" % i + error_logs += log + + log = net['r1'].getStdErr('pbrd') + if log: + error_logs += "r%s PBRd StdErr Output:\n" % i + error_logs += log + log = net['r%s' % i].getStdErr('zebra') if log: error_logs += "r%s Zebra StdErr Output:\n" @@ -1182,6 +1198,19 @@ def test_shutdown_check_stderr(): log = net['r1'].getStdErr('bgpd') if log: print("\nBGPd StdErr Log:\n" + log) + + log = net['r1'].getStdErr('nhrpd') + if log: + print("\nNHRPd StdErr Log:\n" + log) + + log = net['r1'].getStdErr('pbrd') + if log: + print("\nPBRd StdErr Log:\n" + log) + + log = net['r1'].getStdErr('babeld') + if log: + print("\nBABELd StdErr Log:\n" + log) + if (net['r1'].daemon_available('ldpd')): log = net['r1'].getStdErr('ldpd') if log: From a85a3d1ef9651e454d05bc84a91c3f464efa3fe0 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Tue, 25 Aug 2020 10:51:47 -0400 Subject: [PATCH 2/3] tests: remove some unused imports Remove unused imports from some topojson tests Signed-off-by: Mark Stapp --- tests/topotests/bgp_multi_vrf_topo2/test_bgp_multi_vrf_topo2.py | 1 - .../evpn_type5_test_topo1/test_evpn_type5_chaos_topo1.py | 1 - tests/topotests/evpn_type5_test_topo1/test_evpn_type5_topo1.py | 1 - 3 files changed, 3 deletions(-) diff --git a/tests/topotests/bgp_multi_vrf_topo2/test_bgp_multi_vrf_topo2.py b/tests/topotests/bgp_multi_vrf_topo2/test_bgp_multi_vrf_topo2.py index 27c4430a52..c36e66a60e 100755 --- a/tests/topotests/bgp_multi_vrf_topo2/test_bgp_multi_vrf_topo2.py +++ b/tests/topotests/bgp_multi_vrf_topo2/test_bgp_multi_vrf_topo2.py @@ -70,7 +70,6 @@ from lib.common_config import ( create_route_maps, shutdown_bringup_interface, start_router_daemons, - kill_router_daemons, create_static_routes, create_vrf_cfg, create_interfaces_cfg, diff --git a/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_chaos_topo1.py b/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_chaos_topo1.py index 941593e51f..e913105e43 100755 --- a/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_chaos_topo1.py +++ b/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_chaos_topo1.py @@ -62,7 +62,6 @@ from lib.common_config import ( verify_rib, step, start_router_daemons, - kill_router_daemons, create_static_routes, create_vrf_cfg, create_route_maps, diff --git a/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_topo1.py b/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_topo1.py index 8892d13eff..9e385823fc 100755 --- a/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_topo1.py +++ b/tests/topotests/evpn_type5_test_topo1/test_evpn_type5_topo1.py @@ -68,7 +68,6 @@ from lib.common_config import ( create_route_maps, verify_cli_json, start_router_daemons, - kill_router_daemons, create_static_routes, stop_router, start_router, From 942a224eae08e68fda403f3caba1dab4b006b894 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Tue, 25 Aug 2020 10:52:17 -0400 Subject: [PATCH 3/3] tests: fix router stop logic Change the public router stop method to always do a two-phase shutdown - once without waiting and a second time with a wait. Ordinary callers need to use this approach when stopping routers. Move the detailed internal details to a private method that tests should not call directly. Signed-off-by: Mark Stapp --- tests/topotests/lib/topogen.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/topotests/lib/topogen.py b/tests/topotests/lib/topogen.py index 37b9715010..a6cc5280ec 100644 --- a/tests/topotests/lib/topogen.py +++ b/tests/topotests/lib/topogen.py @@ -335,9 +335,7 @@ class Topogen(object): logger.info("stopping topology: {}".format(self.modname)) errors = "" for gear in self.gears.values(): - gear.stop(False, False) - for gear in self.gears.values(): - errors += gear.stop(True, False) + errors += gear.stop() if len(errors) > 0: assert "Errors found post shutdown - details follow:" == 0, errors @@ -703,14 +701,26 @@ class TopoRouter(TopoGear): return result - def stop(self, wait=True, assertOnError=True): + def __stop_internal(self, wait=True, assertOnError=True): """ - Stop router: + Stop router, private internal version * Kill daemons """ - self.logger.debug("stopping") + self.logger.debug("stopping: wait {}, assert {}".format( + wait, assertOnError)) return self.tgen.net[self.name].stopRouter(wait, assertOnError) + + def stop(self): + """ + Stop router cleanly: + * Signal daemons twice, once without waiting, and then a second time + with a wait to ensure the daemons exit cleanly + """ + self.logger.debug("stopping") + self.__stop_internal(False, False) + return self.__stop_internal() + def startDaemons(self, daemons): """ Start Daemons: to start specific daemon(user defined daemon only) @@ -819,8 +829,7 @@ class TopoRouter(TopoGear): if memleak_file is None: return - self.stop(False, False) - self.stop(wait=True) + self.stop() self.logger.info("running memory leak report") self.tgen.net[self.name].report_memory_leaks(memleak_file, testname)