[PATCH 00/12] Reporting and debugging improvements

[PATCH 00/12] Reporting and debugging improvements

[Ian Jackson]

I have been working on a new script called sg-report-host-history.
This is the result so far.

The series so far consists of mostly useful improvements, and one
bugfix to the existing reporting arrangements.

Everything but the last patch is IMO ready to go in right now.  

Thanks,
Ian.

[OSSTEST PATCH 01/12] Osstest.pm: Provide new db_prepare helper with built-in debugging

[Ian Jackson]

No callers, so no functional change, as yet.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest.pm |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Osstest.pm b/Osstest.pm
index e8bd77b..7d6949f 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -35,7 +35,7 @@ BEGIN {
                       getmethod
                       postfork
                       $dbh_tests db_retry db_retry_retry db_retry_abort
-                      db_begin_work
+                      db_begin_work db_prepare
                       ensuredir get_filecontents_core_quiet system_checked
                       nonempty visible_undef show_abs_time
                       );
@@ -268,6 +268,13 @@ sub db_retry ($$$;$$) {
     return $r;
 }
 
+sub db_prepare ($) {
+    # caller must ensure global filehandle DEBUG is open
+    my ($stmt) = @_;
+    print ::DEBUG "DB PREPARING:\n$stmt\n";
+    return $dbh_tests->prepare($stmt);
+}
+
 sub postfork () {
     $mjobdb->jobdb_postfork();
 }
-- 
1.7.10.4

[OSSTEST PATCH 02/12] sg-report-job-history: Use db_prepare

[Ian Jackson]

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-job-history |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sg-report-job-history b/sg-report-job-history
index 409e3d5..e285caa 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -67,7 +67,7 @@ END
     die "$flight ? @$branches ?" if @$branches!=1;
     @branches= @$branches;
 
-    my $selectq= $dbh_tests->prepare(<<END);
+    my $selectq= db_prepare(<<END);
         SELECT job FROM jobs WHERE flight=? ORDER BY JOB
 END
     $selectq->execute($flight);
@@ -84,7 +84,7 @@ if (defined($flight)) {
     push @jobs, $job;
 }
 
-our $failstepq= $dbh_tests->prepare(<<END);
+our $failstepq= db_prepare(<<END);
         SELECT * FROM steps
          WHERE flight=? AND job=?
            AND status!='pass'
@@ -120,7 +120,7 @@ sub run_getinfo ($) {
     }
 }
 
-our $revisionsq= $dbh_tests->prepare(<<END);
+our $revisionsq= db_prepare(<<END);
         SELECT * FROM runvars
          WHERE flight=? AND job=?
            AND name LIKE E'built\\_revision\\_\%'
@@ -136,7 +136,7 @@ sub add_revisions ($$$$) {
     }
 }
 
-our $buildsq= $dbh_tests->prepare(<<END);
+our $buildsq= db_prepare(<<END);
         SELECT * FROM runvars
          WHERE flight=? AND job=?
            AND name LIKE E'\%buildjob'
@@ -171,13 +171,13 @@ END
          WHERE ($cond)
       ORDER BY flight DESC
 END
-    my $flightsq= $dbh_tests->prepare(<<END);
+    my $flightsq= db_prepare(<<END);
         SELECT * $fromstuff
          LIMIT $limit
 END
     $flightsq->execute(@params);
 
-    my $hostsq= $dbh_tests->prepare(<<END);
+    my $hostsq= db_prepare(<<END);
         SELECT DISTINCT name
 	 FROM runvars
 	 JOIN flights USING (flight)
@@ -195,7 +195,7 @@ END
 	push @hostvarcols, $hostvar;
     }
 
-    my $hostq= $dbh_tests->prepare(<<END);
+    my $hostq= db_prepare(<<END);
         SELECT val FROM runvars WHERE flight=? AND job=? AND name=?
 END
 
-- 
1.7.10.4

[OSSTEST PATCH 03/12] cs-bisection-step: Use db_prepare

[Ian Jackson]

And do away with one ad-hoc statement dump.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 cs-bisection-step |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/cs-bisection-step b/cs-bisection-step
index 67a92b7..bf41f81 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -203,7 +203,7 @@ END
 	       AND flight = ?
 END
 
-    my $sth= $dbh_tests->prepare(<<END);
+    my $sth= db_prepare(<<END);
 
         SELECT url.val AS uval,
 	       rev.val AS rval,
@@ -303,7 +303,7 @@ sub findbasics () {
             ? "flight = $specflights{$fp}" : "TRUE";
     }
 
-    my $latestq= $dbh_tests->prepare(<<END);
+    my $latestq= db_prepare(<<END);
             SELECT flights.flight, steps.status,
                    $flight_is_not_broken AS notbroken
                 FROM flights JOIN steps USING (flight)
