[OSSTEST PATCH 00/13] Immediately retry failing tests

[OSSTEST PATCH 00/13] Immediately retry failing tests

[Ian Jackson]

We discussed this at the Xen Summit.  What I do here is immediate
retry the jobs with regressions, and then reanalyse the original full
flight.  If the retries showed the failures were heisenbugs, this will
let them though.

This should reduce the negative impact on development, of heisenbugs,
but it won't do anything to help keep them out of the tree.

I think this approach was basically already agreed, but I'm CCing the
committers here for form's sake.

 README.dev          |  9 +++----
 cr-daily-branch     | 58 ++++++++++++++++++++++++++++++++++++++++++---
 cr-disk-report      |  2 +-
 cr-try-bisect-adhoc |  2 +-
 cri-args-hostlists  | 28 +++++++++++++++-------
 cs-bisection-step   |  4 ++--
 sg-report-flight    | 35 +++++++++++++++++++++++----
 sg-run-job          |  9 ++++++-
 8 files changed, 121 insertions(+), 26 deletions(-)

-- 
2.20.1

[OSSTEST PATCH 01/13] Honour OSSTEST_SIMULATE=2 to actually run dummy flight

[Ian Jackson]

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cri-args-hostlists | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index 994e00c0..6cdff53f 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -68,8 +68,8 @@ fi
 
 execute_flight () {
         case "x$OSSTEST_SIMULATE" in
-        x|x0)   ;;
-        *)      echo SIMULATING - NOT EXECUTING $1 $2
+        x|x0|x2)   ;;
+        *)      echo SIMULATING $OSSTEST_SIMULATE - NOT EXECUTING $1 $2
                 return
                 ;;
         esac
-- 
2.20.1

[OSSTEST PATCH 02/13] Honour OSSTEST_SIMULATE_FAIL in sg-run-job

[Ian Jackson]

