Merge pull request #603 from scottkmaxwell/fix-signal-command

Fix signal command and stopping procedure

circus/commands/base.py

@@ -1,9 +1,8 @@
-import signal
 import copy
 import textwrap
 import time
 
-from circus.exc import MessageError, ArgumentError
+from circus.exc import MessageError
 from circus.commands import errors
 
 
@@ -94,14 +93,6 @@ def _get_watcher(self, arbiter, watcher_name):
         except KeyError:
             raise MessageError("program %s not found" % watcher_name)
 
-    def _get_signal(self, sig):
-        if sig.lower() in ('quit', 'hup', 'kill', 'term', 'ttin', 'ttou',
-                           'usr1', 'usr2'):
-            return getattr(signal, "SIG%s" % sig.upper())
-        elif sig.isdigit():
-            return int(sig)
-        raise ArgumentError("signal %r not supported" % sig)
-
     def validate(self, props):
         if not self.properties:
             return

circus/commands/sendsignal.py

@@ -56,8 +56,8 @@ class Signal(Command):
                     "child_pid": <childpid>,
             }
 
-        It is also possible to send a signal to all the processes of the
-        watcher and its childs::
+        It is also possible to send a signal to all the children of the
+        watcher::
 
             {
                 "command": "signal",
@@ -67,7 +67,7 @@ class Signal(Command):
                     "children": True
             }
 
