Pārlūkot izejas kodu

- Process config reloading implemented by Anders Quist. The
supervisorctl program now has the commands "add" and "drop".
"add <programname>" adds the process group implied by <programname>
in the config file. "drop <programname>" removes the process
group from the running configuration (it must already be stopped).
This makes it possible to add processes to and remove processes from
a running supervisord without restarting the supervisord process.

Chris McDonough 17 gadi atpakaļ
vecāks
revīzija
cf0324d21c

+ 8 - 0
CHANGES.txt

@@ -1,5 +1,13 @@
 Next Release
 
+  - Process config reloading implemented by Anders Quist.  The
+    supervisorctl program now has the commands "add" and "drop".
+    "add <programname>" adds the process group implied by <programname>
+    in the config file.  "drop <programname>" removes the process
+    group from the running configuration (it must already be stopped).
+    This makes it possible to add processes to and remove processes from
+    a running supervisord without restarting the supervisord process.
+
   - Fixed a bug where opening the HTTP servers would fail silently
     for socket errors other than errno.EADDRINUSE.
 

+ 80 - 39
src/supervisor/options.py

@@ -96,6 +96,7 @@ class Options:
         self.default_map = {}
         self.required_map = {}
         self.environ_map = {}
+        self.attr_priorities = {}
         self.add(None, None, "h", "help", self.help)
         self.add("configfile", None, "c:", "configuration=")
 
@@ -216,6 +217,12 @@ class Options:
             if required:
                 self.required_map[name] = required
 
