Fix memory leak detection and reporting which accidentally was dropped a month ago

Signed-off-by: Martin Winter <mwinter@opensourcerouting.org>

tests/topotests/README.md

@@ -81,8 +81,8 @@ And create frr User and frrvty group as follows:
 
 	py.test -s -v --tb=no
 
-All test_* scripts in subdirectories are detected and executed (unless disabled in
+All test_* scripts in subdirectories are detected and executed (unless
-`pytest.ini` file)
+disabled in `pytest.ini` file)
 
 `--tb=no` disables the python traceback which might be irrelevant unless the
 test script itself is debugged
@@ -101,6 +101,38 @@ For the simulated topology, see the description in the python file
 If you need to clear the mininet setup between tests (if it isn't cleanly
 shutdown), then use the `mn -c` command to clean up the environment
 
+#### (Optional) StdErr log from daemos after exit
+
+To enable the reporting of any messages seen on StdErr after the
+daemons exit, the following env variable can be set.
+
+	export TOPOTESTS_CHECK_STDERR=Yes
+
+(The value doesn't matter at this time. The check is if the env variable
+exists or not)
+There is no pass/fail on this reporting. The Output will be reported to
+the console
+
+	export TOPOTESTS_CHECK_MEMLEAK="/home/mydir/memleak_"
+
+This will enable the check and output to console and the writing of
+the information to files with the given prefix (followed by testname),
+ie `/home/mydir/memcheck_test_bgp_multiview_topo1.txt` in case of a 
+memory leak.
+
+#### (Optional) Collect Memory Leak Information
+
+FreeRangeRouting processes have the capabilities to report remaining memory
+allocations upon exit. To enable the reporting of the memory, define an
+enviroment variable `TOPOTESTS_CHECK_MEMLEAK` with the file prefix, ie
+
+	export TOPOTESTS_CHECK_MEMLEAK="/home/mydir/memleak_"
+
+This will enable the check and output to console and the writing of
+the information to files with the given prefix (followed by testname),
+ie `/home/mydir/memcheck_test_bgp_multiview_topo1.txt` in case of a 
+memory leak.
+
 ## License
 
 All the configs and scripts are licensed under a ISC-style license. See

tests/topotests/all-protocol-startup/test_all_protocol_startup.py

@@ -810,11 +810,13 @@ def test_shutdown_check_stderr():
     print("******************************************\n")
 
     if os.environ.get('TOPOTESTS_CHECK_STDERR') is None:
-        print("SKIPPED (Disabled) - TOPOTESTS_CHECK_STDERR undefined\n")
+        print("SKIPPED final check on StdErr output: Disabled (TOPOTESTS_CHECK_STDERR undefined)\n")
-        pytest.skip('Skipping test for Stderr output and memory leaks')
+        pytest.skip('Skipping test for Stderr output')
 
     thisDir = os.path.dirname(os.path.realpath(__file__))
 
+    print("thisDir=" + thisDir)
+
     net['r1'].stopRouter()
 
     log = net['r1'].getStdErr('ripd')
@@ -836,6 +838,25 @@ def test_shutdown_check_stderr():
     print("\nZebra StdErr Log:\n" + log)
 
 
+def test_shutdown_check_memleak():
+    global fatal_error
+    global net
+
+    # Skip if previous fatal error condition is raised
+    if (fatal_error != ""):
+        pytest.skip(fatal_error)
+
+    if os.environ.get('TOPOTESTS_CHECK_MEMLEAK') is None:
+        print("SKIPPED final check on Memory leaks: Disabled (TOPOTESTS_CHECK_MEMLEAK undefined)\n")
+        pytest.skip('Skipping test for memory leaks')
+    
+    thisDir = os.path.dirname(os.path.realpath(__file__))
+
+    for i in range(1, 2):
+        net['r%s' % i].stopRouter()
+        net['r%s' % i].report_memory_leaks(os.environ.get('TOPOTESTS_CHECK_MEMLEAK'), os.path.basename(__file__))
+
+
 if __name__ == '__main__':
 
     setLogLevel('info')

tests/topotests/bgp_multiview_topo1/test_bgp_multiview_topo1.py

@@ -310,6 +310,7 @@ def test_bgp_routingTable():
     # For debugging after starting FRR/Quagga daemons, uncomment the next line
     # CLI(net)
 
+
 def test_shutdown_check_stderr():
     global fatal_error
     global net
@@ -319,7 +320,8 @@ def test_shutdown_check_stderr():
         pytest.skip(fatal_error)
 
     if os.environ.get('TOPOTESTS_CHECK_STDERR') is None:
