[PATCH OSSTEST v4 0/7] Misc standalone wrapper improvements

[PATCH OSSTEST v4 0/7] Misc standalone wrapper improvements

[Ian Campbell]

Fixed up quoting of $@ (generally in a new patch, then also in the bits of
the old patches).

Added "standalone: do not rotate empty log files." posted separately.

Ian.

[PATCH OSSTEST v4 1/7] cs-adjust-flight: `branch' command ought to be `branch-set'

[Ian Campbell]

Also add a doc string and since this op is not a change adjust the doc
comment accordingly.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: New patch.
---
 cs-adjust-flight | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/cs-adjust-flight b/cs-adjust-flight
index 834e2c8..a94ed5f 100755
--- a/cs-adjust-flight
+++ b/cs-adjust-flight
@@ -3,9 +3,9 @@
 # destination flight must already exist
 #
 # args:
-#   <dst-flight> [<change> ...]
+#   <dst-flight> [<op> ...]
 #
-# <change>:
+# <op>:
 #   copy <flight>
 #   copy-jobs <flight> <job-spec>
 #   jobs-list <job-spec>
@@ -16,6 +16,7 @@
 #   runvar-perlop <job-spec> <var-spec> <perl-expr>
 #   recipe-set <job-spec> <new-value>
 #   intended-blessing <intended-blessing>
+#   branch-set <new-branch>
 #
 # <foo-spec>:
 #   <foo-name>
@@ -330,7 +331,7 @@ sub change__intended_blessing {
     verbose "$dstflight blessing set to $blessing\n";
 }
 
-sub change__branch {
+sub change__branch_set {
     die unless @changes >= 1;
     my $branch = shift @changes;
 
-- 
2.5.3

[PATCH OSSTEST v4 2/7] cs-adjust-flight: Add job-status to report job stats

[Ian Campbell]

The return code of sg-run-job does not reflect the state of the job,
which is instead written to the database. For the benefit of running
tests in a loop until failure add a command to retrieve the status to
stdout.

Add a get-job-status command to the standalone helper script.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Don't both capturing status into a variable just to echo it.

v3: Some incidental docs stuff moved to a more appropriate (new)
patch. Only print the job if there might be multiple (i.e. the jobs
argument is some sort of regex)

v2: Replaces "standalone: Add get-job-status to pick status out of
standalone.db" with a variant using cs-adjust-flight
---
 cs-adjust-flight | 19 +++++++++++++++++++
 standalone       | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/cs-adjust-flight b/cs-adjust-flight
index a94ed5f..4bfef48 100755
--- a/cs-adjust-flight
+++ b/cs-adjust-flight
@@ -17,6 +17,7 @@
 #   recipe-set <job-spec> <new-value>
 #   intended-blessing <intended-blessing>
 #   branch-set <new-branch>
+#   job-status <job-spec>
 #
 # <foo-spec>:
 #   <foo-name>
@@ -342,6 +343,24 @@ sub change__branch_set {
     verbose "$dstflight branch set to $branch\n";
 }
 
+sub change__job_status {
+    die unless @changes >= 1;
+    my $jobs = shift @changes;
+
+    my $q = $dbh_tests->prepare(<<END);
+        SELECT status
+          FROM jobs
+         WHERE flight = ? AND job = ?
+END
+    for_jobs($dstflight, $jobs, sub {
+	my ($job) = @_;
+	$q->execute($dstflight, $job);
+	my ($s) = $q->fetchrow_array();
+	print "$job " or die $! if defined spec_re($jobs);
+	print "$s\n" or die $!;
+    });
+}
+
 sub changes () {
     debug("CHANGES...\n");
 
diff --git a/standalone b/standalone
index f64462e..98a323f 100755
--- a/standalone
+++ b/standalone
@@ -35,6 +35,11 @@ Operations:
   hosts next time. Otherwise osstest will complain if you change the
   host(s) which a job is running on on successive runs.
 
+* get-job-status [cf] [JOB]
+
+  Prints the status (pass, fail, running, etc) of the given job to
+  stdout.
+
 Options:
 
 -c FILE, --config=FILE        Use FILE as configuration file
@@ -140,8 +145,9 @@ if [ $reuse -eq 0 ]; then
     read
 fi
 
-if [ ! -f standalone.db ] ; then
-    echo "No standalone.db? Run standalone-reset." >&2
+db="standalone.db"
+if [ ! -f $db ] ; then
+    echo "No $db? Run standalone-reset." >&2
     exit 1
 fi
 
@@ -199,6 +205,14 @@ with_logging() {
     fi
 }
 
+job_status() {
+    flight=$1; shift
+    job=$1; shift
+
+    OSSTEST_CONFIG=$config \
+        ./cs-adjust-flight $flight job-status $job
+}
+
 # other potential ops:
 # - run standalone reset
 
@@ -304,6 +318,20 @@ case $op in
 	OSSTEST_JOB=$job \
 	    with_logging logs/$flight/$job.$ts.log ./$ts $hosts $@
 	;;
+
+    get-job-status)
+	need_flight;
+
+	if [ $# -ne 1 ] ; then
+	    echo "get-job-status: Need job" >&2
+	    exit 1
+	fi
+
+	job=$1; shift
+
+	job_status $flight $job
+
+	;;
     *)
 	echo "Unknown op $op" ; exit 1 ;;
 esac
-- 
2.5.3

Re: [PATCH OSSTEST v4 2/7] cs-adjust-flight: Add job-status to report job stats

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v4 2/7] cs-adjust-flight: Add job-status to report job stats"):
> The return code of sg-run-job does not reflect the state of the job,
> which is instead written to the database. For the benefit of running
> tests in a loop until failure add a command to retrieve the status to
> stdout.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v4 3/7] standalone: Check job status at end of run-job.

