introduce noop and refactor checks in all workers

lib/App/Netdisco/Worker/Plugin/Arpnip.pm

@@ -13,7 +13,7 @@ register_worker({ stage => 'check' }, sub {
   return Status->error('arpnip failed: unable to interpret device param')
     unless defined $device;
 
-  return Status->done("arpnip skipped: $device not yet discovered")
+  return Status->error("arpnip skipped: $device not yet discovered")
     unless $device->in_storage;
 
   return Status->defer("arpnip skipped: $device is pseudo-device")
@@ -25,7 +25,7 @@ register_worker({ stage => 'check' }, sub {
   return Status->defer("arpnip deferred: $device is not arpnipable")
     unless is_arpnipable_now($device);
 
-  return Status->done('Arpnip is able to run.');
+  return Status->done('Arpnip is able to run');
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/Arpnip/Nodes.pm

@@ -11,7 +11,7 @@ use Dancer::Plugin::DBIC 'schema';
 use Time::HiRes 'gettimeofday';
 use NetAddr::MAC ();
 
-register_worker({ stage => 'check', driver => 'snmp' }, sub {
+register_worker({ stage => 'main', driver => 'snmp' }, sub {
   my ($job, $workerconf) = @_;
 
   my $device = $job->device;

lib/App/Netdisco/Worker/Plugin/Arpwalk.pm

@@ -8,6 +8,10 @@ use App::Netdisco::JobQueue qw/jq_queued jq_insert/;
 use Dancer::Plugin::DBIC 'schema';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Arpwalk is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
 
   my %queued = map {$_ => 1} jq_queued('arpnip');

lib/App/Netdisco/Worker/Plugin/Contact.pm

@@ -7,9 +7,14 @@ use aliased 'App::Netdisco::Worker::Status';
 use App::Netdisco::Transport::SNMP;
 
 register_worker({ stage => 'check' }, sub {
+  return Status->error('Missing device (-d).')
+    unless defined (shift)->device;
+  return Status->done('Contact is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $data) = map {$job->$_} qw/device extra/;
-  return Status->error('Missing device (-d).') if !defined $device;
 
   # snmp connect using rw community
   my $snmp = App::Netdisco::Transport::SNMP->writer_for($device)

lib/App/Netdisco/Worker/Plugin/Delete.pm

@@ -7,9 +7,14 @@ use aliased 'App::Netdisco::Worker::Status';
 use App::Netdisco::Util::Device 'delete_device';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->error('Missing device (-d).')
+    unless defined (shift)->device;
+  return Status->done('Delete is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $port, $extra) = map {$job->$_} qw/device port extra/;
-  return Status->error('Missing device (-d).') if !defined $device;
 
   $port = ($port ? 1 : 0);
   delete_device($device, $port, $extra);

lib/App/Netdisco/Worker/Plugin/Discover.pm

@@ -22,7 +22,7 @@ register_worker({ stage => 'check' }, sub {
   return Status->defer("discover deferred: $device is not discoverable")
     unless is_discoverable_now($device);
 
-  return Status->done('discover is able to run.');
+  return Status->done('Discover is able to run.');
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/DiscoverAll.pm

@@ -8,6 +8,10 @@ use App::Netdisco::JobQueue qw/jq_queued jq_insert/;
 use Dancer::Plugin::DBIC 'schema';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Discoverall is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
 
   my %queued = map {$_ => 1} jq_queued('discover');

lib/App/Netdisco/Worker/Plugin/Expire.pm

@@ -8,6 +8,10 @@ use Dancer::Plugin::DBIC 'schema';
 use App::Netdisco::Util::Statistics 'update_stats';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Expire is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
 
   if (setting('expire_devices') and setting('expire_devices') > 0) {

lib/App/Netdisco/Worker/Plugin/ExpireNodes.pm

@@ -7,10 +7,13 @@ use aliased 'App::Netdisco::Worker::Status';
 use Dancer::Plugin::DBIC 'schema';
 
 register_worker({ stage => 'check' }, sub {
-  my ($job, $workerconf) = @_;
+  return Status->error('Missing device (-d).')
+    unless defined (shift)->device;
+  return Status->done('ExpireNodes is able to run');
+});
 
-  return Status->error('nbtstat failed: unable to interpret device param')
+register_worker({ stage => 'main' }, sub {
-    if !defined $job->device;
+  my ($job, $workerconf) = @_;
 
   schema('netdisco')->txn_do(sub {
     schema('netdisco')->resultset('Node')->search({

lib/App/Netdisco/Worker/Plugin/Graph.pm

@@ -7,9 +7,12 @@ use aliased 'App::Netdisco::Worker::Status';
 use App::Netdisco::Util::Graph ();
 
 register_worker({ stage => 'check' }, sub {
-  my ($job, $workerconf) = @_;
+  return Status->done('Graph is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   App::Netdisco::Util::Graph::graph();
-  return Status->done('Generated graph data.');
+  return Status->done('Generated graph data');
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/Location.pm

@@ -7,9 +7,14 @@ use aliased 'App::Netdisco::Worker::Status';
 use App::Netdisco::Transport::SNMP;
 
 register_worker({ stage => 'check' }, sub {
+  return Status->error('Missing device (-d).')
+    unless defined (shift)->device;
+  return Status->done('Location is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $data) = map {$job->$_} qw/device extra/;
-  return Status->error('Missing device (-d).') if !defined $device;
 
   # snmp connect using rw community
   my $snmp = App::Netdisco::Transport::SNMP->writer_for($device)

lib/App/Netdisco/Worker/Plugin/Macsuck.pm

@@ -13,7 +13,7 @@ register_worker({ stage => 'check' }, sub {
   return Status->error('macsuck failed: unable to interpret device param')
     unless defined $device;
 
-  return Status->done("macsuck skipped: $device not yet discovered")
+  return Status->error("macsuck skipped: $device not yet discovered")
     unless $device->in_storage;
 
   return Status->defer("macsuck skipped: $device is pseudo-device")

lib/App/Netdisco/Worker/Plugin/Macwalk.pm

@@ -8,6 +8,10 @@ use App::Netdisco::JobQueue qw/jq_queued jq_insert/;
 use Dancer::Plugin::DBIC 'schema';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Macwalk is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
 
   my %queued = map {$_ => 1} jq_queued('macsuck');

lib/App/Netdisco/Worker/Plugin/Monitor.pm

@@ -7,9 +7,13 @@ use aliased 'App::Netdisco::Worker::Status';
 use App::Netdisco::Util::NodeMonitor ();
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Monitor is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   App::Netdisco::Util::NodeMonitor::monitor();
-  return Status->done('Generated monitor data.');
+  return Status->done('Generated monitor data');
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/Nbtwalk.pm

@@ -8,6 +8,10 @@ use App::Netdisco::JobQueue qw/jq_queued jq_insert/;
 use Dancer::Plugin::DBIC 'schema';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Nbtwalk is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
 
   my %queued = map {$_ => 1} jq_queued('nbtstat');

lib/App/Netdisco/Worker/Plugin/PortControl.pm

@@ -8,18 +8,26 @@ use App::Netdisco::Transport::SNMP;
 use App::Netdisco::Util::Port ':all';
 
 register_worker({ stage => 'check' }, sub {
+  my ($job, $workerconf) = @_;
+  return Status->error('Missing device (-d).') unless defined $job->device;
+  return Status->error('Missing port (-p).') unless defined $job->port;
+  return Status->error('Missing status (-e).') unless defined $job->subaction;
+
+  my $port = get_port($job->device, $job->port)
+    or return Status->error(sprintf "Unknown port name [%s] on device %s",
+                              $job->port, $job->device);
+
+  my $vlan_reconfig_check = vlan_reconfig_check($port);
+  return Status->error("Cannot alter vlan: $vlan_reconfig_check")
+    if $vlan_reconfig_check;
+
+  return Status->done('PortControl is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $pn) = map {$job->$_} qw/device port/;
-  return Status->error('Missing device (-d).') if !defined $device;
+  my $port = get_port($device, $pn);
-  return Status->error('Missing port (-p).') if !defined $pn;
-  return Status->error('Missing status (-e).') if !defined $job->subaction;
-
-  my $port = get_port($device, $pn)
-    or return Status->error("Unknown port name [$pn] on device $device");
-
-  my $port_reconfig_check = port_reconfig_check($port);
-  return Status->error("Cannot alter port: $port_reconfig_check")
-    if $port_reconfig_check;
 
   # need to remove "-other" which appears for power/portcontrol
   (my $sa = $job->subaction) =~ s/-\w+//;

lib/App/Netdisco/Worker/Plugin/PortName.pm

@@ -8,18 +8,27 @@ use App::Netdisco::Transport::SNMP;
 use App::Netdisco::Util::Port ':all';
 
 register_worker({ stage => 'check' }, sub {
+  my ($job, $workerconf) = @_;
+  return Status->error('Missing device (-d).') unless defined $job->device;
+  return Status->error('Missing port (-p).') unless defined $job->port;
+  return Status->error('Missing name (-e).') unless defined $job->subaction;
+
+  my $port = get_port($job->device, $job->port)
+    or return Status->error(sprintf "Unknown port name [%s] on device %s",
+                              $job->port, $job->device);
+
+  return Status->done('PortName is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $pn, $data) = map {$job->$_} qw/device port extra/;
-  return Status->error('Missing device (-d).') if !defined $device;
-  return Status->error('Missing port (-p).') if !defined $pn;
-  return Status->error('Missing name (-e).') if !defined $data;
 
   # snmp connect using rw community
   my $snmp = App::Netdisco::Transport::SNMP->writer_for($device)
     or return Status->defer("failed to connect to $device to update alias");
 
-  my $port = get_port($device, $pn)
+  my $port = get_port($device, $pn);
-    or return Status->error("Unknown port name [$pn] on device $device");
 
   my $iid = get_iid($snmp, $port)
     or return Status->error("Failed to get port ID for [$pn] from $device");

lib/App/Netdisco/Worker/Plugin/Power.pm

@@ -9,20 +9,28 @@ use App::Netdisco::Util::Port ':all';
 
 register_worker({ stage => 'check' }, sub {
   my ($job, $workerconf) = @_;
-  my ($device, $pn) = map {$job->$_} qw/device port/;
+  return Status->error('Missing device (-d).') unless defined $job->device;
-  return Status->error('Missing device (-d).') if !defined $device;
+  return Status->error('Missing port (-p).') unless defined $job->port;
-  return Status->error('Missing port (-p).') if !defined $pn;
+  return Status->error('Missing status (-e).') unless defined $job->subaction;
-  return Status->error('Missing status (-e).') if !defined $job->subaction;
 
-  my $port = get_port($device, $pn)
+  my $port = get_port($job->device, $job->port)
-    or return Status->error("Unknown port name [$pn] on device $device");
+    or return Status->error(sprintf "Unknown port name [%s] on device %s",
+                              $job->port, $job->device);
+
+  my $vlan_reconfig_check = vlan_reconfig_check($port);
+  return Status->error("Cannot alter vlan: $vlan_reconfig_check")
+    if $vlan_reconfig_check;
 
   return Status->error("No PoE service on port [$pn] on device $device")
     unless $port->power;
 
-  my $port_reconfig_check = port_reconfig_check($port);
+  return Status->done('Power is able to run');
-  return Status->error("Cannot alter port: $port_reconfig_check")
+});
-    if $port_reconfig_check;
+
+register_worker({ stage => 'main' }, sub {
+  my ($job, $workerconf) = @_;
+  my ($device, $pn) = map {$job->$_} qw/device port/;
+  my $port = get_port($device, $pn);
 
   # munge data
   (my $data = $job->subaction) =~ s/-\w+//; # remove -other

lib/App/Netdisco/Worker/Plugin/Psql.pm

@@ -5,6 +5,10 @@ use App::Netdisco::Worker::Plugin;
 use aliased 'App::Netdisco::Worker::Status';
 
 register_worker({ stage => 'check' }, sub {
+  return Status->done('Psql is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $port, $extra) = map {$job->$_} qw/device port extra/;
 

lib/App/Netdisco/Worker/Plugin/Renumber.pm

@@ -8,15 +8,23 @@ use NetAddr::IP qw/:rfc3021 :lower/;
 use App::Netdisco::Util::Device qw/get_device renumber_device/;
 
 register_worker({ stage => 'check' }, sub {
+  return Status->error('Missing device (-d).')
+    unless defined (shift)->device;
+
+  my $new_ip = NetAddr::IP->new($job->extra);
+  unless ($new_ip and $new_ip->addr ne '0.0.0.0') {
+      return Status->error("Bad host or IP: ".($job->extra || '0.0.0.0'));
+  }
+
+  return Status->done('Renumber is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $port, $extra) = map {$job->$_} qw/device port extra/;
-  return Status->error('Missing device (-d).') if !defined $device;
-  my $old_ip = $device->ip;
 
+  my $old_ip = $device->ip;
   my $new_ip = NetAddr::IP->new($extra);
-  unless ($new_ip and $new_ip->addr ne '0.0.0.0') {
-      return Status->error("Bad host or IP: ".($extra || '0.0.0.0'));
-  }
 
   my $new_dev = get_device($new_ip->addr);
   if ($new_dev and $new_dev->in_storage and ($new_dev->ip ne $device->ip)) {

lib/App/Netdisco/Worker/Plugin/Show.pm

@@ -8,9 +8,14 @@ use Data::Printer ();
 use App::Netdisco::Transport::SNMP;
 
 register_worker({ stage => 'check' }, sub {
+  return Status->error('Missing device (-d).')
+    unless defined (shift)->device;
+  return Status->done('Show is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   my ($job, $workerconf) = @_;
   my ($device, $port, $extra) = map {$job->$_} qw/device port extra/;
-  return Status->error('Missing device (-d).') if !defined $device;
 
   $extra ||= 'interfaces'; my $class = undef;
   ($class, $extra) = split(/::([^:]+)$/, $extra);
@@ -26,7 +31,7 @@ register_worker({ stage => 'check' }, sub {
   Data::Printer::p($i->$extra);
 
   return Status->done(
-    sprintf "Showed %s response from %s.", $extra, $device->ip);
+    sprintf "Showed %s response from %s", $extra, $device->ip);
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/Stats.pm

@@ -7,9 +7,12 @@ use aliased 'App::Netdisco::Worker::Status';
 use App::Netdisco::Util::Statistics ();
 
 register_worker({ stage => 'check' }, sub {
-  my ($job, $workerconf) = @_;
+  return Status->done('Stats is able to run');
+});
+
+register_worker({ stage => 'main' }, sub {
   App::Netdisco::Util::Statistics::update_stats();
-  return Status->done('Updated statistics.');
+  return Status->done('Updated statistics');
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/Test.pm

@@ -19,13 +19,12 @@ register_worker({ stage => 'check' }, sub {
 register_worker({ stage => 'early' }, sub {
   my ($job, $workerconf) = @_;
   debug 'Test (early) ran successfully.';
-  return Status->done('Test (early) ran successfully.');
+  return Status->error('Test (early) ran successfully.');
 });
 
 register_worker(sub {
   my ($job, $workerconf) = @_;
-  debug 'Test (undefined) ran successfully.';
+  return Status->noop('Test (undefined) ran successfully.');
-  return Status->done('Test (undefined) ran successfully.');
 });
 
 true;

lib/App/Netdisco/Worker/Plugin/Test/Core.pm

@@ -25,7 +25,7 @@ register_worker({ stage => 'early' }, sub {
 register_worker(sub {
   my ($job, $workerconf) = @_;
   debug 'Test (undefined) ran successfully.';
-  return Status->done('Test (undefined) ran successfully.');
+  return Status->error('Test (undefined) ran successfully.');
 });
 
 true;

lib/App/Netdisco/Worker/Runner.pm

@@ -18,7 +18,7 @@ has 'job' => (
 
 has 'jobstat' => (
   is => 'rw',
-  default => sub { Status->error("check phase did not pass for this action") },
+  default => sub { Status->error('check phase did not pass for this action') },
 );
 
 after 'run', 'run_workers' => sub {
@@ -62,10 +62,12 @@ sub run {
   my $guard = guard { set(device_auth => \@userconf) };
   set(device_auth => \@newuserconf);
 
+  my $store = Dancer::Factory::Hook->instance();
   $self->run_workers('nd2_core_check');
-  return if $self->jobstat->not_ok;
+  return if scalar @{ $store->get_hooks_for('nd2_core_check') }
+            and $self->jobstat->not_ok;
 
-  $self->jobstat( Status->error("no worker succeeded during main phase") );
+  $self->jobstat( Status->error('no worker succeeded during main phase') );
   $self->run_workers("nd2_core_${_}") for qw/early main user/;
 }
 
@@ -88,7 +90,7 @@ sub run_workers {
       $self->jobstat($retval)
         if ($phase =~ m/^(?:check|main)$/)
            and ref $retval eq 'App::Netdisco::Worker::Status'
-           and $self->jobstat->not_ok;
+           and $retval->level >= $self->jobstat->level;
     }
     # errors at most phases are ignored
     catch { $self->jobstat->error($_) if $phase eq 'check' };

lib/App/Netdisco/Worker/Status.pm

@@ -3,6 +3,8 @@ package App::Netdisco::Worker::Status;
 use strict;
 use warnings;
 
+use Dancer qw/:moose :syntax !error/;
+
 use Moo;
 use namespace::clean;
 
@@ -61,6 +63,18 @@ sub error { (shift)->_make_new('error', @_) }
 sub done  { (shift)->_make_new('done', @_)  }
 sub defer { (shift)->_make_new('defer', @_) }
 
+=head2 noop
+
+Simply logs a message at debug level if passed, and returns true. Used for
+consistency with other Status class methods but really does nothing.
+
+=cut
+
+sub noop {
+  debug $_[1] if $_[1];
+  return 1;
+}
+
 =head2 is_ok
 
 Returns true if status is C<done>.
@@ -77,4 +91,16 @@ Returns true if status is C<error> or C<defer>.
 
 sub not_ok { return (not $_[0]->is_ok) }
 
+=head2 level
+
+A numeric constant for the status, to allow comparison.
+
+=cut
+
+sub level {
+  my $self = shift;
+  return (($self->status eq 'done') ? 3
+                                    : ($self->status eq 'defer') ? 2 : 1);
+}
+
 1;