From 58cd488ccc2ec22c30ae5e646a6220864f807f83 Mon Sep 17 00:00:00 2001 From: Oliver Gorwits Date: Wed, 13 Sep 2017 19:22:40 +0100 Subject: [PATCH] refactor layer and pseudo checks --- lib/App/Netdisco/DB/Result/Device.pm | 23 +++++++++++++++++++ lib/App/Netdisco/Util/Device.pm | 14 +++-------- lib/App/Netdisco/Worker/Plugin/Arpnip.pm | 5 +++- .../Netdisco/Worker/Plugin/Arpnip/Nodes.pm | 3 --- .../Netdisco/Worker/Plugin/Arpnip/Subnets.pm | 3 --- lib/App/Netdisco/Worker/Plugin/Discover.pm | 10 ++++---- .../Worker/Plugin/Discover/WithNodes.pm | 4 ++-- lib/App/Netdisco/Worker/Plugin/Macsuck.pm | 5 +++- .../Netdisco/Worker/Plugin/Macsuck/Nodes.pm | 3 --- .../Worker/Plugin/Macsuck/WirelessNodes.pm | 3 --- 10 files changed, 40 insertions(+), 33 deletions(-) diff --git a/lib/App/Netdisco/DB/Result/Device.pm b/lib/App/Netdisco/DB/Result/Device.pm index 63f43dbd..6733426f 100644 --- a/lib/App/Netdisco/DB/Result/Device.pm +++ b/lib/App/Netdisco/DB/Result/Device.pm @@ -193,6 +193,29 @@ __PACKAGE__->might_have( =head1 ADDITIONAL METHODS +=head2 is_pseudo + +Returns true if the vendor of the device is "netdisco". + +=cut + +sub not_pseudo { + my $device = shift; + return (defined $device->vendor and $device->vendor eq 'netdisco'); +} + +=head2 has_layer( $number ) + +Returns true if the device provided sysServices and supports the given layer. + +=cut + +sub has_layer { + my ($device, $layer) = @_; + return unless $layer and $layer =~ m/^[1-8]$/; + return $device->layers and substr($device->layers, (9-$layer), 1) == 1; +} + =head2 renumber( $new_ip ) Will update this device and all related database records to use the new IP diff --git a/lib/App/Netdisco/Util/Device.pm b/lib/App/Netdisco/Util/Device.pm index a45c631f..7b6ded33 100644 --- a/lib/App/Netdisco/Util/Device.pm +++ b/lib/App/Netdisco/Util/Device.pm @@ -192,12 +192,10 @@ sub is_discoverable_now { my ($ip, $remote_type) = @_; my $device = get_device($ip) or return 0; - if ($device->in_storage) { - if ($device->since_last_discover and setting('discover_min_age') - and $device->since_last_discover < setting('discover_min_age')) { + if ($device->since_last_discover and setting('discover_min_age') + and $device->since_last_discover < setting('discover_min_age')) { - return _bail_msg("is_discoverable: time since last discover less than discover_min_age"); - } + return _bail_msg("is_discoverable: time since last discover less than discover_min_age"); } return is_discoverable(@_); @@ -242,9 +240,6 @@ sub is_arpnipable_now { my ($ip) = @_; my $device = get_device($ip) or return 0; - return _bail_msg("is_arpnipable: cannot arpnip an undiscovered device") - if not $device->in_storage; - if ($device->since_last_arpnip and setting('arpnip_min_age') and $device->since_last_arpnip < setting('arpnip_min_age')) { @@ -293,9 +288,6 @@ sub is_macsuckable_now { my ($ip) = @_; my $device = get_device($ip) or return 0; - return _bail_msg("is_macsuckable: cannot macsuck an undiscovered device") - if not $device->in_storage; - if ($device->since_last_macsuck and setting('macsuck_min_age') and $device->since_last_macsuck < setting('macsuck_min_age')) { diff --git a/lib/App/Netdisco/Worker/Plugin/Arpnip.pm b/lib/App/Netdisco/Worker/Plugin/Arpnip.pm index d678f745..2f8e6d6f 100644 --- a/lib/App/Netdisco/Worker/Plugin/Arpnip.pm +++ b/lib/App/Netdisco/Worker/Plugin/Arpnip.pm @@ -17,7 +17,10 @@ register_worker({ stage => 'check' }, sub { unless $device->in_storage; return Status->defer("arpnip skipped: $device is pseudo-device") - if $device->vendor and $device->vendor eq 'netdisco'; + if $device->is_pseudo; + + return Status->defer("arpnip skipped: $device has no layer 3 capability") + unless $device->has_layer(3); return Status->defer("arpnip deferred: $device is not arpnipable") unless is_arpnipable_now($device); diff --git a/lib/App/Netdisco/Worker/Plugin/Arpnip/Nodes.pm b/lib/App/Netdisco/Worker/Plugin/Arpnip/Nodes.pm index 8e7ad585..96411775 100644 --- a/lib/App/Netdisco/Worker/Plugin/Arpnip/Nodes.pm +++ b/lib/App/Netdisco/Worker/Plugin/Arpnip/Nodes.pm @@ -18,9 +18,6 @@ register_worker({ stage => 'check', driver => 'snmp' }, sub { my $snmp = App::Netdisco::Transport::SNMP->reader_for($device) or return Status->defer("arpnip failed: could not SNMP connect to $device"); - return Status->defer("Skipped arpnip for device $device without layer 3 capability") - unless $snmp->has_layer(3); - # get v4 arp table my $v4 = get_arps($device, $snmp->at_paddr, $snmp->at_netaddr); # get v6 neighbor cache diff --git a/lib/App/Netdisco/Worker/Plugin/Arpnip/Subnets.pm b/lib/App/Netdisco/Worker/Plugin/Arpnip/Subnets.pm index f4a70087..c2a398ca 100644 --- a/lib/App/Netdisco/Worker/Plugin/Arpnip/Subnets.pm +++ b/lib/App/Netdisco/Worker/Plugin/Arpnip/Subnets.pm @@ -17,9 +17,6 @@ register_worker({ stage => 'second', driver => 'snmp' }, sub { my $snmp = App::Netdisco::Transport::SNMP->reader_for($device) or return Status->defer("arpnip failed: could not SNMP connect to $device"); - return Status->defer("Skipped arpnip for device $device without layer 3 capability") - unless $snmp->has_layer(3); - # get directly connected networks my @subnets = gather_subnets($device, $snmp); # TODO: IPv6 subnets diff --git a/lib/App/Netdisco/Worker/Plugin/Discover.pm b/lib/App/Netdisco/Worker/Plugin/Discover.pm index c6b89c2a..030b1b55 100644 --- a/lib/App/Netdisco/Worker/Plugin/Discover.pm +++ b/lib/App/Netdisco/Worker/Plugin/Discover.pm @@ -13,15 +13,13 @@ register_worker({ stage => 'check' }, sub { return Status->error('discover failed: unable to interpret device param') unless defined $device; - my $host = $device->ip; - return Status->error("discover failed: no device param (need -d ?)") - if $host eq '0.0.0.0'; + if $device->ip eq '0.0.0.0'; - return Status->defer("discover skipped: $host is pseudo-device") - if $device->vendor and $device->vendor eq 'netdisco'; + return Status->defer("discover skipped: $device is pseudo-device") + if $device->is_pseudo; - return Status->defer("discover deferred: $host is not discoverable") + return Status->defer("discover deferred: $device is not discoverable") unless is_discoverable_now($device); return Status->done('discover is able to run.'); diff --git a/lib/App/Netdisco/Worker/Plugin/Discover/WithNodes.pm b/lib/App/Netdisco/Worker/Plugin/Discover/WithNodes.pm index 8b190a21..6abafe51 100644 --- a/lib/App/Netdisco/Worker/Plugin/Discover/WithNodes.pm +++ b/lib/App/Netdisco/Worker/Plugin/Discover/WithNodes.pm @@ -14,7 +14,7 @@ register_worker({ stage => 'second' }, sub { # arpniped/macsucked, queue those jobs now if ($device->in_storage and $job->subaction and $job->subaction eq 'with-nodes') { - if (!defined $device->last_macsuck) { + if (!defined $device->last_macsuck and $device->has_layer(2)) { jq_insert({ device => $device->ip, action => 'macsuck', @@ -23,7 +23,7 @@ register_worker({ stage => 'second' }, sub { }); } - if (!defined $device->last_arpnip) { + if (!defined $device->last_arpnip and $device->has_layer(3)) { jq_insert({ device => $device->ip, action => 'arpnip', diff --git a/lib/App/Netdisco/Worker/Plugin/Macsuck.pm b/lib/App/Netdisco/Worker/Plugin/Macsuck.pm index db9cb752..6ba26e3d 100644 --- a/lib/App/Netdisco/Worker/Plugin/Macsuck.pm +++ b/lib/App/Netdisco/Worker/Plugin/Macsuck.pm @@ -17,7 +17,10 @@ register_worker({ stage => 'check' }, sub { unless $device->in_storage; return Status->defer("macsuck skipped: $device is pseudo-device") - if $device->vendor and $device->vendor eq 'netdisco'; + if $device->is_pseudo; + + return Status->defer("arpnip skipped: $device has no layer 2 capability") + unless $device->has_layer(2); return Status->defer("macsuck deferred: $device is not macsuckable") unless is_macsuckable_now($device); diff --git a/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm b/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm index ced47e43..152dc11a 100644 --- a/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm +++ b/lib/App/Netdisco/Worker/Plugin/Macsuck/Nodes.pm @@ -21,9 +21,6 @@ register_worker({ stage => 'check', driver => 'snmp' }, sub { my $snmp = App::Netdisco::Transport::SNMP->reader_for($device) or return Status->defer("macsuck failed: could not SNMP connect to $device"); - return Status->defer("Skipped macsuck for device $device without layer 2 capability") - unless $snmp->has_layer(2); - # would be possible just to use now() on updated records, but by using this # same value for them all, we can if we want add a job at the end to # select and do something with the updated set (see set archive, below) diff --git a/lib/App/Netdisco/Worker/Plugin/Macsuck/WirelessNodes.pm b/lib/App/Netdisco/Worker/Plugin/Macsuck/WirelessNodes.pm index 87542901..558a6d05 100644 --- a/lib/App/Netdisco/Worker/Plugin/Macsuck/WirelessNodes.pm +++ b/lib/App/Netdisco/Worker/Plugin/Macsuck/WirelessNodes.pm @@ -15,9 +15,6 @@ register_worker({ stage => 'second', driver => 'snmp' }, sub { my $snmp = App::Netdisco::Transport::SNMP->reader_for($device) or return Status->defer("macsuck failed: could not SNMP connect to $device"); - return Status->defer("Skipped macsuck for device $device without layer 2 capability") - unless $snmp->has_layer(2); - my $now = 'to_timestamp('. (join '.', gettimeofday) .')'; my $cd11_txrate = $snmp->cd11_txrate;