[Ian Campbell]

Check if the job passed and if not (so status is fail, broken, running
etc) then return an error.

This is convenient for scripting.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 standalone | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/standalone b/standalone
index 98a323f..7c7fde9 100755
--- a/standalone
+++ b/standalone
@@ -300,6 +300,11 @@ case $op in
 	OSSTEST_HOST_REUSE=$reuse \
 	OSSTEST_SIMULATE=$dryrun \
 	    with_logging logs/$flight/$job.log ./sg-run-job $job
+
+	status=`job_status $flight $job`
+	echo "$flight.$job status = $status" >&2
+	if [ "x$status" != xpass ] ; then exit 1; fi
+
 	;;
     run-test)
 	need_flight; need_host
-- 
2.5.3

[PATCH OSSTEST v4 4/7] standalone: Correctly quote $@ where it is used.

[Ian Campbell]

Else arguments with spaces become multiple arguments.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 standalone | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/standalone b/standalone
index 7c7fde9..25e23ca 100755
--- a/standalone
+++ b/standalone
@@ -197,7 +197,7 @@ with_logging() {
     local log=$1; shift
     ensure_logs
     savelog -c 300 "$log" >/dev/null
-    $@ 2>&1 | tee "$log"
+    "$@" 2>&1 | tee "$log"
     rc=${PIPESTATUS[0]}
     if [ $rc -ne 0 ] ; then
 	echo "FAILED rc=${rc}" >&2
@@ -240,7 +240,7 @@ case $op in
         OSSTEST_FLIGHT=$flight \
         OSSTEST_CONFIG=$config \
         OSSTEST_NO_BASELINE=$nobaseline \
-            with_logging logs/$flight/make-flight.log ./cr-daily-branch $@ $branch
+            with_logging logs/$flight/make-flight.log ./cr-daily-branch "$@" $branch
         ;;
 
     set-paths)
@@ -321,7 +321,7 @@ case $op in
 	OSSTEST_FLIGHT=$flight \
 	OSSTEST_HOST_REUSE=$reuse \
 	OSSTEST_JOB=$job \
-	    with_logging logs/$flight/$job.$ts.log ./$ts $hosts $@
+	    with_logging logs/$flight/$job.$ts.log ./$ts $hosts "$@"
 	;;
 
     get-job-status)
-- 
2.5.3

Re: [PATCH OSSTEST v4 4/7] standalone: Correctly quote $@ where it is used.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v4 4/7] standalone: Correctly quote $@ where it is used."):
> Else arguments with spaces become multiple arguments.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v4 5/7] standalone: Make it possible to pass options to run-test

[Ian Campbell]

Currently the remainder of the comnand line is passed after the host=
ident, which allows for other idents to be given, which isn't all that
useful in practice.

Instead arrange that any additional options up to a "--" marker are
passed before host= and anything after are passed after.

Since the options themselves have a leading -- this can confuse the
scripts own option parsing, meaning you may need more than one "--"
marker, the first to separate the standalone helper args from the ts
args and a second to separate from any ident optiopns.

./standalone run-test -h $HOST -- test-amd64-amd64-xl-xsm ts-host-install --rescue -- guest=debian

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: Correctly quote "$@" and "${FOO[@]}"

v3: Use an array var for correct whitespace handling.
---
 standalone | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/standalone b/standalone
index 25e23ca..31514f9 100755
--- a/standalone
+++ b/standalone
@@ -317,11 +317,18 @@ case $op in
 	job=$1; shift
 	ts=$1; shift
 
+	options=()
+	for i in "$@" ; do
+	    if [ x$i = x-- ] ; then shift; break ; fi
+	    options+=("$i")
+	    shift
+	done
+
 	OSSTEST_CONFIG=$config \
 	OSSTEST_FLIGHT=$flight \
 	OSSTEST_HOST_REUSE=$reuse \
 	OSSTEST_JOB=$job \
-	    with_logging logs/$flight/$job.$ts.log ./$ts $hosts "$@"
+	    with_logging logs/$flight/$job.$ts.log ./$ts "${options[@]}" $hosts "$@"
 	;;
 
     get-job-status)
-- 
2.5.3

Re: [PATCH OSSTEST v4 5/7] standalone: Make it possible to pass options to run-test

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v4 5/7] standalone: Make it possible to pass options to run-test"):
> Currently the remainder of the comnand line is passed after the host=
> ident, which allows for other idents to be given, which isn't all that
> useful in practice.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v4 6/7] standalone: Use fail() from mgi-common in most places

