BIND 10 trac213, updated. 18e970e16c5044da8b4a7d2c800f0b7baeab9f96 [213] Unify _old_config and _components

BIND 10 source code commits bind10-changes at lists.isc.org
Sat Oct 22 12:59:59 UTC 2011


The branch, trac213 has been updated
       via  18e970e16c5044da8b4a7d2c800f0b7baeab9f96 (commit)
       via  0b145510ca7b6d4cfe8bc43cd6de2563907dfca3 (commit)
       via  72f4baca540cc17e18da4632cb4d32df29f3a9a3 (commit)
       via  86123d1dc31432d176eb54fa300eb65e269df0f4 (commit)
       via  7e874ac36e4086fc0ff9b50537ffdbaeb685ed09 (commit)
       via  f0f4387faa4f6246546ee4b79e6289dd370913d1 (commit)
       via  13c03c7116df55fa0aad790c2b2a88f3743ba95b (commit)
       via  65b9917a960e8b49a947bed1886d1331155b95f5 (commit)
       via  5d4e05531e443e355fbf8369a37efc239d1c95c4 (commit)
       via  c92981134284041b71efc68cff49fead91368e47 (commit)
       via  60c6d07decbe759bb57da7dfafc79e71c52a9c6c (commit)
       via  5634285ef8bed69dcceab61e84b7aefdf1c1ef5d (commit)
       via  e0c15795fa09d93fa8c6e3aa0722ca9ed01b61a0 (commit)
       via  27f88f2ed0a0a7541f3ea9c6d95db5c805e4b062 (commit)
       via  1adb9636b2ba1314140411cd142f9b2f95afede9 (commit)
       via  439b8e22a099e641bbe9236bc44beed78634568d (commit)
      from  7f150769d5e3485cd801f0b5ab9b1d3b25aae520 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 18e970e16c5044da8b4a7d2c800f0b7baeab9f96
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:57:54 2011 +0200

    [213] Unify _old_config and _components

commit 0b145510ca7b6d4cfe8bc43cd6de2563907dfca3
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:36:30 2011 +0200

    [213] Extend __check_dead

commit 72f4baca540cc17e18da4632cb4d32df29f3a9a3
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:32:16 2011 +0200

    [213] Use running(), not _running
    
    As it was a leftover.

commit 86123d1dc31432d176eb54fa300eb65e269df0f4
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:25:49 2011 +0200

    [213] Added check for missing specs

commit 7e874ac36e4086fc0ff9b50537ffdbaeb685ed09
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:23:22 2011 +0200

    [213] Variable names, docs

commit f0f4387faa4f6246546ee4b79e6289dd370913d1
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:18:58 2011 +0200

    [213] Some documentation of data structures

commit 13c03c7116df55fa0aad790c2b2a88f3743ba95b
Merge: 65b9917a960e8b49a947bed1886d1331155b95f5 7f150769d5e3485cd801f0b5ab9b1d3b25aae520
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 14:17:50 2011 +0200

    Merge branch 'trac213' of git+ssh://git.bind10.isc.org/var/bind10/git/bind10 into work/bossstart
    
    Conflicts:
    	src/lib/python/isc/bind10/component.py

commit 65b9917a960e8b49a947bed1886d1331155b95f5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 13:28:24 2011 +0200

    [213] Pass as parameter, not modify afterwards
    
    It is cleaner to pass the parameter to parent class constructor than
    overwrite the protected member afterwards.

commit 5d4e05531e443e355fbf8369a37efc239d1c95c4
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 13:25:29 2011 +0200

    [213] Remove leftover check

commit c92981134284041b71efc68cff49fead91368e47
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 13:24:42 2011 +0200

    [213] Mark the internal methods as protected

commit 60c6d07decbe759bb57da7dfafc79e71c52a9c6c
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 13:15:52 2011 +0200

    [213] More docs about customizing component
    
    Some methods are marked when and if they could be overriden. It also
    mentions the _start_func hook.

commit 5634285ef8bed69dcceab61e84b7aefdf1c1ef5d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 12:58:40 2011 +0200

    [213] Documentation about being dead

commit e0c15795fa09d93fa8c6e3aa0722ca9ed01b61a0
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 11:31:55 2011 +0200

    [213] State transitions & picture

commit 27f88f2ed0a0a7541f3ea9c6d95db5c805e4b062
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 11:12:10 2011 +0200

    [213] Move special components out

commit 1adb9636b2ba1314140411cd142f9b2f95afede9
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 10:51:15 2011 +0200

    [213] Use constants for commands and times in component

commit 439b8e22a099e641bbe9236bc44beed78634568d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Sat Oct 22 10:44:09 2011 +0200

    [213] Don't kill components that no longer run

-----------------------------------------------------------------------

Summary of changes:
 src/bin/bind10/bind10_src.py.in                   |   17 +-
 src/bin/bind10/tests/bind10_test.py.in            |   13 +
 src/lib/python/isc/bind10/Makefile.am             |    2 +-
 src/lib/python/isc/bind10/component.py            |  291 ++++++++++++---------
 src/lib/python/isc/bind10/special_component.py    |   93 +++++++
 src/lib/python/isc/bind10/tests/component_test.py |  166 ++++++++-----
 6 files changed, 392 insertions(+), 190 deletions(-)
 create mode 100644 src/lib/python/isc/bind10/special_component.py

