[OSSTEST PATCH] ts-hosts-allocate-Executive: Do not allocate specific host with wrong blessing

[OSSTEST PATCH] ts-hosts-allocate-Executive: Do not allocate specific host with wrong blessing

[Ian Jackson]

If the job contains a runvar specifying a specific host, still check
that host's blessing.  Otherwise bisections can run on unblessed
hosts.  They should fail instead.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index dd9fcf2..88da504 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -279,26 +279,26 @@ sub hid_find_possibilities ($) {
 
     my $use= $hid->{OverrideUse} || $r{ $hid->{Ident} };
 
-    my $findhostsq;
-    if (defined $use) {
-        print DEBUG "HID $hid->{Ident} USE $use\n";
-	$findhostsq= $dbh_tests->prepare(<<END);
-	    SELECT *
-	      FROM resources
-	     WHERE restype=? AND resname=? AND shareix=0
-END
-        $findhostsq->execute('host',$use);
-    } else {
-        print DEBUG "HID $hid->{Ident} INTENDED $fi->{intended}\n";
-	$findhostsq= $dbh_tests->prepare(<<END);
+    print DEBUG "HID $hid->{Ident} INTENDED $fi->{intended}\n";
+    my $findhostsq = <<END;
 	    SELECT *
 	      FROM resources JOIN hostflags
 		ON (restype='host' AND shareix=0 AND
 		    resname=hostname AND hostflag=?)
 END
-        $findhostsq->execute("blessed-$fi->{intended}");
+    my @findhostsqvars = ("blessed-$fi->{intended}");
+
+    if (defined $use) {
+        print DEBUG "HID $hid->{Ident} USE $use\n";
+	$findhostsq .= <<END;
+	     WHERE restype='host' AND resname=? AND shareix=0
+END
+        push @findhostsqvars, $use;
     }
 
+    $findhostsq= $dbh_tests->prepare($findhostsq);
+    $findhostsq->execute(@findhostsqvars);
+
     my $equivstatusq= $dbh_tests->prepare(<<END);
             SELECT flight, job, val, status
 	      FROM flights f
@@ -423,7 +423,7 @@ END
 
     if (!@candidates) {
         if (defined $use) {
-            logm("specified host $use for $hid->{Ident} nonexistent?");
+ logm("specified host $use for $hid->{Ident} nonexistent or unblessed?");
         } else {
             logm("no suitable host for $hid->{Ident} (out of $any)");
         }
-- 
1.7.10.4

Re: [OSSTEST PATCH] ts-hosts-allocate-Executive: Do not allocate specific host with wrong blessing

[Ian Campbell]

On Tue, 2015-09-29 at 15:14 +0100, Ian Jackson wrote:
> If the job contains a runvar specifying a specific host, still check
> that host's blessing.  Otherwise bisections can run on unblessed
> hosts.  They should fail instead.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>
>      if (!@candidates) {
>          if (defined $use) {
> -            logm("specified host $use for $hid->{Ident} nonexistent?");
> + logm("specified host $use for $hid->{Ident} nonexistent or unblessed?");

Strange indent due to the length of the message, rather than by mistake, I 
suppose?

Ian.

Re: [OSSTEST PATCH] ts-hosts-allocate-Executive: Do not allocate specific host with wrong blessing

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH] ts-hosts-allocate-Executive: Do not allocate specific host with wrong blessing"):
> On Tue, 2015-09-29 at 15:14 +0100, Ian Jackson wrote:
> > If the job contains a runvar specifying a specific host, still check
> > that host's blessing.  Otherwise bisections can run on unblessed
> > hosts.  They should fail instead.
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >      if (!@candidates) {
> >          if (defined $use) {
> > -            logm("specified host $use for $hid->{Ident} nonexistent?");
> > + logm("specified host $use for $hid->{Ident} nonexistent or unblessed?");
> 
> Strange indent due to the length of the message, rather than by mistake, I 
> suppose?

Yes.  It seemed better than the other options.

Thanks,
Ian.

[OSSTEST PATCH 2/1] ts-hosts-allocate-Executive: Finish a couple of transactions

[Ian Jackson]

Call $equivflagscheckq->finish() at the end, rather than in the
candidate search loop.  This avoids missing a `next' control path.

Call $resprop_q->finish() at all.

One of these is responsible for this message:

  DBI::db=HASH(0x88e79c4)->disconnect invalidates 1 active statement
  handle (either destroy statement handles or call finish on them
  before disconnecting) at Osstest/Executive.pm line 708, <GEN4> line
  53.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ts-hosts-allocate-Executive |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 88da504..13a0652 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -366,7 +366,6 @@ END
 		push @{ $candrow->{Warnings} },
 		    "host has multiple equivalence class flags";
 	    }
-	    $equivflagscheckq->finish();
 	}
 
         print DEBUG "$dbg FLAGS MISSINGFLAGS: @missingflags.\n";
@@ -419,7 +418,9 @@ END
 	print DEBUG "$dbg CANDIDATE.\n";
     }
     $findhostsq->finish();
+    $equivflagscheckq->finish();
     $equivstatusq->finish();
+    $resprop_q->finish();
 
     if (!@candidates) {
         if (defined $use) {
-- 
1.7.10.4

[OSSTEST PATCH 3/1] ts-hosts-allocate-Executive: Print more info about booking to main log

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index aeb8c25..3475862 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -743,9 +743,19 @@ sub alloc_resources {
 	    }]);
 
 	    if ($bookinglist && $ok!=-1) {
+		my %prstart;
+		foreach my $book (@{ $bookinglist->{Bookings} }) {
+		    my $pr = $book->{Reso};
+		    $pr .= " [$book->{Xinfo}]" if defined $book->{Xinfo};
+		    push @{ $prstart{ $book->{Start }} }, $pr;
+		}
+		my $pr = '';
+		foreach my $start (sort { $a <=> $b } keys %prstart) {
+		    $pr .= " \@$start @{ $prstart{$start} }";
+		}
 		my $jbookings= to_json($bookinglist);
                 chomp($jbookings);
-                logm("resource allocation: booking.");
+                logm("resource allocation: booking$pr.");
 		$debugm->("bookings = ", $jbookings);
 
 		printf $qserv "book-resources %d\n", length $jbookings
-- 
1.7.10.4

Re: [OSSTEST PATCH 3/1] ts-hosts-allocate-Executive: Print more info about booking to main log

[Ian Campbell]

On Tue, 2015-09-29 at 16:27 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  Osstest/Executive.pm |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
> index aeb8c25..3475862 100644
> --- a/Osstest/Executive.pm
> +++ b/Osstest/Executive.pm
> @@ -743,9 +743,19 @@ sub alloc_resources {
>  	    }]);
>  
>  	    if ($bookinglist && $ok!=-1) {
> +		my %prstart;
> +		foreach my $book (@{ $bookinglist->{Bookings} }) {
> +		    my $pr = $book->{Reso};
> +		    $pr .= " [$book->{Xinfo}]" if defined $book
> ->{Xinfo};
> +		    push @{ $prstart{ $book->{Start }} }, $pr;

                                                   ^ stray space?

> +		}
> +		my $pr = '';
> +		foreach my $start (sort { $a <=> $b } keys %prstart) {
> +		    $pr .= " \@$start @{ $prstart{$start} }";

Even with one reso and its Xinfo this is going to be quite a long line,
isn't it?

I don't think it really matters in the context, so in any case

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

> +		}
>  		my $jbookings= to_json($bookinglist);
>                  chomp($jbookings);
> -                logm("resource allocation: booking.");
> +                logm("resource allocation: booking$pr.");
>  		$debugm->("bookings = ", $jbookings);
>  
>  		printf $qserv "book-resources %d\n", length $jbookings

Re: [OSSTEST PATCH 3/1] ts-hosts-allocate-Executive: Print more info about booking to main log

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 3/1] ts-hosts-allocate-Executive: Print more info about booking to main log"):
> On Tue, 2015-09-29 at 16:27 +0100, Ian Jackson wrote:
> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > +		    push @{ $prstart{ $book->{Start }} }, $pr;
> 
>                                                    ^ stray space?

Space in wrong place.  Should have read:

		    push @{ $prstart{ $book->{Start} } }, $pr;

(That's cosmetic only.)

> > +		my $pr = '';
> > +		foreach my $start (sort { $a <=> $b } keys %prstart) {
> > +		    $pr .= " \@$start @{ $prstart{$start} }";
> 
> Even with one reso and its Xinfo this is going to be quite a long line,
> isn't it?
> 
> I don't think it really matters in the context, so in any case

Output looks like this:

2015-09-29 14:59:53 Z host allocation: planned start in 829 seconds.
2015-09-29 14:59:53 Z resource allocation: booking @829 host scape-moth [host].
2015-09-29 14:59:54 Z resource allocation: we are in the plan.

I see that in this case it _just_ fits into 80 columns :-).  But the
host allocation log is generally quite wide.  I think if this one line
wraps it's probably worthwhile.

Ian.

Re: [OSSTEST PATCH 2/1] ts-hosts-allocate-Executive: Finish a couple of transactions

[Ian Campbell]

On Tue, 2015-09-29 at 16:27 +0100, Ian Jackson wrote:
> Call $equivflagscheckq->finish() at the end, rather than in the
> candidate search loop.  This avoids missing a `next' control path.
> 
> Call $resprop_q->finish() at all.
> 
> One of these is responsible for this message:
> 
>   DBI::db=HASH(0x88e79c4)->disconnect invalidates 1 active statement
>   handle (either destroy statement handles or call finish on them
>   before disconnecting) at Osstest/Executive.pm line 708, <GEN4> line
>   53.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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