From 59cc4077d6b7084cfe29e1280bd8521b4b933d89 Mon Sep 17 00:00:00 2001
From: Philip Withnall <philip.withnall@collabora.co.uk>
Date: Tue, 22 Sep 2015 13:34:14 +0100
Subject: [PATCH] traffic-control: Ignore failure to kill wget processes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It’s possible for wget to finish downloading a file before being killed
by the test harness (for example, if the download is particularly fast.

These failures to kill processes don’t indicate failure of the test
case, so should be ignored.

Additionally, prevent client.py from exiting early due to its D-Bus
timeout expiring — tcmmd never sends a reply to the method call, so
client.py should not expect one. This requires ignoring the need for a
reply, and also running the dbus-python main loop correctly rather than
sleeping and potentially ignoring incoming D-Bus traffic.

Bug: https://bugs.apertis.org/show_bug.cgi?id=327
Differential Revision: https://phabricator.apertis.org/D509
Signed-off-by: Philip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
---
 traffic-control/manual/client.py         | 13 ++++++++++---
 traffic-control/manual/run-tcmmd-test.sh | 20 ++++++++++----------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/traffic-control/manual/client.py b/traffic-control/manual/client.py
index e7baa5d..55ef09e 100755
--- a/traffic-control/manual/client.py
+++ b/traffic-control/manual/client.py
@@ -4,6 +4,10 @@ import sys
 import argparse
 import dbus
 import time
+from gi.repository import GObject
+from dbus.mainloop.glib import DBusGMainLoop
+
+DBusGMainLoop(set_as_default=True)
 
 parser = argparse.ArgumentParser(description='Testing tcmmd')
 parser.add_argument('-S','--ipsrc', help='IP source',required=True)
@@ -27,10 +31,13 @@ iface = dbus.Interface(remote_object, "org.tcmmd.ManagedConnections")
 #        str(args.ipdst), int(args.tcpdport),
 #        int(args.priorityrate), int(args.backgroundrate)))
 
+# Do not expect a reply.
 ret = iface.SetFixedPolicy(str(args.ipsrc), int(args.tcpsport),
                            str(args.ipdst), int(args.tcpdport),
                            int(args.priorityrate),
-                           int(args.backgroundrate))
-
-time.sleep(3600)
+                           int(args.backgroundrate),
+                           ignore_reply=True)
 
+# Loop forever until killed by the test harness.
+loop = GObject.MainLoop()
+loop.run()
diff --git a/traffic-control/manual/run-tcmmd-test.sh b/traffic-control/manual/run-tcmmd-test.sh
index 61be6a9..0dc21cc 100755
--- a/traffic-control/manual/run-tcmmd-test.sh
+++ b/traffic-control/manual/run-tcmmd-test.sh
@@ -36,7 +36,7 @@ start_rule() {
     --ipsrc "$ENFORCED_IPSRC" --ipdst "$ENFORCED_IPDST" \
     --tcpsport "$ENFORCED_TCPSPORT" --tcpdport "$ENFORCED_TCPDPORT" \
     --backgroundrate "$ENFORCED_BACKGROUND_RATE" \
-    --priorityrate "$ENFORCED_PRIORITY_RATE" &> /dev/null &
+    --priorityrate "$ENFORCED_PRIORITY_RATE" &
   CLIENT_PID=$!
 
   say "Please look at the download rate."
@@ -46,12 +46,12 @@ start_rule() {
   PROC_KILL_LIST+=($WGET_PID)
 
   sleep $WGET_TEST_DURATION
-  kill $WGET_PID
+  kill $WGET_PID || true
   sleep 0.2
   whine "Was the limit of $EXPECTED_BACKGROUND_RATE correctly enforced [yes/no]?"
   read v
   if [ "$v" != "yes" ] ; then
-    kill $CLIENT_PID
+    kill $CLIENT_PID || true
     return 1
   fi
 
@@ -63,12 +63,12 @@ start_rule() {
   PROC_KILL_LIST+=($WGET_PID)
 
   sleep $WGET_TEST_DURATION
-  kill $WGET_PID
+  kill $WGET_PID || true
   sleep 0.2
   whine "Was the limit of $EXPECTED_PRIORITY_RATE correctly enforced [yes/no]?"
   read v
   if [ "$v" != "yes" ] ; then
-    kill $CLIENT_PID
+    kill $CLIENT_PID || true
     return 1
   fi
 
@@ -86,13 +86,13 @@ check_decent_speed() {
   PROC_KILL_LIST+=($WGET_PID)
 
   sleep $WGET_TEST_DURATION
-  kill -9 $WGET_PID
+  kill -9 $WGET_PID || true
   wait $WGET_PID || true
   sleep 0.2
   whine "Was it a decent speed (1MB/s or more) [yes/no]?"
   read v
   if [ "$v" != "yes" ] ; then
-    sudo kill $TCMMD_PID
+    sudo kill $TCMMD_PID || true
     return 1
   fi
 
@@ -112,7 +112,7 @@ check_tcmmd() {
   # there is no point to run the main tests if we don't have a good connectivity
   check_decent_speed "check decent speed before the main tests"
   if [ $? != 0 ] ; then
-    sudo kill $TCMMD_PID
+    sudo kill $TCMMD_PID || true
     return 1
   fi
 
@@ -165,11 +165,11 @@ check_tcmmd() {
   done
   echo
 
-  sudo kill $TCMMD_PID
+  sudo kill $TCMMD_PID || true
 
   check_decent_speed "check if connectivity is not broken after killing tcmmd"
   if [ $? != 0 ] ; then
-    sudo kill $TCMMD_PID
+    sudo kill $TCMMD_PID || true
     return 1
   fi
 
-- 
GitLab