BIND 10 trac213, updated. 5e3d007b0b08f340e646a2df9073b31cd3c76476 [213] Fix parameters in tests

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Oct 25 10:48:48 UTC 2011


The branch, trac213 has been updated
       via  5e3d007b0b08f340e646a2df9073b31cd3c76476 (commit)
       via  c3a5acc65768a1d87c102159baae0d04f8c14790 (commit)
       via  1c4e66cfdfab4fb4608f2b8d18a25e28e7a70adc (commit)
       via  7db8a3e327aa6eb8fdc5fed2abb7f52b030fe6f8 (commit)
       via  fd3c952098c46d84c9a277b1409442813a263876 (commit)
       via  b108bc9f9231872d4f3e0fa768b8c0e4506a2b95 (commit)
       via  c5cef09ac250129340f357a9ea2dd798d290be4d (commit)
       via  8b349f6730bf85ccfb37d368aa18db4f6c0aaa1b (commit)
       via  4b584e952e14a40e81b7e360c75cd787ba988481 (commit)
       via  702e2dd653a315141e01147ac4cc2a6c06fab673 (commit)
      from  5d38929255f7d8cca95020672a2b72273a07de1d (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 5e3d007b0b08f340e646a2df9073b31cd3c76476
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 12:47:09 2011 +0200

    [213] Fix parameters in tests

commit c3a5acc65768a1d87c102159baae0d04f8c14790
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 12:42:43 2011 +0200

    [213] Pass and log exit code

commit 1c4e66cfdfab4fb4608f2b8d18a25e28e7a70adc
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 12:25:59 2011 +0200

    [213] Doc string for register_process

commit 7db8a3e327aa6eb8fdc5fed2abb7f52b030fe6f8
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 11:49:57 2011 +0200

    [213] Document strange behaivour and test

commit fd3c952098c46d84c9a277b1409442813a263876
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 11:29:35 2011 +0200

    [213] Return a TODO comment

commit b108bc9f9231872d4f3e0fa768b8c0e4506a2b95
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 11:21:39 2011 +0200

    [213] Drop single-line function

commit c5cef09ac250129340f357a9ea2dd798d290be4d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 11:15:43 2011 +0200

    [213] Return dropping of root privileges
    
    But it won't work well :-(

commit 8b349f6730bf85ccfb37d368aa18db4f6c0aaa1b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 11:07:30 2011 +0200

    [213] Comments

commit 4b584e952e14a40e81b7e360c75cd787ba988481
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 10:59:01 2011 +0200

    [213] Better variable name

commit 702e2dd653a315141e01147ac4cc2a6c06fab673
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Oct 25 10:53:20 2011 +0200

    [213] A docstring update

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

Summary of changes:
 src/bin/bind10/bind10_messages.mes                |    4 +
 src/bin/bind10/bind10_src.py.in                   |   62 +++++++++++++--------
 src/bin/bind10/tests/bind10_test.py.in            |   32 ++++++++--
 src/lib/python/isc/bind10/component.py            |   10 +++-
 src/lib/python/isc/bind10/tests/component_test.py |   22 ++++----
 5 files changed, 86 insertions(+), 44 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/bind10/bind10_messages.mes b/src/bin/bind10/bind10_messages.mes
index 994af16..b04e191 100644
--- a/src/bin/bind10/bind10_messages.mes
+++ b/src/bin/bind10/bind10_messages.mes
@@ -20,6 +20,10 @@ The boss process is starting up and will now check if the message bus
 daemon is already running. If so, it will not be able to start, as it
 needs a dedicated message bus.
 
+% BIND10_COMPONENT_FAILED component %1 (pid %2) failed with %3 exit status
+The process terminated, but the bind10 boss didn't expect it to, which means
+it must have failed.
+
 % BIND10_COMPONENT_RESTART component %1 is about to restart
 The named component failed previously and we will try to restart it to provide
 as flawless service as possible, but it should be investigated what happened,
diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in
index bbb0eac..7df7df4 100755
--- a/src/bin/bind10/bind10_src.py.in
+++ b/src/bin/bind10/bind10_src.py.in
@@ -354,11 +354,7 @@ class BoB:
         """
             Reads the parameters associated with the BoB module itself.
 
-            At present these are the components to start although arguably this
-            information should be in the configuration for the appropriate
-            module itself. (However, this would cause difficulty in the case of
-            xfrin/xfrout and zone manager as we don't need to start those if we
-            are not running the authoritative server.)
+            This means the the list of components we should be running.
         """
         logger.info(BIND10_READING_BOSS_CONFIGURATION)
 
@@ -410,11 +406,11 @@ class BoB:
             Start the message queue and connect to the command channel.
         """
         self.log_starting("b10-msgq")
-        c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], self.c_channel_env,
+        msgq_proc = ProcessInfo("b10-msgq", ["b10-msgq"], self.c_channel_env,
                                 True, not self.verbose, uid=self.uid,
                                 username=self.username)
-        c_channel.spawn()
-        self.log_started(c_channel.pid)
+        msgq_proc.spawn()
+        self.log_started(msgq_proc.pid)
 
         # Now connect to the c-channel
         cc_connect_start = time.time()
@@ -428,7 +424,7 @@ class BoB:
                 self.cc_session = isc.cc.Session(self.msgq_socket_file)
             except isc.cc.session.SessionError:
                 time.sleep(0.1)
-        return c_channel
+        return msgq_proc
 
     def start_cfgmgr(self):
         """
@@ -491,8 +487,6 @@ class BoB:
 
             ... where -v is appended if verbose is enabled.  This method
             generates the arguments from the name and starts the process.
-
-            The port and address arguments are for log messages only.
         """
         # Set up the command arguments.
         args = [name]
@@ -502,11 +496,10 @@ class BoB:
         # ... and start the process
         return self.start_process(name, args, self.c_channel_env)
 
-    # The next few methods start up the rest of the BIND-10 processes.
-    # Although many of these methods are little more than a call to
-    # start_simple, they are retained (a) for testing reasons and (b) as a place
-    # where modifications can be made if the process start-up sequence changes
-    # for a given process.
+    # The next few methods start up some of the BIND-10 processes.
+    # These are the ones that need to be passed some parameters, so
+    # using a start_simple is not enough. However, in future, we should
+    # get rid of these parameters and they could be removed then.
 
     def start_auth(self):
         """
@@ -555,17 +548,31 @@ class BoB:
             Starts up all the processes.  Any exception generated during the
             starting of the processes is handled by the caller.
         """
+        # Start the real core (sockcreator, msgq, cfgmgr)
         self._component_configurator.startup(self.__core_components)
-        # TODO: Once everything uses the socket creator, we can drop root
-        # privileges right now
 
+        # Connect to the msgq. This is not a process, so it's not handled
+        # inside the configurator.
         c_channel_env = self.c_channel_env
         self.start_ccsession(c_channel_env)
 
         # Extract the parameters associated with Bob.  This can only be
         # done after the CC Session is started.
+        #
+        # This will start all the other configured processes.
         self.read_bind10_config()
 
+        # FIXME: This is currently the only place we can reasonably drop
+        # root privileges. But that's wrong, as everything will run as root.
+        # If we put it before the read_bind10_config, the auth and resolver
+        # will not run as root, which means they can't get their privileged
+        # sockets.
+        #
+        # Once the socket creator is working fully (and is used), this can go
+        # directly to the function starting socket creator.
+        if self.uid is not None:
+            posix.setuid(self.uid)
+
     def startup(self):
         """
             Start the BoB instance.
@@ -602,10 +609,6 @@ class BoB:
         self.__started = True
         return None
 
-    def stop_all_processes(self):
-        """Stop all processes."""
-        self._component_configurator.shutdown()
-
     def stop_process(self, process, recipient):
         """
         Stop the given process, friendly-like. The process is the name it has
@@ -620,6 +623,13 @@ class BoB:
         """
         Stop the Boss instance from a components' request. The exitcode
         indicates the desired exit code.
+
+        If we did not start yet, it raises an exception, which is meant
+        to propagate through the component and configurator to the startup
+        routine and abort the startup imediatelly. If it is started up already,
+        we just mark it so we terminate soon.
+
+        It does set the exit code in both cases.
         """
         if self.__stopping:
             return
@@ -635,7 +645,7 @@ class BoB:
         self.__stopping = True
         # first try using the BIND 10 request to stop
         try:
-            self.stop_all_processes()
+            self._component_configurator.shutdown()
         except:
             pass
         # XXX: some delay probably useful... how much is uncertain
@@ -703,7 +713,7 @@ class BoB:
                 # Tell it it failed, but only if it matters at all (eg. it is
                 # running and we are running - if not, it should stop anyway)
                 if component.running() and self.runnable:
-                    component.failed()
+                    component.failed(exit_status)
             else:
                 logger.info(BIND10_UNKNOWN_CHILD_PROCESS_ENDED, pid)
 
@@ -745,6 +755,10 @@ class BoB:
         return next_restart
 
     def register_process(self, pid, info):
+        """
+        Put another process into boss to watch over it.  When the process
+        dies, the info.failed() is called with the exit code.
+        """
         self.processes[pid] = info
 
 # global variables, needed for signal handlers
diff --git a/src/bin/bind10/tests/bind10_test.py.in b/src/bin/bind10/tests/bind10_test.py.in
index e67d13b..df620f0 100644
--- a/src/bin/bind10/tests/bind10_test.py.in
+++ b/src/bin/bind10/tests/bind10_test.py.in
@@ -302,14 +302,9 @@ class TestBossComponents(unittest.TestCase):
         # Check it rejected it
         self.assertEqual(1, result['result'][0])
 
-        # Stop it
-        orig = bob._component_configurator.shutdown
-        bob._component_configurator.shutdown = self.__nullary_hook
-        self.__called = False
         # We can't call shutdown, that one relies on the stuff in main
-        bob.stop_all_processes()
-        bob._component_configurator.shutdown = orig
-        self.assertTrue(self.__called)
+        # We check somewhere else that the shutdown is actually called
+        # from there (the test_kills).
 
     def test_kills(self):
         """
@@ -333,8 +328,31 @@ class TestBossComponents(unittest.TestCase):
                 return "Immortal"
         bob.processes = {}
         bob.register_process(1, ImmortalComponent())
+
+        # While at it, we check the configurator shutdown is actually called
+        orig = bob._component_configurator.shutdown
+        bob._component_configurator.shutdown = self.__nullary_hook
+        self.__called = False
+
         bob.shutdown()
+
         self.assertEqual([False, True], killed)
+        self.assertTrue(self.__called)
+
+        bob._component_configurator.shutdown = orig
+
+    def test_component_shutdown(self):
+        """
+        Test the component_shutdown sets all variables accordingly.
+        """
+        bob = MockBob()
+        self.assertRaises(Exception, bob.component_shutdown, 1)
+        self.assertEqual(1, bob.exitcode)
+        bob._BoB__started = True
+        bob.runnable = True
+        bob.component_shutdown(2)
+        self.assertEqual(2, bob.exitcode)
+        self.assertFalse(bob.runnable)
 
     def test_init_config(self):
         """
diff --git a/src/lib/python/isc/bind10/component.py b/src/lib/python/isc/bind10/component.py
index d11b175..7add835 100644
--- a/src/lib/python/isc/bind10/component.py
+++ b/src/lib/python/isc/bind10/component.py
@@ -137,7 +137,7 @@ class Component:
             self._start_internal()
         except Exception as e:
             logger.error(BIND10_COMPONENT_START_EXCEPTION, self.name(), e)
-            self.failed()
+            self.failed(None)
             raise
 
     def _start_internal(self):
@@ -196,8 +196,10 @@ class Component:
         name as well).
         """
         self._boss.stop_process(self._process, self._address)
+        # TODO Some way to wait for the process that doesn't want to
+        # terminate and kill it would prove nice (or add it to boss somewhere?)
 
-    def failed(self):
+    def failed(self, exit_code):
         """
         Notify the component it crashed. This will be called from boss object.
 
@@ -209,7 +211,11 @@ class Component:
         down with error exit status. A dead component can't be started again.
 
         Otherwise the component will try to restart.
+
+        The exit code is used for logging. It might be None.
         """
+        logger.error(BIND10_COMPONENT_FAILED, self.name(), self.pid(),
+                     exit_code if exit_code is not None else "unknown")
         if not self.running():
             raise ValueError("Can't fail component that isn't running")
         self.__state = STATE_STOPPED
diff --git a/src/lib/python/isc/bind10/tests/component_test.py b/src/lib/python/isc/bind10/tests/component_test.py
index 339b39f..1ac28cf 100644
--- a/src/lib/python/isc/bind10/tests/component_test.py
+++ b/src/lib/python/isc/bind10/tests/component_test.py
@@ -179,7 +179,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertFalse(component.running())
         # We can't stop or fail the component yet
         self.assertRaises(ValueError, component.stop)
-        self.assertRaises(ValueError, component.failed)
+        self.assertRaises(ValueError, component.failed, 1)
 
     def __check_started(self, component):
         """
@@ -206,7 +206,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         # Nor started
         self.assertRaises(ValueError, component.start)
         # Nor it can fail again
-        self.assertRaises(ValueError, component.failed)
+        self.assertRaises(ValueError, component.failed, 1)
 
     def __check_restarted(self, component):
         """
@@ -254,7 +254,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         # Check it can't be stopped twice
         self.assertRaises(ValueError, component.stop)
         # Or failed
-        self.assertRaises(ValueError, component.failed)
+        self.assertRaises(ValueError, component.failed, 1)
         # But it can be started again if it is stopped
         # (no more checking here, just it doesn't crash)
         component.start()
@@ -287,7 +287,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         component.start()
         self.__check_started(component)
         # Pretend the component died
-        component.failed()
+        component.failed(1)
         # It should bring down the whole server
         self.__check_dead(component)
 
@@ -303,7 +303,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.__check_started(component)
         self._timeskip()
         # Pretend the component died some time later
-        component.failed()
+        component.failed(1)
         # Check the component is still dead
         self.__check_dead(component)
 
@@ -319,7 +319,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         component.start()
         self.__check_started(component)
         # Make it fail right away.
-        component.failed()
+        component.failed(1)
         self.__check_dead(component)
 
     def test_start_fail_needed_later(self):
@@ -335,7 +335,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         # Make it fail later on
         self.__start_called = False
         self._timeskip()
-        component.failed()
+        component.failed(1)
         self.__check_restarted(component)
 
     def test_start_fail_dispensable(self):
@@ -349,7 +349,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.__check_started(component)
         # Make it fail right away
         self.__start_called = False
-        component.failed()
+        component.failed(1)
         self.__check_restarted(component)
 
     def test_start_fail_dispensable(self):
@@ -365,7 +365,7 @@ class ComponentTests(BossUtils, unittest.TestCase):
         # Make it fail later on
         self.__start_called = False
         self._timeskip()
-        component.failed()
+        component.failed(1)
         self.__check_restarted(component)
 
     def test_fail_core(self):
@@ -678,7 +678,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         plan = configurator._build_plan({}, {
             'component': {
                 'kind': 'needed',
-                'params': [1, 2],
+                'params': ["1", "2"],
                 'address': 'address'
             }
         })
@@ -687,7 +687,7 @@ class ConfiguratorTest(BossUtils, unittest.TestCase):
         self.assertEqual('component', plan[0]['name'])
         component = plan[0]['component']
         self.assertEqual('component', component.name())
-        self.assertEqual([1, 2], component._params)
+        self.assertEqual(["1", "2"], component._params)
         self.assertEqual('address', component._address)
         self.assertEqual('needed', component._kind)
         # We don't use isinstance on purpose, it would allow a descendant




More information about the bind10-changes mailing list