-        pytest.skip('Skipping test for Stderr output and memory leaks')
+        print("SKIPPED final check on StdErr output: Disabled (TOPOTESTS_CHECK_STDERR undefined)\n")
+        pytest.skip('Skipping test for Stderr output')
 
     thisDir = os.path.dirname(os.path.realpath(__file__))
 
@@ -334,6 +336,24 @@ def test_shutdown_check_stderr():
     print("\nZebra StdErr Log:\n" + log)
 
 
+def test_shutdown_check_memleak():
+    global fatal_error
+    global net
+
+    # Skip if previous fatal error condition is raised
+    if (fatal_error != ""):
+        pytest.skip(fatal_error)
+
+    if os.environ.get('TOPOTESTS_CHECK_MEMLEAK') is None:
+        print("SKIPPED final check on Memory leaks: Disabled (TOPOTESTS_CHECK_MEMLEAK undefined)\n")
+        pytest.skip('Skipping test for memory leaks')
+    
+    thisDir = os.path.dirname(os.path.realpath(__file__))
+
+    net['r1'].stopRouter()
+    net['r1'].report_memory_leaks(os.environ.get('TOPOTESTS_CHECK_MEMLEAK'), os.path.basename(__file__))
+
+
 if __name__ == '__main__':
 
     setLogLevel('info')

tests/topotests/ldp-topo1/test_ldp_topo1.py

@@ -724,7 +724,8 @@ def test_shutdown_check_stderr():
         pytest.skip(fatal_error)
 
     if os.environ.get('TOPOTESTS_CHECK_STDERR') is None:
-        pytest.skip('Skipping test for Stderr output and memory leaks')
+        print("SKIPPED final check on StdErr output: Disabled (TOPOTESTS_CHECK_STDERR undefined)\n")
+        pytest.skip('Skipping test for Stderr output')
 
     thisDir = os.path.dirname(os.path.realpath(__file__))
 
@@ -741,6 +742,24 @@ def test_shutdown_check_stderr():
         print("\nRouter r%s Zebra StdErr Log:\n%s" % (i, log))
 
 
+def test_shutdown_check_memleak():
+    global fatal_error
+    global net
+
+    # Skip if previous fatal error condition is raised
+    if (fatal_error != ""):
+        pytest.skip(fatal_error)
+
+    if os.environ.get('TOPOTESTS_CHECK_MEMLEAK') is None:
+        print("SKIPPED final check on Memory leaks: Disabled (TOPOTESTS_CHECK_MEMLEAK undefined)\n")
+        pytest.skip('Skipping test for memory leaks')
+    
+    thisDir = os.path.dirname(os.path.realpath(__file__))
+
+    for i in range(1, 5):
+        net['r%s' % i].stopRouter()
+        net['r%s' % i].report_memory_leaks(os.environ.get('TOPOTESTS_CHECK_MEMLEAK'), os.path.basename(__file__))
+
 
 if __name__ == '__main__':
 

tests/topotests/lib/topotest.py

@@ -23,6 +23,7 @@
 #
 
 import os
+import errno
 import re
 import sys
 import glob
@@ -51,6 +52,27 @@ def int2dpid(dpid):
                         'please either specify a dpid or use a '
                         'canonical switch name such as s23.')
 
+def pid_exists(pid):
+    "Check whether pid exists in the current process table."
+
+    if pid <= 0:
+        return False
+    try:
+        os.kill(pid, 0)
+    except OSError as err:
+        if err.errno == errno.ESRCH:
+            # ESRCH == No such process
+            return False
+        elif err.errno == errno.EPERM:
+            # EPERM clearly means there's a process to deny access to
+            return True
+        else:
+            # According to "man 2 kill" possible error values are
+            # (EINVAL, EPERM, ESRCH)
+            raise
+    else:
+        return True
+
 def addRouter(topo, name):
     "Adding a FRRouter (or Quagga) to Topology"
 
@@ -119,7 +141,9 @@ class Router(Node):
         rundaemons = self.cmd('ls -1 /var/run/%s/*.pid' % self.routertype)
         if rundaemons is not None:
             for d in StringIO.StringIO(rundaemons):
-                self.cmd('kill -7 `cat %s`' % d.rstrip())
+                daemonpid = self.cmd('cat %s' % d.rstrip()).rstrip()
+                if pid_exists(int(daemonpid)):
+                    self.cmd('kill -7 %s' % daemonpid)
                     self.waitOutput()
     def removeIPs(self):
         for interface in self.intfNames():
@@ -261,6 +285,34 @@ class Router(Node):
         "Return the type of Router (frr or quagga)"
 
         return self.routertype
