fix: Support CONTACT after PROLOG on first connection

g-bougard · g-bougard · commit f5a62664b727 · 2025-02-13T19:20:52.000+01:00
Reworked target responses caching for event handling Closes #851
diff --git a/Changes b/Changes
@@ -2,6 +2,10 @@ Revision history for GLPI agent
 
 1.13 not yet released
 
+core:
+* fix #851: glpi-agent should also try to request CONTACT after GLPI 10+ answer on PROLOG
+* Reworked target responses caching for event handling
+
 inventory:
 * On windows, don't cache system is not 64 bits for the service lifetime as this can
   be the result of a failed WMI call at the service start.
diff --git a/lib/GLPI/Agent.pm b/lib/GLPI/Agent.pm
@@ -203,24 +203,17 @@ sub terminate {
         if ($self->{current_task});
 }
 
-sub runTarget {
-    my ($self, $target, $responses_only) = @_;
+sub getContact {
+    my ($self, $target, $plannedTasks) = @_;
 
-    if ($target->isType('local') || $target->isType('server')) {
-        $self->{logger}->info("target $target->{id}: " . $target->getType() . " " . $target->getName());
-    }
+    my $response;
 
-    # the prolog/contact dialog must be done once for all tasks,
-    # but only for server targets
-    my ($response, $contact_response);
-    my $client;
-    my @plannedTasks = $target->plannedTasks();
     if ($target->isGlpiServer()) {
         GLPI::Agent::HTTP::Client::GLPI->require();
         return $self->{logger}->error("GLPI Protocol library can't be loaded")
             if $EVAL_ERROR;
 
-        $client = GLPI::Agent::HTTP::Client::GLPI->new(
+        my $client = GLPI::Agent::HTTP::Client::GLPI->new(
             logger  => $self->{logger},
             config  => $self->{config},
             agentid => uuid_to_string($self->{agentid}),
@@ -236,7 +229,7 @@ sub runTarget {
             $httpd_conf{"httpd-port"} = $self->{server}->{port};
         }
 
-        my %enabled = map { lc($_) => 1 } @plannedTasks;
+        my %enabled = map { lc($_) => 1 } @{$plannedTasks};
         my $contact = GLPI::Agent::Protocol::Contact->new(
             logger              => $self->{logger},
             deviceid            => $self->{deviceid},
@@ -306,14 +299,14 @@ sub runTarget {
                 $self->{_disabled_remoteinventory} = delete $disabled{remoteinventory};
                 # Never disable inventory if force option is used
                 delete $disabled{inventory} if $self->{config}->{force};
-                @plannedTasks = grep { ! exists($disabled{lc($_)}) } @plannedTasks;
+                $response->{plannedTasks} = [ grep { ! exists($disabled{lc($_)}) } @{$plannedTasks} ];
             } elsif (!ref($disabled)) {
                 $disabled = lc($disabled);
                 # Only disable inventory if force option is not set
                 if ($disabled ne "inventory" || !$self->{config}->{force}) {
-                    @plannedTasks = grep {
-                        lc($_) ne $disabled
-                    } @plannedTasks;
+                    $response->{plannedTasks} = [
+                        grep { lc($_) ne $disabled } @{$plannedTasks}
+                    ];
                 }
             }
         }
@@ -322,7 +315,7 @@ sub runTarget {
         # Handle tasks informations returned by server in CONTACT answer
         if (ref($tasks) eq "HASH") {
             # Only keep task server support for planned tasks
-            foreach my $task (map { lc($_) } @plannedTasks) {
+            foreach my $task (map { lc($_) } @{$plannedTasks}) {
                 next unless ref($tasks->{$task}) eq 'HASH';
 
                 # Keep task supporting announced by server
@@ -354,15 +347,17 @@ sub runTarget {
                 }
             }
         }
-
-        # Keep contact response
-        $contact_response = $response;
     }
 
-    # By default, PROLOG request could be avoided when communicating with a GLPI server
-    # But it still may be required if we detect server supports any task due to glpiinventory plugin
-    if ($target->isType('server') && $target->doProlog()) {
+    return $response;
+}
+
+sub getProlog {
+    my ($self, $target) = @_;
+
+    my $response;
 
+    if ($target->isType('server')) {
         return unless GLPI::Agent::HTTP::Client::OCS->require();
 
         my $agentid;
@@ -372,7 +367,7 @@ sub runTarget {
         $agentid = uuid_to_string($self->{agentid})
             unless $target->isGlpiServer();
 
-        $client = GLPI::Agent::HTTP::Client::OCS->new(
+        my $client = GLPI::Agent::HTTP::Client::OCS->new(
             logger  => $self->{logger},
             config  => $self->{config},
             agentid => $agentid,
@@ -401,7 +396,6 @@ sub runTarget {
             unless ($target->isGlpiServer()) {
                 $self->{logger}->info("$target->{id} answer shows it supports GLPI Agent protocol");
                 $target->isGlpiServer('true');
-                return $self->runTarget($target) unless $response->expiration;
             }
         } else {
             # update target
@@ -413,20 +407,72 @@ sub runTarget {
         }
     }
 
-    # Used when running tasks after a taskrun event
-    if ($responses_only) {
-        return {
-            contact  => $contact_response,
-            response => $response
-        };
+    return $response;
+}
+
+sub runTarget {
+    my ($self, $target, $responses_only) = @_;
+
+    if ($target->isType('local') || $target->isType('server')) {
+        $self->{logger}->info("target $target->{id}: " . $target->getType() . " " . $target->getName());
+    }
+
+    # the prolog/contact dialog must be done once for all tasks,
+    # but only for server targets
+    my @plannedTasks = $target->plannedTasks();
+    my @requests = ();
+    my $responses = {};
+    push @requests, 'CONTACT' if $target->isGlpiServer();
+    push @requests, 'PROLOG' if !@requests && $target->isType('server');
+    my %requested = qw(CONTACT 0 PROLOG 0);
+
+    while (@requests) {
+        my $request = shift @requests;
+        next if $responses->{$request};
+
+        my $response;
+        $requested{$request} = 1;
+
+        if ($request eq 'CONTACT') {
+
+            $response = $self->getContact($target, \@plannedTasks);
+            # Still return on error
+            return $response if $response && !ref($response);
+
+            # Update plannedTasks
+            if (ref($response) && $response->{plannedTasks}) {
+                my $plannedTasks = delete $response->{plannedTasks};
+                @plannedTasks = @{$plannedTasks};
+            }
+
+            # Check condition where we also need to request PROLOG after a CONTACT
+            push @requests, 'PROLOG'
+                if ref($response) && $target->doProlog() && !$requested{PROLOG};
+
+        # By default, PROLOG request could be avoided when communicating with a GLPI server
+        # But it still may be required if we detect server supports any task due to glpiinventory plugin
+        } elsif ($request eq 'PROLOG') {
+
+            $response = $self->getProlog($target, \@plannedTasks);
+            # Still return on error
+            return $response if $response && !ref($response);
+
+            push @requests, 'CONTACT'
+                if ref($response) && $target->isGlpiServer() && !$requested{CONTACT};
+        }
+
+        $responses->{$request} = $response if ref($response);
     }
 
+    # Used when running tasks after a taskrun event
+    return $responses if $responses_only;
+
     foreach my $name (@plannedTasks) {
-        my $server_response = $response;
-        if ($contact_response) {
+        my $server_response = $responses->{PROLOG} // $responses->{CONTACT};
+        if ($responses->{CONTACT}) {
             # Be sure to use expected response for task
             my $task_server = $target->getTaskServer($name) // 'glpi';
-            $server_response = $contact_response
+            $server_response = $responses->{CONTACT}
                 if $task_server eq 'glpi';
         }
         eval {
diff --git a/lib/GLPI/Agent/Daemon.pm b/lib/GLPI/Agent/Daemon.pm
@@ -117,7 +117,6 @@ sub run {
 
     # background mode: work on a targets list copy, but loop while
     # the list really exists so we can stop quickly when asked for
-    my $responses;
     while ($self->getTargets()) {
         my $time = time();
 
@@ -131,7 +130,8 @@ sub run {
 
         if ($target->paused()) {
 
-            undef $responses;
+            # Reset target responses
+            $target->responses({});
 
             # Leave immediately if we passed in terminate method
             last if $self->{_terminate};
@@ -140,20 +140,24 @@ sub run {
             # Always remove event from list
             $target->delEvent($event);
 
+            my $responses = $target->responses();
+
             # Contact server if required and cache responses
             if ($event->taskrun) {
-                if (!defined($responses)) {
-                    eval {
-                        $responses = $self->runTarget($target, "contact-only");
-                    };
+                if ((!ref($responses) || !$responses->{CONTACT}) && $target->isGlpiServer()) {
+                    $responses->{CONTACT} = $self->getContact($target, [$target->plannedTasks()]);
+                }
+                if ((!ref($responses) || !$responses->{PROLOG}) && $target->isType('server') && $event->task =~ /^net(discovery|inventory)$/i) {
+                    $responses->{PROLOG} = $self->getProlog($target);
                 }
                 # Fail event on no expected response from server
-                unless (ref($responses)) {
+                unless (ref($responses) && (ref($responses->{CONTACT}) || ref($responses->{PROLOG}))) {
                     $logger->error("Failed to handle run event for ".$event->task) if $logger && $event->task;
                     next;
                 }
-            } else {
-                undef $responses;
+
+                # Keep target responses
+                $target->responses($responses);
             }
 
             eval {
@@ -170,6 +174,9 @@ sub run {
                 $target->setNextRunDateFromNow();
                 $target->resetNextRunDate();
 
+                # This is also safe to reset target responses
+                $target->responses({});
+
                 if ($logger) {
                     my $date = $target->getFormatedNextRunDate();
                     my $id   = $target->id();
@@ -183,8 +190,6 @@ sub run {
 
         } elsif ($time >= $target->getNextRunDate()) {
 
-            undef $responses;
-
             my $net_error = 0;
             eval {
                 $net_error = $self->runTarget($target);
@@ -275,11 +280,11 @@ sub runTargetEvent {
         $target->triggerRunTasksNow($event);
 
     } elsif (ref($responses)) {
-        my $server_response = $responses->{response};
-        if ($responses->{contact}) {
+        my $server_response = $responses->{PROLOG};
+        if ($responses->{CONTACT}) {
             # Be sure to use expected response for task
             my $task_server = $target->getTaskServer($task) // 'glpi';
-            $server_response = $responses->{contact}
+            $server_response = $responses->{CONTACT}
                 if $task_server eq 'glpi';
         }
         eval {
diff --git a/lib/GLPI/Agent/Target.pm b/lib/GLPI/Agent/Target.pm
@@ -215,6 +215,9 @@ sub triggerRunTasksNow {
 
         $self->addEvent(GLPI::Agent::Event->new(%event), 1);
     }
+
+    # Also reset cached responses
+    delete $self->{_responses};
 }
 
 sub addEvent {
@@ -461,6 +464,12 @@ sub getTaskVersion {
     return $self->{_glpi};
 }
 
+sub responses {
+    my ($self, $responses) = @_;
+    return $self->{_responses} unless defined($responses);
+    $self->{_responses} = $responses;
+}
+
 1;
 __END__
 

Original file line number	Diff line number	Diff line change
`@@ -215,6 +215,9 @@ sub triggerRunTasksNow {`
`215`	`215`
`216`	`216`	`$self->addEvent(GLPI::Agent::Event->new(%event), 1);`
`217`	`217`	`}`
	`218`	`+`
	`219`	`+ # Also reset cached responses`
	`220`	`+ delete $self->{_responses};`
`218`	`221`	`}`
`219`	`222`
`220`	`223`	`sub addEvent {`
`@@ -461,6 +464,12 @@ sub getTaskVersion {`
`461`	`464`	`return $self->{_glpi};`
`462`	`465`	`}`
`463`	`466`
	`467`	`+sub responses {`
	`468`	`+ my ($self, $responses) = @_;`
	`469`	`+ return $self->{_responses} unless defined($responses);`
	`470`	`+ $self->{_responses} = $responses;`
	`471`	`+}`
	`472`	`+`
`464`	`473`	`1;`
`465`	`474`	`__END__`
`466`	`475`