Add logic to daemonizer to be able to kill what it daemonizes

scripts/slave/daemonizer.py

Index: scripts/slave/daemonizer.py
diff --git a/scripts/slave/daemonizer.py b/scripts/slave/daemonizer.py
index 2a75f98f7f4b9fda82b0ea80f3dfb21a5a1dce7d..6c000a0298f1892a227f4ad54d0982c47a556d34 100755
--- a/scripts/slave/daemonizer.py
+++ b/scripts/slave/daemonizer.py
@@ -12,7 +12,7 @@ USAGE:
 
   daemonizer.py [options] -- <script> [args]
 
-  - options are options to this script. Note, currently there are none!
+  - options are options to this script.
   - script is the script to daemonize or run in the background
   - args are the arguments that one might want to pass the <script>
 """
@@ -20,12 +20,38 @@ USAGE:
 # TODO(sivachandra): Enhance this script by enforcing a protocol of
 # communication between the parent (this script) and the daemon script.
 
+import argparse
+import logging
 import os
+import signal
 import subprocess
 import sys
 
-
-def daemonize():
+def start(cmd, pid_file_path):

[luqui, 2015/09/08 18:18:10]

seems like this action ought to be called 'restart'

[bpastene, 2015/09/08 20:50:47]

Done.

+  # Check for the pid_file to see if the daemon's already running
+  # and restart it if it is
+  if (pid_file_path == None):

[luqui, 2015/09/08 18:18:10]

You don't need parens around if's conditional in python.

[bpastene, 2015/09/08 20:50:47]

Done.

+    logging.error('pid_file_path arg must be specified when starting a daemon')
+    return 1
+  try:
+    with open(pid_file_path, 'r') as pid_file:
+      pid = int(pid_file.readline())
+  except (IOError, ValueError):
+    pid = None
+
+  if (pid):

[luqui, 2015/09/08 18:18:10]

no parens

[bpastene, 2015/09/08 20:50:47]

Done.

+    logging.info(
+        "%s pid file already exists, attempting to kill process %d",
+        pid_file_path,
+        pid)
+    try:
+      os.kill(pid, signal.SIGTERM)
+    except (OSError):

[luqui, 2015/09/08 18:18:10]

no parens

[bpastene, 2015/09/08 20:50:47]

Done.

+      logging.exception("Unable to kill old daemon process")
+
+  return daemonize(cmd, pid_file_path)
+
+def daemonize(cmd, pid_file_path):
   """This function is based on the Python recipe provided here:
   http://www.jejik.com/articles/2007/02/a_simple_unix_linux_daemon_in_python/
   """
@@ -66,28 +92,62 @@ def daemonize():
   os.dup2(so.fileno(), sys.stdout.fileno())
   os.dup2(se.fileno(), sys.stderr.fileno())
 
+  proc = subprocess.Popen(cmd)
 
-def print_usage(err_msg):
-  print >> sys.stderr, err_msg
-  sys.exit('Usage: daemonizer.py [options] -- arg0 [argN...]')
+  # write pid to file if applicable
+  if (pid_file_path):
+    try:
+      with open(pid_file_path, 'w+') as pid_file:

[luqui, 2015/09/08 18:18:10]

I think you mean 'w' mode instead of 'w+'.

[bpastene, 2015/09/08 20:50:47]

Done.

+        pid_file.write('%s' % str(proc.pid))
+    except (IOError):
+      logging.exception("Unable to write pid to file")
 
+  proc.communicate()

[luqui, 2015/09/08 18:18:10]

This will make daemonize block, which is the opposite of what is good

[bpastene, 2015/09/08 20:50:47]

Previously, it would do just that, and return the child's return code once it
terminated. But, as far as I can tell, that information isn't used anywhere (not
that it even could be since this is a fork of a dead fork). I'll go ahead and
change it.

[luqui, 2015/09/08 21:15:57]

Oh I misunderstood, I missed the fork() above.  I think commuicate() is correct
after all.

+  return proc.returncode
 
-def main():
+def stop(pid_file_path):
+  if (pid_file_path == None):
+    logging.error("pid_file_path arg must be specified when stopping a daemon")
+    return 1
   try:
-    idx = sys.argv.index('--')
-  except ValueError:
-    print_usage('Separator -- not found')
-
-  cmd = sys.argv[idx+1:]
-  if not cmd:
-    print_usage('arg0 not specified for sub command')
-
-  # TODO(sivachandra): When required in the future, use optparse to parse args
-  # from sys.argv[:idx]
-
-  daemonize()
-  return subprocess.call(cmd)
+    with open(pid_file_path) as pid_file:
+      pid = int(pid_file.readline())
+    logging.info('Sending SIGTERM to %d', pid)
+    os.kill(pid, signal.SIGTERM)
+    os.remove(pid_file_path)
+  except (IOError, OSError):
+    logging.exception('Error terminating daemon process')

[luqui, 2015/09/08 18:18:10]

Log the exception here so we have some idea what the error was.

[bpastene, 2015/09/08 20:50:47]

logging.exception() automatically logs the exception details and stack trace of
the caught exception

[luqui, 2015/09/08 21:15:57]

Oh cool :-)

 
+def main():
+  parser = argparse.ArgumentParser(
+      description='Launch, or shutdown, a daemon process.')
+  parser.add_argument(
+      '--action',
+      default='daemonize',
+      choices=['start','stop','daemonize'],
+      help='What action to take. Both start and stop attempt to write & read '
+      'the pid to a file so it can kill or restart it, while daemonize simply '
+      'fires and forgets.')
+  parser.add_argument(
+      '--pid-file-path',
+      type=str,
+      default=None,
+      help='Path of tmp file to store the daemon\'s pid.')
+  parser.add_argument(
+      '--', dest='',
+      required=False,
+      help='Optional delimiter dividing daemonizer options with the command. '
+      'This is here to ensure it\'s backwards compatible with the previous '
+      'version of daemonizer.')
+  parser.add_argument('cmd', help='Command (+ args) to daemonize', nargs='*')
+  args = parser.parse_args()
+
+  if (args.action == 'start'):
+    return start(args.cmd, args.pid_file_path)
+  elif (args.action == 'daemonize'):
+    return daemonize(args.cmd, None)
+  elif (args.action == 'stop'):
+    return stop(args.pid_file_path)
 
 if __name__ == '__main__':
   sys.exit(main())