diff --git a/Changes b/Changes index 39dbb699e..61317cdc5 100644 --- a/Changes +++ b/Changes @@ -20,6 +20,8 @@ remoteinventory: * Fix remoteGlob function for ssh remote inventory as it was preventing storage inventory to work properly when accessing remote via ssh command * Don't try to register/update remotes when provided via --remote glpi-agent option +* Support 'remote-workers' configuration to define how many remoteinventory we can + run in parallel netdiscovery/netinventory: * Avoid to record invalid MAC Address from Netbios during netdiscovery task diff --git a/bin/glpi-agent b/bin/glpi-agent index 494b34594..7f218f5e0 100755 --- a/bin/glpi-agent +++ b/bin/glpi-agent @@ -59,6 +59,7 @@ GetOptions( 'list-categories', 'listen', 'remote=s', + 'remote-workers=i', 'set-forcerun', 'scan-homedirs', 'scan-profiles', @@ -262,8 +263,9 @@ glpi-agent [options] [--server server|--local path] --credentials set credentials to support database inventory RemoteInventory task specific options: - --remote specify a list of remotes to process in place + --remote=REMOTE[,REMOTE]... specify a list of remotes to process in place of remotes managed via glpi-remote command + --remote-workers=COUNT maximum number of workers for remoteinventory task Package deployment task specific options: --no-p2p do not use peer to peer to download diff --git a/contrib/unix/glpi-agent-appimage-hook b/contrib/unix/glpi-agent-appimage-hook index 45a487f2c..4c67ea03a 100755 --- a/contrib/unix/glpi-agent-appimage-hook +++ b/contrib/unix/glpi-agent-appimage-hook @@ -96,6 +96,7 @@ GetOptions( 'httpd-port=s', 'httpd-trust=s', 'remote=s', + 'remote-workers=i', 'scan-homedirs', 'scan-profiles', 'server|s=s', @@ -575,7 +576,8 @@ glpi-agent-linux-installer.AppImage [options] =head2 Remote inventory task specific options - --remote=REMOTE setup remote for which request remote inventory + --remote=REMOTE[,REMOTE]... list of remotes for remoteinventory task + --remote-workers=COUNT maximum number of workers for remoteinventory task =head2 Deploy task specific options diff --git a/contrib/unix/installer/Getopt.pm b/contrib/unix/installer/Getopt.pm index 11f05d9f0..90c79fa95 100644 --- a/contrib/unix/installer/Getopt.pm +++ b/contrib/unix/installer/Getopt.pm @@ -35,6 +35,8 @@ my @options = ( 'httpd-port=s', 'httpd-trust=s', 'reinstall', + 'remote=s', + 'remote-workers=i', 'runnow', 'scan-homedirs', 'scan-profiles', @@ -122,6 +124,10 @@ glpi-agent-linux-installer [options] --backend-collect-timeout=TIME set timeout for inventory modules execution (30) -t --tag=TAG configure tag to define in inventories + RemoteInventory specific options: + --remote=REMOTE[,REMOTE]... list of remotes for remoteinventory task + --remote-workers=COUNT maximum number of workers for remoteinventory task + Package deployment task specific options: --no-p2p set to not use peer to peer to download deploy task packages diff --git a/contrib/windows/glpi-agent-packaging.pl b/contrib/windows/glpi-agent-packaging.pl index 132e49f82..730ceb987 100644 --- a/contrib/windows/glpi-agent-packaging.pl +++ b/contrib/windows/glpi-agent-packaging.pl @@ -482,6 +482,7 @@ sub _tree2xml { $result .= $ident ." ". qq[ \n]; $result .= $ident ." ". qq[ \n]; $result .= $ident ." ". qq[ \n]; + $result .= $ident ." ". qq[ \n]; $result .= $ident ." ". qq[ \n]; $result .= $ident ." ". qq[ \n]; $result .= $ident ." ". qq[ \n]; diff --git a/contrib/windows/packaging/MSI_main-v2.wxs.tt b/contrib/windows/packaging/MSI_main-v2.wxs.tt index 027c7af69..59e0e2057 100644 --- a/contrib/windows/packaging/MSI_main-v2.wxs.tt +++ b/contrib/windows/packaging/MSI_main-v2.wxs.tt @@ -256,6 +256,12 @@ ""]]> + + + + + ""]]> + diff --git a/lib/GLPI/Agent/Config.pm b/lib/GLPI/Agent/Config.pm index 6d173ca6f..3f7a376bd 100644 --- a/lib/GLPI/Agent/Config.pm +++ b/lib/GLPI/Agent/Config.pm @@ -22,6 +22,8 @@ my $default = { 'conf-reload-interval' => 0, 'debug' => undef, 'delaytime' => 3600, + 'remote-scheduling' => 0, + 'remote-workers' => 1, 'force' => undef, 'html' => undef, 'json' => undef, diff --git a/lib/GLPI/Agent/Target.pm b/lib/GLPI/Agent/Target.pm index 85c04c8ca..28aaaedd0 100644 --- a/lib/GLPI/Agent/Target.pm +++ b/lib/GLPI/Agent/Target.pm @@ -52,7 +52,7 @@ sub _init { # handle persistent state $self->_loadState(); - $self->{nextRunDate} = $self->_computeNextRunDate() + $self->{nextRunDate} = $self->computeNextRunDate() if (!$self->{nextRunDate} || $self->{nextRunDate} < time-$self->getMaxDelay()); $self->_saveState(); @@ -117,7 +117,7 @@ sub resetNextRunDate { return if delete $self->{_expiration}; $self->{_nextrundelay} = 0; - $self->{nextRunDate} = $self->_computeNextRunDate(); + $self->{nextRunDate} = $self->computeNextRunDate(); $self->_saveState(); } @@ -274,7 +274,7 @@ sub isGlpiServer { # compute a run date, as current date and a random delay # between maxDelay / 2 and maxDelay -sub _computeNextRunDate { +sub computeNextRunDate { my ($self) = @_; my $ret; diff --git a/lib/GLPI/Agent/Task/RemoteInventory.pm b/lib/GLPI/Agent/Task/RemoteInventory.pm index f324db81c..689cee324 100644 --- a/lib/GLPI/Agent/Task/RemoteInventory.pm +++ b/lib/GLPI/Agent/Task/RemoteInventory.pm @@ -5,6 +5,8 @@ use warnings; use parent 'GLPI::Agent::Task::Inventory'; +use Parallel::ForkManager; + use GLPI::Agent::Tools; use GLPI::Agent::Task::RemoteInventory::Remotes; @@ -17,6 +19,9 @@ sub isEnabled { return 0; } + # Always enable remoteinventory task if remote option is set + return 1 if $self->{config}->{remote}; + my $remotes = GLPI::Agent::Task::RemoteInventory::Remotes->new( config => $self->{config}, storage => $self->{target}->getStorage(), @@ -24,16 +29,16 @@ sub isEnabled { ); my $errors = 0; - foreach my $remote ($remotes->getall()) { + while (my $remote = $remotes->next()) { my $error = $remote->checking_error(); last unless $error; my $deviceid = $remote->deviceid or next; $self->{logger}->debug("Skipping remote inventory task execution for $deviceid: $error"); - # We want to retry in a hour - $self->{target}->setNextRunDateFromNow(3600); - $remote->expiration($self->{target}->getNextRunDate()); - $remotes->store(); + if ($self->{config}->{"remote-scheduling"}) { + $remotes->retry($remote, $self->{target}->getMaxDelay()); + $remotes->store(); + } $errors++; } @@ -43,8 +48,10 @@ sub isEnabled { return 1; } - $self->{logger}->debug("Remote inventory task execution disabled: ".(!$count ? - "no remote to inventory" : "all $count remotes are failing")); + $self->{logger}->debug( + "Remote inventory task execution disabled: ". + ($count ? "all $count remotes are failing" : "no remote to inventory") + ); return 0; } @@ -58,43 +65,76 @@ sub run { logger => $self->{logger}, ); - # Handle only one reachable remote at a time - my $remote; - while ($remote = $remotes->next()) { - my $error = $remote->checking_error(); - last unless $error; - my $deviceid = $remote->deviceid - or next; - $self->{logger}->debug("Skipping remote inventory task execution for $deviceid: $error"); - # We want to retry in a hour - $self->{target}->setNextRunDateFromNow(3600); - $remote->expiration($self->{target}->getNextRunDate()); - $remotes->store(); - } - return unless $remote; + my $worker_count = $remotes->count() > 1 ? $self->{config}->{'remote-workers'} : 0; my $start = time; - $self->{deviceid} = $remote->deviceid(); + my $manager = Parallel::ForkManager->new($worker_count); + $manager->set_waitpid_blocking_sleep(0); - # Set now we are remote - $self->setRemote($remote->protocol()); + if ($worker_count) { + $manager->run_on_start( + sub { + my ($pid, $remote) = @_; + $self->{logger}->debug("Starting remoteinventory worker[$pid] to handle ".$remote->safe_url()); + } + ); + } - setRemoteForTools($remote); + $manager->run_on_finish( + sub { + my ($pid, $ret, $remote) = @_; + my $remoteid = $remote->safe_url(); + if ($ret) { + $self->{logger}->debug("Remoteinventory worker[$pid] failed to handle $remoteid") if $worker_count; + # We want to schedule a retry but limited by target max delay + $remotes->retry($remote, $self->{target}->getMaxDelay()); + } else { + $self->{logger}->debug("Remoteinventory worker[$pid] finished to handle $remoteid") if $worker_count; + $remote->expiration($self->{target}->computeNextRunDate()); + } + # Store new remotes scheduling if required + $remotes->store(); + } + ); - $self->SUPER::run(%params); + while (my $remote = $remotes->next()) { + $manager->start($remote) and next; - $remote->disconnect(); + my $error = $remote->checking_error(); + my $deviceid = $remote->deviceid; - resetRemoteForTools(); + my $remoteid = $deviceid // $remote->safe_url(); + $self->{logger}->{prefix} = "[worker $$] $remoteid, " if $worker_count; + if ($error || !$deviceid) { + $self->{logger}->debug("Skipping remote inventory task execution for $remoteid: $error"); + $manager->finish(1); + # In the case we have only one remote, finish won't leave the loop, so always last here + last; + } - my $timing = time - $start; - $self->{logger}->debug("Remote inventory run in $timing seconds"); + $self->{deviceid} = $deviceid; + + # Set now we are remote + $self->setRemote($remote->protocol()); + + setRemoteForTools($remote); + + $self->SUPER::run(%params); + + $remote->disconnect(); + + resetRemoteForTools(); - # Set expiration from target for the remote before storing remotes - $self->{target}->resetNextRunDate(); - $remote->expiration($self->{target}->getNextRunDate()); - $remotes->store(); + delete $self->{logger}->{prefix} if $worker_count; + + $manager->finish(); + } + + $manager->wait_all_children(); + + my $timing = time - $start; + $self->{logger}->debug("Remote inventory task run in $timing seconds"); } 1; diff --git a/lib/GLPI/Agent/Task/RemoteInventory/Remote.pm b/lib/GLPI/Agent/Task/RemoteInventory/Remote.pm index 71600a201..5d361148e 100644 --- a/lib/GLPI/Agent/Task/RemoteInventory/Remote.pm +++ b/lib/GLPI/Agent/Task/RemoteInventory/Remote.pm @@ -116,6 +116,17 @@ sub mode { return $self->{_mode} // ''; } +sub retry { + my ($self, $delay) = @_; + + if (defined($delay)) { + $self->{_retry} = $delay; + $self->expiration(time+$delay) if $delay; + } + + return $self->{_retry} ? $self : 0; +} + sub resetmode { my ($self) = @_; delete $self->{_mode}; diff --git a/lib/GLPI/Agent/Task/RemoteInventory/Remotes.pm b/lib/GLPI/Agent/Task/RemoteInventory/Remotes.pm index baf662f0f..b826aff28 100644 --- a/lib/GLPI/Agent/Task/RemoteInventory/Remotes.pm +++ b/lib/GLPI/Agent/Task/RemoteInventory/Remotes.pm @@ -76,19 +76,59 @@ sub getall { return values %{$self->{_remotes}}; } +sub sort { + my ($self) = @_; + + return unless $self->{_list} && @{$self->{_list}} > 1; + + $self->{_list} = [ + sort { $a->expiration() <=> $b->expiration() } @{$self->{_list}} + ]; +} + sub next { my ($self) = @_; - my @remotes = $self->getall() - or return; + # next API now always initialize an internal list + unless ($self->{_list}) { + my @remotes = $self->getall() + or return; + + $self->{_list} = \@remotes; - my ($remote) = sort { $a->expiration() <=> $b->expiration() } @remotes; + $self->sort(); + } - return unless $remote->expiration() <= time || $self->{_config}->{force}; + my $remote = shift @{$self->{_list}} + or return; + + # Skip scheduling check if forcing and not re-trying a failed remote + unless ($self->{_config}->{force} && !$remote->retry()) { + return if ($self->{_config}->{'remote-scheduling'} || $remote->retry()) && $remote->expiration() > time; + } return $remote; } +sub retry { + my ($self, $remote, $maxdelay) = @_; + + return unless $self->{_list}; + + # Add one hour to last retry time + my $timeout = $remote->retry() // 0; + $timeout += 3600; + # But don't retry if the delay is overdue + return if $timeout > $maxdelay; + + push @{$self->{_list}}, $remote->retry($timeout); + + # Always sort in case of a not empty big list running for a bigger time than + # the retry expiration but not if forcing so retry always occurs after the + # last pending remote + $self->sort() unless $self->{_config}->{force}; +} + sub store { my ($self) = @_; diff --git a/t/tasks/remoteinventory.t b/t/tasks/remoteinventory.t new file mode 100644 index 000000000..7bea7ec59 --- /dev/null +++ b/t/tasks/remoteinventory.t @@ -0,0 +1,123 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use lib 't/lib'; + +use UNIVERSAL::require; +use File::Temp qw(tempdir); + +use Test::Exception; +use Test::More; +use Test::MockModule; +use Test::Deep qw(cmp_deeply); +use Test::NoWarnings; + +use GLPI::Agent;; +use GLPI::Agent::Logger; +use GLPI::Agent::Config; +use GLPI::Agent::Target; + +GLPI::Agent::Task::RemoteInventory->use(); + +# Base config for a Test logger and debug +my %baseconfig = ( + logger => 'Test', + debug => 1, +); + +my $agent = GLPI::Agent->new( + datadir => tempdir(CLEANUP => 1), + vardir => tempdir(CLEANUP => 1), + libdir => 'blib/lib', +); + +my $runs; + +my $inventory_module = Test::MockModule->new('GLPI::Agent::Task::Inventory'); +$inventory_module->mock('run', sub {}); + +# Store API is called while a remote has been processed +my $remotes_module = Test::MockModule->new('GLPI::Agent::Task::RemoteInventory::Remotes'); +$remotes_module->mock('store', sub { $runs++ }); + +my $ssh_remote_module = Test::MockModule->new('GLPI::Agent::Task::RemoteInventory::Remote::Ssh'); +$ssh_remote_module->mock('checking_error', sub { 0 }); + +my %test_cases = ( + notarget => { + config => { + listen => 1, + }, + enabled => 0, + runs => 0, + }, + "local-one-remote-in-config" => { + config => { + local => ".", + remote => "ssh://remote-1", + }, + enabled => 1, + runs => 1, + }, + "local-4-remotes-in-config" => { + config => { + local => ".", + remote => "ssh://remote-2,ssh://remote-3,ssh://remote-4,ssh://remote-5", + }, + enabled => 1, + runs => 4, + }, + "local-20-remotes-in-config-with-5-workers" => { + config => { + local => ".", + remote => join(",", map { "ssh://remote-$_"} 6..25), + 'remote-workers' => 5, + }, + enabled => 1, + runs => 20, + }, +); + +plan tests => 3 * (scalar keys %test_cases) + 2; + +foreach my $test_case (sort keys(%test_cases)) { + + my $test = $test_cases{$test_case}; + $runs = 0; + + # Forget agent configuration + delete $agent->{config}; + + $agent->init( + options => { + %baseconfig, + %{$test->{config}} + } + ); + + my $task; + lives_ok { + $task = GLPI::Agent::Task::RemoteInventory->new( + config => $agent->{config}, + datadir => $agent->{datadir}, + logger => $agent->{logger}, + target => $agent->{targets}->[0], + deviceid => $agent->{deviceid}, + ); + } "$test_case: create remoteinventory task"; + + my $enabled = $task->isEnabled(); + + is($enabled, $test->{enabled}, "$test_case: ".($test->{enabled} ? "enabled" : "disabled")." test"); + + $runs = 0; + $task->run(); + + is($runs, $test->{runs}, "$test_case: runs count test"); +} + +# No remote has been stored in local storage +my $remotes = $agent->{vardir}."/__LOCAL__/remotes.dump"; +ok(! -e $remotes, "No remotes.dump file in local target");