From: Christian Hopps Date: Sat, 22 Apr 2023 01:59:33 +0000 (-0400) Subject: tests: fix check for daemon startup, remove sleep hack X-Git-Tag: base_9.0~147^2~5 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=260268c45cd5f705d72ab2c195b7ed32c80ea40a;p=matthieu%2Ffrr.git tests: fix check for daemon startup, remove sleep hack - Remove the .pid and .vty files and then wait for them to show back up. - Fix broken BGP GR test to not fail now that it's bug is exposed. It only worked b/c when starting a daemon the pid file still existed and blocked the bogus second BGP launch from happening. Signed-off-by: Christian Hopps --- diff --git a/tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-3.py b/tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-3.py index f8a01a1c9d..1a8f8302ff 100644 --- a/tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-3.py +++ b/tests/topotests/bgp_gr_functionality_topo1/test_bgp_gr_functionality_topo1-3.py @@ -1159,7 +1159,7 @@ def test_BGP_GR_TC_31_2_p1(request): reset_config_on_routers(tgen) logger.info( - "[Phase 1] : Test Setup " "[Disable Mode]R1-----R2[Restart Mode] initialized " + "[Phase 1] : Test Setup " "[Disable Mode]R1-----R2[Helper Mode] initialized " ) # Configure graceful-restart @@ -1251,7 +1251,7 @@ def test_BGP_GR_TC_31_2_p1(request): tc_name, result ) - logger.info("[Phase 2] : R2 Goes from Disable to Restart Mode ") + logger.info("[Phase 2] : R1 Goes from Disable to Restart Mode ") # Configure graceful-restart input_dict = { @@ -1356,31 +1356,7 @@ def test_BGP_GR_TC_31_2_p1(request): }, } - # here the verify_graceful_restart fro the neighbor would be - # "NotReceived" as the latest GR config is not yet applied. - for addr_type in ADDR_TYPES: - result = verify_graceful_restart( - tgen, topo, addr_type, input_dict, dut="r1", peer="r2" - ) - assert result is True, "Testcase {} : Failed \n Error {}".format( - tc_name, result - ) - - for addr_type in ADDR_TYPES: - # Verifying RIB routes - next_hop = next_hop_per_address_family( - tgen, dut, peer, addr_type, NEXT_HOP_IP_2 - ) - input_topo = {key: topo["routers"][key] for key in ["r2"]} - result = verify_rib(tgen, addr_type, dut, input_topo, next_hop, protocol) - assert result is True, "Testcase {} : Failed \n Error {}".format( - tc_name, result - ) - - logger.info("[Phase 6] : R1 is about to come up now ") - start_router_daemons(tgen, "r1", ["bgpd"]) - - logger.info("[Phase 4] : R1 is UP now, so time to collect GR stats ") + logger.info("[Phase 4] : R1 is UP and GR state is correct ") for addr_type in ADDR_TYPES: result = verify_graceful_restart( diff --git a/tests/topotests/lib/topotest.py b/tests/topotests/lib/topotest.py index b034852de5..05827040aa 100644 --- a/tests/topotests/lib/topotest.py +++ b/tests/topotests/lib/topotest.py @@ -30,6 +30,7 @@ from copy import deepcopy import lib.topolog as topolog from lib.micronet_compat import Node from lib.topolog import logger +from munet.base import Timeout from lib import micronet @@ -1769,18 +1770,30 @@ class Router(Node): daemons_list.append(daemon) tail_log_files = [] + check_daemon_files = [] def start_daemon(daemon, extra_opts=None): daemon_opts = self.daemons_options.get(daemon, "") + + # get pid and vty filenames and remove the files + m = re.match(r"(.* |^)-n (\d+)( ?.*|$)", daemon_opts) + dfname = daemon if not m else "{}-{}".format(daemon, m.group(2)) + runbase = "/var/run/{}/{}".format(self.routertype, dfname) + # If this is a new system bring-up remove the pid/vty files, otherwise + # do not since apparently presence of the pidfile impacts BGP GR + self.cmd_status("rm -f {0}.pid {0}.vty".format(runbase)) + rediropt = " > {0}.out 2> {0}.err".format(daemon) if daemon == "snmpd": binary = "/usr/sbin/snmpd" cmdenv = "" cmdopt = "{} -C -c /etc/frr/snmpd.conf -p ".format( daemon_opts - ) + "/var/run/{}/snmpd.pid -x /etc/frr/agentx".format(self.routertype) + ) + "{}.pid -x /etc/frr/agentx".format(runbase) + # check_daemon_files.append(runbase + ".pid") else: binary = os.path.join(self.daemondir, daemon) + check_daemon_files.extend([runbase + ".pid", runbase + ".vty"]) cmdenv = "ASAN_OPTIONS=" if asan_abort: @@ -1844,8 +1857,6 @@ class Router(Node): logger.info( "%s: %s %s launched in gdb window", self, self.routertype, daemon ) - # Need better check for daemons running. - time.sleep(5) else: if daemon != "snmpd": cmdopt += " -d " @@ -1904,9 +1915,22 @@ class Router(Node): start_daemon(daemon) # Check if daemons are running. - rundaemons = self.cmd("ls -1 /var/run/%s/*.pid" % self.routertype) - if re.search(r"No such file or directory", rundaemons): - return "Daemons are not running" + wait_time = 30 if (gdb_routers or gdb_daemons) else 10 + timeout = Timeout(wait_time) + for remaining in timeout: + if not check_daemon_files: + break + check = check_daemon_files[0] + if self.path_exists(check): + check_daemon_files.pop(0) + continue + self.logger.debug("Waiting {}s for {} to appear".format(remaining, check)) + time.sleep(0.5) + + if check_daemon_files: + assert False, "Timeout({}) waiting for {} to appear on {}".format( + wait_time, check_daemon_files[0], self.name + ) # Update the permissions on the log files self.cmd("chown frr:frr -R {}/{}".format(self.logdir, self.name))