+    def report_memory_leaks(self, filename_prefix, testscript):
+        "Report Memory Leaks to file prefixed with given string"
+
+        leakfound = False
+        filename = filename_prefix + re.sub(r"\.py", "", testscript) + ".txt"
+        for daemon in self.daemons:
+            if (self.daemons[daemon] == 1):
+                log = self.getStdErr(daemon)
+                if "memstats" in log:
+                    # Found memory leak
+                    print("\nRouter %s %s StdErr Log:\n%s" % (self.name, daemon, log))        
+                    if not leakfound:
+                        leakfound = True
+                        # Check if file already exists
+                        fileexists = os.path.isfile(filename)
+                        leakfile = open(filename, "a")
+                        if not fileexists:
+                            # New file - add header
+                            leakfile.write("# Memory Leak Detection for topotest %s\n\n" % testscript)
+                        leakfile.write("## Router %s\n" % self.name)
+                    leakfile.write("### Process %s\n" % daemon)
+                    log = re.sub("core_handler: ", "", log)
+                    log = re.sub(r"(showing active allocations in memory group [a-zA-Z0-9]+)", r"\n#### \1\n", log)
+                    log = re.sub("memstats:  ", "    ", log)
+                    leakfile.write(log)
+                    leakfile.write("\n")
+        if leakfound:
+            leakfile.close()
 
 
 class LegacySwitch(OVSSwitch):

tests/topotests/ospf6-topo1/test_ospf6_topo1.py

@@ -380,7 +380,8 @@ def test_shutdown_check_stderr():
         pytest.skip(fatal_error)
 
     if os.environ.get('TOPOTESTS_CHECK_STDERR') is None:
-        pytest.skip('Skipping test for Stderr output and memory leaks')
+        print("SKIPPED final check on StdErr output: Disabled (TOPOTESTS_CHECK_STDERR undefined)\n")
+        pytest.skip('Skipping test for Stderr output')
 
     thisDir = os.path.dirname(os.path.realpath(__file__))
 
@@ -395,6 +396,25 @@ def test_shutdown_check_stderr():
         print("\nRouter r%s Zebra StdErr Log:\n%s" % (i, log))
 
 
+def test_shutdown_check_memleak():
+    global fatal_error
+    global net
+
+    # Skip if previous fatal error condition is raised
+    if (fatal_error != ""):
+        pytest.skip(fatal_error)
+
+    if os.environ.get('TOPOTESTS_CHECK_MEMLEAK') is None:
+        print("SKIPPED final check on Memory leaks: Disabled (TOPOTESTS_CHECK_MEMLEAK undefined)\n")
+        pytest.skip('Skipping test for memory leaks')
+    
+    thisDir = os.path.dirname(os.path.realpath(__file__))
+
+    for i in range(1, 5):
+        net['r%s' % i].stopRouter()
+        net['r%s' % i].report_memory_leaks(os.environ.get('TOPOTESTS_CHECK_MEMLEAK'), os.path.basename(__file__))
+
+
 if __name__ == '__main__':
 
     setLogLevel('info')

tests/topotests/ripng-topo1/test_ripng_topo1.py

@@ -338,7 +338,8 @@ def test_shutdown_check_stderr():
         pytest.skip(fatal_error)
 
     if os.environ.get('TOPOTESTS_CHECK_STDERR') is None:
-        pytest.skip('Skipping test for Stderr output and memory leaks')
+        print("SKIPPED final check on StdErr output: Disabled (TOPOTESTS_CHECK_STDERR undefined)\n")
+        pytest.skip('Skipping test for Stderr output')
 
     thisDir = os.path.dirname(os.path.realpath(__file__))
 
@@ -353,6 +354,24 @@ def test_shutdown_check_stderr():
     print("\nZebra StdErr Log:\n" + log)
 
 
+def test_shutdown_check_memleak():
+    global fatal_error
+    global net
+
+    # Skip if previous fatal error condition is raised
+    if (fatal_error != ""):
+        pytest.skip(fatal_error)
+
+    if os.environ.get('TOPOTESTS_CHECK_MEMLEAK') is None:
+        print("SKIPPED final check on Memory leaks: Disabled (TOPOTESTS_CHECK_MEMLEAK undefined)\n")
+        pytest.skip('Skipping test for memory leaks')
+    
+    thisDir = os.path.dirname(os.path.realpath(__file__))
+
+    net['r1'].stopRouter()
+    net['r1'].report_memory_leaks(os.environ.get('TOPOTESTS_CHECK_MEMLEAK'), os.path.basename(__file__))
+
+
 if __name__ == '__main__':
 
     setLogLevel('info')