[Ian Campbell]

Functional change is simply to prepend "$0: ", to change the exit
code for unknown operation and to slightly alter the error message
when no arguments are given.

A few "exit 0" and "exit $rc" remain.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: New patch
---
 standalone | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/standalone b/standalone
index 31514f9..27e2e7d 100755
--- a/standalone
+++ b/standalone
@@ -58,10 +58,11 @@ Options:
 EOF
 }
 
+. ./mgi-common
+
 if [ $# -lt 1 ] ; then
-    echo "Need an operation" >&2
     usage
-    exit 1
+    fail "No arguments given"
 fi
 
 op=$1 ; shift
@@ -123,7 +124,7 @@ while true ; do
 	--baseline)nobaseline=n; shift 1;;
 	--help)      usage; exit 0;;
         --) shift ; break ;;
-        *) echo "Internal error!" ; exit 1 ;;
+        *) fail "Internal error!" ;;
 
     esac
 done
@@ -147,34 +148,29 @@ fi
 
 db="standalone.db"
 if [ ! -f $db ] ; then
-    echo "No $db? Run standalone-reset." >&2
-    exit 1
+    fail "No $db? Run standalone-reset."
 fi
 
 if [ -z "$config" ] ; then
-    echo "No config specified." >&2
-    exit 1
+    fail "No config specified."
 fi
 IFS_saved=$IFS
 IFS=:
 for c in $config ; do
     if [ -z "$c" -o ! -r "$c" ] ; then
-        echo "Cannot read config $c." >&2
-        exit 1
+        fail "Cannot read config $c."
     fi
 done
 IFS=$IFS_saved
 
 need_flight() {
     if [ -z "$flight" ] ; then
-	echo "run-job: Need a flight" >&2
-	exit 1
+	fail "run-job: Need a flight"
     fi
 }
 need_host() {
     if [ "x$hosts" = x ] ; then
-	echo "run-job: Need a host" >&2
-	exit 1
+	fail "run-job: Need a host"
     fi
 }
 
@@ -227,8 +223,7 @@ case $op in
         need_flight
 
         if [ $# -lt 1 ] ; then
-            echo "make-flight: Need branch" >&2
-            exit 1
+            fail "make-flight: Need branch"
         fi
 
         branch=$1; shift
@@ -247,8 +242,7 @@ case $op in
 	need_flight
 
 	if [ $# -lt 1 ] ; then
-	    echo "set-paths: Need job" >&2
-	    exit 1
+	    fail "set-paths: Need job"
 	fi
 
 	job=$1; shift
@@ -275,8 +269,7 @@ case $op in
 	need_flight;
 
 	if [ $# -lt 1 ] ; then
-	    echo "run-job: Need job" >&2
-	    exit 1
+	    fail "run-job: Need job"
 	fi
 
 	job=$1; shift
@@ -287,8 +280,7 @@ case $op in
 	need_flight; need_host
 
 	if [ $# -lt 1 ] ; then
-	    echo "run-job: Need job" >&2
-	    exit 1
+	    fail "run-job: Need job"
 	fi
 
 	job=$1; shift
@@ -310,8 +302,7 @@ case $op in
 	need_flight; need_host
 
 	if [ $# -lt 2 ] ; then
-	    echo "run-test: Need job + test" >&2
-	    exit 1
+	    fail "run-test: Need job + test"
 	fi
 
 	job=$1; shift
@@ -335,8 +326,7 @@ case $op in
 	need_flight;
 
 	if [ $# -ne 1 ] ; then
-	    echo "get-job-status: Need job" >&2
-	    exit 1
+	    fail "get-job-status: Need job"
 	fi
 
 	job=$1; shift
@@ -345,5 +335,5 @@ case $op in
 
 	;;
     *)
-	echo "Unknown op $op" ; exit 1 ;;
+	fail "Unknown op $op" ;;
 esac
-- 
2.5.3

[PATCH OSSTEST v4 7/7] standalone: do not rotate empty log files.

[Ian Campbell]

By passing -n to savelog.

In particular this prevents the creation of an empty $log.0 on first
use when $log doesn't exist.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Put -n in correct place
---
 standalone | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/standalone b/standalone
index 27e2e7d..20a6ad5 100755
--- a/standalone
+++ b/standalone
@@ -192,7 +192,7 @@ ensure_logs() {
 with_logging() {
     local log=$1; shift
     ensure_logs
-    savelog -c 300 "$log" >/dev/null
+    savelog -c 300 -n "$log" >/dev/null
     "$@" 2>&1 | tee "$log"
     rc=${PIPESTATUS[0]}
     if [ $rc -ne 0 ] ; then
-- 
2.5.3

Re: [PATCH OSSTEST v4 7/7] standalone: do not rotate empty log files.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v4 7/7] standalone: do not rotate empty log files."):
> By passing -n to savelog.
> 
> In particular this prevents the creation of an empty $log.0 on first
> use when $log doesn't exist.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>