[PATCH OSSTEST v2 0/5] add a flight summary html report

[PATCH OSSTEST v2 0/5] add a flight summary html report

[Ian Campbell]

This series tries to add a simply flight summary. An example of the
output can be found at:
http://xenbits.xen.org/people/ianc/tmp/fsummary-v2.html

The actually HTML generation could likely do with more work, however I'm
sending this now to get feedback on the ms-planner and
ts-hosts-allocate-Executive changes in particular, since further
development would be easier if they were in use in real life. I've
tested them on the Cambridge instance.

Ian.

[PATCH OSSTEST v2 1/5] ms-planner: Propagate a booking's Job to the plan

[Ian Campbell]

This needs to be done in several places:

- When booking resources (cmd: book-resources), to initially propagate
  from the booking (e.g. from ts-hosts-allocate-Executive's input).
- On reset (cmd: reset) so that the Events corresponding to actual
  allocations retain there Job.
- When retrieving the plan (cmd: get-plan), so it would be available
  for logging etc (I don't think this is currently used)

This patch has been deployed on the Cambridge instance for testing
with no ill-effects.

cmd_reset does not include a ->Job for jobs which are "(preparing)",
corresponding to a job which is going to use a shared host which is
currently being installed by another job. I was unable to figure out a
way to include these.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 README.planner |  2 ++
 ms-planner     | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/README.planner b/README.planner
index ec4dce8..73e97ea 100644
--- a/README.planner
+++ b/README.planner
@@ -213,6 +213,7 @@ Plan is:
 			End	abs planned end time, *
 				 omitted for rogue tasks
 			Live	boolean, for reporting purposes only
+			Job	flight.job which made the allocation
 
 	Events
 		map from
@@ -231,6 +232,7 @@ Plan is:
 						Shareix
 						Task
 						Live
+			Job	flight.job which made the allocation
 
 	* = internal to plan
 	** = computed by launder_check_plan
diff --git a/ms-planner b/ms-planner
index 1dd01a9..02a5464 100755
--- a/ms-planner
+++ b/ms-planner
@@ -350,16 +350,19 @@ END
                      Shareix => $shareix,
                      Live => $alloc->{Live},
                     );
-        
+
+	my $job = $alloc->{Job};
 	push @{ $plan->{Events}{$reso} }, {
 	    Time => $alloc->{Start},
 	    Info => $alloc->{Info},
+	    ($job ? (Job => $job) : ()),
 	    Type => Start,
 	    ($share ? (Share => { %$share }) : ()),
 	    Allocated => { @ealloc },
 	}, {
 	    Time => $end_exptd,
 	    Info => $alloc->{Info},
+	    ($job ? (Job => $job) : ()),
 	    Type => End,
 	    ($share ? (Share => { %$share }) : ()),
 	    Allocated => { @ealloc },
@@ -385,10 +388,12 @@ sub cmd_get_plan () {
 	my @jevts;
 	foreach my $evt (@$evts) {
 	    my $share= $evt->{Share};
+	    my $job= $evt->{Job};
 	    push @jevts, {
 		Time => $evt->{Time} - $plan->{Start},
 		Type => $evt->{Type},
 		Avail => $evt->{Avail},
+		($job ? (Job => $job) : ()),
 		($share ? (Share => $share) : ()),
 	    };
 	}
@@ -450,6 +455,7 @@ sub cmd_book_resources () {
 	my $starttime= parse_reltime2abs($jbook->{Start});
 	my $endtime= parse_reltime2abs($jbook->{End});
 	my $reso= parse_reso($jbook->{Reso});
+	my $job= $jbook->{Job};
 
 	die unless $starttime < $endtime;
 
@@ -483,6 +489,7 @@ sub cmd_book_resources () {
                 Start => $starttime,
                 End => $endtime,
                 Live => 1,
+                ($job ? (Job => $job) : ()),
             };
 	}
 
@@ -491,6 +498,7 @@ sub cmd_book_resources () {
 		Info => $info,
 		(@share ? (Share => { @share }) : ()),
 		(@alloc ? (Allocated => { @alloc }) : ()),
+		($job ? (Job => $job) : ()),
 		@_,
 	    };
 	};
-- 
2.1.4

[PATCH OSSTEST v2 2/5] ms-planner: Expose the plan start in json.

[Ian Campbell]

Without this the Start and End times cannot be interpreted.

This patch has been deployed on the Cambridge instance for testing
with no ill-effects.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 ms-planner | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ms-planner b/ms-planner
index 02a5464..f38f05b 100755
--- a/ms-planner
+++ b/ms-planner
@@ -382,7 +382,7 @@ sub cmd_get_plan () {
     get_current_plan();
     launder_check_plan();
 
-    my $jplan= { Events=>{} };
+    my $jplan= { Events=>{}, Start => $plan->{Start} };
 
     while (my ($reso,$evts) = each %{ $plan->{Events} }) {
 	my @jevts;
-- 
2.1.4

[PATCH OSSTEST v2 3/5] ts-hosts-allocate-Executive: Add the requesting Job to the booking

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 ts-hosts-allocate-Executive | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 7a9411d..6583191 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -669,6 +669,7 @@ sub compute_booking_list () {
 		Shares => $sel->{SharedMaxTasks},
 	    }) : ()),
 	};
+	$book->{Job}= "$flight.$job";
 	$book->{Reso}= "$sel->{restype} $sel->{resname}";
 	$book->{Xinfo}= $hid->{Ident};
 	$book->{Start}= $best->{Start},
-- 
2.1.4

[PATCH OSSTEST v2 4/5] Osstest::Executive: Export get_plan

[Ian Campbell]

To retrieve the current plan from the queue daemon.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 Osstest/Executive.pm | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index ecd38c3..f29f778 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -48,7 +48,7 @@ BEGIN {
                       findtask @all_lock_tables
                       report_run_getinfo report_altcolour
                       report_blessingscond report_find_push_age_info
-                      tcpconnect_queuedaemon plan_search
+                      tcpconnect_queuedaemon get_plan plan_search
                       alloc_resources alloc_resources_rollback_begin_work
                       resource_check_allocated resource_shared_mark_ready
                       duration_estimator
@@ -477,6 +477,18 @@ sub tcpconnect_queuedaemon () {
     return $qserv;
 }
 
+sub get_plan ($) {
+    my ($qserv) = @_;
+
+    print $qserv "get-plan\n" or die $!;
+    $_= <$qserv>; defined && m/^OK get-plan (\d+)\s/ or die "$_ ?";
+
+    my $jplanlen= $1;
+    my $jplan;
+    read($qserv, $jplan, $jplanlen) == $jplanlen or die $!;
+    return $jplan;
+}
+
 sub plan_search ($$$$) {
     my ($plan, $dbgprint, $duration, $requestlist) = @_;
     #
@@ -674,12 +686,7 @@ sub alloc_resources {
 
 	    db_retry($flight,'running', $dbh_tests, \@all_lock_tables,
 		     [ sub {
-		print $qserv "get-plan\n" or die $!;
-		$_= <$qserv>; defined && m/^OK get-plan (\d+)\s/ or die "$_ ?";
-
-		my $jplanlen= $1;
-		my $jplan;
-		read($qserv, $jplan, $jplanlen) == $jplanlen or die $!;
+		my $jplan = get_plan($qserv);
 		my $jplanprint= $jplan;
 		chomp $jplanprint;
 		logm("resource allocation: obtained base plan.");
-- 
2.1.4

[PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights

[Ian Campbell]

From: Ian Campbell <ijc@hellion.org.uk>

Jobs are categorised by a new ->Job field. This is added by
ts-hosts-alllocate-Executive and propagated byu the planner after
recent patches. It contains $flight.$job.

Jobs which do not include this are anonymous and are listed
separately.

Jobs which the plan describes as "(preparing)" (which I think means they
are waiting for another job to regroove build host for sharing) are
currently classified as anonymous until they are done preparing, at
which point they become correctly classified again. I can't see how
to figure out the correct ->{Job} at this point in the planner.

TODO: Hook up to cron (via for-maintjobs.git) or otherwise arrange for
this to be run when the plan is updated.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
Example output:
http://xenbits.xen.org/people/ianc/tmp/fsummary-v2.html

This could surely use better Perl and produce better output, however
I'm sending it now because it would be useful for further development
if some or all of the preceding patches could go into production and
this serves as an example of why I think I want them.

v2:
  - Get the plan from the queue daemon.
  - Do not parse ->Info, instead expect a new ->Job field
  - Handle multiple resources for a job.
---
 ms-flights-summary | 315 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ms-planner         |   1 +
 2 files changed, 316 insertions(+)
 create mode 100755 ms-flights-summary

diff --git a/ms-flights-summary b/ms-flights-summary
new file mode 100755
index 0000000..76eb4b1
--- /dev/null
+++ b/ms-flights-summary
@@ -0,0 +1,315 @@
+#!/usr/bin/perl
+
+use strict qw(vars refs);
+
+use Osstest;
+use Osstest::Executive;
+
+use JSON;
+use POSIX;
+use Data::Dumper;
+use HTML::Entities;
+
+# $flights{NNNN} = {
+#     Nr => NNNN,
+#     Stats => {
+#         Pass => <NRPASS>,
+#         Fail => <NRFAIL>,
+#         ...
+#     },
+#     Started => ...,
+#     Blessing => ...,
+#     Branch => ...,
+#     Intended => ...,
+#     ExpectedEnd => ...,
+#     Jobs => {
+#         JNAME1 => {
+#             Reso => {
+#                 RNAME1 => {
+#                     Start => ...,
+#                     End => ...,
+#                 },
+#                 RNAME2 => { ... },
+#             },
+#             OverallTime => { Start => ..., End => ... },
+#             Receipe => ...,
+#             Status => pass|fail|...,
+#         },
+#         JNAME2 => { ... },
+#     },
+# };
+our %flights;
+
+# As for $flights{NNN}{Jobs} but each Job only has the Reso and
+# OverallTime keys
+our %anon_jobs;
+
+our $plan;
+
+sub get_current_plan () {
+    my $qserv = tcpconnect_queuedaemon();
+    my $jplan = get_plan($qserv);
+    $plan= from_json($jplan);
+    #print STDERR Dumper($plan);
+}
+
+# Find all the flights referenced by an Event and insert into %flights.
+sub enumerate_flights() {
+    while (my ($reso,$evts) = each %{ $plan->{Events} }) {
+	foreach my $evt ( @{$evts} ) {
+	    next unless $evt->{Type} =~ m/^(Start|End)$/;
+	    next unless $evt->{Job};
+	    $evt->{Job} =~ m/^([0-9]+)\.(.*)/ or die;
+
+	    my $f = $1;
+
+	    next if $flights{$f};
+
+	    my $flightinfo= $dbh_tests->selectrow_hashref(<<END);
+	    SELECT started,blessing,branch,intended FROM flights
+		WHERE flight=$f
+END
+            $flights{$f}= { Nr => $f,
+			    Stats => {},
+	                    Jobs => {} };
+            foreach my $fld (qw(started blessing branch intended)) {
+                $flights{$f}->{ucfirst($fld)}= $flightinfo->{$fld};
+	    };
+	    $flights{$f}->{ExpectedEnd} = 0;
+	}
+    }
+}
+
+# Enumerate all jobs of every known flight and populate the
+# corresponding ->{Jobs}.
+sub enumerate_jobs($) {
+    my ($f) = @_;
+
+    $f->{Jobs} = {};
+
+    my $jobs= $dbh_tests->selectall_arrayref(<<END, { Slice => {} });
+        SELECT job,recipe,status FROM jobs
+            WHERE flight=$f->{Nr}
+END
+
+    for my $row (@{$jobs}) {
+	$f->{Jobs}{$row->{job}} =
+	{
+	    Status => $row->{status},
+	    Recipe => $row->{recipe},
+	    Reso => {},
+	};
+    }
+}
+
+# Gather statistics around number of pass/fail/etc jobs in each
+# flight.
+sub gather_stats($) {
+    my ($f) = @_;
+
+    my $stats= $dbh_tests->selectall_arrayref(<<END);
+        SELECT status,COUNT(*) FROM jobs
+	    WHERE flight=$f->{Nr}
+            GROUP BY status
+END
+    for my $row (@{$stats}) {
+	my ($stat,$count) = @$row;
+	$f->{Stats}{ucfirst($stat)} = $count;
+    }
+}
+
+sub add_event($$$$) {
+    my ($job,$reso,$type,$time) = @_;
+
+    die unless $type =~ m/^(Start|End)/;
+
+    $job->{OverallTime} //= {};
+
+    $job->{Reso}{$reso} //= {};
+    $job->{Reso}{$reso}{$type} = $time;
+
+    my $cmp = $type eq "Start" ?
+	sub { $_[0] < $_[1] ? $_[0] : $_[1] } :
+	sub { $_[0] > $_[1] ? $_[0] : $_[1] };
+
+    $job->{OverallTime}{$type} //= $time;
+
+    $job->{OverallTime}{$type} =
+	$cmp->($time, $job->{OverallTime}{$type});
+}
+
+# Walk all events in the plan and update the corresponding flight/job
+# with the timespan. Events relating to unknown jobs are added to
+# %anon_jobs.
+sub gather_events() {
+    while (my ($reso,$evts) = each %{ $plan->{Events} }) {
+	foreach my $evt ( @{$evts} ) {
+	    my ($f,$job);
+	    next unless $evt->{Type} =~ m/^(Start|End)$/;
+	    if ( $evt->{Job} ) {
+		my $j;
+		$evt->{Job} =~ m/^([0-9]+)\.(.*)/ or die;
+		($fnum,$j) = ($1,$2);
+		goto anon unless $flights{$fnum};
+		$f = $flights{$fnum};
+		goto anon unless $f->{Jobs}{$j};
+		$job = $f->{Jobs}{$j};
+	    } else {
+ anon:
+		# The JSON version of the plan does not include
+		# $evt->{Info}, the best we have got is the resource
+		# itself.
+		$anon_jobs{$reso} //= { Reso => {} };
+		$job = $anon_jobs{$reso}
+	    }
+
+	    my $time = $plan->{Start} + $evt->{Time};
+
+	    add_event($job, $reso, $evt->{Type}, $time);
+
+	    if ($f && $evt->{Type} eq "End") {
+		$f->{ExpectedEnd} =
+		    ($evt->{Time} > $f->{ExpectedEnd}) ?
+		    $evt->{Time} : $f->{ExpectedEnd}
+	    }
+	}
+    }
+}
+
+###########
+
+csreadconfig();
+
+get_current_plan();
+
+enumerate_flights();
+
+foreach my $f (keys %flights) {
+    enumerate_jobs($flights{$f});
+    gather_stats($flights{$f});
+}
+
+gather_events();
+
+############
+
+my @cols = ("Job",
+	    #"Recipe",
+	    "Status", "Resource", "Scheduled");#, "Start", "End", "Host");
+
+sub fmttime($)
+{
+    my ($t) = @_;
+    return strftime("%Y-%b-%d %a %H:%M:%S", gmtime $t);
+    #return strftime("%+", gmtime $t);
+}
+
+sub cols_hdr() {
+    printf("<tr>\n");
+    printf(" <th align='left'>%s</th>\n", encode_entities($_)) foreach @cols;
+    printf("</tr>\n");
+}
+sub flight_hdr($) {
+    my $text = encode_entities(shift);
+    printf("<tr><td colspan=%d><b>$text</b></td></tr>\n", scalar @cols);
+}
+sub job_cell($) {
+    my $text = encode_entities(shift);
+    printf(" <td><b>$text</b></td>\n");
+}
+sub rawcell($) {
+    my $text = shift;
+    printf(" <td>$text</td>\n");
+}
+sub cell($) {
+    rawcell(encode_entities(shift));
+}
+
+sub fmt_timespan($) {
+    my ($s) = @_;
+
+    if ( $s->{Start} && $s->{End} ) {
+	return join(" - ", map { fmttime($s->{$_}) } qw(Start End));
+    } else {
+	return "";
+    }
+}
+
+sub row($$$;$$) {
+    my ($omitjob,$job,$status, $reso,$tspan) = @_;
+
+    print "<tr>\n";
+
+    if ($omitjob) {
+	print "<td></td>" foreach (1..2);
+    } else {
+	job_cell($job);
+	cell($status // "???");
+    }
+    rawcell($reso) if $reso;
+    cell(fmt_timespan($tspan)) if $tspan;
+
+    print "</tr>\n";
+}
+
+sub do_one_job($$) {
+    my ($job,$info) = @_;
+
+    my $nrreso = scalar keys %{ $info->{Reso} };
+
+#    my $prinfo = sub {
+#	job_cell($job);
+#	cell($info->{Status} // "???");
+#    };
+
+#    my $row = sub {
+#	print "<tr>\n";
+#	$prinfo->();
+#	$prinfo = sub { print "<td></td>" foreach (1..2) };
+#	rawcell($_[0].$_[1]) if $_[1];
+#	cell(fmt_timespan($_[2])) if $_[2];
+#	print "</tr>\n";
+#    };
+
+    if ($nrreso == 0) {
+	row(0,$job,$info->{Status});
+    } else {
+	my $pfx = '';
+	my $omitjob = 0;
+	if ($nrreso > 1) {
+	    row(0, $job, $info->{Status}, "Overall", $info->{OverallTime});
+	    $omitjob = 1;
+	    $pfx = "&mdash;";
+	}
+	while (my ($reso,$rinf) = each %{ $info->{Reso} }) {
+	    row($omitjob, $job, $info->{Status}, $pfx.encode_entities($reso), $rinf)
+	}
+    }
+}
+
+printf("<p>Based on plan at ".fmttime($plan->{Start})."</p>\n");
+
+printf("<table>\n");
+
+foreach my $f (sort keys %flights) {
+    my $fi = $flights{$f};
+    flight_hdr("Flight: $f [$fi->{Branch} $fi->{Intended}]");
+    flight_hdr("Started: ".fmttime($fi->{Started}));
+    flight_hdr("Expected end: ".fmttime($fi->{ExpectedEnd}));
+    flight_hdr(
+	join("; ", map { "$_: $fi->{Stats}{$_}" } (keys %{$fi->{Stats}}))
+	);
+    cols_hdr();
+    do_one_job($_, $fi->{Jobs}{$_})
+	foreach sort { $a cmp $b } keys %{$fi->{Jobs}};
+    print("<tr><td>&nbsp</td></tr>\n");
+}
+print "\n";
+
+flight_hdr("Anonymous Jobs");
+cols_hdr();
+foreach my $j (sort keys %anon_jobs) {
+    do_one_job($j, $anon_jobs{$j});
+}
+
+print "</table>\n";
diff --git a/ms-planner b/ms-planner
index f38f05b..35d430b 100755
--- a/ms-planner
+++ b/ms-planner
@@ -289,6 +289,7 @@ END
 	    $info= "rogue task $arow->{subtask}";
 	}
 	$plan->{Allocations}{$reskey}= {
+	    # Can we find a Job here?
             Task => $arow->{owntaskid},
 	    Info => $info,
 	    Start => $plan->{Start},
-- 
2.1.4

Re: [PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights

[Ian Campbell]

On Tue, 2015-07-14 at 14:58 +0100, Ian Campbell wrote:
> This could surely use better Perl and produce better output, however
> I'm sending it now because it would be useful for further development
> if some or all of the preceding patches could go into production and
> this serves as an example of why I think I want them.

This was even more WIP than I meant to send out, sorry.

Here is a v3 of just this one patch which is a bit less so...

----8<-----

>From b72688c114cfd0ba55b547e0634965d4c744dfc9 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 14 Jul 2015 15:32:54 +0100
Subject: [PATCH] ms-flights-summary: Produce an HTML report of all active
 flights

Jobs are categorised by a new ->Job field. This is added by
ts-hosts-alllocate-Executive and propagated byu the planner after
recent patches. It contains $flight.$job.

Jobs which do not include this are anonymous and are listed
separately.

Jobs which the plan describes as "(preparing)" (which I think means they
are waiting for another job to regroove build host for sharing) are
currently classified as anonymous until they are done preparing, at
which point they become correctly classified again. I can't see how
to figure out the correct ->{Job} at this point in the planner.

TODO: Hook up to cron (via for-maintjobs.git) or otherwise arrange for
this to be run when the plan is updated.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
Example output:
http://xenbits.xen.org/people/ianc/tmp/fsummary-v3.html

This could surely use better Perl and produce better output, however
I'm sending it now because it would be useful for further development
if some or all of the preceding patches could go into production and
this serves as an example of why I think I want them.

v3:
  - Author/S-o-b using correct hat.
  - Much improved output, somewhat improved code (v2 was a bit more
    WIP even than I had intended to send out).
  - perl -w
v2:
  - Get the plan from the queue daemon.
  - Do not parse ->Info, instead expect a new ->Job field
  - Handle multiple resources for a job.

nicer output

wip
---
 ms-flights-summary | 309 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ms-planner         |   1 +
 2 files changed, 310 insertions(+)
 create mode 100755 ms-flights-summary

diff --git a/ms-flights-summary b/ms-flights-summary
new file mode 100755
index 0000000..651d13d
--- /dev/null
+++ b/ms-flights-summary
@@ -0,0 +1,309 @@
+#!/usr/bin/perl -w
+
+use strict qw(vars refs);
+
+use Osstest;
+use Osstest::Executive qw(:DEFAULT :colours);
+
+use JSON;
+use POSIX;
+use Data::Dumper;
+use HTML::Entities;
+
+# $flights{NNNN} = {
+#     Nr => NNNN,
+#     Stats => {
+#         Pass => <NRPASS>,
+#         Fail => <NRFAIL>,
+#         ...
+#     },
+#     Started => ...,
+#     Blessing => ...,
+#     Branch => ...,
+#     Intended => ...,
+#     ExpectedEnd => ...,
+#     Jobs => {
+#         JNAME1 => {
+#             Reso => {
+#                 RNAME1 => {
+#                     Start => ...,
+#                     End => ...,
+#                 },
+#                 RNAME2 => { ... },
+#             },
+#             OverallTime => { Start => ..., End => ... },
+#             Receipe => ...,
+#             Status => pass|fail|...,
+#         },
+#         JNAME2 => { ... },
+#     },
+# };
+our %flights;
+
+# As for $flights{NNN}{Jobs} but each Job only has the Reso and
+# OverallTime keys
+our %anon_jobs;
+
+our $plan;
+
+sub get_current_plan () {
+    my $qserv = tcpconnect_queuedaemon();
+    my $jplan = get_plan($qserv);
+    $plan= from_json($jplan);
+    #print STDERR Dumper($plan);
+}
+
+# Find all the flights referenced by an Event and insert into %flights.
+sub enumerate_flights() {
+    while (my ($reso,$evts) = each %{ $plan->{Events} }) {
+	foreach my $evt ( @{$evts} ) {
+	    next unless $evt->{Type} =~ m/^(Start|End)$/;
+	    next unless $evt->{Job};
+	    $evt->{Job} =~ m/^([0-9]+)\.(.*)/ or die;
+
+	    my $f = $1;
+
+	    next if $flights{$f};
+
+	    my $flightinfo= $dbh_tests->selectrow_hashref(<<END);
+	    SELECT started,blessing,branch,intended FROM flights
+		WHERE flight=$f
+END
+            $flights{$f}= { Nr => $f,
+			    Stats => {},
+	                    Jobs => {} };
+            foreach my $fld (qw(started blessing branch intended)) {
+                $flights{$f}->{ucfirst($fld)}= $flightinfo->{$fld};
+	    };
+	    $flights{$f}->{ExpectedEnd} = 0;
+	}
+    }
+}
+
+# Enumerate all jobs of every known flight and populate the
+# corresponding ->{Jobs}.
+sub enumerate_jobs($) {
+    my ($f) = @_;
+
+    $f->{Jobs} = {};
+
+    my $jobs= $dbh_tests->selectall_arrayref(<<END, { Slice => {} });
+        SELECT job,recipe,status FROM jobs
+            WHERE flight=$f->{Nr}
+END
+
+    for my $row (@{$jobs}) {
+	$f->{Jobs}{$row->{job}} =
+	{
+	    Status => $row->{status},
+	    Recipe => $row->{recipe},
+	    Reso => {},
+	};
+    }
+}
+
+# Gather statistics around number of pass/fail/etc jobs in each
+# flight.
+sub gather_stats($) {
+    my ($f) = @_;
+
+    my $stats= $dbh_tests->selectall_arrayref(<<END);
+        SELECT status,COUNT(*) FROM jobs
+	    WHERE flight=$f->{Nr}
+            GROUP BY status
+END
+    for my $row (@{$stats}) {
+	my ($stat,$count) = @$row;
+	$f->{Stats}{ucfirst($stat)} = $count;
+    }
+}
+
+sub add_event($$$$) {
+    my ($job,$reso,$type,$time) = @_;
+
+    die unless $type =~ m/^(Start|End)/;
+
+    $job->{OverallTime} //= {};
+
+    $job->{Reso}{$reso} //= {};
+    $job->{Reso}{$reso}{$type} = $time;
+
+    my $cmp = $type eq "Start" ?
+	sub { $_[0] < $_[1] ? $_[0] : $_[1] } :
+	sub { $_[0] > $_[1] ? $_[0] : $_[1] };
+
+    $job->{OverallTime}{$type} //= $time;
+
+    $job->{OverallTime}{$type} =
+	$cmp->($time, $job->{OverallTime}{$type});
+}
+
+# Walk all events in the plan and update the corresponding flight/job
+# with the timespan. Events relating to unknown jobs are added to
+# %anon_jobs.
+sub gather_events() {
+    while (my ($reso,$evts) = each %{ $plan->{Events} }) {
+	foreach my $evt ( @{$evts} ) {
+	    my ($f,$job);
+	    next unless $evt->{Type} =~ m/^(Start|End)$/;
+	    if ( $evt->{Job} ) {
+		my ($fnum,$j);
+		$evt->{Job} =~ m/^([0-9]+)\.(.*)/ or die;
+		($fnum,$j) = ($1,$2);
+		goto anon unless $flights{$fnum};
+		$f = $flights{$fnum};
+		goto anon unless $f->{Jobs}{$j};
+		$job = $f->{Jobs}{$j};
+	    } else {
+ anon:
+		# The JSON version of the plan does not include
+		# $evt->{Info}, the best we have got is the resource
+		# itself.
+		$anon_jobs{$reso} //= { Reso => {} };
+		$job = $anon_jobs{$reso}
+	    }
+
+	    my $time = $plan->{Start} + $evt->{Time};
+
+	    add_event($job, $reso, $evt->{Type}, $time);
+
+	    if ($f && $evt->{Type} eq "End") {
+		$f->{ExpectedEnd} =
+		    $time > $f->{ExpectedEnd} ?
+		    $time : $f->{ExpectedEnd};
+	    }
+	}
+    }
+}
+
+###########
+
+csreadconfig();
+
+get_current_plan();
+
+enumerate_flights();
+
+foreach my $f (keys %flights) {
+    enumerate_jobs($flights{$f});
+    gather_stats($flights{$f});
+}
+
+gather_events();
+
+############
+
+my @cols = ("Job", "Status", "Resource", "Scheduled");
+
+sub fmttime($)
+{
+    my ($t) = @_;
+    return "Unknown" if !$t;
+    return strftime("%Y-%b-%d %a %H:%M:%S", gmtime $t);
+}
+
+sub fmt_timespan($) {
+    my ($s) = @_;
+
+    return undef unless $s;
+
+    if ( $s->{Start} || $s->{End} ) {
+	return join(" &mdash; ",
+		    map { encode_entities(fmttime($s->{$_})) }
+		    qw(Start End));
+    } else {
+	return "???";
+    }
+}
+
+sub cols_hdr() {
+    printf("<tr bgcolor=#808080>\n");
+    printf(" <th align='left'>%s</th>\n", encode_entities($_)) foreach @cols;
+    printf("</tr>\n");
+}
+
+sub flight_hdr($) {
+    my $text = encode_entities(shift);
+    printf("    <tr><td colspan=%d><b>$text</b></td></tr>\n", scalar @cols);
+}
+
+sub cell($;$$) {
+    my ($text,$bgcolour,$tag) = @_;
+    $text //= '';
+    $bgcolour = $bgcolour ? "bgcolor=$bgcolour" : '';
+    $text = "<$tag>$text</$tag>" if $tag;
+    printf(" <td valign=top $bgcolour>$text</td>\n");
+}
+
+sub do_one_job($$$) {
+    my ($alt,$job,$info) = @_;
+    my $status = $info->{Status} // "???";
+
+    my %bgcolours = (
+	pass => $green,
+	fail => $red,
+    );
+    my $bgcolour = report_altcolour(${$alt});
+
+    my @resos = sort keys %{ $info->{Reso} };
+
+    my ($resos,$spans);
+    my $resopfx = '';
+    if (@resos > 1) {
+	$resos = "Overall<br>\n    ";
+	$resopfx = "&mdash; ";
+	$spans = fmt_timespan($info->{OverallTime})."<br>\n    ";
+    }
+
+    $resos .= join "<br>\n    ", map { "$resopfx$_" } @resos;
+    $spans .= join "<br>\n    ", map { fmt_timespan($info->{Reso}{$_}) } @resos;
+
+    print "<tr $bgcolour>\n";
+
+    my $tag = $status eq "running" ? "b" : undef;
+
+    cell(encode_entities($job), undef, $tag);
+    cell(encode_entities($status), $bgcolours{$status}, $tag);
+    cell($resos);
+    cell($spans);
+
+    print "</tr>\n";
+    ${$alt} ^= 1;
+}
+
+printf("<p>Based on plan at ".fmttime($plan->{Start})."</p>\n");
+
+printf("<table border='0' cellspacing='0' rules=all>\n");
+
+foreach my $f (sort keys %flights) {
+    my $fi = $flights{$f};
+    my $alt = 0;
+
+    printf ("<tr><td colspan=%d>\n  <table>\n", scalar @cols);
+    print ("    <tr><td>&nbsp;</td></tr>\n");
+    flight_hdr("Flight: $f [$fi->{Branch} $fi->{Intended}]");
+    flight_hdr("Started: ".fmttime($fi->{Started}));
+    flight_hdr("Expected end: ".fmttime($fi->{ExpectedEnd}));
+    flight_hdr(
+	join("; ", map { "$_: $fi->{Stats}{$_}" } (sort keys %{$fi->{Stats}}))
+	);
+    print ("  </table>\n</td></tr>\n");
+
+    cols_hdr();
+
+    do_one_job(\$alt,$_, $fi->{Jobs}{$_})
+	foreach sort { $a cmp $b } keys %{$fi->{Jobs}};
+}
+print "\n";
+
+printf ("<tr><td colspan=%d>\n  <table>\n", scalar @cols);
+print ("   <tr><td>&nbsp;</td></tr>\n");
+flight_hdr("Anonymous Jobs");
+print ("  </table>\n</td></tr>\n");
+cols_hdr();
+my $alt = 0;
+foreach my $j (sort keys %anon_jobs) {
+    do_one_job(\$alt,$j, $anon_jobs{$j});
+}
+
+print "</table>\n";
diff --git a/ms-planner b/ms-planner
index f38f05b..35d430b 100755
--- a/ms-planner
+++ b/ms-planner
@@ -289,6 +289,7 @@ END
 	    $info= "rogue task $arow->{subtask}";
 	}
 	$plan->{Allocations}{$reskey}= {
+	    # Can we find a Job here?
             Task => $arow->{owntaskid},
 	    Info => $info,
 	    Start => $plan->{Start},
-- 
2.1.4

Re: [PATCH OSSTEST v2 2/5] ms-planner: Expose the plan start in json.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 2/5] ms-planner: Expose the plan start in json."):
> Without this the Start and End times cannot be interpreted.
> 
> This patch has been deployed on the Cambridge instance for testing
> with no ill-effects.

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

Re: [PATCH OSSTEST v2 1/5] ms-planner: Propagate a booking's Job to the plan

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 1/5] ms-planner: Propagate a booking's Job to the plan"):
> This needs to be done in several places:
> 
> - When booking resources (cmd: book-resources), to initially propagate
>   from the booking (e.g. from ts-hosts-allocate-Executive's input).
> - On reset (cmd: reset) so that the Events corresponding to actual
>   allocations retain there Job.
                       ^^^^^
                       their

> - When retrieving the plan (cmd: get-plan), so it would be available
>   for logging etc (I don't think this is currently used)

get-plan is used by each job in turn to fetch the existing plan so
they can edit it.  So the parenthetical should be removed.

> This patch has been deployed on the Cambridge instance for testing
> with no ill-effects.

This doesn't currently actually supply a Job anywhere ?  Ah yes I see
that in 3/5, but it would be worth mentioning in the commit message.


Aside from these three observations about the commit message.

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

Ian.

Re: [PATCH OSSTEST v2 3/5] ts-hosts-allocate-Executive: Add the requesting Job to the booking

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 3/5] ts-hosts-allocate-Executive: Add the requesting Job to the booking"):
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

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

Re: [PATCH OSSTEST v2 4/5] Osstest::Executive: Export get_plan

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 4/5] Osstest::Executive: Export get_plan"):
> To retrieve the current plan from the queue daemon.

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

Re: [PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights"):
> This could surely use better Perl and produce better output, however
> I'm sending it now because it would be useful for further development
> if some or all of the preceding patches could go into production and
> this serves as an example of why I think I want them.

I think it's pretty good actually.  I have some minor stylistic
comments.  I haven't inspected the output, but as you say we can
improve it later.


> +	    my $flightinfo= $dbh_tests->selectrow_hashref(<<END);
> +	    SELECT started,blessing,branch,intended FROM flights
> +		WHERE flight=$f
> +END

If you indent this by another 4, it would be a bit easier to read.

> +            $flights{$f}= { Nr => $f,
> +			    Stats => {},
> +	                    Jobs => {} };

This layout is rather unidiomatic.

  +            $flights{$f}= {
  +                Nr => $f,
  +                Stats => {},
  +                Jobs => {},
  +            };

if it won't go on one line.  (And there's one later where you have the
{ on the next line indented as if it were a code block, which is
odder.)

> +	    if ( $evt->{Job} ) {
> +		my $j;
> +		$evt->{Job} =~ m/^([0-9]+)\.(.*)/ or die;
> +		($fnum,$j) = ($1,$2);
> +		goto anon unless $flights{$fnum};
> +		$f = $flights{$fnum};
> +		goto anon unless $f->{Jobs}{$j};
> +		$job = $f->{Jobs}{$j};
> +	    } else {
> + anon:

Surely you mean

  +		$f = $flights{$fnum};
  +		$job = $f->{Jobs}{$j};
  +	    }
  +	    if (!$job) {

?

> +my @cols = ("Job",
> +	    #"Recipe",
> +	    "Status", "Resource", "Scheduled");#, "Start", "End", "Host");

qw() ?

> +sub fmttime($)
> +{

{ should be on previous line, and space before (.

> +#    my $prinfo = sub {
> +#	job_cell($job);
> +#	cell($info->{Status} // "???");
> +#    };
> +
> +#    my $row = sub {
> +#	print "<tr>\n";
> +#	$prinfo->();
> +#	$prinfo = sub { print "<td></td>" foreach (1..2) };
> +#	rawcell($_[0].$_[1]) if $_[1];
> +#	cell(fmt_timespan($_[2])) if $_[2];
> +#	print "</tr>\n";
> +#    };

Are you deliberately leaving this here, commented out ?


> +	while (my ($reso,$rinf) = each %{ $info->{Reso} }) {
> +	    row($omitjob, $job, $info->{Status}, $pfx.encode_entities($reso), $rinf)

Line is rather long.


> diff --git a/ms-planner b/ms-planner
> index f38f05b..35d430b 100755
> --- a/ms-planner
> +++ b/ms-planner
> @@ -289,6 +289,7 @@ END
>  	    $info= "rogue task $arow->{subtask}";
>  	}
>  	$plan->{Allocations}{$reskey}= {
> +	    # Can we find a Job here?
>              Task => $arow->{owntaskid},
>  	    Info => $info,

I don't understand this comment.

Ian.

Re: [PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights

[Ian Campbell]

On Fri, 2015-07-31 at 16:50 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2 5/5] ms-flights-summary: Produce 
> an HTML report of all active flights"):
> > This could surely use better Perl and produce better output, however
> > I'm sending it now because it would be useful for further development
> > if some or all of the preceding patches could go into production and
> > this serves as an example of why I think I want them.
> 
> I think it's pretty good actually.  I have some minor stylistic
> comments.  I haven't inspected the output, but as you say we can
> improve it later.

Thanks, I actually posted a v3 shortly after which rewrote a lot of this.

I'll try and apply those of your comments which are relevant to the v3
though.

> > diff --git a/ms-planner b/ms-planner
> > index f38f05b..35d430b 100755
> > --- a/ms-planner
> > +++ b/ms-planner
> > @@ -289,6 +289,7 @@ END
> >  	    $info= "rogue task $arow->{subtask}";
> >  	}
> >  	$plan->{Allocations}{$reskey}= {
> > +	    # Can we find a Job here?
> >              Task => $arow->{owntaskid},
> >  	    Info => $info,
> 
> I don't understand this comment.

This didn't really belong in this patch. It was in relation to this
paragraph from patch #1's commit message:

    cmd_reset does not include a ->Job for jobs which are "(preparing)",
    corresponding to a job which is going to use a shared host which is
    currently being installed by another job. I was unable to figure out a
    way to include these.

The upshot is that N-1 of the N jobs which are going to share a host sit in
the "Anonymous Jobs" list until the first job has finished with the actual
installation of the host, instead of being associated with the proper job.

I wasn't able to figure out how to get the original Job to include in those
records in the allocations.

Ian.

Re: [PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST v2 5/5] ms-flights-summary: Produce an HTML report of all active flights"):
> On Fri, 2015-07-31 at 16:50 +0100, Ian Jackson wrote:
> > I think it's pretty good actually.  I have some minor stylistic
> > comments.  I haven't inspected the output, but as you say we can
> > improve it later.
> 
> Thanks, I actually posted a v3 shortly after which rewrote a lot of this.

Oh, sorry.

> I'll try and apply those of your comments which are relevant to the v3
> though.

Fair enough.

> > > +	    # Can we find a Job here?
> > >              Task => $arow->{owntaskid},
> > >  	    Info => $info,
> > 
> > I don't understand this comment.
> 
> This didn't really belong in this patch. It was in relation to this
> paragraph from patch #1's commit message:
> 
>     cmd_reset does not include a ->Job for jobs which are "(preparing)",
>     corresponding to a job which is going to use a shared host which is
>     currently being installed by another job. I was unable to figure out a
>     way to include these.
> 
> The upshot is that N-1 of the N jobs which are going to share a host sit in
> the "Anonymous Jobs" list until the first job has finished with the actual
> installation of the host, instead of being associated with the proper job.
> 
> I wasn't able to figure out how to get the original Job to include in those
> records in the allocations.

Yes.  I think this will do for now.

Ian.