+    def _set(self, attr, value, prio):
+        current = self.attr_priorities.get(attr, -1)
+        if prio >= current:
+            setattr(self, attr, value)
+            self.attr_priorities[attr] = prio
+
     def realize(self, args=None, doc=None,
                 progname=None, raise_getopt_errs=True):
         """Realize a configuration.
@@ -264,13 +271,11 @@ class Options:
             if name and arg is not None:
                 if getattr(self, name) is not None:
                     self.usage("conflicting command line option %r" % opt)
-                setattr(self, name, arg)
+                self._set(name, arg, 2)
 
         # Process environment variables
         for envvar in self.environ_map.keys():
             name, handler = self.environ_map[envvar]
-            if name and getattr(self, name, None) is not None:
-                continue
             if os.environ.has_key(envvar):
                 value = os.environ[envvar]
                 if handler is not None:
@@ -280,11 +285,14 @@ class Options:
                         self.usage("invalid environment value for %s %r: %s"
                                    % (envvar, value, msg))
                 if name and value is not None:
-                    setattr(self, name, value)
+                    self._set(name, value, 1)
 
         if self.configfile is None:
             self.configfile = self.default_configfile()
 
+        self.process_config_file()
+
+    def process_config_file(self):
         # Process config file
         if not hasattr(self.configfile, 'read'):
             self.here = os.path.abspath(os.path.dirname(self.configfile))
@@ -297,7 +305,7 @@ class Options:
         # Copy config options to attributes of self.  This only fills
         # in options that aren't already set from the command line.
         for name, confname in self.names_list:
-            if confname and getattr(self, name) is None:
+            if confname:
                 parts = confname.split(".")
                 obj = self.configroot
                 for part in parts:
@@ -305,7 +313,7 @@ class Options:
                         break
                     # Here AttributeError is not a user error!
                     obj = getattr(obj, part)
-                setattr(self, name, obj)
+                self._set(name, obj, 0)
 
         # Process defaults
         for name, value in self.default_map.items():
@@ -396,6 +404,7 @@ class ServerOptions(Options):
         self.add("profile_options", "supervisord.profile_options",
                  "", "profile_options=", profile_options, default=None)
         self.pidhistory = {}
+        self.process_group_configs = []
         self.parse_warnings = []
 
     def getLogger(self, filename, level, fmt, rotating=False, maxbytes=0,
@@ -432,7 +441,6 @@ class ServerOptions(Options):
 
         self.pidfile = normalize_path(pidfile)
 
-        self.process_group_configs = section.process_group_configs
         self.rpcinterface_factories = section.rpcinterface_factories
 
         self.serverurl = None
@@ -463,6 +471,28 @@ class ServerOptions(Options):
 
         self.identifier = section.identifier
 
+    def diff_process_groups(self, new):
+        cur = self.process_group_configs
+
+        curdict = dict(zip([cfg.name for cfg in cur], cur))
+        newdict = dict(zip([cfg.name for cfg in new], new))
+
+        added   = [cand for cand in new if cand.name not in curdict]
+        removed = [cand for cand in cur if cand.name not in newdict]
+
+        changed = [cand for cand in new
+                   if cand != curdict.get(cand.name, cand)]
+
+        return added, changed, removed
+
+    def process_config_file(self):
+        Options.process_config_file(self)
+
+        new = self.configroot.supervisord.process_group_configs
+        changes = self.diff_process_groups(new)
+        self.process_group_configs = new
+        return changes
+
     def read_config(self, fp):
         section = self.configroot.supervisord
         if not hasattr(fp, 'read'):
@@ -1442,39 +1472,34 @@ class Config:
                                                  self.name)
     
 class ProcessConfig(Config):
-    def __init__(self, options, name, command, directory, umask,
-                 priority, autostart, autorestart, startsecs, startretries, uid,
-                 stdout_logfile, stdout_capture_maxbytes,
-                 stdout_logfile_backups, stdout_logfile_maxbytes,
-                 stderr_logfile, stderr_capture_maxbytes,
-                 stderr_logfile_backups, stderr_logfile_maxbytes,
-                 stopsignal, stopwaitsecs, exitcodes, redirect_stderr,
-                 environment=None, serverurl=None):
+    req_param_names = [
+        'name', 'uid', 'command', 'directory', 'umask', 'priority',
+        'autostart', 'autorestart', 'startsecs', 'startretries',
+        'stdout_logfile', 'stdout_capture_maxbytes',
+        'stdout_logfile_backups', 'stdout_logfile_maxbytes',
+        'stderr_logfile', 'stderr_capture_maxbytes',
+        'stderr_logfile_backups', 'stderr_logfile_maxbytes',
+        'stopsignal', 'stopwaitsecs', 'exitcodes', 'redirect_stderr' ]
+    optional_param_names = [ 'environment', 'serverurl' ]
+
+    def __init__(self, options, **params):
         self.options = options
-        self.name = name
-        self.command = command
-        self.directory = directory
-        self.umask = umask
-        self.priority = priority
-        self.autostart = autostart
-        self.autorestart = autorestart
-        self.startsecs = startsecs
-        self.startretries = startretries
-        self.uid = uid
-        self.stdout_logfile = stdout_logfile
-        self.stdout_capture_maxbytes = stdout_capture_maxbytes
-        self.stdout_logfile_backups = stdout_logfile_backups
-        self.stdout_logfile_maxbytes = stdout_logfile_maxbytes
-        self.stderr_logfile = stderr_logfile
-        self.stderr_capture_maxbytes = stderr_capture_maxbytes
-        self.stderr_logfile_backups = stderr_logfile_backups
-        self.stderr_logfile_maxbytes = stderr_logfile_maxbytes
-        self.stopsignal = stopsignal
-        self.stopwaitsecs = stopwaitsecs
-        self.exitcodes = exitcodes
-        self.redirect_stderr = redirect_stderr
-        self.environment = environment
-        self.serverurl = serverurl
+        for name in self.req_param_names:
+            setattr(self, name, params[name])
+        for name in self.optional_param_names:
+            setattr(self, name, params.get(name, None))
+
+    def __eq__(self, other):
+        if not isinstance(other, ProcessConfig):
+            return False
+
+        for name in self.req_param_names + self.optional_param_names:
+            if Automatic in [getattr(self, name), getattr(other, name)] :
+                continue
+            if getattr(self, name) != getattr(other, name):
+                return False
+
+        return True
 
     def create_autochildlogs(self):
         # temporary logfiles which are erased at start time
@@ -1556,6 +1581,22 @@ class ProcessGroupConfig(Config):
         self.priority = priority
         self.process_configs = process_configs
 
+    def __eq__(self, other):
+        if not isinstance(other, ProcessGroupConfig):
+            return False
+
+        if self.name != other.name:
+            return False
+        if self.priority != other.priority:
+            return False
+        if self.process_configs != other.process_configs:
+            return False
+
+        return True
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
     def after_setuid(self):
         for config in self.process_configs:
             config.create_autochildlogs()

+ 40 - 0
src/supervisor/rpcinterface.py

@@ -162,6 +162,46 @@ class SupervisorNamespaceRPCInterface:
         self.supervisord.options.mood = SupervisorStates.RESTARTING
         return True
 
+    def reloadConfig(self):
+        """
+        Reload configuration
+
+        @return boolean result  always return True unless error
+        """
+        self._update('reloadConfig')
+        added, changed, removed = self.supervisord.options.process_config_file()
+
+        added = [group.name for group in added]
+        changed = [group.name for group in changed]
+        removed = [group.name for group in removed]
+        return [[added, changed, removed]] # cannot return len > 1, apparently
+
+    def addProcess(self, name):
+        """ Update the config for a running process from config file.
+
+        @param string name         name of process to start
+        @return boolean result     true if successful
+        """
+        self._update('addProcess')
+
+        for config in self.supervisord.options.process_group_configs:
+            if config.name == name:
+                return self.supervisord.add_process_group(config)
+
+        return False
+
+    def removeProcess(self, name):
+        """ Remove a stopped process from the active configuration.
+
+        @param string name         name of process to remove
+        @return boolean result     Indicates wether the removal was successful
+        """
+        self._update('removeProcess')
+        if name not in self.supervisord.process_groups:
+            return False
+
+        return self.supervisord.remove_process_group(name)
+
     def _getAllProcesses(self, lexical=False):
         # if lexical is true, return processes sorted in lexical order,
         # otherwise, sort in priority order

+ 68 - 0
src/supervisor/supervisorctl.py

@@ -646,6 +646,74 @@ class DefaultControllerPlugin(ControllerPluginBase):
     def help_reload(self):
         self.ctl.output("reload \t\tRestart the remote supervisord.")
 
+    def _formatChanges(self, (added, changed, dropped)):
+        changedict = {}
+        for n, t in [(added, 'available'),
+                     (changed, 'changed'),
+                     (dropped, 'disappeared')]:
+            changedict.update(dict(zip(n, [t] * len(n))))
+
+        if changedict:
+            for name in sorted(changedict):
+                self.ctl.output("%s: %s" % (name, changedict[name]))
+        else:
+            self.ctl.output("No config updates to proccesses")
+
+    def do_reread(self, arg):
+        supervisor = self.ctl.get_supervisor()
+        try:
+            result = supervisor.reloadConfig()
+        except xmlrpclib.Fault, e:
+            if e.faultCode == xmlrpc.Faults.SHUTDOWN_STATE:
+                self.ctl.output('ERROR: already shutting down')
+        else:
+            self._formatChanges(result[0])
+
+    def help_reread(self):
+        self.ctl.output("reread \t\t\tReload the daemon's configuration files")
+
+    def do_add(self, arg):
+        names = arg.strip().split()
+
+        supervisor = self.ctl.get_supervisor()
+        for name in names:
+            result = supervisor.addProcess(name)
+            if result:
+                self.ctl.output("%s: added process" % name)
+            else:
+                self.ctl.output("%s: ERROR (no such group or already active)" % name)
+
+    def help_add(self):
+        self.ctl.output("add <name> [...]\tActivates any updates in config for process/group")
+
+    def do_drop(self, arg):
+        names = arg.strip().split()
+
+        supervisor = self.ctl.get_supervisor()
+        for name in names:
+            result = supervisor.removeProcess(name)
+            if result:
+                self.ctl.output("%s: dropped" % name)
+            else:
+                self.ctl.output("%s: ERROR (no such group or already dropped)" % name)
+
+    def help_drop(self):
+        self.ctl.output("drop <name> [...]\tRemoves process/group from active config")
+
+    def do_refresh(self, arg):
+        pass
+
+    def help_refresh(self):
+        self.ctl.output("refresh\t\t\tReload config and stop, drop, "
+                        "add and restart \n"
+                        "\t\t\tprocesses as necessary")
+
+    def do_avail(self, arg):
+        pass
+
+    def help_avail(self):
+        self.ctl.output("avail\t\t\tDisplay all configured processes")
+
     def _clearresult(self, result):
         name = result['name']
         code = result['status']

+ 15 - 5
src/supervisor/supervisord.py

@@ -87,9 +87,6 @@ class Supervisor:
             # clean up old automatic logs
             self.options.clear_autochildlogdir()
 
-        for config in self.options.process_group_configs:
-            config.after_setuid()
-
         self.run()
 
     def run(self):
@@ -98,8 +95,7 @@ class Supervisor:
         events.clear()
         try:
             for config in self.options.process_group_configs:
-                name = config.name
-                self.process_groups[name] = config.make_group()
+                self.add_process_group(config)
             self.options.process_environment()
             self.options.openhttpservers(self)
             self.options.setsignals()
@@ -112,6 +108,20 @@ class Supervisor:
         finally:
             self.options.cleanup()
 
+    def add_process_group(self, config):
+        name = config.name
+        if name not in self.process_groups:
+            config.after_setuid()
+            self.process_groups[name] = config.make_group()
+            return True
+        return False
+
+    def remove_process_group(self, name):
+        if self.process_groups[name].get_unstopped_processes():
+            return False
+        del self.process_groups[name]
+        return True
+
     def get_process_map(self):
         process_map = {}
         pgroups = self.process_groups.values()

+ 16 - 0
src/supervisor/tests/base.py

@@ -766,6 +766,18 @@ class DummySupervisorRPCNamespace:
         from supervisor import xmlrpc
         raise Fault(xmlrpc.Faults.SHUTDOWN_STATE, '')
 
+    def reloadConfig(self):
+        return [[['added'], ['changed'], ['dropped']]]
+
+    def addProcess(self, name):
+        if hasattr(self, 'processes'):
+            self.processes.append(name)
+        else:
+            self.processes = [name]
+
+    def removeProcess(self, name):
+        self.processes.remove(name)
+
     def clearProcessStdoutLog(self, name):
         from xmlrpclib import Fault
         from supervisor import xmlrpc
@@ -821,6 +833,10 @@ class DummyPGroupConfig:
     def make_group(self):
         return DummyProcessGroup(self)
 
+    def __repr__(self):
+        return '<%s instance at %s named %s>' % (self.__class__, id(self),
+                                                 self.name)
+
 class DummyFCGIGroupConfig(DummyPGroupConfig):
     def __init__(self, options, name, priority, pconfigs, socket_manager):
         DummyPGroupConfig.__init__(self, options, name, priority, pconfigs)

+ 221 - 0
src/supervisor/tests/test_options.py

@@ -13,10 +13,81 @@ from supervisor.tests.base import DummySupervisor
 from supervisor.tests.base import DummyLogger
 from supervisor.tests.base import DummyOptions
 from supervisor.tests.base import DummyPConfig
+from supervisor.tests.base import DummyPGroupConfig
 from supervisor.tests.base import DummyProcess
 from supervisor.tests.base import DummySocketManager
 from supervisor.tests.base import lstrip
 
+class OptionTests(unittest.TestCase):
+
+    def _makeOptions(self):
+        from cStringIO import StringIO
+        from supervisor.options import Options
+        from supervisor.datatypes import integer
+
+        class MyOptions(Options):
+            master = {
+                'other': 41 }
+            def __init__(self):
+                Options.__init__(self)
+                class Foo(object): pass
+                self.configroot = Foo()
+
+            def read_config(self, fp):
+                # Pretend we read it from file:
+                self.configroot.__dict__.update(self.default_map)
+                self.configroot.__dict__.update(self.master)
+
+        options = MyOptions()
+        options.configfile = StringIO()
+        options.add(name='anoption', confname='anoption',
+                    short='o', long='option', default='default')
+        options.add(name='other', confname='other', env='OTHER',
+                    short='p:', long='other=', handler=integer)
+        return options
+
+    def test_options_and_args_order(self):
+        # Only config file exists
+        options = self._makeOptions()
+        options.realize([])
+        self.assertEquals(options.anoption, 'default')
+        self.assertEquals(options.other, 41)
+
+        # Env should trump config
+        options = self._makeOptions()
+        os.environ['OTHER'] = '42'
+        options.realize([])
+        self.assertEquals(options.other, 42)
+
+        # Opt should trump both env (still set) and config
+        options = self._makeOptions()
+        options.realize(['-p', '43'])
+        self.assertEquals(options.other, 43)
+        del os.environ['OTHER']
+
+    def test_config_reload(self):
+        options = self._makeOptions()
+        options.realize([])
+        self.assertEquals(options.other, 41)
+        options.master['other'] = 42
+        options.process_config_file()
+        self.assertEquals(options.other, 42)
+
+    def test__set(self):
+        from supervisor.options import Options
+        options = Options()
+        options._set('foo', 'bar', 0)
+        self.assertEquals(options.foo, 'bar')
+        self.assertEquals(options.attr_priorities['foo'], 0)
+        options._set('foo', 'baz', 1)
+        self.assertEquals(options.foo, 'baz')
+        self.assertEquals(options.attr_priorities['foo'], 1)
+        options._set('foo', 'gazonk', 0)
+        self.assertEquals(options.foo, 'baz')
+        self.assertEquals(options.attr_priorities['foo'], 1)
+        options._set('foo', 'gazonk', 1)
+        self.assertEquals(options.foo, 'gazonk')
+
 class ClientOptionsTests(unittest.TestCase):
     def _getTargetClass(self):
         from supervisor.options import ClientOptions
@@ -272,6 +343,156 @@ class ServerOptionsTests(unittest.TestCase):
         self.assertEqual(instance.minfds, 2048)
         self.assertEqual(instance.minprocs, 300)
 
+    def test_reload(self):
+        from cStringIO import StringIO
+        text = lstrip("""\
+        [supervisord]
+        user=root
+
+        [program:one]
+        command = /bin/cat
+
+        [program:two]
+        command = /bin/dog
+
+        [program:four]
+        command = /bin/sheep
+
+        [group:thegroup]
+        programs = one,two
+        """)
+
+        instance = self._makeOne()
+        instance.configfile = StringIO(text)
+        instance.realize(args=[])
+
+        section = instance.configroot.supervisord
+        self.assertEqual(len(section.process_group_configs), 2)
+        cat = section.process_group_configs[0]
+        self.assertEqual(len(cat.process_configs), 2)
+        self.assertTrue(section.process_group_configs is
+                        instance.process_group_configs)
+
+        text = lstrip("""\
+        [supervisord]
+        user=root
+
+        [program:one]
+        command = /bin/cat
+
+        [program:three]
+        command = /bin/pig
+
+        [group:thegroup]
+        programs = three
+        """)
+        instance.configfile = StringIO(text)
+        instance.process_config_file()
+
+        section = instance.configroot.supervisord
+        self.assertEqual(len(section.process_group_configs), 2)
+        cat = section.process_group_configs[0]
+        self.assertEqual(len(cat.process_configs), 1)
+        proc = cat.process_configs[0]
+        self.assertEqual(proc.name, 'three')
+        self.assertEqual(proc.command, '/bin/pig')
+
+        cat = section.process_group_configs[1]
+        self.assertEqual(len(cat.process_configs), 1)
+        proc = cat.process_configs[0]
+        self.assertEqual(proc.name, 'one')
+        self.assertEqual(proc.command, '/bin/cat')
+        self.assertTrue(section.process_group_configs is
+                        instance.process_group_configs)
+
+    def test_diff_add_remove(self):
+        options = self._makeOne()
+
+        pconfig = DummyPConfig(options, 'process1', 'process1')
+        group1 = DummyPGroupConfig(options, 'group1', pconfigs=[pconfig])
+
+        pconfig = DummyPConfig(options, 'process2', 'process2')
+        group2 = DummyPGroupConfig(options, 'group2', pconfigs=[pconfig])
+
+        new = [group1, group2]
+
+        added, changed, removed = options.diff_process_groups([])
+        self.assertEqual(added, options.process_group_configs)
+        self.assertEqual(removed, [])
+
+        options.process_group_configs = list(new)
+        added, changed, removed = options.diff_process_groups(new)
+        self.assertEqual(added, [])
+        self.assertEqual(changed, [])
+        self.assertEqual(removed, [])
+
+        pconfig = DummyPConfig(options, 'process3', 'process3')
+        new_group1 = DummyPGroupConfig(options, pconfigs=[pconfig])
+
+        pconfig = DummyPConfig(options, 'process4', 'process4')
+        new_group2 = DummyPGroupConfig(options, pconfigs=[pconfig])
+
+        new = [group2, new_group1, new_group2]
+
+        added, changed, removed = options.diff_process_groups(new)
+        self.assertEqual(added, [new_group1, new_group2])
+        self.assertEqual(changed, [])
+        self.assertEqual(removed, [group1])
+
+    def test_diff_changed(self):
+        from supervisor.options import ProcessConfig, ProcessGroupConfig
+
+        options = self._makeOne()
+
+        def make_pconfig(name, command, **params):
+            result = {
+                'name': name, 'command': command,
+                'directory': None, 'umask': None, 'priority': 999, 'autostart': True,
+                'autorestart': True, 'startsecs': 10, 'startretries': 999,
+                'uid': None, 'stdout_logfile': None, 'stdout_capture_maxbytes': 0,
+                'stdout_logfile_backups': 0, 'stdout_logfile_maxbytes': 0,
+                'stderr_logfile': None, 'stderr_capture_maxbytes': 0,
+                'stderr_logfile_backups': 0, 'stderr_logfile_maxbytes': 0,
+                'redirect_stderr': False,
+                'stopsignal': None, 'stopwaitsecs': 10,
+                'exitcodes': (0,2), 'environment': None, 'serverurl': None }
+            result.update(params)
+            return ProcessConfig(options, **result)
+
+        def make_gconfig(name, pconfigs):
+            return ProcessGroupConfig(options, name, 25, pconfigs)
+
+        pconfig = make_pconfig('process1', 'process1', uid='new')
+        group1 = make_gconfig('group1', [pconfig])
+
+        pconfig = make_pconfig('process2', 'process2')
+        group2 = make_gconfig('group2', [pconfig])
+        new = [group1, group2]
+
+        pconfig = make_pconfig('process1', 'process1', uid='old')
+        group3 = make_gconfig('group1', [pconfig])
+
+        pconfig = make_pconfig('process2', 'process2')
+        group4 = make_gconfig('group2', [pconfig])
+        options.process_group_configs = [group4, group3]
+
+        added, changed, removed = options.diff_process_groups(new)
+
+        self.assertEqual([added, removed], [[], []])
+        self.assertEqual(changed, [group1])
+
+        options = self._makeOne()
+        pconfig1 = make_pconfig('process1', 'process1')
+        pconfig2 = make_pconfig('process2', 'process2')
+        group1 = make_gconfig('group1', [pconfig1, pconfig2])
+        new = [group1]
+
+        options.process_group_configs = [make_gconfig('group1', [pconfig1])]
+
+        added, changed, removed = options.diff_process_groups(new)
+        self.assertEqual([added, removed], [[], []])
+        self.assertEqual(changed, [group1])
+
     def test_readFile_failed(self):
         from supervisor.options import readFile
         try:

+ 52 - 0
src/supervisor/tests/test_rpcinterfaces.py

@@ -218,6 +218,58 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
         self.assertEqual(value, True)
         self.assertEqual(supervisord.options.mood, 0)
 
+    def test_reloadConfig(self):
+        options = DummyOptions()
+        supervisord = DummySupervisor(options)
+        interface = self._makeOne(supervisord)
+
+        changes = [ [DummyPGroupConfig(options, 'added')],
+                    [DummyPGroupConfig(options, 'changed')],
+                    [DummyPGroupConfig(options, 'dropped')] ]
+
+        supervisord.options.process_config_file = \
+            lambda : changes
+
+        value = interface.reloadConfig()
+        self.assertEqual(value, [[['added'], ['changed'], ['dropped']]])
+
+    def test_addProcess(self):
+        from supervisor.supervisord import Supervisor
+        options = DummyOptions()
+        supervisord = Supervisor(options)
+        pconfig = DummyPConfig(options, 'foo', __file__, autostart=False)
+        gconfig = DummyPGroupConfig(options, 'group1', pconfigs=[pconfig])
+        supervisord.options.process_group_configs = [gconfig]
+
+        interface = self._makeOne(supervisord)
+
+        result = interface.addProcess('group1')
+        self.assertTrue(result)
+        self.assertEqual(supervisord.process_groups.keys(), ['group1'])
+
+        result = interface.addProcess('group1')
+        self.assertTrue(not result)
+        self.assertEqual(supervisord.process_groups.keys(), ['group1'])
+
+        result = interface.addProcess('asdf')
+        self.assertTrue(not result)
+        self.assertEqual(supervisord.process_groups.keys(), ['group1'])
+
+    def test_removeProcess(self):
+        from supervisor.supervisord import Supervisor
+        options = DummyOptions()
+        supervisord = Supervisor(options)
+        pconfig = DummyPConfig(options, 'foo', __file__, autostart=False)
+        gconfig = DummyPGroupConfig(options, 'group1', pconfigs=[pconfig])
+        supervisord.options.process_group_configs = [gconfig]
+
+        interface = self._makeOne(supervisord)
+
+        interface.addProcess('group1')
+        result = interface.removeProcess('group1')
+        self.assertTrue(result)
+        self.assertEqual(supervisord.process_groups.keys(), [])
+
     def test_startProcess_already_started(self):
         from supervisor import xmlrpc
         options = DummyOptions()

+ 29 - 0
src/supervisor/tests/test_supervisorctl.py

@@ -460,6 +460,35 @@ class TestDefaultControllerPlugin(unittest.TestCase):
         self.assertEqual(result, None)
         self.assertEqual(options._server.supervisor._shutdown, True)
 
+    def test__formatChanges(self):
+        plugin = self._makeOne()
+        # Don't explode, plz
+        plugin._formatChanges([['added'], ['changed'], ['dropped']])
+        plugin._formatChanges([[], [], []])
+
+    def test_reread(self):
+        plugin = self._makeOne()
+        calls = []
+        plugin._formatChanges = lambda x: calls.append(x)
+        result = plugin.do_reread(None)
+        self.assertEqual(result, None)
+        self.assertEqual(calls[0], [['added'], ['changed'], ['dropped']])
+
+    def test_add(self):
+        plugin = self._makeOne()
+        result = plugin.do_add('foo')
+        self.assertEqual(result, None)
+        supervisor = plugin.ctl.options._server.supervisor
+        self.assertEqual(supervisor.processes, ['foo'])
+
+    def test_drop(self):
+        plugin = self._makeOne()
+        supervisor = plugin.ctl.options._server.supervisor
+        supervisor.processes = ['foo']
+        result = plugin.do_drop('foo')
+        self.assertEqual(result, None)
+        self.assertEqual(supervisor.processes, [])
+
     def test_pid(self):
         plugin = self._makeOne()
         result = plugin.do_pid('')

+ 38 - 0
src/supervisor/tests/test_supervisord.py

@@ -173,6 +173,44 @@ class SupervisordTests(unittest.TestCase):
         self.assertEqual(options.logger.data[0],
                          'received SIGUSR1 indicating nothing')
 
+    def test_add_process_group(self):
+        options = DummyOptions()
+        pconfig = DummyPConfig(options, 'foo', 'foo', '/bin/foo')
+        gconfig = DummyPGroupConfig(options,'foo', pconfigs=[pconfig])
+        options.process_group_configs = [gconfig]
+        supervisord = self._makeOne(options)
+
+        self.assertEqual(supervisord.process_groups, {})
+
+        result = supervisord.add_process_group(gconfig)
+        self.assertEqual(supervisord.process_groups.keys(), ['foo'])
+        self.assertTrue(result)
+
+        group = supervisord.process_groups['foo']
+        result = supervisord.add_process_group(gconfig)
+        self.assertEqual(group, supervisord.process_groups['foo'])
+        self.assertTrue(not result)
+
+    def test_remove_process_group(self):
+        from supervisor.states import ProcessStates
+        options = DummyOptions()
+        pconfig = DummyPConfig(options, 'foo', 'foo', '/bin/foo')
+        gconfig = DummyPGroupConfig(options, 'foo', pconfigs=[pconfig])
+        supervisord = self._makeOne(options)
+
+        self.assertRaises(KeyError, supervisord.remove_process_group, 'asdf')
+
+        supervisord.add_process_group(gconfig)
+        result = supervisord.remove_process_group('foo')
+        self.assertEqual(supervisord.process_groups, {})
+        self.assertTrue(result)
+
+        supervisord.add_process_group(gconfig)
+        supervisord.process_groups['foo'].unstopped_processes = [DummyProcess(None)]
+        result = supervisord.remove_process_group('foo')
+        self.assertEqual(supervisord.process_groups.keys(), ['foo'])
+        self.assertTrue(not result)
+
     def test_runforever_emits_generic_startup_event(self):
         from supervisor import events
         L = []