-        Last, you can send a signal to the process *and* its children, with
+        Lastly, you can send a signal to the process *and* its children, with
         the *recursive* option::
 
             {
@@ -85,63 +85,51 @@ class Signal(Command):
 
         ::
 
-            $ circusctl signal <name> [<process>] [<pid>] [--children]
-                    [recursive] <signum>
+            $ circusctl signal <name> [<pid>] [--children]
+                    [--recursive] <signum>
 
         Options:
         ++++++++
 
         - <name>: the name of the watcher
         - <pid>: integer, the process id.
-        - <signum>: the signal number to send.
+        - <signum>: the signal number (or name) to send.
         - <childpid>: the pid of a child, if any
         - <children>: boolean, send the signal to all the children
         - <recursive>: boolean, send the signal to the process and its children
 
-        Allowed signals are:
-
-            - 3:    QUIT
-            - 15:   TERM
-            - 9:    KILL
-            - 1:    HUP
-            - 21:   TTIN
-            - 22:   TTOU
-            - 30:   USR1
-            - 31:   USR2
     """
 
     name = "signal"
-    options = [('', 'children', True, "Only signal children of the process")]
+    options = [('', 'children', False, "Only signal children of the process"),
+               ('', 'recursive', False, "Signal parent and children")]
     properties = ['name', 'signum']
 
     def message(self, *args, **opts):
         largs = len(args)
-        if largs < 2 or largs > 4:
+        if largs < 2 or largs > 3:
             raise ArgumentError("Invalid number of arguments")
 
-        if len(args) == 4:
-            signum = self._get_signal(args[3])
-            return self.make_message(name=args[0], process=args[1],
-                                     pid=args[2], signum=signum)
-        elif len(args) == 3:
-            signum = self._get_signal(args[2])
-            children = opts.get("children", False)
-            return self.make_message(name=args[0], process=args[1],
-                                     signum=signum, children=children)
-        else:
-            signum = self._get_signal(args[1])
-            return self.make_message(name=args[0], signum=signum)
+        props = {
+            'name': args[0],
+            'children': opts.get("children", False),
+            'recursive': opts.get("recursive", False),
+            'signum': args[-1],
+        }
+        if len(args) == 3:
+            props['pid'] = args[1]
+        return self.make_message(**props)
 
     def execute(self, arbiter, props):
         name = props.get('name')
         watcher = self._get_watcher(arbiter, name)
         signum = props.get('signum')
-        pid = props.get('pid', None)
+        pids = [props['pid']] if 'pid' in props else watcher.get_active_pids()
         childpid = props.get('childpid', None)
         children = props.get('children', False)
         recursive = props.get('recursive', False)
 
-        if pid:
+        for pid in pids:
             if childpid:
                 watcher.send_signal_child(pid, childpid, signum)
             elif children:
@@ -153,20 +141,14 @@ def execute(self, arbiter, props):
                 if recursive:
                     # also send to the children
                     watcher.send_signal_children(pid, signum)
-        else:
-            # send to all the pids for this watcher
-            watcher.send_signal_processes(signum)
 
     def validate(self, props):
         super(Signal, self).validate(props)
 
-        if 'statspid' in props and not 'pid' in props:
+        if 'childpid' in props and not 'pid' in props:
             raise ArgumentError('cannot specify childpid without pid')
-        if 'children' in props and not 'pid' in props:
-            raise ArgumentError('cannot specify children without pid')
-
-        if 'signum' in props:
-            try:
-                props['signum'] = to_signum(props['signum'])
-            except ValueError:
-                raise MessageError('signal invalid')
+
+        try:
+            props['signum'] = to_signum(props['signum'])
+        except ValueError:
+            raise MessageError('signal invalid')

circus/commands/set.py

@@ -87,7 +87,7 @@ def validate(self, props):
 
         options = props['options']
         if not isinstance(options, dict):
-            raise MessageError("'options' property shoudl be an object")
+            raise MessageError("'options' property should be an object")
 
         for key, val in options.items():
             validate_option(key, val)

circus/commands/util.py

@@ -23,6 +23,8 @@ def convert_option(key, val):
         return util.to_bool(val)
     elif key == "stop_signal":
         return util.to_signum(val)
+    elif key == "stop_children":
+        return util.to_bool(val)
     elif key == "shell":
         return util.to_bool(val)
     elif key == "copy_env":
@@ -80,7 +82,8 @@ def convert_option(key, val):
 
 def validate_option(key, val):
     valid_keys = ('numprocesses', 'warmup_delay', 'working_dir', 'uid',
-                  'gid', 'send_hup', 'shell', 'env', 'cmd', 'copy_env',
+                  'gid', 'send_hup', 'stop_signal', 'stop_children',
+                  'shell', 'env', 'cmd', 'copy_env',
                   'flapping_attempts', 'flapping_window', 'retry_in',
                   'max_retry', 'graceful_timeout', 'stdout_stream',
                   'stderr_stream', 'max_age', 'max_age_variance', 'respawn',
@@ -98,7 +101,7 @@ def _valid_prefix():
         raise MessageError('unknown key %r' % key)
 
     if key in ('numprocesses', 'flapping_attempts', 'max_retry', 'max_age',
-               'max_age_variance'):
+               'max_age_variance', 'stop_signal'):
         if not isinstance(val, int):
             raise MessageError("%r isn't an integer" % key)
 
@@ -111,7 +114,7 @@ def _valid_prefix():
         if not isinstance(val, int) and not isinstance(val, string_types):
             raise MessageError("%r isn't an integer or string" % key)
 
-    if key in ('send_hup', 'shell', 'copy_env', 'respawn'):
+    if key in ('send_hup', 'shell', 'copy_env', 'respawn', 'stop_children'):
         if not isinstance(val, bool):
             raise MessageError("%r isn't a valid boolean" % key)
 

circus/config.py

@@ -24,6 +24,7 @@ def watcher_defaults():
         'gid': None,
         'send_hup': False,
         'stop_signal': signal.SIGTERM,
+        'stop_children': False,
         'max_retry': 5,
         'graceful_timeout': 30,
         'rlimits': dict(),
@@ -221,6 +222,9 @@ def _upper(items):
                                                bool)
                 elif opt == 'stop_signal':
                     watcher['stop_signal'] = to_signum(val)
+                elif opt == 'stop_children':
+                    watcher['stop_children'] = dget(section, 'stop_children',
+                                                    False, bool)
                 elif opt == 'check_flapping':
                     watcher['check_flapping'] = dget(section, 'check_flapping',
                                                      True, bool)

circus/process.py

@@ -313,10 +313,12 @@ def is_child(self, pid):
     @debuglog
     def send_signal_child(self, pid, signum):
         """Send signal *signum* to child *pid*."""
-        children = dict([(child.pid, child)
-                         for child in self._worker.get_children()])
-
-        children[pid].send_signal(signum)
+        children = dict((child.pid, child)
+                        for child in self._worker.get_children())
+        try:
+            children[pid].send_signal(signum)
+        except KeyError:
+            raise NoSuchProcess(pid)
 
     @debuglog
     def send_signal_children(self, signum):

circus/stream/__init__.py

@@ -3,9 +3,9 @@
 
 from datetime import datetime
 try:
-    from queue import Queue
+    from queue import Queue, Empty
 except ImportError:
-    from Queue import Queue
+    from Queue import Queue, Empty  # NOQA
 
 from circus.util import resolve_name
 from circus.stream.file_stream import FileStream

circus/tests/config/issue594.ini

@@ -7,5 +7,6 @@ statsd = true
 
 [watcher:my_app]
 cmd = boo
-graceful_timeout=30
-stop_signal=INT
+graceful_timeout = 30
+stop_signal = INT
+stop_children = true

circus/tests/support.py

@@ -28,6 +28,9 @@
 from circus.client import AsyncCircusClient, make_message
 from circus.stream import QueueStream
 
+if 'ASYNC_TEST_TIMEOUT' not in os.environ:
+    os.environ['ASYNC_TEST_TIMEOUT'] = '10'
+
 
 class EasyTestSuite(TestSuite):
     def __init__(self, name):