fiddle about with runner logic to fix exit states

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,

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 {

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 {

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/) {
+  $self->run_workers('nd2_core_check');
-    my $hookname = "nd2_core_${stage}";
+  return if $self->jobstat->not_ok;
-    $self->run_workers($hookname);
+
-    return if $stage eq 'check' and $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 $phase = $hook) =~ s/^nd2_core_//;
-  my $main  = ($hook eq 'nd2_core_main');
 
-  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)
+      my $retval = $worker->($self->job);
-        if ($check or $main) and ref $retval eq 'App::Netdisco::Worker::Status';
-    }
-    catch { $self->jobstat->error($_) if $check };
 
-    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;
   }
 }
 

lib/App/Netdisco/Worker/Status.pm

@@ -6,23 +6,40 @@ use warnings;
 use Moo;
 use namespace::clean;
 
-foreach my $slot (qw/
+has 'status' => (
-      done_slot
+  is => 'rw',
-      error_slot
+  default => undef,
-      defer_slot
+  clearer => 1,
-    /) {
+);
-
-  has $slot => (
-    is => 'rw',
-    default => 0,
-  );
-}
 
 has 'log' => (
   is => 'rw',
   default => '',
 );
 
+=head1 INTRODUCTION
+
+The status can be:
+
+=over 4
+
+=item * C<done>
+
+At C<check> phase, indicates the action may continue. At other phases,
+indicates the worker has completed without error or has no work to do.
+
+=item * C<error>
+
+Indicates that there is an error condition. Also used to quit a worker without
+side effects that C<done> and C<defer> have.
+
+=item * C<defer>
+
+Quits a worker. If the final recorded outcome for a device is C<defer> 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->status($status);
-  $new->$slot(1);
   return $new;
 }
 
-sub error { (shift)->_make_new(@_, 'error_slot') }
+sub error { (shift)->_make_new('error', @_) }
-sub done  { (shift)->_make_new(@_, 'done_slot')  }
+sub done  { (shift)->_make_new('done', @_)  }
-sub defer { (shift)->_make_new(@_, 'defer_slot') }
+sub defer { (shift)->_make_new('defer', @_) }
 
 =head2 is_ok
 
-Returns true if C<done> is true and C<error> and C<defer> have not been set.
+Returns true if status is C<done>.
 
 =cut
 
-sub is_ok { return ($_[0]->done_slot
+sub is_ok { return $_[0]->status eq 'done' }
-  and not $_[0]->error_slot and not $_[0]->defer_slot) }
 
 =head2 not_ok
 
-Returns the logical inversion of C<ok>.
+Returns true if status is C<error> or C<defer>.
 
 =cut
 
 sub not_ok { return (not $_[0]->is_ok) }
 
-=head2 status
-
-Returns text equivalent of C<done>, C<defer>, or C<error>.
-
-=cut
-
-sub status {
-  my $self = shift;
-  return (
-    $self->done_slot ? 'done'
-                     : $self->defer_slot ? 'defer'
-                                         : 'error'
-  );
-}
-
 1;