Linux daemon updates (#425)

* Get rid of Proc::PID::File - Fix #424 * Also make filename optional to --pidfile option. * Check against pid file only if --pidfile is used as service manager like systemd doesn't expect with handle ourself to run or not. * Fix PID file checking even in foreground mode. * Be sure to keep libdir in @inc before daemonize.
fusioninventory · Dec 18, 2017 · cce59cd · cce59cd
1 parent a77b72e
commit cce59cd
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 37 deletions.
diff --git a/Changes b/Changes
@@ -6,9 +6,13 @@ core:
   logged in journal
 * Get rid of confdir setup in setup.pm
 * Update syslog name to fullname agent
-* Get rid of List::Util module dependency
+* Get rid of List::Util & Proc::PID::File module dependencies
 * Try to load more recent IDS database files if found in well-known places
-* Fixed daemon pid filename
+* Fixed default daemon pid filename
+* When --pidfile is used, don't permit to manually start daemon even in foreground
+  unless --pidfile parameter is different
+* Makes --pidfile filename optional to compute a default one
+* Check if we need to include libdir while daemonize
 * Class refactoring: Get rid of discouraged 'use base' syntax in favor of lighter
   'use parent' and as fields pragma is not used (see 'base' man)
 * Logger refactoring: no more an Exporter based class to simplify its usage and

diff --git a/Makefile.PL b/Makefile.PL
@@ -45,7 +45,6 @@ recommends 'LWP::Protocol::https' => '0';
 
 if ($OSNAME ne 'MSWin32') {
     recommends 'Proc::Daemon'         => '0';
-    recommends 'Proc::PID::File'      => '0';
 } else {
     recommends 'Win32::Daemon'        => '0';
     recommends 'Win32::Unicode::File' => '0';

diff --git a/bin/fusioninventory-agent b/bin/fusioninventory-agent
@@ -47,7 +47,7 @@ GetOptions(
     'no-task=s',
     'no-p2p',
     'password|p=s',
-    'pidfile=s',
+    'pidfile:s',
     'proxy|P=s',
     'httpd-ip=s',
     'httpd-port=s',
@@ -637,11 +637,12 @@ Don't fork in background.
 
 This is only useful when running as a daemon.
 
-=item B<--pidfile>=I<FILE>
+=item B<--pidfile>[=I<FILE>]
 
-Store pid in I<FILE>.
+Store pid in I<FILE> or in default PID file.
 
-This is only useful when running as a daemon.
+This is only useful when running as a daemon and still not managed with a system
+service manager like systemd.
 
 =item B<--tag>=I<TAG>
 

diff --git a/lib/FusionInventory/Agent/Daemon.pm b/lib/FusionInventory/Agent/Daemon.pm
@@ -40,6 +40,9 @@ sub init {
 sub reinit {
     my ($self) = @_;
 
+    # Update PID file modification time so we can expire it
+    utime undef,undef,$self->{pidfile} if $self->{pidfile};
+
     $self->{logger}->debug('agent reinit');
 
     $self->{lastConfigLoad} = time;
@@ -60,11 +63,26 @@ sub reinit {
 sub run {
     my ($self) = @_;
 
+    my $config = $self->{config};
+    my $logger = $self->{logger};
+
     $self->setStatus('waiting');
 
     my @targets = $self->getTargets();
 
-    $self->{logger}->debug("Running in background mode");
+    if ($logger) {
+        if ($config->{'no-fork'}) {
+            $logger->debug2("Waiting in mainloop");
+            foreach my $target (@targets) {
+                my $date = $target->getFormatedNextRunDate();
+                my $type = $target->_getType();
+                my $name = $target->_getName();
+                $logger->debug2("$type target next run: $date - $name");
+            }
+        } else {
+            $logger->debug("Running in background mode");
+        }
+    }
 
     # background mode: work on a targets list copy, but loop while
     # the list really exists so we can stop quickly when asked for
@@ -86,14 +104,20 @@ sub run {
             eval {
                 $net_error = $self->runTarget($target);
             };
-            $self->{logger}->error($EVAL_ERROR) if $EVAL_ERROR;
+            $logger->error($EVAL_ERROR) if ($EVAL_ERROR && $logger);
             if ($net_error) {
                 # Prefer to retry early on net error
                 $target->setNextRunDateFromNow(60);
             } else {
                 $target->resetNextRunDate();
             }
 
+            if ($logger && $config->{'no-fork'}) {
+                my $date = $target->getFormatedNextRunDate();
+                my $type = $target->_getType();
+                $logger->debug2("$type target scheduled: $date");
+            }
+
             # Leave immediately if we passed in terminate method
             last unless $self->getTargets();
 
@@ -178,50 +202,87 @@ sub createDaemon {
 
     $logger->info("$PROVIDER Agent starting");
 
-    my $pidfile = $config->{pidfile} ||
-        $self->{vardir} . '/'.lc($PROVIDER).'-agent.pid';
+    my $pidfile = $config->{pidfile};
 
-    if ($self->isAlreadyRunning($pidfile)) {
-        $logger->error("$PROVIDER Agent is already running, exiting...") if $logger;
-        exit 1;
+    if (defined($pidfile) && $pidfile eq "") {
+        # Set to default pidfile only when needed
+        $pidfile = $self->{vardir} . '/'. lc($PROVIDER). '-agent.pid';
+        $logger->debug("Using $pidfile as default PID file") if $logger;
+    } elsif (!$pidfile) {
+        $logger->debug("Skipping running daemon control based on PID file checking") if $logger;
     }
 
-    if (!$config->{'no-fork'}) {
+    # Expire PID file if daemon is not running while conf-reload-interval is
+    # in use and PID file has not been update since, including a minute safety gap
+    if ($pidfile && -e $pidfile && $self->{config}->{'conf-reload-interval'}) {
+        my $mtime = (stat($pidfile))[9];
+        if ($mtime && $mtime < time - $self->{config}->{'conf-reload-interval'} - 60) {
+            $logger->info("$pidfile PID file expired") if $logger;
+            unlink $pidfile;
+        }
+    }
 
-        Proc::Daemon->require();
-        if ($EVAL_ERROR) {
-            $logger->error("Failed to load Proc::Daemon: $EVAL_ERROR") if $logger;
-            exit 1;
+    my $daemon;
+
+    Proc::Daemon->require();
+    if ($EVAL_ERROR) {
+        $logger->debug("Failed to load recommended Proc::Daemon library: $EVAL_ERROR") if $logger;
+
+        # Eventually check running process from pid found in pid file
+        if ($pidfile) {
+            my $pid = getFirstLine(file => $pidfile);
+
+            if ($pid && int($pid)) {
+                $logger->debug2("Last running daemon started with PID $pid") if $logger;
+                if ($pid != $$ && kill(0, $pid)) {
+                    $logger->error("$PROVIDER Agent is already running, exiting...") if $logger;
+                    exit 1;
+                }
+                $logger->debug("$PROVIDER Agent with PID $pid is dead") if $logger;
+            }
         }
 
+    } else {
         # If we use relative path, we must stay in the current directory
         my $workdir = substr($self->{libdir}, 0, 1) eq '/' ? '/' : getcwd();
 
-        Proc::Daemon::Init(
-            {
-                work_dir => $workdir,
-                pid_file => $pidfile
-            }
+        # Be sure to keep libdir in includes or we can fail to load need libraries
+        unshift @INC, $self->{libdir}
+            if ($workdir eq '/' && ! first { $_ eq $self->{libdir} } @INC);
+
+        $daemon = Proc::Daemon->new(
+            work_dir => $workdir,
+            pid_file => $pidfile
         );
 
-        $logger->debug("$PROVIDER Agent daemonized") if $logger;
+        # Use Proc::Daemon API to check daemon status but it always return false
+        # if pidfile is not used
+        if ($daemon->Status()) {
+            $logger->error("$PROVIDER Agent is already running, exiting...") if $logger;
+            exit 1;
+        }
     }
-}
 
-sub isAlreadyRunning {
-    my ($self, $pidfile) = @_;
+    if ($config->{'no-fork'} || !$daemon) {
+        # Still keep current PID in PID file to permit Proc::Daemon to check status
+        if ($pidfile) {
+            if (open(PID, ">", $pidfile)) {
+                print PID "$$\n";
+                close(PID);
+            } elsif ($logger) {
+                $logger->debug("Can't write PID file: $!");
+                undef $pidfile;
+            }
+        }
+        $logger->debug("$PROVIDER Agent started in foreground") if $logger;
 
-    Proc::PID::File->require();
-    if ($EVAL_ERROR) {
-        $self->{logger}->debug(
-            'Proc::PID::File unavailable, unable to check for running agent'
-        );
-        return 0;
+    } elsif (my $pid = $daemon->Init()) {
+        $logger->debug("$PROVIDER Agent daemonized with PID $pid") if $logger;
+        exit 0;
     }
 
-    my $pid = Proc::PID::File->new();
-    $pid->{path} = $pidfile;
-    return $pid->alive();
+    # From here we can enable our pidfile deletion on terminate
+    $self->{pidfile} = $pidfile;
 }
 
 sub sleep {
@@ -307,6 +368,9 @@ sub terminate {
         kill 'TERM', $self->{current_runtask};
         delete $self->{current_runtask};
     }
+
+    # Remove pidfile
+    unlink $self->{pidfile} if $self->{pidfile};
 }
 
 1;