[PATCH 1/4] testbot/Engine: Make sure not to schedule jobs on VMs undergoing maintenance!
After a series of revert failures the scheduler would put the VM in maintenance mode but still put it in the list of VMs that can be reverted to run a new task. Now such VMs are put in the busy list so no task is scheduled on them. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/WineTestBot/Engine/Scheduler.pm | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/testbot/lib/WineTestBot/Engine/Scheduler.pm b/testbot/lib/WineTestBot/Engine/Scheduler.pm index 3ce90895..1b42ea9a 100644 --- a/testbot/lib/WineTestBot/Engine/Scheduler.pm +++ b/testbot/lib/WineTestBot/Engine/Scheduler.pm @@ -274,15 +274,22 @@ sub _CheckAndClassifyVMs() { my $Errors = ($VM->Errors || 0) + 1; $VM->Errors($Errors); - $NewStatus = "maintenance" if ($Errors >= $MaxVMErrors); + if ($Errors >= $MaxVMErrors) + { + $NewStatus = "maintenance"; + $Sched->{busyvms}->{$VMKey} = 1; + } } $VM->Status($NewStatus); $VM->KillChild(); $VM->Save(); $VM->RecordResult($Sched->{records}, "boterror stuck process"); - $Sched->{lambvms}->{$VMKey} = 1; - $Host->{dirty}++; - $Host->{active}++; + if ($NewStatus eq "dirty") + { + $Sched->{lambvms}->{$VMKey} = 1; + $Host->{dirty}++; + $Host->{active}++; + } } elsif ($VM->Status =~ /^(?:dirty|running|reverting)$/) { -- 2.20.1
Include the reason why the VM was put offline in the notification email. Add NotifyAdministrator() to the Utils module to simplify sending the notification. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/LibvirtTool.pl | 25 +-------------- testbot/bin/WineRunBuild.pl | 6 +++- testbot/bin/WineRunReconfig.pl | 6 +++- testbot/bin/WineRunTask.pl | 6 +++- testbot/bin/WineRunWineTest.pl | 6 +++- testbot/lib/WineTestBot/Engine/Scheduler.pm | 5 +++ testbot/lib/WineTestBot/Utils.pm | 34 ++++++++++++++++++++- testbot/lib/WineTestBot/VMs.pm | 5 +++ 8 files changed, 64 insertions(+), 29 deletions(-) diff --git a/testbot/bin/LibvirtTool.pl b/testbot/bin/LibvirtTool.pl index 4000c147..e348834a 100755 --- a/testbot/bin/LibvirtTool.pl +++ b/testbot/bin/LibvirtTool.pl @@ -43,6 +43,7 @@ $Name0 =~ s+^.*/++; use WineTestBot::Config; use WineTestBot::Log; +use WineTestBot::Utils; use WineTestBot::VMs; my $Debug; @@ -58,30 +59,6 @@ sub Error(@) LogMsg @_; } -sub NotifyAdministrator($$) -{ - my ($Subject, $Body) = @_; - - if (open(my $fh, "|/usr/sbin/sendmail -oi -t -odq")) - { - LogMsg "Notifying administrator: $Subject\n"; - print $fh <<"EOF"; -From: $RobotEMail -To: $AdminEMail -Subject: $Subject - -$Body -EOF - close($fh); - } - else - { - LogMsg "Could not send administrator notification: $!\n"; - LogMsg " Subject: $Subject\n"; - LogMsg " Body: $Body\n"; - } -} - # # Setup and command line processing diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index fdcf770b..1d57c440 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -280,10 +280,14 @@ sub FatalTAError($$) my $IsPoweredOn = $VM->GetDomain()->IsPoweredOn(); if (!defined $IsPoweredOn) { - # The VM host is not accessible anymore so mark the VM as offline and + # The VM host is not accessible anymore so put the VM offline and # requeue the task. This does not count towards the task's tries limit # since neither the VM nor the task are at fault. Error("$ErrMessage\n"); + NotifyAdministrator("Putting the ". $VM->Name ." VM offline", + "A TestAgent operation to the ". $VM->Name ." VM failed:\n". + "\n$ErrMessage\n". + "So the VM has been put offline and the TestBot will try to regain access to it."); WrapUpAndExit('queued'); } diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl index 44919fa6..3a0b3f3e 100755 --- a/testbot/bin/WineRunReconfig.pl +++ b/testbot/bin/WineRunReconfig.pl @@ -282,10 +282,14 @@ sub FatalTAError($$) my $IsPoweredOn = $VM->GetDomain()->IsPoweredOn(); if (!defined $IsPoweredOn) { - # The VM host is not accessible anymore so mark the VM as offline and + # The VM host is not accessible anymore so put the VM offline and # requeue the task. This does not count towards the task's tries limit # since neither the VM nor the task are at fault. Error("$ErrMessage\n"); + NotifyAdministrator("Putting the ". $VM->Name ." VM offline", + "A TestAgent operation to the ". $VM->Name ." VM failed:\n". + "\n$ErrMessage\n". + "So the VM has been put offline and the TestBot will try to regain access to it."); WrapUpAndExit('queued'); } diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index 6a718494..53e9e500 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -332,10 +332,14 @@ sub FatalTAError($$;$) my $IsPoweredOn = $VM->GetDomain()->IsPoweredOn(); if (!defined $IsPoweredOn) { - # The VM host is not accessible anymore so mark the VM as offline and + # The VM host is not accessible anymore so put the VM offline and # requeue the task. This does not count towards the task's tries limit # since neither the VM nor the task are at fault. Error("$ErrMessage\n"); + NotifyAdministrator("Putting the ". $VM->Name ." VM offline", + "A TestAgent operation to the ". $VM->Name ." VM failed:\n". + "\n$ErrMessage\n". + "So the VM has been put offline and the TestBot will try to regain access to it."); WrapUpAndExit('queued'); } diff --git a/testbot/bin/WineRunWineTest.pl b/testbot/bin/WineRunWineTest.pl index 74b974ae..508605b3 100755 --- a/testbot/bin/WineRunWineTest.pl +++ b/testbot/bin/WineRunWineTest.pl @@ -332,10 +332,14 @@ sub FatalTAError($$;$) my $IsPoweredOn = $VM->GetDomain()->IsPoweredOn(); if (!defined $IsPoweredOn) { - # The VM host is not accessible anymore so mark the VM as offline and + # The VM host is not accessible anymore so put the VM offline and # requeue the task. This does not count towards the task's tries limit # since neither the VM nor the task are at fault. Error("$ErrMessage\n"); + NotifyAdministrator("Putting the ". $VM->Name ." VM offline", + "A TestAgent operation to the ". $VM->Name ." VM failed:\n". + "\n$ErrMessage\n". + "So the VM has been put offline and the TestBot will try to regain access to it."); WrapUpAndExit('queued'); } diff --git a/testbot/lib/WineTestBot/Engine/Scheduler.pm b/testbot/lib/WineTestBot/Engine/Scheduler.pm index 1b42ea9a..39d23425 100644 --- a/testbot/lib/WineTestBot/Engine/Scheduler.pm +++ b/testbot/lib/WineTestBot/Engine/Scheduler.pm @@ -37,6 +37,7 @@ use WineTestBot::Engine::Events; use WineTestBot::Jobs; use WineTestBot::Log; use WineTestBot::RecordGroups; +use WineTestBot::Utils; use WineTestBot::VMs; @@ -277,6 +278,10 @@ sub _CheckAndClassifyVMs() if ($Errors >= $MaxVMErrors) { $NewStatus = "maintenance"; + NotifyAdministrator("Putting the $VMKey VM in maintenance mode", + "The last $Errors revert operations timed out.\n\n". + "No further operation will be attempted until an administrator has put\n". + "the VM back online."); $Sched->{busyvms}->{$VMKey} = 1; } } diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index e28a34b8..83f60fa0 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -30,7 +30,8 @@ use Exporter 'import'; our @EXPORT = qw(MakeSecureURL SecureConnection GenerateRandomString OpenNewFile CreateNewFile CreateNewLink CreateNewDir DurationToString BuildEMailRecipient IsValidFileName - BuildTag SanitizeTag LocaleName ShQuote ShArgv2Cmd); + BuildTag SanitizeTag LocaleName NotifyAdministrator + ShQuote ShArgv2Cmd); use Fcntl; @@ -276,6 +277,37 @@ sub BuildTag($;$) } +# +# EMail helper +# + +sub NotifyAdministrator($$) +{ + my ($Subject, $Body) = @_; + + if (open(my $fh, "|/usr/sbin/sendmail -oi -t -odq")) + { + require WineTestBot::Log; + WineTestBot::Log::LogMsg("Notifying administrator: $Subject\n"); + print $fh <<"EOF"; +From: $RobotEMail +To: $AdminEMail +Subject: $Subject + +$Body +EOF + close($fh); + } + else + { + require WineTestBot::Log; + WineTestBot::Log::LogMsg("Could not send administrator notification: $!\n"); + WineTestBot::Log::LogMsg(" Subject: $Subject\n"); + WineTestBot::Log::LogMsg(" Body: $Body\n"); + } +} + + # # Shell helpers # diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm index 5d738e2a..78e066fe 100644 --- a/testbot/lib/WineTestBot/VMs.pm +++ b/testbot/lib/WineTestBot/VMs.pm @@ -452,6 +452,11 @@ sub Run($$$$$$) # Reset the Status and ChildPid since the exec failed $self->Status("offline"); + NotifyAdministrator("Putting the ". $self->Name ." VM offline", + "Could not run $Tool on the ". $self->Name ." VM. The failed command is:\n\n". + ShArgv2Cmd(@$Args) ."\n\n". + "An administrator should investigate why the command failed but the\n". + "TestBot will still try to continue using the VM.\n"); $self->ChildDeadline(undef); $self->ChildPid(undef); my ($ErrProperty, $ErrMessage) = $self->Save(); -- 2.20.1
Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/Feedback.pl | 17 ++++++----------- testbot/web/Register.pl | 11 +---------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/testbot/web/Feedback.pl b/testbot/web/Feedback.pl index 2adc4cd4..36f0a2b0 100644 --- a/testbot/web/Feedback.pl +++ b/testbot/web/Feedback.pl @@ -26,6 +26,7 @@ our @ISA = qw(ObjectModel::CGI::FreeFormPage); use ObjectModel::BasicPropertyDescriptor; use WineTestBot::Config; +use WineTestBot::Utils; sub _initialize($$$) @@ -83,17 +84,11 @@ sub OnSend($) return !1; } - open (SENDMAIL, "|/usr/sbin/sendmail -oi -t -odq"); - print SENDMAIL <<"EOF"; -From: $RobotEMail -To: $AdminEMail -Subject: winetestbot feedback - -EOF - print SENDMAIL "Name: ", $self->GetParam("Name"), "\n"; - print SENDMAIL "EMail: ", $self->GetParam("EMail"), "\n\n"; - print SENDMAIL "Remarks:\n", $self->GetParam("Remarks"), "\n"; - close(SENDMAIL); + NotifyAdministrator("winetestbot feedback", + "Name: ". $self->GetParam("Name") ."\n". + "EMail: ". $self->GetParam("EMail") ."\n\n". + "Remarks:\n". + $self->GetParam("Remarks")); return 1; } diff --git a/testbot/web/Register.pl b/testbot/web/Register.pl index 207728e7..23f57a9b 100644 --- a/testbot/web/Register.pl +++ b/testbot/web/Register.pl @@ -142,16 +142,7 @@ sub OnSendRequest($) my $URL = ($UseSSL ? "https://" : "http://") . $WebHostName . "/admin/UserDetails.pl?Key=" . uri_escape($self->GetParam("Name")); $Msg .= "\nTo approve or deny the request, please go to " . $URL; - - open (SENDMAIL, "|/usr/sbin/sendmail -oi -t -odq"); - print SENDMAIL <<"EOF"; -From: $RobotEMail -To: $AdminEMail -Subject: winetestbot account request - -$Msg -EOF - close(SENDMAIL); + NotifyAdministrator("winetestbot account request", $Msg); return 1; } -- 2.20.1
Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/scripts/CheckWineTestBot.pl | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/testbot/scripts/CheckWineTestBot.pl b/testbot/scripts/CheckWineTestBot.pl index 3ca4fde9..aa271db2 100755 --- a/testbot/scripts/CheckWineTestBot.pl +++ b/testbot/scripts/CheckWineTestBot.pl @@ -36,6 +36,7 @@ sub BEGIN } use WineTestBot::Config; +use WineTestBot::Utils; use WineTestBot::Engine::Notify; my $rc = 0; @@ -47,27 +48,21 @@ if (! PingEngine()) sleep 5; } - open (SENDMAIL, "|/usr/sbin/sendmail -oi -t -odq"); - print SENDMAIL <<"EOF"; -From: $RobotEMail -To: $AdminEMail -Subject: WineTestBot engine died - -EOF + my $Body; if ($> != 0) { - print SENDMAIL "Insufficient permissions to restart the engine\n"; + $Body = "Insufficient permissions to restart the engine\n"; $rc = 1; } elsif (PingEngine()) { - print SENDMAIL "The engine was restarted successfully\n"; + $Body = "The engine was restarted successfully\n"; } else { - print SENDMAIL "Unable to restart the engine\n"; + $Body = "Unable to restart the engine\n"; $rc = 1; } - close(SENDMAIL); + NotifyAdministrator("WineTestBot engine died", $Body); } exit($rc); -- 2.20.1
participants (1)
-
Francois Gouget