This is a Tcl list of globs for <job>.<step>, and allows for
simulating particular test failures.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 sg-run-job | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sg-run-job b/sg-run-job
index dd76d4f2..c64ae026 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -406,7 +406,14 @@ proc spawn-ts {iffail testid args} {
     jobdb::spawn-step-commit $flight $jobinfo(job) $stepno $testid
 
     set xprefix {}
-    if {[var-or-default env(OSSTEST_SIMULATE) 0]} { set xprefix echo }
+    if {[var-or-default env(OSSTEST_SIMULATE) 0]} {
+	set xprefix echo
+	foreach ent [var-or-default env(OSSTEST_SIMULATE_FAIL) {}] {
+	    if {[string match $ent $jobinfo(job).$testid]} {
+		set xprefix OSSTEST_SIMULATE_FAIL
+	    }
+	}
+    }
 
     set log [jobdb::step-log-filename $flight $jobinfo(job) $stepno $ts]
     set redirects {}
-- 
2.20.1

[OSSTEST PATCH 03/13] sg-report-job-history: eval $DAILY_BRANCH_PREEXEC_HOOK

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

Put the call to this debugging/testing variable inside an eval.  This
allows a wider variety of stunts.  The one in-tree reference is
already compatible with this new semantics.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cr-daily-branch | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cr-daily-branch b/cr-daily-branch
index b8f221ee..23060588 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -472,7 +472,7 @@ sgr_args+=" $EXTRA_SGR_ARGS"
 
 date >&2
 : $flight $branch $OSSTEST_BLESSING $sgr_args
-$DAILY_BRANCH_PREEXEC_HOOK
+eval "$DAILY_BRANCH_PREEXEC_HOOK"
 execute_flight $flight $OSSTEST_BLESSING
 date >&2
 
-- 
2.20.1

[OSSTEST PATCH 04/13] cri-args-hostlists: New debug var $OSSTEST_REPORT_JOB_HISTORY_RUN

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

No effect if this is empty.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cri-args-hostlists | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index 6cdff53f..7019c0c7 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -121,6 +121,7 @@ start_email () {
 
 	date >&2
 
+	$OSSTEST_REPORT_JOB_HISTORY_RUN \
 	with-lock-ex -w $globallockdir/report-lock \
 	  ./sg-report-job-history --report-processing-start-time \
 	    --html-dir=$job_html_dir --flight=$flight
-- 
2.20.1

[OSSTEST PATCH 05/13] cri-args-hostlists: Break out report_flight and publish_logs

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

NFC.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 cri-args-hostlists | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index 7019c0c7..52e39f33 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -128,10 +128,7 @@ start_email () {
 
 	date >&2
 
-	./sg-report-flight --report-processing-start-time \
-	        --html-dir=$flight_html_dir/$flight/ \
-		--allow=allow.all --allow=allow.$branch \
-		$sgr_args $flight >tmp/$flight.report
+	report_flight $flight
 	./cr-fold-long-lines tmp/$flight.report
 
 	date >&2
@@ -144,11 +141,23 @@ start_email () {
 	date >&2
 }
 
+report_flight () {
+	local flight=$1
+	./sg-report-flight --html-dir=$flight_html_dir/$flight/ \
+		--allow=allow.all --allow=allow.$branch \
+		$sgr_args $flight >tmp/$flight.report
+}
+
+publish_logs () {
+	local flight=$1
+	./cr-publish-flight-logs ${OSSTEST_PUSH_HARNESS- --push-harness} \
+	    $flight >&2
+}
+
 publish_send_email () {
 	local flight=$1
+	publish_logs $flight
 	exec >&2
-	./cr-publish-flight-logs ${OSSTEST_PUSH_HARNESS- --push-harness} \
-	    $flight
 	send_email tmp/$flight.email
 }
 
-- 
2.20.1

[OSSTEST PATCH 06/13] sg-report-flight: Break out printout_flightheader

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-flight | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/sg-report-flight b/sg-report-flight
index a07e03cb..4b33facb 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -783,6 +783,14 @@ sub includes ($) {
     }
 }    
 
+sub printout_flightheader ($) {
+    my ($r) = @_;
+    bodyprint <<END;
+flight $r->{Flight} $branch $r->{FlightInfo}{blessing} [$r->{FlightInfo}{intended}]
+$c{ReportHtmlPubBaseUrl}/$r->{Flight}/
+END
+}
+
 sub printout {
     my ($r, @failures) = @_;
     $header_text = '';
@@ -793,10 +801,9 @@ sub printout {
 $r->{Flight}: $r->{OutcomeSummary}
 END
     includes(\@includebeginfiles);
-    bodyprint <<END;
-flight $r->{Flight} $branch $r->{FlightInfo}{blessing} [$r->{FlightInfo}{intended}]
-$c{ReportHtmlPubBaseUrl}/$r->{Flight}/
-END
+
+    printout_flightheader($r);
+
     if (defined $r->{Overall}) {
         bodyprint "\n";
         bodyprint $r->{Overall};
-- 
2.20.1

[OSSTEST PATCH 07/13] sg-report-flight: Provide --refer-to-flight option

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

This just generates an extra heading and URL at the top of the output.
In particular, it doesn't affect the algorithms which calculate
regressions.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-flight | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/sg-report-flight b/sg-report-flight
index 4b33facb..d9f0b964 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -42,6 +42,7 @@ our $htmldir;
 our $want_info_headers;
 our ($branch, $branches_cond_q);
 our @allows;
+our (@refer_to_flights);
 our (@includebeginfiles,@includefiles);
 
 open DEBUG, ">/dev/null";
@@ -66,6 +67,8 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
         push @includebeginfiles, $1;
     } elsif (m/^--include=(.*)$/) {
         push @includefiles, $1;
+    } elsif (m/^--refer-to-flight=(.*)$/) {
+        push @refer_to_flights, $1;
     } elsif (restrictflight_arg($_)) {
         # Handled by Executive
     } elsif (m/^--allow=(.*)$/) {
@@ -504,6 +507,16 @@ END
         die unless defined $specflight;
     }
 }
+sub find_refer_to_flights () {
+    my $ffq = $dbh_tests->prepare("SELECT * FROM flights WHERE flight=?");
+    @refer_to_flights = map {
+	my $flight = $_;
+	$ffq->execute($flight);
+	my $row = $ffq->fetchrow_hashref();
+	die "refer to flight $flight not found\n" unless $row;
+	{ Flight => $flight, FlightInfo => $row };
+    } @refer_to_flights;
+}
 
 sub examineflight ($) {
     my ($flight) = @_;
@@ -804,6 +817,10 @@ END
 
     printout_flightheader($r);
 
+    foreach my $ref_r (@refer_to_flights) {
+	printout_flightheader($ref_r);
+    }
+
     if (defined $r->{Overall}) {
         bodyprint "\n";
         bodyprint $r->{Overall};
@@ -1878,6 +1895,7 @@ db_retry($dbh_tests, [], sub {
     if (defined $mro) {
 	open MRO, "> $mro.new" or die "$mro.new $!";
     }
+    find_refer_to_flights();
     findspecflight();
     my $fi= examineflight($specflight);
     my @fails= justifyfailures($fi);
-- 
2.20.1

[OSSTEST PATCH 08/13] sg-report-flight: Nicer output for --refer-to-flight option

[Ian Jackson]

Sort the flight summary lines together, before the URLs.  This makes
it considerably easier to read.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 sg-report-flight | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/sg-report-flight b/sg-report-flight
index d9f0b964..f6ace190 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -796,12 +796,17 @@ sub includes ($) {
     }
 }    
 
-sub printout_flightheader ($) {
-    my ($r) = @_;
-    bodyprint <<END;
+sub printout_flightheaders {
+    foreach my $r (@_) {
+	bodyprint <<END;
 flight $r->{Flight} $branch $r->{FlightInfo}{blessing} [$r->{FlightInfo}{intended}]
+END
+    }
+    foreach my $r (@_) {
+	bodyprint <<END;
 $c{ReportHtmlPubBaseUrl}/$r->{Flight}/
 END
+    }
 }
 
 sub printout {
@@ -814,12 +819,7 @@ sub printout {
 $r->{Flight}: $r->{OutcomeSummary}
 END
     includes(\@includebeginfiles);
-
-    printout_flightheader($r);
-
-    foreach my $ref_r (@refer_to_flights) {
-	printout_flightheader($ref_r);
-    }
+    printout_flightheaders $r, @refer_to_flights;
 
     if (defined $r->{Overall}) {
         bodyprint "\n";
-- 
2.20.1

[OSSTEST PATCH 09/13] Introduce real-retry blessing

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

Nothing produces this yet.  (There's play-retry as well of course but
we don't need to document that really.)

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 README.dev          | 9 +++++----
 cr-daily-branch     | 3 ++-
 cr-disk-report      | 2 +-
 cr-try-bisect-adhoc | 2 +-
 cs-bisection-step   | 4 ++--
 sg-report-flight    | 2 +-
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/README.dev b/README.dev
index 2cbca109..3d09b3c6 100644
--- a/README.dev
+++ b/README.dev
@@ -381,10 +381,11 @@ These are the principal (intended) blessings:
    commissioning, and that blessing removed and replaced with `real'
    when the hosts are ready.
 
- * `real-bisect' and `adhoc-bisect': These are found only as the
-   blessing of finished flights.  (This is achieved by passing
-   *-bisect to sg-execute-flight.)  This allows the archaeologist
-   tools to distinguish full flights from bisection steps.
+ * `real-bisect', `real-retry', `adhoc-bisect': These are found only
+   as the blessing of finished flights.  (This is achieved by passing
+   *-bisect or *-retry to sg-execute-flight.)  This allows the
+   archaeologist tools to distinguish full flights from bisection
+   steps and retries.
 
    The corresponding intended blessing (as found in the `intended'
    column of the flights table) is `real'.  So the hosts used by the
diff --git a/cr-daily-branch b/cr-daily-branch
index 23060588..285ea361 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -76,7 +76,8 @@ case $branch in
 	treeurl=`./ap-print-url $branch`;;
 esac
 
-blessings_arg=--blessings=${DAILY_BRANCH_TESTED_BLESSING:-real}
+blessings_arg=${DAILY_BRANCH_TESTED_BLESSING:-real}
+blessings_arg=--blessings=${blessings_arg},${blessings_arg}-retest
 sgr_args+=" $blessings_arg"
 
 force_baseline='' # Non-empty = indication why we are forcing baseline.
diff --git a/cr-disk-report b/cr-disk-report
index 543d35bf..d76fd72f 100755
--- a/cr-disk-report
+++ b/cr-disk-report
@@ -38,7 +38,7 @@ our $graphs_px=0;
 our $graphs_py=0;
 open DEBUG, ">/dev/null" or die $!;
 
-our @blessings = qw(real real-bisect);
+our @blessings = qw(real real-retry real-bisect);
 # for these blessings column is       "<blessing> <branch>"
 # for other blessings column is       "<intended> [<blessing>]"
 
diff --git a/cr-try-bisect-adhoc b/cr-try-bisect-adhoc
index caadfd80..c2cfa475 100755
--- a/cr-try-bisect-adhoc
+++ b/cr-try-bisect-adhoc
@@ -49,7 +49,7 @@ export OSSTEST_BLESSING=adhoc
 
 compute_state_callback () {
 	compute_state_core \
-        	--blessings=real,real-bisect,adhoc-bisect \
+        	--blessings=real,real-retry,real-bisect,adhoc-bisect \
                 $bisect "$@" $branch $job $testid
 }
 
diff --git a/cs-bisection-step b/cs-bisection-step
index 762966da..8b391448 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -7,7 +7,7 @@
 # usage:
 #   ./cs-bisection-setup [<options>] <branch> <job> <testid>
 # options, usually:
-#      --blessings=real,real-bisect
+#      --blessings=real,real-retry,real-bisect
 #
 # First entry in --blessings list is the blessing of the basis
 # (non-bisection) flights.  This should not be the same as the
@@ -45,7 +45,7 @@ use HTML::Entities;
 use Osstest::Executive;
 use URI::Escape;
 
-our @blessings= qw(real real-bisect);
+our @blessings= qw(real real-retry real-bisect);
 our @revtuplegenargs= ();
 our $broken;
 
diff --git a/sg-report-flight b/sg-report-flight
index f6ace190..18643df6 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -120,7 +120,7 @@ die if defined $specver{this}{flight};
 die if defined $specver{that}{flight} &&
     grep { $_ ne 'flight' } keys %{ $specver{that} };
 
-push @blessings, 'real', 'real-bisect' unless @blessings;
+push @blessings, 'real', 'real-retry', 'real-bisect' unless @blessings;
 
 csreadconfig();
 
-- 
2.20.1

[OSSTEST PATCH 10/13] cri-args-hostlists: Move flight_html_dir variable

[Ian Jackson]

This is only used in report_flight.  We are going to want to call
report_flight from outside start_email, without having to set that
variable ourselves.

The variable isn't actually used in start_email.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cri-args-hostlists | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index 52e39f33..52cac195 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -113,7 +113,6 @@ start_email () {
 	printf '%s\n' "`getconfig EmailStdHeaders`"
 	printf 'Subject: %s' "${subject_prefix:-[$branch test] }"
 
-	local flight_html_dir=$OSSTEST_HTMLPUB_DIR/
 	local job_html_dir=$OSSTEST_HTML_DIR/
 	local host_html_dir=$OSSTEST_HTML_DIR/host/
 
@@ -143,6 +142,7 @@ start_email () {
 
 report_flight () {
 	local flight=$1
+	local flight_html_dir=$OSSTEST_HTMLPUB_DIR/
 	./sg-report-flight --html-dir=$flight_html_dir/$flight/ \
 		--allow=allow.all --allow=allow.$branch \
 		$sgr_args $flight >tmp/$flight.report
-- 
2.20.1

[OSSTEST PATCH 11/13] cr-daily-branch: Immediately retry failing tests

[Ian Jackson]

From: Ian Jackson <ian.jackson@eu.citrix.com>

We exclude the self-tests because we don't want to miss breakage, and
the Xen smoke tests because they will be run again RSN anyway.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cr-daily-branch | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/cr-daily-branch b/cr-daily-branch
index 285ea361..bea8734e 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -472,12 +472,58 @@ esac
 sgr_args+=" $EXTRA_SGR_ARGS"
 
 date >&2
+original_start=`date +%s`
+
 : $flight $branch $OSSTEST_BLESSING $sgr_args
 eval "$DAILY_BRANCH_PREEXEC_HOOK"
 execute_flight $flight $OSSTEST_BLESSING
 date >&2
 
-start_email $flight $branch "$sgr_args" "$subject_prefix"
+default_immediate_retry=$wantpush
+
+case "$branch" in
+*smoke*)	default_immediate_retry=false ;;
+osstest)	default_immediate_retry=false ;;
+*)		;;
+esac
+
+: ${OSSTEST_IMMEDIATE_RETRY:-$default_immediate_retry}
+
+while true; do
+	start_email $flight $branch "$sgr_args" "$subject_prefix"
+	if grep '^tolerable$' $mrof >/dev/null 2>&1; then break; fi
+	if ! $OSSTEST_IMMEDIATE_RETRY; then break; fi
+	OSSTEST_IMMEDIATE_RETRY=false
+	retry_jobs=$(
+		perl <$mrof -wne '
+			next unless m/^regression (\S+) /;
+			my $j = $1;
+			next if $j =~ m/^build/;
+			$r{$j}++;
+			END {
+				print "copy-jobs '$flight' $_ "
+					foreach sort keys %r;
+			}'
+	)
+	if [ "x$retry_jobs" = x ]; then break; fi
+
+	rflight=$(
+		./cs-adjust-flight new:$OSSTEST_BLESSING \
+			branch-set $branch \
+			$retry_jobs
+	)
+
+	./mg-adjust-flight-makexrefs -v $rflight \
+		--branch=$branch --revision-osstest=$narness_rev \
+		'^build-*' --debug --blessings=real
+
+	export OSSTEST_RESOURCE_WAITSTART=$original_start
+	execute_flight $rflight $OSSTEST_BLESSING-retest
+	report_flight $rflight
+	publish_logs $rflight
+
+	sgr_args+=" --refer-to-flight=$rflight"
+done
 
 push=false
 if grep '^tolerable$' $mrof >/dev/null 2>&1; then push=$wantpush; fi
-- 
2.20.1

[OSSTEST PATCH 12/13] Honour OSSTEST_SIMULATE_FAIL_RETRY for immediate retries

[Ian Jackson]

This is primarily useful for debugging the immediate-retry logic, but
it seems churlish to delete it again.

Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cr-daily-branch | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cr-daily-branch b/cr-daily-branch
index bea8734e..3e58d465 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -517,6 +517,10 @@ while true; do
 		--branch=$branch --revision-osstest=$narness_rev \
 		'^build-*' --debug --blessings=real
 
+	if [ "${OSSTEST_SIMULATE_FAIL_RETRY+y}" = y ]; then
+		export OSSTEST_SIMULATE_FAIL="${OSSTEST_SIMULATE_FAIL_RETRY}"
+	fi
+
 	export OSSTEST_RESOURCE_WAITSTART=$original_start
 	execute_flight $rflight $OSSTEST_BLESSING-retest
 	report_flight $rflight
-- 
2.20.1

[OSSTEST PATCH 13/13] cr-daily-branch: Do not do immediate retry of failing xtf flights

[Ian Jackson]

CC: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Ian Jackson <iwj@xenproject.org>
---
 cr-daily-branch | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cr-daily-branch b/cr-daily-branch
index 3e58d465..9b1961bd 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -484,6 +484,7 @@ default_immediate_retry=$wantpush
 case "$branch" in
 *smoke*)	default_immediate_retry=false ;;
 osstest)	default_immediate_retry=false ;;
+xtf*)		default_immediate_retry=false ;;
 *)		;;
 esac
 
-- 
2.20.1