From 3b32e8431219fd4f889e5d035069d024e64cfbf7 Mon Sep 17 00:00:00 2001 From: Oliver Gorwits Date: Sun, 1 Oct 2017 08:18:13 +0100 Subject: [PATCH] fiddle about with runner logic to fix exit states --- bin/netdisco-do | 2 +- lib/App/Netdisco/Worker/Plugin/Test.pm | 2 +- lib/App/Netdisco/Worker/Plugin/Test/Core.pm | 2 +- lib/App/Netdisco/Worker/Runner.pm | 35 +++++----- lib/App/Netdisco/Worker/Status.pm | 73 +++++++++++---------- 5 files changed, 60 insertions(+), 54 deletions(-) diff --git a/bin/netdisco-do b/bin/netdisco-do index 44694006..418b386c 100755 --- a/bin/netdisco-do +++ b/bin/netdisco-do @@ -139,7 +139,7 @@ foreach my $host (@hostlist) { $job->log("error running job: $_"); }; - if ($job->log eq 'no worker for this action was successful') { + if ($job->log eq 'check phase did not pass for this action') { pod2usage( -msg => (sprintf 'error: %s is not a valid action', $action), -verbose => 2, diff --git a/lib/App/Netdisco/Worker/Plugin/Test.pm b/lib/App/Netdisco/Worker/Plugin/Test.pm index 4a680f78..d0c27863 100644 --- a/lib/App/Netdisco/Worker/Plugin/Test.pm +++ b/lib/App/Netdisco/Worker/Plugin/Test.pm @@ -7,7 +7,7 @@ use aliased 'App::Netdisco::Worker::Status'; register_worker({ stage => 'main' }, sub { my ($job, $workerconf) = @_; debug 'Test (main) ran successfully.'; - return Status->done('Test (main) ran successfully.'); + return Status->done('Test (main) ran successfully (1).'); }); register_worker({ stage => 'check' }, sub { diff --git a/lib/App/Netdisco/Worker/Plugin/Test/Core.pm b/lib/App/Netdisco/Worker/Plugin/Test/Core.pm index f63eff24..7f227733 100644 --- a/lib/App/Netdisco/Worker/Plugin/Test/Core.pm +++ b/lib/App/Netdisco/Worker/Plugin/Test/Core.pm @@ -7,7 +7,7 @@ use aliased 'App::Netdisco::Worker::Status'; register_worker({ stage => 'main' }, sub { my ($job, $workerconf) = @_; debug 'Test (main) ran successfully.'; - return Status->done('Test (main) ran successfully.'); + return Status->done('Test (main) ran successfully (2).'); }); register_worker({ stage => 'check' }, sub { diff --git a/lib/App/Netdisco/Worker/Runner.pm b/lib/App/Netdisco/Worker/Runner.pm index a8f61c97..33f38758 100644 --- a/lib/App/Netdisco/Worker/Runner.pm +++ b/lib/App/Netdisco/Worker/Runner.pm @@ -18,7 +18,7 @@ has 'job' => ( has 'jobstat' => ( is => 'rw', - default => sub { Status->error("no worker for this action was successful") }, + default => sub { Status->error("check phase did not pass for this action") }, ); after 'run', 'run_workers' => sub { @@ -62,33 +62,38 @@ sub run { my $guard = guard { set(device_auth => \@userconf) }; set(device_auth => \@newuserconf); - foreach my $stage (qw/check early main user/) { - my $hookname = "nd2_core_${stage}"; - $self->run_workers($hookname); - return if $stage eq 'check' and $self->jobstat->not_ok; - } + $self->run_workers('nd2_core_check'); + return if $self->jobstat->not_ok; + + $self->jobstat( Status->error("no worker succeeded during main phase") ); + $self->run_workers("nd2_core_${_}") for qw/early main user/; } sub run_workers { my $self = shift; my $hook = shift or return $self->jobstat->error('missing hook param'); my $store = Dancer::Factory::Hook->instance(); - my $check = ($hook eq 'nd2_core_check'); - my $main = ($hook eq 'nd2_core_main'); + (my $phase = $hook) =~ s/^nd2_core_//; - return unless scalar @{ $store->get_hooks_for($hook) }; debug "running workers for hook: $hook"; foreach my $worker (@{ $store->get_hooks_for($hook) }) { try { - my $retval = $worker->($self->job); # could die or return undef or a scalar or Status or another class - $self->jobstat($retval) - if ($check or $main) and ref $retval eq 'App::Netdisco::Worker::Status'; - } - catch { $self->jobstat->error($_) if $check }; + my $retval = $worker->($self->job); - last if $check and $self->jobstat->is_ok; + # update (save) the status if we're in check or main phases + #  check because it's a gatekeeper, main because it's the retval + $self->jobstat($retval) + if ($phase =~ m/^(?:check|main)$/) + and ref $retval eq 'App::Netdisco::Worker::Status' + and $self->jobstat->not_ok; + } + # errors at most phases are ignored + catch { $self->jobstat->error($_) if $phase eq 'check' }; + + # any successful check is a GO! + last if $phase eq 'check' and $self->jobstat->is_ok; } } diff --git a/lib/App/Netdisco/Worker/Status.pm b/lib/App/Netdisco/Worker/Status.pm index 010d17b2..6b8b706d 100644 --- a/lib/App/Netdisco/Worker/Status.pm +++ b/lib/App/Netdisco/Worker/Status.pm @@ -6,23 +6,40 @@ use warnings; use Moo; use namespace::clean; -foreach my $slot (qw/ - done_slot - error_slot - defer_slot - /) { - - has $slot => ( - is => 'rw', - default => 0, - ); -} +has 'status' => ( + is => 'rw', + default => undef, + clearer => 1, +); has 'log' => ( is => 'rw', default => '', ); +=head1 INTRODUCTION + +The status can be: + +=over 4 + +=item * C + +At C phase, indicates the action may continue. At other phases, +indicates the worker has completed without error or has no work to do. + +=item * C + +Indicates that there is an error condition. Also used to quit a worker without +side effects that C and C have. + +=item * C + +Quits a worker. If the final recorded outcome for a device is C several +times in a row, then it may be skipped from further jobs. + +=back + =head1 METHODS =head2 done, error, defer @@ -32,48 +49,32 @@ Shorthand for new() with setting param, accepts log as arg. =cut sub _make_new { - my ($self, $log, $slot) = @_; + my ($self, $status, $log) = @_; + die unless $status; my $new = (ref $self ? $self : $self->new()); $new->log($log); - $new->$_(0) for (qw/done_slot error_slot defer_slot/); - $new->$slot(1); + $new->status($status); return $new; } -sub error { (shift)->_make_new(@_, 'error_slot') } -sub done { (shift)->_make_new(@_, 'done_slot') } -sub defer { (shift)->_make_new(@_, 'defer_slot') } +sub error { (shift)->_make_new('error', @_) } +sub done { (shift)->_make_new('done', @_) } +sub defer { (shift)->_make_new('defer', @_) } =head2 is_ok -Returns true if C is true and C and C have not been set. +Returns true if status is C. =cut -sub is_ok { return ($_[0]->done_slot - and not $_[0]->error_slot and not $_[0]->defer_slot) } +sub is_ok { return $_[0]->status eq 'done' } =head2 not_ok -Returns the logical inversion of C. +Returns true if status is C or C. =cut sub not_ok { return (not $_[0]->is_ok) } -=head2 status - -Returns text equivalent of C, C, or C. - -=cut - -sub status { - my $self = shift; - return ( - $self->done_slot ? 'done' - : $self->defer_slot ? 'defer' - : 'error' - ); -} - 1;