@@ -316,7 +316,7 @@ sub findbasics () {
                      $maxflight_cond
                ORDER BY notbroken DESC, flights.started DESC
 END
-    my $basisq= $dbh_tests->prepare(<<END);
+    my $basisq= db_prepare(<<END);
         SELECT * FROM flights JOIN steps USING (flight)
 	   WHERE job = ?
 	     AND testid = ?
@@ -553,14 +553,14 @@ END
 
     print STDERR "Searching for test results:\n";
 
-    my $stepq= $dbh_tests->prepare("
+    my $stepq= db_prepare("
        SELECT * FROM steps
           WHERE flight = ?
             AND job = ?
             AND testid = ?
     ");
 
-    my $jobq= $dbh_tests->prepare("
+    my $jobq= db_prepare("
        SELECT * FROM flights JOIN jobs USING (flight)
           WHERE job = ?
             AND $blessingscond
@@ -1059,7 +1059,7 @@ sub preparejob ($$$) {
 END
     my (@trevisions) = split / /, $choose->{Rtuple};
 
-    my $treeq= $dbh_tests->prepare(<<END);
+    my $treeq= db_prepare(<<END);
         SELECT name FROM runvars
           WHERE  flight=? AND job=?
             AND  name = ?
@@ -1082,7 +1082,7 @@ END
 
     # Check for subjobs:
 
-    my $jobq= $dbh_tests->prepare(<<END);
+    my $jobq= db_prepare(<<END);
         SELECT name, val FROM runvars
             WHERE  flight=? AND job=?
               AND  name LIKE '%job';
@@ -1156,7 +1156,7 @@ END
 
     $dbh_tests->do("DROP TABLE bisection_runvars");
 
-    my $jobsetq= $dbh_tests->prepare(<<END);
+    my $jobsetq= db_prepare(<<END);
         UPDATE runvars SET val=?
             WHERE  flight=? AND job=? AND name=?
 END
@@ -1185,7 +1185,7 @@ sub populateflight () {
     }
 
     my $removehosts= 
-    my $addhost= $dbh_tests->prepare(<<END);
+    my $addhost= db_prepare(<<END);
         INSERT INTO runvars (flight,job,name,val,synth)
                      VALUES (?,     ?,  ?,   ?,  'f')
 END
@@ -1244,8 +1244,7 @@ END
               LIMIT $maxequalflights + 1;
 END
 
-    print DEBUG "equalflightsq:\n$equalflightsqt";
-    my $equalflightsq = $dbh_tests->prepare($equalflightsqt);
+    my $equalflightsq = db_prepare($equalflightsqt);
 
     db_retry($popflight,'constructing', $dbh_tests,[qw(flights)], sub {
         print STDERR "Checking for flail (since $repro_firstflight)...\n";
-- 
1.7.10.4

[OSSTEST PATCH 04/12] sg-report-flight: Use db_prepare

[Ian Jackson]

And do away with two commented-out ad-hoc statement dumps.

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

diff --git a/sg-report-flight b/sg-report-flight
index 80777af..bdaf1c8 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -163,7 +163,7 @@ sub findaflight ($$$$$) {
         return undef;
     }
 
-    my $checkq= $dbh_tests->prepare(<<END);
+    my $checkq= db_prepare(<<END);
         SELECT status FROM steps WHERE flight=? AND job=? AND testid=?
 END
 
@@ -188,11 +188,10 @@ END
       ) AS sub
       ORDER BY blessing ASC, flight DESC
 END
-    #print DEBUG "===\n$flightsq\n===\n";
-    $flightsq= $dbh_tests->prepare($flightsq);
+    $flightsq= db_prepare($flightsq);
     $flightsq->execute(@blessings);
 
-    my $buildflightsq= $dbh_tests->prepare(<<END);
+    my $buildflightsq= db_prepare(<<END);
         SELECT val FROM runvars
 	 WHERE flight = ?
            AND name LIKE '%buildjob'
@@ -204,10 +203,9 @@ END
 		  AND name=?
 		GROUP BY flight, job, val
 END
-    #print DEBUG "===\n$mismatchq\n===\n";
-    $revisionsq= $dbh_tests->prepare($revisionsq);
+    $revisionsq= db_prepare($revisionsq);
 
-    my $revisionsosstestq= $dbh_tests->prepare(<<END);
+    my $revisionsosstestq= db_prepare(<<END);
             SELECT harness AS val FROM flights_harness_touched
                 WHERE flight=?
 END
@@ -319,7 +317,7 @@ END
     };
     $jobs= [ sort { $colmap->($a) cmp $colmap->($b) } @$jobs ];
 
-    my $stepsq= $dbh_tests->prepare(<<END);
+    my $stepsq= db_prepare(<<END);
         SELECT * FROM steps
             WHERE flight=$flight AND job=?
             ORDER BY stepno
@@ -441,7 +439,7 @@ END
             });
         }
 
-        my $revh= $dbh_tests->prepare(<<END);
+        my $revh= db_prepare(<<END);
             SELECT * FROM runvars
                 WHERE flight=$flight AND job='$j->{job}'
                   AND name like 'built_revision_%'
@@ -623,7 +621,7 @@ sub justifyfailures ($;$) {
               AND $blessingscond
             LIMIT 1
 END
-    $anypassq= $dbh_tests->prepare($anypassq);
+    $anypassq= db_prepare($anypassq);
 
     my $duration_estimator= duration_estimator($branch, $blessings[0]);
     foreach my $failv (@failures) {
@@ -802,7 +800,7 @@ sub htmloutjob ($$) {
     my $title= "Info on flight $fi->{Flight} job $job";
     my $branch= $fi->{FlightInfo}{branch};
 
-    our $htmlout_jobq ||= $dbh_tests->prepare(<<END);
+    our $htmlout_jobq ||= db_prepare(<<END);
         SELECT * FROM jobs WHERE flight = ? AND job = ?
 END
     $htmlout_jobq->execute($fi->{Flight}, $job);
@@ -826,7 +824,7 @@ END
 END
 
     my %issteplog;
-    my $stepsq= $dbh_tests->prepare(<<END);
+    my $stepsq= db_prepare(<<END);
         SELECT * FROM steps
                 WHERE flight=? AND job=?
              ORDER BY stepno ASC
@@ -892,7 +890,7 @@ END
 <h2>Test control variables</h2>
 <table rules=all><tr><th>Name</th><th>Value</th><th>Source</th></tr>
 END
-    my $varsq= $dbh_tests->prepare(<<END);
+    my $varsq= db_prepare(<<END);
         SELECT * FROM runvars
                 WHERE flight=? AND job=?
              ORDER BY synth, name
@@ -1086,7 +1084,7 @@ END
     }
     my $started_str = show_abs_time $fi->{FlightInfo}{started};
 
-    our $htmlout_finishedq ||= $dbh_tests->prepare(<<END);
+    our $htmlout_finishedq ||= db_prepare(<<END);
         SELECT
  (SELECT max(finished) FROM steps WHERE flight=?)                 max,
  (SELECT count(*) FROM steps WHERE finished IS NULL AND flight=?) unfinished
@@ -1110,7 +1108,7 @@ END
 <td>Test harness revision(s):</td><td>
 END
 
-    our $htmlout_touchedq ||= $dbh_tests->prepare(<<END);
+    our $htmlout_touchedq ||= db_prepare(<<END);
         SELECT harness FROM flights_harness_touched WHERE flight=?
 END
     $htmlout_touchedq->execute($fi->{Flight});
-- 
1.7.10.4

[OSSTEST PATCH 05/12] sg-report-job-history: Add a debugging statement

[Ian Jackson]

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

diff --git a/sg-report-job-history b/sg-report-job-history
index e285caa..f5f3496 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -150,6 +150,8 @@ sub altcolour ($) {
 sub processjobbranch ($$) {
     my ($j,$bra) = @_;
 
+    print DEBUG "processjobbranch('$j',", ($bra ? "'$bra'" : 'undef'), ")\n";
+
     my %rev_grid_col;
     my @rev_grid_cols;
     my @test_rows;
-- 
1.7.10.4

[OSSTEST PATCH 06/12] sg-report-job-history: Slightly prettify sql

[Ian Jackson]

No functional change apart from slightly better debug output.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-job-history |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sg-report-job-history b/sg-report-job-history
index f5f3496..e4f35bf 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -160,9 +160,7 @@ sub processjobbranch ($$) {
     my $cond = "job = ? AND $blessingscond";
     my (@params) = ($j, @blessings);
     if (defined $bra) {
-        $cond .= <<END;
-           AND branch = ?
-END
+        $cond .= " AND branch = ?";
         push @params, $bra;
     }
     my $limit= 100;
-- 
1.7.10.4

[OSSTEST PATCH 07/12] sg-report-job-history: Cope if history too short

[Ian Jackson]

If there have been less than 99 relevant flights, the inner SELECT (to
determine the minimum flight number) would return NULL.  And anything
> NULL is NULL and NULL is treated as false.  So the host runvar
identification would break.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-job-history |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sg-report-job-history b/sg-report-job-history
index e4f35bf..76cb146 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -182,10 +182,11 @@ END
 	 FROM runvars
 	 JOIN flights USING (flight)
 	WHERE ($cond)
-	  AND flight >= (
+	  AND flight >= COALESCE(
+             (
 	      SELECT flight $fromstuff
 	      LIMIT 1 OFFSET $offset
-          )
+	     ), 0)
      ORDER BY name;
 END
     $hostsq->execute(@params, @params); # sql text contains $cond twice
-- 
1.7.10.4

[OSSTEST PATCH 08/12] sg-report-job-history: Show start time

[Ian Jackson]

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-job-history |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sg-report-job-history b/sg-report-job-history
index 76cb146..bbb85fd 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -251,7 +251,8 @@ END
         print H "<html><head><title>$title</title></head><body>\n";
         print H "<h1>$title</h1>\n";
         print H "<table rules=all>";
-        print H "<tr><th>flight</th><th>branch</th><th>failure</th>\n";
+        print H "<tr><th>started</th><th>flight</th>",
+	        "<th>branch</th><th>failure</th>\n";
         print H "<th>", (join ", ", @hostvarcols), "</th>\n";
         foreach my $c (@rev_grid_cols) {
             print H "<th>".encode_entities($c)."</th>\n";
@@ -264,6 +265,10 @@ END
             my $colour= "bgcolor=\"$r->{Colour}\"";
             my $altcolour= altcolour($alternate);
             print H "<tr $altcolour>";
+	    my $started = $r->{Flight}{started};
+	    print H "<td>";
+	    print H show_abs_time $started if $started;
+	    print H "</td>\n";
             my $flt= $r->{Flight}{flight};
             $url= "$c{ReportHtmlPubBaseUrl}/$flt";
             print H "<td><a href=\"$url\">$flt</a></td>\n";
-- 
1.7.10.4

[OSSTEST PATCH 09/12] reporting: Move report_run_getinfo and some colours into Executive.pm

[Ian Jackson]

This is to support a forthcoming sg-report-host-history.

No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Executive.pm  |   45 +++++++++++++++++++++++++++++++++++++++++++++
 sg-report-job-history |   48 ++++--------------------------------------------
 2 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 118a91a..90c615a 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -45,6 +45,7 @@ BEGIN {
     @ISA         = qw(Exporter);
     @EXPORT      = qw(get_harness_rev grabrepolock_reexec
                       findtask @all_lock_tables
+                      report_run_getinfo report_altcolour
                       tcpconnect_queuedaemon plan_search
                       alloc_resources alloc_resources_rollback_begin_work
                       resource_check_allocated resource_shared_mark_ready
@@ -188,6 +189,50 @@ sub opendb ($) {
     return $dbh;
 }
 
+#---------- history reporting ----------
+
+our $green=  '#008800';
+our $red=    '#ff8888';
+our $yellow= '#ffff00';
+our $purple= '#ff00ff';
+
+sub report_run_getinfo ($) {
+    # $f is a joined flight/job row, must contain at least
+    #    flight job status
+    my ($f) = @_;
+    my $status= $f->{status};
+    if ($status eq 'pass') {
+        return { Summary => "($status)", Colour => $green };
+    } elsif ($status eq 'fail') {
+	our $failstepq //= db_prepare(<<END);
+	    SELECT * FROM steps
+	     WHERE flight=? AND job=?
+	       AND status!='pass'
+	  ORDER BY stepno
+	     LIMIT 1
+END
+        $failstepq->execute($f->{flight}, $f->{job});
+        my $fs= $failstepq->fetchrow_hashref();
+        if (!defined $fs) {
+            return { Summary => "(unknown)", Colour => $yellow };
+        } elsif ($fs->{status} eq 'fail') {
+            return { Summary => "$fs->{testid}", Colour => $red };
+        } else {
+            return { Summary => "$fs->{testid} $fs->{status}",
+                     Colour => $red };
+        }
+    } elsif ($status eq 'blocked') {
+        return { Summary => "blocked", Colour => $purple },
+    } else {
+        return { Summary => "($f->{status})", Colour => $yellow };
+    }
+}
+
+sub report_altcolour ($) {
+    my ($bool) = @_;
+    return "bgcolor=\"#".(qw(d0d0d0 ffffff))[$bool]."\"";
+}
+
 #---------- host (and other resource) allocation ----------
 
 our $taskid;
diff --git a/sg-report-job-history b/sg-report-job-history
index bbb85fd..18821b3 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -25,6 +25,7 @@ use IO::Handle;
 use HTML::Entities;
 
 use Osstest::TestSupport;
+use Osstest::Executive;
 
 our (@blessings,@branches);
 our $limit= 500;
@@ -84,42 +85,6 @@ if (defined($flight)) {
     push @jobs, $job;
 }
 
-our $failstepq= db_prepare(<<END);
-        SELECT * FROM steps
-         WHERE flight=? AND job=?
-           AND status!='pass'
-      ORDER BY stepno
-         LIMIT 1
-END
-
-our $green=  '#008800';
-our $red=    '#ff8888';
-our $yellow= '#ffff00';
-our $purple= '#ff00ff';
-
-sub run_getinfo ($) {
-    my ($f) = @_;
-    my $status= $f->{status};
-    if ($status eq 'pass') {
-        return { Summary => "($status)", Colour => $green };
-    } elsif ($status eq 'fail') {
-        $failstepq->execute($f->{flight}, $f->{job});
-        my $fs= $failstepq->fetchrow_hashref();
-        if (!defined $fs) {
-            return { Summary => "(unknown)", Colour => $yellow };
-        } elsif ($fs->{status} eq 'fail') {
-            return { Summary => "$fs->{testid}", Colour => $red };
-        } else {
-            return { Summary => "$fs->{testid} $fs->{status}",
-                     Colour => $red };
-        }
-    } elsif ($status eq 'blocked') {
-        return { Summary => "blocked", Colour => $purple },
-    } else {
-        return { Summary => "($f->{status})", Colour => $yellow };
-    }
-}
-
 our $revisionsq= db_prepare(<<END);
         SELECT * FROM runvars
          WHERE flight=? AND job=?
@@ -142,11 +107,6 @@ our $buildsq= db_prepare(<<END);
            AND name LIKE E'\%buildjob'
 END
 
-sub altcolour ($) {
-    my ($bool) = @_;
-    return "bgcolor=\"#".(qw(d0d0d0 ffffff))[$bool]."\"";
-}
-
 sub processjobbranch ($$) {
     my ($j,$bra) = @_;
 
@@ -201,7 +161,7 @@ END
 END
 
     while (my $f= $flightsq->fetchrow_hashref()) {
-        my $ri= run_getinfo($f);
+        my $ri= report_run_getinfo($f);
 
 	$ri->{Hosts} = [ ];
 	foreach my $hostvar (@hostvarcols) {
@@ -263,7 +223,7 @@ END
         my @alt_revs= ('0')x $#rev_grid_cols;
         foreach my $r (@test_rows) {
             my $colour= "bgcolor=\"$r->{Colour}\"";
-            my $altcolour= altcolour($alternate);
+            my $altcolour= report_altcolour($alternate);
             print H "<tr $altcolour>";
 	    my $started = $r->{Flight}{started};
 	    print H "<td>";
@@ -286,7 +246,7 @@ END
                     !defined($v) ? 0 :
                     $last_revs[$i] eq $v;
                 $alt_revs[$i] ^= !$same;
-                print H "<td ".altcolour($alt_revs[$i]).">";
+                print H "<td ".report_altcolour($alt_revs[$i]).">";
                 if (defined $v) {
                     my $vp= $v;
                     if (defined $lastrev && $v eq $lastrev) {
-- 
1.7.10.4

[OSSTEST PATCH 10/12] reporting: Add colours as optional export tag, and provide $blue

[Ian Jackson]

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Executive.pm |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 90c615a..6edbfee 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -52,9 +52,9 @@ BEGIN {
                       duration_estimator
                       db_pg_dsn opendb opendb_state
                       );
-    %EXPORT_TAGS = ( );
+    %EXPORT_TAGS = ( colours => [qw($green $red $yellow $purple $blue)] );
 
-    @EXPORT_OK   = qw();
+    @EXPORT_OK   = @{ $EXPORT_TAGS{colours} };
 }
 
 # DATABASE TABLE LOCK HIERARCHY
@@ -195,6 +195,7 @@ our $green=  '#008800';
 our $red=    '#ff8888';
 our $yellow= '#ffff00';
 our $purple= '#ff00ff';
+our $blue=   '#0000ff';
 
 sub report_run_getinfo ($) {
     # $f is a joined flight/job row, must contain at least
-- 
1.7.10.4

[OSSTEST PATCH 11/12] reporting: Show slightly better info for broken jobs

[Ian Jackson]

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Executive.pm |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 6edbfee..6c16fdd 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -204,7 +204,8 @@ sub report_run_getinfo ($) {
     my $status= $f->{status};
     if ($status eq 'pass') {
         return { Summary => "($status)", Colour => $green };
-    } elsif ($status eq 'fail') {
+    } elsif ($status eq 'fail' or $status eq 'broken') {
+	my $failcolour = $status eq 'fail' ? $red : $yellow;
 	our $failstepq //= db_prepare(<<END);
 	    SELECT * FROM steps
 	     WHERE flight=? AND job=?
@@ -217,10 +218,12 @@ END
         if (!defined $fs) {
             return { Summary => "(unknown)", Colour => $yellow };
         } elsif ($fs->{status} eq 'fail') {
-            return { Summary => "$fs->{testid}", Colour => $red };
+            return { Summary => "$fs->{testid}", Colour => $failcolour };
+        } elsif ($fs->{status} eq 'broken') {
+            return { Summary => "$fs->{testid} broken", Colour => $yellow };
         } else {
             return { Summary => "$fs->{testid} $fs->{status}",
-                     Colour => $red };
+                     Colour => $failcolour };
         }
     } elsif ($status eq 'blocked') {
         return { Summary => "blocked", Colour => $purple },
-- 
1.7.10.4

[OSSTEST PATCH 12/12] RFC: sg-report-job-history

[Ian Jackson]

This script is still a work in progress.

Things which I know are still needed:
 * --flight= option to make it automatically report host usage for a flight
 * handle db deadlock report (by putting whole script in a db retry)
 * hook this into the automatic reporting in cr-*

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-report-host-history |  243 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 243 insertions(+)
 create mode 100755 sg-report-host-history

diff --git a/sg-report-host-history b/sg-report-host-history
new file mode 100755
index 0000000..7aacb42
--- /dev/null
+++ b/sg-report-host-history
@@ -0,0 +1,243 @@
+#!/usr/bin/perl -w
+
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2013 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+use strict qw(vars);
+
+use DBI;
+use Osstest;
+use IO::Handle;
+use HTML::Entities;
+
+use Osstest::Executive qw(:DEFAULT :colours);
+
+our $limit= 200;
+our $flightlimit;
+our $htmlout = ".";
+our @blessings;
+
+open DEBUG, ">/dev/null";
+
+my $namecond= "(name = 'host' or name like '%_host')";
+csreadconfig();
+
+while (@ARGV && $ARGV[0] =~ m/^-/) {
+    $_= shift @ARGV;
+    last if m/^--?$/;
+    if (m/^--(limit)\=([1-9]\d*)$/) {
+        $$1= $2;
+    } elsif (m/^--flight-limit\=([1-9]\d*)$/) {
+	$flightlimit= $1;
+    } elsif (m/^--blessings?=(.*)$/) {
+        push @blessings, split ',', $1;
+    } elsif (m/^--html-dir=(.*)$/) {
+        $htmlout= $1;
+    } elsif (m/^--debug/) {
+        open DEBUG, ">&2" or die $!;
+        DEBUG->autoflush(1);
+    } else {
+        die "$_ ?";
+    }
+}
+@blessings= qw(real) if !@blessings;
+
+@ARGV or die $!;
+
+$dbh_tests->begin_work;
+
+if (!$flightlimit) {
+    my $flagscond =
+	'('.join(' OR ', map { "f.hostflag = 'blessed-$_'" } @blessings).')';
+    my $nhostsq = db_prepare(<<END);
+        SELECT count(*)
+	  FROM resources r
+	 WHERE restype='host'
+	   AND EXISTS (SELECT 1
+		         FROM hostflags f
+		        WHERE f.hostname=r.resname
+		          AND $flagscond)
+END
+    $nhostsq->execute();
+    my ($nhosts) = $nhostsq->fetchrow_array();
+    print DEBUG "COUNTED $nhosts hosts\n";
+    $flightlimit = $nhosts * $limit * 2;
+}
+
+my $minflightsq = db_prepare(<<END);
+    SELECT flight
+      FROM (
+	SELECT flight
+	  FROM flights
+	 ORDER BY flight DESC
+	 LIMIT $flightlimit
+      ) f
+      ORDER BY flight ASC
+      LIMIT 1
+END
+$minflightsq->execute();
+my ($minflight) = $minflightsq->fetchrow_array();
+$minflight //= 0;
+
+our $flightcond = "(flight > $minflight)";
+
+$dbh_tests->do("SET LOCAL enable_seqscan=false");
+# Otherwise the PostgreSQL query planner likes to do a complete scan
+# of the runvars table, rather than walking backwards through the
+# flights until it has what we've told it is enough.
+
+my $runvarq = db_prepare(<<END);
+    SELECT flight, job, name, val
+      FROM runvars
+     WHERE $namecond
+       AND val = ?
+       AND $flightcond
+     ORDER BY flight DESC
+     LIMIT $limit * 2 + 100
+END
+
+my $endedq = db_prepare(<<END);
+    SELECT finished, testid, status AS laststepstatus
+      FROM steps
+     WHERE flight=? AND job=? AND finished IS NOT NULL
+     ORDER BY finished DESC
+     LIMIT 1
+END
+
+my $infoq = db_prepare(<<END);
+    SELECT blessing, branch, intended, status
+      FROM flights
+      JOIN jobs USING (flight)
+     WHERE flight=? AND job=?
+END
+
+my $allocdq = db_prepare(<<END);
+    SELECT testid, finished, status
+      FROM steps
+     WHERE flight=? AND job=?
+       AND (testid='hosts-allocate' OR step='ts-hosts-allocate')
+     ORDER BY finished ASC
+     LIMIT 1
+END
+
+sub jobquery ($$) {
+    my ($q, $jr) = @_;
+    $q->execute($jr->{flight}, $jr->{job});
+    return $q->fetchrow_hashref();
+}
+
+sub reporthost ($) {
+    my ($hostname) = @_;
+
+    die if $hostname =~ m/[^-_.+0-9a-z]/;
+
+    my $html_file= "$htmlout/host.$hostname.html";
+    open H, "> $html_file.new" or die "$html_file $!";
+
+    my $title= "host history $hostname\n";
+    $title= encode_entities($title);
+    print H "<html><head><title>$title</title></head><body>\n";
+    print H "<h1>$title</h1>\n";
+    print H "<table rules=all><tr>\n";
+
+    print H "<th>alloc testid</th><th>alloc completed</th>\n";
+    print H "<th>job finished</th>\n";
+    print H "<th>role</th>\n";
+
+    print H "<th>flight</th>\n";
+    print H "<th>branch</th><th>intended</th><th>blessing</th>\n";
+
+    print H "<th>job</th><th>failure</th>\n";
+
+    print H "</tr>\n";
+
+    my @rows;
+    $runvarq->execute($hostname);
+
+    print DEBUG "FIRST PASS\n";
+    while (my $jr= $runvarq->fetchrow_hashref()) {
+	print DEBUG "JOB $jr->{flight}.$jr->{job} ";
+
+	my $endedrow = jobquery($endedq, $jr);
+	if (!$endedrow) {
+	    print "no-finished\n";
+	    next;
+	}
+	print DEBUG join " ", map { $endedrow->{$_} } sort keys %$endedrow;
+	print DEBUG ".\n";
+
+	push @rows, { %$jr, %$endedrow };
+    }
+
+    print DEBUG "FOUND ", (scalar @rows), " ROWS\n";
+
+    @rows = sort { $b->{finished} <=> $a->{finished} } @rows;
+    $#rows = $limit-1 if @rows > $limit;
+
+    my $alternate = 0;
+    foreach my $jr (@rows) {
+	print DEBUG "JOB $jr->{flight}.$jr->{job}\n";
+
+	my $ir = jobquery($infoq, $jr);
+	my $ar = jobquery($allocdq, $jr);
+
+	my $altcolour = report_altcolour($alternate);
+	print H "<tr $altcolour>";
+
+	if (!defined $ar->{testid}) {
+	    print H "<td bgcolor=\"$red\"></td>";
+	    print H "<td>?</td>";
+	} else {
+	    if ($ar->{status} eq 'pass') {
+		print H "<td>$ar->{testid}</td>";
+		print H "<td>", (show_abs_time $ar->{finished}), "</td>";
+	    } elsif ($ar->{status} eq 'running') {
+		print H "<td bgcolor=\"$blue\">$ar->{testid}</td>";
+		print H "<td>$ar->{status}</td>";
+	    } else {
+		print H "<td bgcolor=\"$red\">$ar->{testid}</td>";
+		print H "<td>$ar->{status}</td>";
+	    }
+	}
+	print H "\n";
+
+	print H "<td>", (show_abs_time $jr->{finished}), "</td>\n";
+	print H "<td>", $jr->{name}, "</td>\n";
+
+	my $url= "$c{ReportHtmlPubBaseUrl}/$jr->{flight}";
+	print H "<td><a href=\"$url\">$jr->{flight}</a></td>\n";
+	$url= "$c{ReportHtmlPubBaseUrl}/$jr->{flight}/".
+	    encode_entities($jr->{job})."/";
+	print H "<td>$ir->{branch}</td>";
+	print H "<td>$ir->{intended}</td>";
+	print H "<td>";
+	print H $ir->{blessing} unless $ir->{blessing} eq 'running';
+	print H "</td>";
+
+	print H "<td><a href=\"$url\">$jr->{job}</td>\n";
+
+	my $ri = report_run_getinfo({ %$jr, %$ir });
+	print H "<td bgcolor=\"$ri->{Colour}\">$ri->{Summary}</td>\n";
+
+	print H "</tr>\n\n";
+	$alternate ^= 1;
+    }
+
+    print H "</table></body></html>\n";
+}
+
+reporthost $_ foreach @ARGV;
-- 
1.7.10.4

Re: [PATCH 00/12] Reporting and debugging improvements

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> I have been working on a new script called sg-report-host-history.
> This is the result so far.
> 
> The series so far consists of mostly useful improvements, and one
> bugfix to the existing reporting arrangements.

Unrelated, but while looking at
http://logs.test-lab.xenproject.org/osstest/results/?C=M;O=A which is
quite long I've often wished that the paths had a few more / instead
of . in them so it was a bit easier to find the specific things I was
after...

Ian.

> Everything but the last patch is IMO ready to go in right now.  
> 
> Thanks,
> Ian.

Re: [OSSTEST PATCH 01/12] Osstest.pm: Provide new db_prepare helper with built-in debugging

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> No callers, so no functional change, as yet.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 02/12] sg-report-job-history: Use db_prepare

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 03/12] cs-bisection-step: Use db_prepare

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> And do away with one ad-hoc statement dump.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 04/12] sg-report-flight: Use db_prepare

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> And do away with two commented-out ad-hoc statement dumps.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 05/12] sg-report-job-history: Add a debugging statement

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 06/12] sg-report-job-history: Slightly prettify sql

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> No functional change apart from slightly better debug output.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 07/12] sg-report-job-history: Cope if history too short

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> If there have been less than 99 relevant flights, the inner SELECT (to
> determine the minimum flight number) would return NULL.  And anything
> > NULL is NULL and NULL is treated as false.  So the host runvar
> identification would break.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 08/12] sg-report-job-history: Show start time

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 09/12] reporting: Move report_run_getinfo and some colours into Executive.pm

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> This is to support a forthcoming sg-report-host-history.
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 10/12] reporting: Add colours as optional export tag, and provide $blue

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 11/12] reporting: Show slightly better info for broken jobs

[Ian Campbell]

On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  Osstest/Executive.pm |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
> index 6edbfee..6c16fdd 100644
> --- a/Osstest/Executive.pm
> +++ b/Osstest/Executive.pm
> @@ -204,7 +204,8 @@ sub report_run_getinfo ($) {
>      my $status= $f->{status};
>      if ($status eq 'pass') {
>          return { Summary => "($status)", Colour => $green };
> -    } elsif ($status eq 'fail') {
> +    } elsif ($status eq 'fail' or $status eq 'broken') {
> +	my $failcolour = $status eq 'fail' ? $red : $yellow;
>  	our $failstepq //= db_prepare(<<END);
>  	    SELECT * FROM steps
>  	     WHERE flight=? AND job=?
> @@ -217,10 +218,12 @@ END
>          if (!defined $fs) {
>              return { Summary => "(unknown)", Colour => $yellow };
>          } elsif ($fs->{status} eq 'fail') {
> -            return { Summary => "$fs->{testid}", Colour => $red };
> +            return { Summary => "$fs->{testid}", Colour => $failcolour };
> +        } elsif ($fs->{status} eq 'broken') {
> +            return { Summary => "$fs->{testid} broken", Colour => $yellow };

Not $failcolour?

(Or alternatively why is the fail case changing if not)

>          } else {
>              return { Summary => "$fs->{testid} $fs->{status}",
> -                     Colour => $red };
> +                     Colour => $failcolour };
>          }
>      } elsif ($status eq 'blocked') {
>          return { Summary => "blocked", Colour => $purple },

Re: [OSSTEST PATCH 11/12] reporting: Show slightly better info for broken jobs

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 11/12] reporting: Show slightly better info for broken jobs"):
> On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
...
> > -    } elsif ($status eq 'fail') {
> > +    } elsif ($status eq 'fail' or $status eq 'broken') {
> > +	my $failcolour = $status eq 'fail' ? $red : $yellow;
> >  	our $failstepq //= db_prepare(<<END);
> >  	    SELECT * FROM steps
> >  	     WHERE flight=? AND job=?
> > @@ -217,10 +218,12 @@ END
> >          if (!defined $fs) {
> >              return { Summary => "(unknown)", Colour => $yellow };
> >          } elsif ($fs->{status} eq 'fail') {
> > -            return { Summary => "$fs->{testid}", Colour => $red };
> > +            return { Summary => "$fs->{testid}", Colour => $failcolour };
> > +        } elsif ($fs->{status} eq 'broken') {
> > +            return { Summary => "$fs->{testid} broken", Colour => $yellow };
> 
> Not $failcolour?

Using $failcolour instead of $yellow there would only make a
difference if the job was `fail' but the step was `broken'.  So this
is a bit of an edge case.

But I think in that situation $red is probably better, so I will
change this.

Thanks,
Ian.

Re: [PATCH 00/12] Reporting and debugging improvements

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH 00/12] Reporting and debugging improvements"):
> On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> > I have been working on a new script called sg-report-host-history.
> > This is the result so far.
> > 
> > The series so far consists of mostly useful improvements, and one
> > bugfix to the existing reporting arrangements.
> 
> Unrelated, but while looking at
> http://logs.test-lab.xenproject.org/osstest/results/?C=M;O=A which is
> quite long I've often wished that the paths had a few more / instead
> of . in them so it was a bit easier to find the specific things I was
> after...

This is a good point.  Would you like me to take that as a todo item
for this series ?

If we feel like changing this then I think there is no serious
compatibility problem: old scripts will write and refer to old paths
and new scripts to new ones.  Eventually all will be new.

Ian.

Re: [OSSTEST PATCH 11/12] reporting: Show slightly better info for broken jobs

[Ian Campbell]

On Tue, 2015-06-16 at 11:13 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 11/12] reporting: Show slightly better info for broken jobs"):
> > On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ...
> > > -    } elsif ($status eq 'fail') {
> > > +    } elsif ($status eq 'fail' or $status eq 'broken') {
> > > +	my $failcolour = $status eq 'fail' ? $red : $yellow;
> > >  	our $failstepq //= db_prepare(<<END);
> > >  	    SELECT * FROM steps
> > >  	     WHERE flight=? AND job=?
> > > @@ -217,10 +218,12 @@ END
> > >          if (!defined $fs) {
> > >              return { Summary => "(unknown)", Colour => $yellow };
> > >          } elsif ($fs->{status} eq 'fail') {
> > > -            return { Summary => "$fs->{testid}", Colour => $red };
> > > +            return { Summary => "$fs->{testid}", Colour => $failcolour };
> > > +        } elsif ($fs->{status} eq 'broken') {
> > > +            return { Summary => "$fs->{testid} broken", Colour => $yellow };
> > 
> > Not $failcolour?
> 
> Using $failcolour instead of $yellow there would only make a
> difference if the job was `fail' but the step was `broken'.  So this
> is a bit of an edge case.
> 
> But I think in that situation $red is probably better, so I will
> change this.

OK. You can put my Ack on it if you like.

Ian.

Re: [PATCH 00/12] Reporting and debugging improvements

[Ian Campbell]

On Tue, 2015-06-16 at 11:18 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 00/12] Reporting and debugging improvements"):
> > On Mon, 2015-06-15 at 18:13 +0100, Ian Jackson wrote:
> > > I have been working on a new script called sg-report-host-history.
> > > This is the result so far.
> > > 
> > > The series so far consists of mostly useful improvements, and one
> > > bugfix to the existing reporting arrangements.
> > 
> > Unrelated, but while looking at
> > http://logs.test-lab.xenproject.org/osstest/results/?C=M;O=A which is
> > quite long I've often wished that the paths had a few more / instead
> > of . in them so it was a bit easier to find the specific things I was
> > after...
> 
> This is a good point.  Would you like me to take that as a todo item
> for this series ?

If you have the tuits, please.

> If we feel like changing this then I think there is no serious
> compatibility problem: old scripts will write and refer to old paths
> and new scripts to new ones.  Eventually all will be new.

I figured eventually we'd need to gc the old ones, or they will clutter
up the directory.