-----------------------------------------------------------------------
diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in
index ae9bf0b..4ce35c0 100755
--- a/src/bin/bind10/bind10_src.py.in
+++ b/src/bin/bind10/bind10_src.py.in
@@ -68,6 +68,7 @@ import isc.net.parse
 import isc.log
 from isc.log_messages.bind10_messages import *
 import isc.bind10.component
+import isc.bind10.special_component
 
 isc.log.init("b10-boss")
 logger = isc.log.Logger("boss")
@@ -242,7 +243,8 @@ class BoB:
         self.config_filename = config_filename
         self.cmdctl_port = cmdctl_port
         self.brittle = brittle
-        self._component_configurator = isc.bind10.component.Configurator(self)
+        self._component_configurator = isc.bind10.component.Configurator(self,
+            isc.bind10.special_component.get_specials())
         self.__core_components = {
             'sockcreator': {
                 'kind': 'core',
@@ -642,6 +644,9 @@ class BoB:
         # next try sending a SIGTERM
         processes_to_stop = list(self.processes.values())
         for component in processes_to_stop:
+            if component.pid() is None:
+                # This isn't running any more for some reason
+                continue
             logger.info(BIND10_SEND_SIGTERM, component.name(),
                         component.pid())
             try:
@@ -651,12 +656,20 @@ class BoB:
                 # finally exited)
                 pass
         # finally, send SIGKILL (unmaskable termination) until everybody dies
-        while self.processes:
+        alive = self.processes # Is there any process alive?
+        # We set alive to false at the start of each killing and reset it
+        # to true whenever we find a component that still lives.
+        while alive:
             # XXX: some delay probably useful... how much is uncertain
             time.sleep(0.1)
             self.reap_children()
             processes_to_stop = list(self.processes.values())
+            alive = False
             for component in processes_to_stop:
+                if component.pid() is None:
+                    # This isn't running any more for some reason
+                    continue
+                alive = True
                 logger.info(BIND10_SEND_SIGKILL, component.name(),
                             component.pid())
                 try:
diff --git a/src/bin/bind10/tests/bind10_test.py.in b/src/bin/bind10/tests/bind10_test.py.in
index 5b84caa..3118db3 100644
--- a/src/bin/bind10/tests/bind10_test.py.in
+++ b/src/bin/bind10/tests/bind10_test.py.in
@@ -329,6 +329,19 @@ class TestBossComponents(unittest.TestCase):
         bob.start_all_processes()
         self.__check_extended(self.__param)
 
+    def test_shutdown_no_pid(self):
+        """
+        Test the boss doesn't fail when ve don't have a PID of a component
+        (which means it's not running).
+        """
+        bob = MockBob()
+        class NoComponent:
+            def pid(self):
+                return None
+        bob.processes = {}
+        bob.register_process(1, NoComponent())
+        bob.shutdown()
+
 class TestBossCmd(unittest.TestCase):
     def test_ping(self):
         """
diff --git a/src/lib/python/isc/bind10/Makefile.am b/src/lib/python/isc/bind10/Makefile.am
index 6a4fef1..c0f1e32 100644
--- a/src/lib/python/isc/bind10/Makefile.am
+++ b/src/lib/python/isc/bind10/Makefile.am
@@ -1,4 +1,4 @@
 SUBDIRS = . tests
 
-python_PYTHON = __init__.py sockcreator.py component.py
+python_PYTHON = __init__.py sockcreator.py component.py special_component.py
 pythondir = $(pyexecdir)/isc/bind10
diff --git a/src/lib/python/isc/bind10/component.py b/src/lib/python/isc/bind10/component.py
index 0fbe21d..9e28305 100644
--- a/src/lib/python/isc/bind10/component.py
+++ b/src/lib/python/isc/bind10/component.py
@@ -22,23 +22,62 @@ Dependencies between them are not yet handled. It might turn out they are
 needed, in that case they will be added sometime in future.
 """
 
-import isc.bind10.sockcreator
 import isc.log
 from isc.log_messages.bind10_messages import *
 import time
-from bind10_config import LIBEXECDIR
-import os
 
 logger = isc.log.Logger("boss")
 DBG_TRACE_DATA = 20
 DBG_TRACE_DETAILED = 80
 
+START_CMD = 'start'
+STOP_CMD = 'stop'
+
+STARTED_OK_TIME = 10
+
+STATE_DEAD = 'dead'
+STATE_STOPPED = 'stopped'
+STATE_RUNNING = 'running'
+
 class Component:
     """
     This represents a single component. It has some defaults of behaviour,
     which should be reasonable for majority of ordinary components, but
     it might be inherited and modified for special-purpose components,
-    like the core modules with different ways of starting up.
+    like the core modules with different ways of starting up. Another
+    way to tweak only the start of the component (eg. by providing some
+    command line parameters) is to set _start_func function from within
+    inherited class.
+
+    The methods are marked if it is expected for them to be overridden.
+
+
+    The component is in one of the three states:
+    - Stopped - it is either not started yet or it was explicitly stopped.
+      The component is created in this state (it must be asked to start
+      explicitly).
+    - Running - after start() was called, it started successfully and is
+      now running.
+    - Dead - it failed and can not be resurrected.
+
+    Init
+      |            stop()
+      |  +-----------------------+
+      |  |                       |
+      v  |  start()  success     |
+    Stopped --------+--------> Running <----------+
+                    |            |                |
+                    |failure     | failed()       |
+                    |            |                |
+                    v            |                |
+                    +<-----------+                |
+                    |                             |
+                    |  kind == dispensable or kind|== needed and failed late
+                    +-----------------------------+
+                    |
+                    | kind == core or kind == needed and it failed too soon
+                    v
+                  Dead
     """
     def __init__(self, process, boss, kind, address=None, params=None):
         """
@@ -70,42 +109,52 @@ class Component:
         """
         if kind not in ['core', 'needed', 'dispensable']:
             raise ValueError('Component kind can not be ' + kind)
-        self.__running = False
-        # Dead like really dead. No resurrection possible.
-        self.__dead = False
+        self.__state = STATE_STOPPED
         self._kind = kind
         self._boss = boss
         self._process = process
         self._start_func = None
         self._address = address
         self._params = params
+        self._procinfo = None
 
     def start(self):
         """
         Start the component for the first time or restart it. If you need to
         modify the way a component is started, do not replace this method,
-        but start_internal. This one does some more bookkeeping around.
+        but _start_internal. This one does some more bookkeeping around.
 
         If you try to start an already running component, it raises ValueError.
         """
-        if self.__dead:
+        if self.__state == STATE_DEAD:
             raise ValueError("Can't resurrect already dead component")
         if self.running():
             raise ValueError("Can't start already running component")
         logger.info(BIND10_COMPONENT_START, self.name())
-        self.__running = True
+        self.__state = STATE_RUNNING
         self.__start_time = time.time()
         try:
-            self.start_internal()
+            self._start_internal()
         except Exception as e:
             logger.error(BIND10_COMPONENT_START_EXCEPTION, self.name(), e)
             self.failed()
             raise
 
-    def start_internal(self):
+    def _start_internal(self):
         """
         This method does the actual starting of a process. If you need to
         change the way the component is started, replace this method.
+
+        You can change the "core" of this function by setting self._start_func
+        to a function without parameters. Such function should start the
+        process and return the procinfo object describing the running process.
+
+        If you don't provide the _start_func, the usual startup by calling
+        boss.start_simple is performed.
+
+        If you override the method completely, you should consider overriding
+        pid and _stop_internal (and possibly _failed_internal and name) as well.
+        You should also register any processes started within boss.
         """
         # This one is not tested. For one, it starts a real process
         # which is out of scope of unit tests, for another, it just
@@ -123,7 +172,7 @@ class Component:
     def stop(self):
         """
         Stop the component. If you need to modify the way a component is
-        stopped, do not replace this method, but stop_internal. This one
+        stopped, do not replace this method, but _stop_internal. This one
         does some more bookkeeping.
 
         If you try to stop a component that is not running, it raises
@@ -134,13 +183,17 @@ class Component:
         if not self.running():
             raise ValueError("Can't stop a component which is not running")
         logger.info(BIND10_COMPONENT_STOP, self.name())
-        self.__running = False
-        self.stop_internal()
+        self.__state = STATE_STOPPED
+        self._stop_internal()
 
-    def stop_internal(self):
+    def _stop_internal(self):
         """
         This is the method that does the actual stopping of a component.
         You can replace this method if you want a different way to do it.
+
+        If you're overriding this one, you probably want to replace the
+        _start_internal and pid methods (and maybe _failed_internal and
+        name as well).
         """
         self._boss.stop_process(self._process, self._address)
 
@@ -150,16 +203,23 @@ class Component:
 
         If you try to call failed on a component that is not running,
         a ValueError is raised.
+
+        If it is a core component or needed component and it was started only
+        recently, the component will become dead and will ask the boss to shut
+        down with error exit status. A dead component can't be started again.
+
+        Otherwise the component will try to restart.
         """
         if not self.running():
             raise ValueError("Can't fail component that isn't running")
-        self.__running = False
-        self.failed_internal()
+        self.__state = STATE_STOPPED
+        self._failed_internal()
         # If it is a core component or the needed component failed to start
         # (including it stopped really soon)
         if self._kind == 'core' or \
-            (self._kind == 'needed' and time.time() - 10 < self.__start_time):
-            self.__dead = True
+            (self._kind == 'needed' and time.time() - STARTED_OK_TIME <
+             self.__start_time):
+            self.__state = STATE_DEAD
             logger.fatal(BIND10_COMPONENT_UNSATISFIED, self.name())
             self._boss.component_shutdown(1)
         # This means we want to restart
@@ -167,7 +227,7 @@ class Component:
             logger.warn(BIND10_COMPONENT_RESTART, self.name())
             self.start()
 
-    def failed_internal(self):
+    def _failed_internal(self):
         """
         This method is called from failed. You can replace it if you need
         some specific behaviour when the component crashes. The default
@@ -183,14 +243,16 @@ class Component:
         Informs if the component is currently running. It assumes the failed
         is called whenever the component really fails and there might be some
         time in between actual failure and the call.
+
+        It is not expected for this method to be overriden.
         """
-        return self.__running
+        return self.__state == STATE_RUNNING
 
     def name(self):
         """
         Returns human-readable name of the component. This is usually the
         name of the executable, but it might be something different in a
-        derived class.
+        derived class (in case it is overriden).
         """
         return self._process
 
@@ -199,88 +261,13 @@ class Component:
         Provides a PID of a process, if the component is real running process.
         This implementation expects it to be a real process, but derived class
         may return None in case the component is something else.
-        """
-        return self._procinfo.pid
 
-# These are specialized components. Some of them are components which need
-# special care (like the message queue or socket creator) or they need
-# some parameters constructed from Boss's command line. They are not tested
-# currently, because it is not clear what to test on them anyway and they just
-# delegate the work for the boss
-class SockCreator(Component):
-    """
-    The socket creator component. Will start and stop the socket creator
-    accordingly.
-    """
-    def start_internal(self):
-        self._boss.curproc = 'b10-sockcreator'
-        self.__creator = isc.bind10.sockcreator.Creator(LIBEXECDIR + ':' +
-                                                        os.environ['PATH'])
-        self._boss.register_process(self.pid(), self)
+        This returns None in case it is not yet running.
 
-    def stop_internal(self):
-        if self.__creator is None:
-            return
-        self.__creator.terminate()
-        self.__creator = None
-
-    def pid(self):
+        You probably want to override this method if you're providing custom
+        _start_internal.
         """
-        Pid of the socket creator. It is provided differently from a usual
-        component.
-        """
-        return self.__creator.pid()
-
-class Msgq(Component):
-    """
-    The message queue. Starting is passed to boss, stopping is not supported
-    and we leave the boss kill it by signal.
-    """
-    def __init__(self, process, boss, kind, address, params):
-        Component.__init__(self, process, boss, kind)
-        self._start_func = boss.start_msgq
-
-    def stop_internal(self):
-        pass # Wait for the boss to actually kill it. There's no stop command.
-
-class CfgMgr(Component):
-    def __init__(self, process, boss, kind, address, params):
-        Component.__init__(self, process, boss, kind)
-        self._start_func = boss.start_cfgmgr
-        self._address = 'ConfigManager'
-
-class Auth(Component):
-    def __init__(self, process, boss, kind, address, params):
-        Component.__init__(self, process, boss, kind)
-        self._start_func = boss.start_auth
-        self._address = 'Auth'
-
-class Resolver(Component):
-    def __init__(self, process, boss, kind, address, params):
-        Component.__init__(self, process, boss, kind)
-        self._start_func = boss.start_resolver
-        self._address = 'Resolver'
-
-class CmdCtl(Component):
-    def __init__(self, process, boss, kind, address, params):
-        Component.__init__(self, process, boss, kind)
-        self._start_func = boss.start_cmdctl
-        self._address = 'Cmdctl'
-
-specials = {
-    'sockcreator': SockCreator,
-    'msgq': Msgq,
-    'cfgmgr': CfgMgr,
-    # TODO: Should these be replaced by configuration in config manager only?
-    # They should not have any parameters anyway
-    'auth': Auth,
-    'resolver': Resolver,
-    'cmdctl': CmdCtl
-}
-"""
-List of specially started components. Each one should be the class than can
-be created for that component.
-"""
+        return self._procinfo.pid if self._procinfo else None
 
 class Configurator:
     """
@@ -292,8 +279,38 @@ class Configurator:
     component. There should be some kind of layer protecting users from ever
     doing so (users must not stop the config manager, message queue and stuff
     like that or the system won't start again).
+
+    The parameters are:
+    * `boss`: The boss we are managing for.
+    * `specials`: Dict of specially started components. Each item is a class
+      representing the component.
+
+    The configuration passed to it (by startup() and reconfigure()) is a
+    dictionary, each item represents one component that should be running.
+    The key is an unique identifier used to reference the component. The
+    value is a dictionary describing the component. All items in the
+    description is optional and they are as follows:
+    * `special` - Some components are started in a special way. If it is
+      present, it specifies which class from the specials parameter should
+      be used to create the component. In that case, some of the following
+      items might be irrelevant, depending on the special component choosen.
+      If it is not there, the basic Component class is used.
+    * `process` - Name of the executable to start. If it is not present,
+      it defaults to the identifier of the component.
+    * `kind` - The kind of component, either of 'core', 'needed' and
+      'dispensable'. This specifies what happens if the component fails.
+      Defaults to despensable.
+    * `address` - The address of the component on message bus. It is used
+      to shut down the component. All special components currently either
+      know their own address or don't need one and ignore it. The common
+      components should provide this.
+    * `params` - The command line parameters of the executable. Defaults
+      to no parameters. It is currently unused.
+    * `priority` - When starting the component, the components with higher
+      priority are started before the ones with lower priority. If it is
+      not present, it defaults to 0.
     """
-    def __init__(self, boss):
+    def __init__(self, boss, specials = {}):
         """
         Initializes the configurator, but nothing is started yet.
 
@@ -302,16 +319,17 @@ class Configurator:
         self.__boss = boss
         # These could be __private, but as we access them from within unittest,
         # it's more comfortable to have them just _protected.
+
+        # They are tuples (configuration, component)
         self._components = {}
-        self._old_config = {}
         self._running = False
+        self.__specials = specials
 
     def __reconfigure_internal(self, old, new):
         """
         Does a switch from one configuration to another.
         """
         self._run_plan(self._build_plan(old, new))
-        self._old_config = new
 
     def startup(self, configuration):
         """
@@ -323,7 +341,7 @@ class Configurator:
             raise ValueError("Trying to start the component configurator " +
                              "twice")
         logger.info(BIND10_CONFIGURATOR_START)
-        self.__reconfigure_internal({}, configuration)
+        self.__reconfigure_internal(self._components, configuration)
         self._running = True
 
     def shutdown(self):
@@ -335,18 +353,21 @@ class Configurator:
                              "configurator while it's not yet running")
         logger.info(BIND10_CONFIGURATOR_STOP)
         self._running = False
-        self.__reconfigure_internal(self._old_config, {})
+        self.__reconfigure_internal(self._components, {})
 
     def reconfigure(self, configuration):
         """
         Changes configuration from the current one to the provided. It
-        starts and stops all the components as needed.
+        starts and stops all the components as needed (eg. if there's
+        a component that was not in the original configuration, it is
+        started, any component that was in the old and is not in the
+        new one is stopped).
         """
         if not self._running:
             raise ValueError("Trying to reconfigure the component " +
                              "configurator while it's not yet running")
         logger.info(BIND10_CONFIGURATOR_RECONFIGURE)
-        self.__reconfigure_internal(self._old_config, configuration)
+        self.__reconfigure_internal(self._components, configuration)
 
     def _build_plan(self, old, new):
         """
@@ -364,18 +385,19 @@ class Configurator:
         # Handle removals of old components
         for cname in old.keys():
             if cname not in new:
-                component = self._components[cname]
+                component = self._components[cname][1]
                 if component.running():
                     plan.append({
-                        'command': 'stop',
+                        'command': STOP_CMD,
                         'component': component,
                         'name': cname
                     })
         # Handle transitions of configuration of what is here
         for cname in new.keys():
             if cname in old:
-                for option in ['special', 'process', 'kind']:
-                    if new[cname].get(option) != old[cname].get(option):
+                for option in ['special', 'process', 'kind', 'address',
+                               'params']:
+                    if new[cname].get(option) != old[cname][0].get(option):
                         raise NotImplementedError('Changing configuration of' +
                                                   ' a running component is ' +
                                                   'not yet supported. Remove' +
@@ -385,21 +407,23 @@ class Configurator:
         plan_add = []
         for cname in new.keys():
             if cname not in old:
-                params = new[cname]
+                component_spec = new[cname]
                 creator = Component
-                if 'special' in params:
+                if 'special' in component_spec:
                     # TODO: Better error handling
-                    creator = specials[params['special']]
-                component = creator(params.get('process', cname), self.__boss,
-                                    params.get('kind', 'dispensable'),
-                                    params.get('address'),
-                                    params.get('params'))
-                priority = params.get('priority', 0)
+                    creator = self.__specials[component_spec['special']]
+                component = creator(component_spec.get('process', cname),
+                                    self.__boss,
+                                    component_spec.get('kind', 'dispensable'),
+                                    component_spec.get('address'),
+                                    component_spec.get('params'))
+                priority = component_spec.get('priority', 0)
                 # We store tuples, priority first, so we can easily sort
                 plan_add.append((priority, {
                     'component': component,
-                    'command': 'start',
+                    'command': START_CMD,
                     'name': cname,
+                    'spec': component_spec
                 }))
         # Push the starts there sorted by priority
         plan.extend([command for (_, command) in sorted(plan_add,
@@ -409,11 +433,15 @@ class Configurator:
         return plan
 
     def running(self):
+        """
+        Returns if the configurator is running (eg. was started by startup and
+        not yet stopped by shutdown).
+        """
         return self._running
 
     def _run_plan(self, plan):
         """
-        Run a plan, created beforehead by _build_plan.
+        Run a plan, created beforehand by _build_plan.
 
         With the start and stop commands, it also adds and removes components
         in _components.
@@ -421,6 +449,13 @@ class Configurator:
         Currently implemented commands are:
         * start
         * stop
+
+        The plan is a list of tasks, each task is a dictionary. It must contain
+        at last 'component' (a component object to work with) and 'command'
+        (the command to do). Currently, both existing commands need 'name' of
+        the component as well (the identifier from configuration). The 'start'
+        one needs the 'spec' to be there, which is the configuration description
+        of the component.
         """
         done = 0
         try:
@@ -428,12 +463,12 @@ class Configurator:
             for task in plan:
                 component = task['component']
                 command = task['command']
-                logger.debug(DBG_TRACE_DETAILED, BIND10_CONFIGURATOR_TASK, command,
-                             component.name())
-                if command == 'start':
+                logger.debug(DBG_TRACE_DETAILED, BIND10_CONFIGURATOR_TASK,
+                             command, component.name())
+                if command == START_CMD:
                     component.start()
-                    self._components[task['name']] = component
-                elif command == 'stop':
+                    self._components[task['name']] = (task['spec'], component)
+                elif command == STOP_CMD:
                     if component.running():
                         component.stop()
                     del self._components[task['name']]
diff --git a/src/lib/python/isc/bind10/special_component.py b/src/lib/python/isc/bind10/special_component.py
new file mode 100644
index 0000000..41e8e13
--- /dev/null
+++ b/src/lib/python/isc/bind10/special_component.py
@@ -0,0 +1,93 @@
+# Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+from isc.bind10.component import Component
+import isc.bind10.sockcreator
+from bind10_config import LIBEXECDIR
+import os
+
+class SockCreator(Component):
+    """
+    The socket creator component. Will start and stop the socket creator
+    accordingly.
+    """
+    def __init__(self, process, boss, kind, address=None, params=None):
+        Component.__init__(self, process, boss, kind)
+        self.__creator = None
+
+    def _start_internal(self):
+        self._boss.curproc = 'b10-sockcreator'
+        self.__creator = isc.bind10.sockcreator.Creator(LIBEXECDIR + ':' +
+                                                        os.environ['PATH'])
+        self._boss.register_process(self.pid(), self)
+
+    def _stop_internal(self):
+        self.__creator.terminate()
+        self.__creator = None
+
+    def pid(self):
+        """
+        Pid of the socket creator. It is provided differently from a usual
+        component.
+        """
+        return self.__creator.pid() if self.__creator else None
+
+class Msgq(Component):
+    """
+    The message queue. Starting is passed to boss, stopping is not supported
+    and we leave the boss kill it by signal.
+    """
+    def __init__(self, process, boss, kind, address=None, params=None):
+        Component.__init__(self, process, boss, kind)
+        self._start_func = boss.start_msgq
+
+    def _stop_internal(self):
+        pass # Wait for the boss to actually kill it. There's no stop command.
+
+class CfgMgr(Component):
+    def __init__(self, process, boss, kind, address=None, params=None):
+        Component.__init__(self, process, boss, kind, 'ConfigManager')
+        self._start_func = boss.start_cfgmgr
+
+class Auth(Component):
+    def __init__(self, process, boss, kind, address=None, params=None):
+        Component.__init__(self, process, boss, kind, 'Auth')
+        self._start_func = boss.start_auth
+
+class Resolver(Component):
+    def __init__(self, process, boss, kind, address=None, params=None):
+        Component.__init__(self, process, boss, kind, 'Resolver')
+        self._start_func = boss.start_resolver
+
+class CmdCtl(Component):
+    def __init__(self, process, boss, kind, address=None, params=None):
+        Component.__init__(self, process, boss, kind, 'Cmdctl')
+        self._start_func = boss.start_cmdctl
+
+def get_specials():
+    """
+    List of specially started components. Each one should be the class than can
+    be created for that component.
+    """
+    return {
+        'sockcreator': SockCreator,
+        'msgq': Msgq,
+        'cfgmgr': CfgMgr,
+        # TODO: Should these be replaced by configuration in config manager only?
+        # They should not have any parameters anyway
+        'auth': Auth,
+        'resolver': Resolver,
+        'cmdctl': CmdCtl
+    }
diff --git a/src/lib/python/isc/bind10/tests/component_test.py b/src/lib/python/isc/bind10/tests/component_test.py
index 8af8ec4..11aee9c 100644
--- a/src/lib/python/isc/bind10/tests/component_test.py
+++ b/src/lib/python/isc/bind10/tests/component_test.py
@@ -14,14 +14,16 @@
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 """
-Tests for the bind10.component module
+Tests for the isc.bind10.component module and the
+isc.bind10.special_component module.
 """
 
 import unittest
 import isc.log
 import time
 import copy
-from isc.bind10.component import Component, Configurator, specials
+from isc.bind10.component import Component, Configurator
+import isc.bind10.special_component
 
 class TestError(Exception):
     """
@@ -56,7 +58,7 @@ class BossUtils:
         Mock function to shut down. We just note we were asked to do so.
         """
         self._shutdown = True
-        self._exitcode = None
+        self._exitcode = exitcode
 
     def _timeskip(self):
         """
@@ -67,6 +69,23 @@ class BossUtils:
         tm = time.time()
         isc.bind10.component.time.time = lambda: tm + 30
 
+    # Few functions that pretend to start something. Part of pretending of
+    # being boss.
+    def start_msgq(self):
+        pass
+
+    def start_cfgmgr(self):
+        pass
+
+    def start_auth(self):
+        pass
+
+    def start_resolver(self):
+        pass
+
+    def start_cmdctl(self):
+        pass
+
 class ComponentTests(BossUtils, unittest.TestCase):
     """
     Tests for the bind10.component.Component class
@@ -84,28 +103,28 @@ class ComponentTests(BossUtils, unittest.TestCase):
 
     def __start(self):
         """
-        Mock function, installed into the component into start_internal.
+        Mock function, installed into the component into _start_internal.
         This only notes the component was "started".
         """
         self.__start_called = True
 
     def __stop(self):
         """
-        Mock function, installed into the component into stop_internal.
+        Mock function, installed into the component into _stop_internal.
         This only notes the component was "stopped".
         """
         self.__stop_called = True
 
     def __fail(self):
         """
-        Mock function, installed into the component into failed_internal.
+        Mock function, installed into the component into _failed_internal.
         This only notes the component called the method.
         """
         self.__failed_called = True
 
     def __fail_to_start(self):
         """
-        Mock function. It can be installed into the component's start_internal
+        Mock function. It can be installed into the component's _start_internal
         to simulate a component that fails to start by raising an exception.
         """
         orig_started = self.__start_called
@@ -125,9 +144,9 @@ class ComponentTests(BossUtils, unittest.TestCase):
         kind of tests and we pretend to be the boss.
         """
         component = Component('No process', self, kind, 'homeless', [])
-        component.start_internal = self.__start
-        component.stop_internal = self.__stop
-        component.failed_internal = self.__fail
+        component._start_internal = self.__start
+        component._stop_internal = self.__stop
+        component._failed_internal = self.__fail
         return component
 
     def test_name(self):
@@ -180,12 +199,14 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(self.__start_called)
         self.assertFalse(self.__stop_called)
         self.assertTrue(self.__failed_called)
-        self.assertNotEqual(0, self._exitcode)
+        self.assertEqual(1, self._exitcode)
         self.assertFalse(component.running())
-        # Surely it can't be stopped again
+        # Surely it can't be stopped when already dead
         self.assertRaises(ValueError, component.stop)
         # Nor started
         self.assertRaises(ValueError, component.start)
+        # Nor it can fail again
+        self.assertRaises(ValueError, component.failed)
 
     def __check_restarted(self, component):
         """
@@ -354,7 +375,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         """
         component = self.__create_component('core')
         self.__check_startup(component)
-        component.start_internal = self.__fail_to_start
+        component._start_internal = self.__fail_to_start
         self.assertRaises(TestError, component.start)
         self.__check_dead(component)
 
@@ -365,7 +386,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         """
         component = self.__create_component('needed')
         self.__check_startup(component)
-        component.start_internal = self.__fail_to_start
+        component._start_internal = self.__fail_to_start
         self.assertRaises(TestError, component.start)
         self.__check_dead(component)
 
@@ -376,7 +397,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         """
         component = self.__create_component('dispensable')
         self.__check_startup(component)
-        component.start_internal = self.__fail_to_start
+        component._start_internal = self.__fail_to_start
         self.assertRaises(TestError, component.start)
         self.__check_restarted(component)
 
@@ -388,6 +409,21 @@ class ComponentTests(BossUtils, unittest.TestCase):
         for kind in ['Core', 'CORE', 'nonsense', 'need ed', 'required']:
             self.assertRaises(ValueError, Component, 'No process', self, kind)
 
+    def test_pid_not_running(self):
+        """
+        Test that a componet that is not yet started doesn't have a PID.
+        But it won't failed if asked for and returns None.
+        """
+        for component_type in [Component,
+                               isc.bind10.special_component.SockCreator,
+                               isc.bind10.special_component.Msgq,
+                               isc.bind10.special_component.CfgMgr,
+                               isc.bind10.special_component.Auth,
+                               isc.bind10.special_component.Resolver,
+                               isc.bind10.special_component.CmdCtl]:
+            component = component_type('none', self, 'needed')
+            self.assertIsNone(component.pid())
+
 class TestComponent(Component):
     """
     A test component. It does not start any processes or so, it just logs
@@ -413,20 +449,20 @@ class TestComponent(Component):
         """
         self.__owner.log.append((self.__name, event))
 
-    def start_internal(self):
+    def _start_internal(self):
         self.log('start')
 
-    def stop_internal(self):
+    def _stop_internal(self):
         self.log('stop')
 
-    def failed_internal(self):
+    def _failed_internal(self):
         self.log('failed')
 
 class FailComponent(Component):
     """
     A mock component that fails whenever it is started.
     """
-    def start_internal(self):
+    def _start_internal(self):
         raise TestError("test error")
 
 class ConfiguratorTest(BossUtils, unittest.TestCase):
@@ -435,15 +471,9 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
     """
     def setUp(self):
         """
-        Insert the special evaluated test components we use and prepare the
-        log. Also provide some data for the tests and prepare us to pretend
-        we're boss.
+        Prepare some test data for the tests.
         """
         BossUtils.setUp(self)
-        # We put our functions inside instead of class constructors,
-        # so we can look into what is happening more easily
-        self.__orig_specials = copy.copy(specials)
-        specials['test'] = self.__component_test
         self.log = []
         # The core "hardcoded" configuration
         self.__core = {
@@ -476,13 +506,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         self.__core_log_start = [('core1', 'start'), ('core3', 'start'),
                                  ('core2', 'start')]
         self.__core_log = self.__core_log_create + self.__core_log_start
-
-    def tearDown(self):
-        """
-        Clean up the special evaluated test components and other stuff.
-        """
-        BossUtils.tearDown(self)
-        specials = self.__orig_specials
+        self.__specials = { 'test': self.__component_test }
 
     def __component_test(self, process, boss, kind, address=None, params=None):
         """
@@ -496,11 +520,10 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         Tests the configurator can be created and it does not create
         any components yet, nor does it remember anything.
         """
-        configurator = Configurator(self)
+        configurator = Configurator(self, self.__specials)
         self.assertEqual([], self.log)
         self.assertEqual({}, configurator._components)
-        self.assertEqual({}, configurator._old_config)
-        self.assertFalse(configurator._running)
+        self.assertFalse(configurator.running())
 
     def test_run_plan(self):
         """
@@ -510,7 +533,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         Also includes one that raises, so we see it just stops there.
         """
         # Prepare the configurator and the plan
-        configurator = Configurator(self)
+        configurator = Configurator(self, self.__specials)
         started = self.__component_test('second', self, 'dispensable')
         started.start()
         stopped = self.__component_test('first', self, 'core')
@@ -519,22 +542,26 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
             {
                 'component': stopped,
                 'command': 'start',
-                'name': 'first'
+                'name': 'first',
+                'spec': {'a': 1}
             },
             {
                 'component': started,
                 'command': 'stop',
-                'name': 'second'
+                'name': 'second',
+                'spec': {}
             },
             {
                 'component': FailComponent('third', self, 'needed'),
                 'command': 'start',
-                'name': 'third'
+                'name': 'third',
+                'spec': {}
             },
             {
                 'component': self.__component_test('fourth', self, 'core'),
                 'command': 'start',
-                'name': 'fourth'
+                'name': 'fourth',
+                'spec': {}
             }
         ]
         # Don't include the preparation into the log
@@ -543,14 +570,27 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         self.assertRaises(TestError, configurator._run_plan, plan)
         # The first two were handled, the rest not, due to the exception
         self.assertEqual([('first', 'start'), ('second', 'stop')], self.log)
-        self.assertEqual({'first': stopped}, configurator._components)
+        self.assertEqual({'first': ({'a': 1}, stopped)},
+                         configurator._components)
+
+    def __build_components(self, config):
+        """
+        Insert the components into the configuration to specify possible
+        Configurator._components.
+
+        Actually, the components are None, but we need something to be there.
+        """
+        result = {}
+        for name in config.keys():
+            result[name] = (config[name], None)
+        return result
 
     def test_build_plan(self):
         """
         Test building the plan correctly. Not complete yet, this grows as we
         add more ways of changing the plan.
         """
-        configurator = Configurator(self)
+        configurator = Configurator(self, self.__specials)
         plan = configurator._build_plan({}, self.__core)
         # This should have created the components
         self.assertEqual(self.__core_log_create, self.log)
@@ -572,7 +612,8 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
             'kind': 'needed'
         }
         self.log = []
-        plan = configurator._build_plan(self.__core, bigger)
+        plan = configurator._build_plan(self.__build_components(self.__core),
+                                        bigger)
         self.assertEqual([('additional', 'init'), ('additional', 'needed')],
                          self.log)
         self.assertEqual(1, len(plan))
@@ -585,7 +626,8 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         # We run the plan so the component is wired into internal structures
         configurator._run_plan(plan)
         self.log = []
-        plan = configurator._build_plan(bigger, self.__core)
+        plan = configurator._build_plan(self.__build_components(bigger),
+                                        self.__core)
         self.assertEqual([], self.log)
         self.assertEqual([{
             'command': 'stop',
@@ -595,7 +637,8 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
 
         # We want to switch a component. So, prepare the configurator so it
         # holds one
-        configurator._run_plan(configurator._build_plan(self.__core, bigger))
+        configurator._run_plan(configurator._build_plan(
+             self.__build_components(self.__core), bigger))
         # Get a different configuration with a different component
         different = copy.copy(self.__core)
         different['another'] = {
@@ -604,7 +647,8 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
             'kind': 'dispensable'
         }
         self.log = []
-        plan = configurator._build_plan(bigger, different)
+        plan = configurator._build_plan(self.__build_components(bigger),
+                                        different)
         self.assertEqual([('another', 'init'), ('another', 'dispensable')],
                          self.log)
         self.assertEqual(2, len(plan))
@@ -651,7 +695,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         Start it with some component and then switch the configuration of the
         component. This will probably raise, as it is not yet supported.
         """
-        configurator = Configurator(self)
+        configurator = Configurator(self, self.__specials)
         compconfig = {
             'special': 'test',
             'process': 'process',
@@ -660,7 +704,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         }
         modifiedconfig = copy.copy(compconfig)
         modifiedconfig[option] = value
-        return configurator._build_plan({'comp': compconfig},
+        return configurator._build_plan({'comp': (compconfig, None)},
                                         {'comp': modifiedconfig})
 
     def test_change_config_plan(self):
@@ -674,6 +718,10 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
                           'not_a_test')
         self.assertRaises(NotImplementedError, self.__do_switch, 'process',
                           'different')
+        self.assertRaises(NotImplementedError, self.__do_switch, 'address',
+                          'different')
+        self.assertRaises(NotImplementedError, self.__do_switch, 'params',
+                          ['different'])
         # This does not change anything on running component, so no need to
         # raise
         self.assertEqual([], self.__do_switch('priority', 5))
@@ -704,7 +752,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
 
         It also checks the components are kept inside the configurator.
         """
-        configurator = Configurator(self)
+        configurator = Configurator(self, self.__specials)
         # Can't reconfigure nor stop yet
         self.assertRaises(ValueError, configurator.reconfigure, self.__core)
         self.assertRaises(ValueError, configurator.shutdown)
@@ -714,8 +762,9 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         self.assertEqual(self.__core_log, self.log)
         for core in self.__core.keys():
             self.assertTrue(core in configurator._components)
-        self.assertEqual(self.__core, configurator._old_config)
-        self.assertTrue(configurator._running)
+            self.assertEqual(self.__core[core],
+                             configurator._components[core][0])
+        self.assertEqual(set(self.__core), set(configurator._components))
         self.assertTrue(configurator.running())
         # It can't be started twice
         self.assertRaises(ValueError, configurator.startup, self.__core)
@@ -724,8 +773,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         # Reconfigure - stop everything
         configurator.reconfigure({})
         self.assertEqual({}, configurator._components)
-        self.assertEqual({}, configurator._old_config)
-        self.assertTrue(configurator._running)
+        self.assertTrue(configurator.running())
         self.__check_shutdown_log()
 
         # Start it again
@@ -734,15 +782,15 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         self.assertEqual(self.__core_log, self.log)
         for core in self.__core.keys():
             self.assertTrue(core in configurator._components)
-        self.assertEqual(self.__core, configurator._old_config)
-        self.assertTrue(configurator._running)
+            self.assertEqual(self.__core[core],
+                             configurator._components[core][0])
+        self.assertEqual(set(self.__core), set(configurator._components))
+        self.assertTrue(configurator.running())
 
         # Do a shutdown
         self.log = []
         configurator.shutdown()
         self.assertEqual({}, configurator._components)
-        self.assertEqual({}, configurator._old_config)
-        self.assertFalse(configurator._running)
         self.assertFalse(configurator.running())
         self.__check_shutdown_log()
 
@@ -755,7 +803,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         (or without priority), it failed as it couldn't compare the dicts
         there. This tests it doesn't crash.
         """
-        configurator = Configurator(self)
+        configurator = Configurator(self, self.__specials)
         configurator._build_plan({}, {"c1": {}, "c2": {}})
 
 if __name__ == '__main__':




More information about the bind10-changes mailing list