Store "tag" and "url" in files in reservation dir

String "Unknown" is used if test run tag or url is not supplied,
so this change should be backward compatible.

+ Used "runtag", as both "test" and "tag" start with taken "-t".
+ Fixed some pylint violations.
+ Docstring explaining the steps taken and reasoning behind two files.
+ Other minor improvements.

Change-Id: If704d02e51374087fe39ad0f69432477d1b310d3
Signed-off-by: Vratko Polak <vrpolak@cisco.com>

2 files changed, 96 insertions, 22 deletions

diff --git a/resources/libraries/bash/function/common.sh b/resources/libraries/bash/function/common.sh
index 08e916f65f..5deab8aa40 100644
--- a/resources/libraries/bash/function/common.sh
+++ b/resources/libraries/bash/function/common.sh
@@ -524,6 +524,10 @@ function reserve_and_cleanup_testbed () {
     # Variables read:
     # - TOPOLOGIES - Array of paths to topology yaml to attempt reservation on.
     # - PYTHON_SCRIPTS_DIR - Path to directory holding the reservation script.
+    # - BUILD_TAG - Any string suitable as filename, identifying
+    #   test run executing this function. May be unset.
+    # - BUILD_URL - Any string suitable as URL, identifying
+    #   test run executing this function. May be unset.
     # Variables set:
     # - TOPOLOGIES - Array of paths to topologies, with failed cleanups removed.
     # - WORKING_TOPOLOGY - Path to topology yaml file of the reserved testbed.
@@ -535,7 +539,10 @@ function reserve_and_cleanup_testbed () {
     while [[ ${TOPOLOGIES[@]} ]]; do
         for topo in "${TOPOLOGIES[@]}"; do
             set +e
-            python "${PYTHON_SCRIPTS_DIR}/topo_reservation.py" -t "${topo}"
+            scrpt="${PYTHON_SCRIPTS_DIR}/topo_reservation.py"
+            opts=("-t" "${topo}" "-r" "${BUILD_TAG:-Unknown}")
+            opts+=("-u" "${BUILD_URL:-Unknown}")
+            python "${scrpt}" "${opts[@]}"
             result="$?"
             set -e
             if [[ "${result}" == "0" ]]; then
diff --git a/resources/tools/scripts/topo_reservation.py b/resources/tools/scripts/topo_reservation.py
index bf31918c73..a04709b2d5 100755
--- a/resources/tools/scripts/topo_reservation.py
+++ b/resources/tools/scripts/topo_reservation.py
@@ -1,6 +1,6 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 
-# Copyright (c) 2018 Cisco and/or its affiliates.
+# Copyright (c) 2019 Cisco and/or its affiliates.
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at:
@@ -22,48 +22,115 @@ As source of truth, TG node from the topology file is used.
 
 import sys
 import argparse
-from resources.libraries.python.ssh import SSH
-from yaml import load
+import yaml
+
+from resources.libraries.python.ssh import exec_cmd
+
 
 RESERVATION_DIR = "/tmp/reservation_dir"
 
+
+def diag_cmd(node, cmd):
+    """Execute cmd, print cmd and stdout, ignore stderr and rc; return None.
+
+    :param node: Node object as parsed from topology file to execute cmd on.
+    :param cmd: Command to execute.
+    :type ssh: dict
+    :type cmd: str
+    """
+    print "+", cmd
+    _, stdout, _ = exec_cmd(node, cmd)
+    print stdout
+
+
 def main():
+    """Parse arguments, perform the action, write useful output, propagate RC.
+
+    If the intended action is cancellation, reservation dir is deleted.
+
+    If the intended action is reservation, the list is longer:
+    1. List contents of reservation dir.
+    2. List contents of test.url file in the dir.
+    3. Create reservation dir.
+    4. Touch file according to -r option.
+    5. Put -u option string to file test.url
+    From these 5 steps, 1 and 2 are performed always, their RC ignored.
+    RC of step 3 gives the overall result.
+    If the result is success, steps 4-5 are executed without any output,
+    their RC is ignored.
+
+    The two files in reservation dir are there for reporting
+    which test run holds the reservation, so people can manually fix the testbed
+    if the rest run has been aborted, or otherwise failed to unregister.
+
+    The two files have different audiences.
+
+    The URL content is useful for people scheduling their test runs
+    and wondering why the reservation takes so long.
+    For them, a URL (if available) to copy and paste into browser
+    to see which test runs are blocking testbeds is the most convenient.
+
+    The "run tag" as a filename is useful for admins accessing the testbed
+    via a graphical terminal, which does not allow copying of text,
+    as they need less keypresses to identify the test run holding the testbed.
+    Also, the listing shows timestamps, which is useful for both audiences.
+    """
     parser = argparse.ArgumentParser()
     parser.add_argument("-t", "--topo", required=True,
                         help="Topology file")
     parser.add_argument("-c", "--cancel", help="Cancel reservation",
                         action="store_true")
+    parser.add_argument("-r", "--runtag", required=False, default="Unknown",
+                        help="Identifier for test run suitable as filename")
+    parser.add_argument("-u", "--url", required=False, default="Unknown",
+                        help="Identifier for test run suitable as URL")
     args = parser.parse_args()
-    topology_file = args.topo
-    cancel_reservation = args.cancel
 
-    work_file = open(topology_file)
-    topology = load(work_file.read())['nodes']
+    with open(args.topo, "r") as topo_file:
+        topology = yaml.load(topo_file.read())['nodes']
 
     # Even if TG is not guaranteed to be a Linux host,
     # we are using it, because testing shows SSH access to DUT
     # during test affects its performance (bursts of lost packets).
     try:
-        tg_node = topology["TG"]
+        tgn = topology["TG"]
     except KeyError:
         print "Topology file does not contain 'TG' node"
         return 1
 
-    ssh = SSH()
-    ssh.connect(tg_node)
-
     # For system reservation we use mkdir it is an atomic operation and we can
     # store additional data (time, client_ID, ..) within reservation directory.
-    if cancel_reservation:
-        ret, _, err = ssh.exec_command("rm -r {}".format(RESERVATION_DIR))
-    else:
-        ret, _, err = ssh.exec_command("mkdir {}".format(RESERVATION_DIR))
-
-    if ret != 0:
-        print("{} unsuccessful:\n{}".
-              format(("Cancellation " if cancel_reservation else "Reservation"),
-                     err))
+    if args.cancel:
+        ret, _, err = exec_cmd(tgn, "rm -r {}".format(RESERVATION_DIR))
+        if ret:
+            print "Cancellation unsuccessful:\n{}".format(err)
+        return ret
+    # Before critical section, output can be outdated already.
+    print "Diagnostic commands:"
+    # -d and * are to supress "total <size>", see https://askubuntu.com/a/61190
+    diag_cmd(tgn, "ls --full-time -cd '{dir}'/*".format(dir=RESERVATION_DIR))
+    diag_cmd(tgn, "head -1 '{dir}/run.url'".format(dir=RESERVATION_DIR))
+    print "Attempting reservation."
+    # Entering critical section.
+    # TODO: Add optional argument to exec_cmd_no_error to make it
+    # sys.exit(ret) instead raising? We do not want to deal with stacktrace.
+    ret, _, err = exec_cmd(tgn, "mkdir '{dir}'".format(dir=RESERVATION_DIR))
+    # Critical section is over.
+    if ret:
+        print "Reservation unsuccessful:\n{}".format(err)
+        return ret
+    # Here the script knows it is the only owner of the testbed.
+    print "Success, writing test run info to reservation dir."
+    # TODO: Add optional argument to exec_cmd_no_error to print message
+    # to console instead raising? We do not want to deal with stacktrace.
+    ret2, _, err = exec_cmd(
+        tgn, "touch '{dir}/{runtag}' && ( echo '{url}' > '{dir}/run.url' )"\
+        .format(dir=RESERVATION_DIR, runtag=args.runtag, url=args.url))
+    if ret2:
+        print "Writing test run info failed, but continuing anyway:\n{}".format(
+            err)
     return ret
 
+
 if __name__ == "__main__":
     sys.exit(main())