[2854] added more behavior to BIND10Server class

src/lib/python/isc/server_common/bind10_server.py.in

@@ -13,32 +13,77 @@
 # NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
+import errno
 import os
+import select
 import signal
 
 import isc.log
+import isc.config
 from isc.server_common.logger import logger
 from isc.log_messages.server_common_messages import *
 
+class BIND10ServerFatal(Exception):
+    """Exception raised when the server program encounters a fatal error."""
+    pass
+
 class BIND10Server:
     """A mixin class for common BIND 10 server implementations.
 
+    It takes care of common initialization such as setting up a module CC
+    session, and running main event loop.  It also handles the "shutdown"
+    command for its normal behavior.  If a specific server class wants to
+    handle this command differently or if it does not support the command,
+    it should override the _command_handler method.
+
+    Specific modules can define module-specific class inheriting this class,
+    instantiate it, and call run() with the module name.
+
     Methods to be implemented in the actual class:
+      _config_handler: config handler method as specified in ModuleCCSession.
+                       must be exception free; errors should be signaled by
+                       the return value.
+      _mod_command_handler: can be optionally defined to handle
+                            module-specific commands.  should conform to
+                            command handlers as specified in ModuleCCSession.
+                            must be exception free; errors should be signaled
+                            by the return value.
 
     """
     # Will be set to True when the server should stop and shut down.
     # Can be read via accessor method 'shutdown', mainly for testing.
     __shutdown = False
-    __module_name = None
+
+    # ModuleCCSession used in the server.  Defined as 'protectd' so tests
+    # can refer to it directly; others should access it via the
+    # 'mod_ccsession' accessor.
+    _mod_cc = None
+
+    # Will be set in run().  Define a tentative value so other methods can
+    # be tested directly.
+    __module_name = ''
+
+    # Basically constant, but allow tests to override it.
+    _select_fn = select.select
 
     @property
     def shutdown(self):
         return self.__shutdown
 
+    @property
+    def mod_ccsession(self):
+        return self._mod_cc
+
     def _setup_ccsession(self):
-        self._cc = isc.config.ModuleCCSession(self._get_specfile_location(),
-                                              self._config_handler,
-                                              self._command_handler)
+        """Create and start module CC session.
+
+        This is essentially private, but allows tests to override it.
+
+        """
+        self._mod_cc = isc.config.ModuleCCSession(
+            self._get_specfile_location(), self._config_handler,
+            self._command_handler)
+        self._mod_cc.start()
 
     def _get_specfile_location(self):
         """Return the path to the module spec file following common convetion.
@@ -54,10 +99,10 @@ class BIND10Server:
             specfile_path = os.environ['B10_FROM_SOURCE'] + '/src/bin/' + \
                 self.__module_name
         else:
-            specfile_path = '@datadir@/@PACKAGE@'\
-                .replace('${datarootdir}', '@datarootdir@')\
-                .replace('${prefix}', '@prefix@')
-        return specfile_path + '/' + self.__module_name
+            specfile_path = '${datarootdir}/bind10'\
+                .replace('${datarootdir}', '${prefix}/share')\
+                .replace('${prefix}', '/Users/jinmei/opt')
+        return specfile_path + '/' + self.__module_name + '.spec'
 
     def _trigger_shutdown(self):
         """Initiate a shutdown sequence.
@@ -67,21 +112,53 @@ class BIND10Server:
         as possible to minimize side effects.  Actual shutdown will take
         place in a normal control flow.
 
-        This method is defined as 'protected' so tests can call it; otherwise
-        it's private.
+        This method is defined as 'protected'.  User classes can use it
+        to shut down the server.
 
         """
         self.__shutdown = True
 
     def _run_internal(self):
+        """Main event loop.
+
+        This method is essentially private, but allows tests to override it.
+
+        """
+
+        logger.info(PYSERVER_COMMON_SERVER_STARTED, self.__module_name)
+        cc_fileno = self._mod_cc.get_socket().fileno()
         while not self.__shutdown:
-            pass
+            try:
+                (reads, _, _) = self._select_fn([cc_fileno], [], [])
+            except select.error as ex:
+                # ignore intterruption by signal; regard other select errors
+                # fatal.
+                if ex.args[0] == errno.EINTR:
+                    continue
+                else:
+                    raise
+            for fileno in reads:
+                if fileno == cc_fileno:
+                    # this shouldn't raise an exception (if it does, we'll
+                    # propagate it)
+                    self._mod_cc.check_command(True)
+
+        self._mod_cc.send_stopping()
+
+    def _command_handler(self, cmd, args):
+        logger.debug(logger.DBGLVL_TRACE_BASIC, PYSERVER_COMMON_COMMAND,
+                     self.__module_name, cmd)
+        if cmd == 'shutdown':
+            self._trigger_shutdown()
+            answer = isc.config.create_answer(0)
+        else:
+            answer = self._mod_command_handler(cmd, args)
 
-    def _config_handler(self):
-        pass
+        return answer
 
-    def _command_handler(self):
-        pass
+    def _mod_command_handler(self, cmd, args):
+        """The default implementation of the module specific command handler"""
+        return isc.config.create_answer(1, "Unknown command: " + str(cmd))
 
     def run(self, module_name):
         """Start the server and let it run until it's told to stop.
@@ -107,9 +184,13 @@ class BIND10Server:
             signal.signal(signal.SIGINT, shutdown_sighandler)
             self._setup_ccsession()
             self._run_internal()
+            logger.info(PYSERVER_COMMON_SERVER_STOPPED, self.__module_name)
             return 0
+        except BIND10ServerFatal as ex:
+            logger.error(PYSERVER_COMMON_SERVER_FATAL, self.__module_name,
+                         ex)
         except Exception as ex:
             logger.error(PYSERVER_COMMON_UNCAUGHT_EXCEPTION, type(ex).__name__,
-                         str(ex))
+                         ex)
 
         return 1

src/lib/python/isc/server_common/server_common_messages.mes

@@ -47,11 +47,15 @@ The destination address and the total size of the message that has
 been transmitted so far (including the 2-byte length field) are shown
 in the log message.
 
+% PYSERVER_COMMON_SERVER_FATAL %1 server has encountered a fatal error: %2
+The BIND 10 server process encountered a fatal error (normally specific to
+the particular program), and is forcing itself to shut down.
+
 % PYSERVER_COMMON_SERVER_STARTED %1 server has started
 The server process has successfully started and is now ready to receive
 commands and updates.
 
-% PYSERVER_COMMON_SERVER_STOPPED %1 server has stopped
+% PYSERVER_COMMON_SERVER_STOPPED %1 server has started
 The server process has successfully stopped and is no longer listening for or
 handling commands.  Normally the process will soon exit.
 

src/lib/python/isc/server_common/tests/Makefile.am

@@ -30,6 +30,7 @@ endif
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/dns/python/.libs \
 	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
+	B10_FROM_SOURCE=$(abs_top_srcdir) \
 	B10_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/server_common/tests/bind10_server_test.py

@@ -14,6 +14,7 @@
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 import unittest
+import errno
 import os
 import signal
 
@@ -22,15 +23,47 @@ import isc.config
 from isc.server_common.bind10_server import *
 from isc.testutils.ccsession_mock import MockModuleCCSession
 
+TEST_FILENO = 42                # arbitrarily chosen
+
 class TestException(Exception):
     """A generic exception class specific in this test module."""
     pass
 
 class MyCCSession(MockModuleCCSession, isc.config.ConfigData):
     def __init__(self, specfile, config_handler, command_handler):
+        # record parameter for later inspection
+        self.specfile_param = specfile
+        self.config_handler_param = config_handler
+        self.command_handler_param = command_handler
+
+        self.check_command_param = None # used in check_command()
+
+        # Initialize some local attributes of MockModuleCCSession, including
+        # 'stopped'
+        MockModuleCCSession.__init__(self)
+
+    def start(self):
         pass
 
+    def check_command(self, nonblock):
+        """Mock check_command(). Just record the param for later inspection."""
+        self.check_command_param = nonblock
+
+    def get_socket(self):
+        return self
+
+    def fileno(self):
+        """Pretending get_socket().fileno()
+
+        Returing an arbitrarily chosen constant.
+
+        """
+        return TEST_FILENO
+
 class MockServer(BIND10Server):
+    def __init__(self):
+        self._select_fn = self.select_wrapper
+
     def _setup_ccsession(self):
         orig_cls = isc.config.ModuleCCSession
         isc.config.ModuleCCSession = MyCCSession
@@ -41,6 +74,19 @@ class MockServer(BIND10Server):
         finally:
             isc.config.ModuleCCSession = orig_cls
 
+    def _config_handler(self):
+        pass
+
+    def mod_command_handler(self, cmd, args):
+        """A sample _mod_command_handler implementation."""
+        self.command_handler_params = (cmd, args) # for inspection
+        return isc.config.create_answer(0)
+
+    def select_wrapper(self, reads, writes, errors):
+        self._trigger_shutdown() # make sure the loop will stop
+        self.select_params = (reads, writes, errors) # record for inspection
+        return [], [], []
+
 class TestBIND10Server(unittest.TestCase):
     def setUp(self):
         self.__server = MockServer()
@@ -57,7 +103,7 @@ class TestBIND10Server(unittest.TestCase):
         """Check the signal handler behavior.
 
         SIGTERM and SIGINT should be caught and should call memmgr's
-        _trigger_shutdown().  This test also indirectly confirms main() calls
+        _trigger_shutdown().  This test also indirectly confirms run() calls
         run_internal().
 
         """
@@ -82,10 +128,111 @@ class TestBIND10Server(unittest.TestCase):
             raise ex_cls('test')
 
         # Test all possible exceptions that are explicitly caught
-        for ex in [TestException]:
+        for ex in [TestException, BIND10ServerFatal]:
             self.__server._run_internal = lambda: exception_raiser(ex)
             self.assertEqual(1, self.__server.run('test'))
 
+    def test_run(self):
+        """Check other behavior of run()"""
+        self.__server._run_internal = lambda: None # prevent looping
+        self.assertEqual(0, self.__server.run('test'))
+        # module CC session should have been setup.
+        self.assertEqual(self.__server.mod_ccsession.specfile_param,
+                         os.environ['B10_FROM_SOURCE'] +
+                         '/src/bin/test/test.spec')
+        self.assertEqual(self.__server.mod_ccsession.config_handler_param,
+                         self.__server._config_handler)
+        self.assertEqual(self.__server.mod_ccsession.command_handler_param,
+                         self.__server._command_handler)
+
+    def test_shutdown_command(self):
+        answer = self.__server._command_handler('shutdown', None)
+        self.assertTrue(self.__server.shutdown)
+        self.assertEqual((0, None), isc.config.parse_answer(answer))
+
+    def test_other_command(self):
+        self.__server._mod_command_handler = self.__server.mod_command_handler
+        answer = self.__server._command_handler('other command', None)
+        # shouldn't be confused with shutdown
+        self.assertFalse(self.__server.shutdown)
+        self.assertEqual((0, None), isc.config.parse_answer(answer))
+        self.assertEqual(('other command', None),
+                         self.__server.command_handler_params)
+
+    def test_other_command_nohandler(self):
+        """Similar to test_other_command, but without explicit handler"""
+        # In this case "unknown command" error should be returned.
+        answer = self.__server._command_handler('other command', None)
+        self.assertEqual(1, isc.config.parse_answer(answer)[0])
+
+    def test_run_internal(self):
+        self.__server._setup_ccsession()
+        self.__server._run_internal()
+        self.assertEqual(([TEST_FILENO], [], []), self.__server.select_params)
+
+    def select_wrapper(self, r, w, e, ex=None, ret=None):
+        """Mock select() function used some of the tests below.
+
+        If ex is not None and it's first call to this method, it raises ex
+        assuming it's an exception.
+
+        If ret is not None, it returns the given value; otherwise it returns
+        all empty lists.
+
+        """
+        self.select_params.append((r, w, e))
+        if ex is not None and len(self.select_params) == 1:
+            raise ex
+        else:
+            self.__server._trigger_shutdown()
+        if ret is not None:
+            return ret
+        return [], [], []
+
+    def test_select_for_command(self):
+        """A normal event iteration, handling one command."""
+        self.select_params = []
+        self.__server._select_fn = \
+            lambda r, w, e: self.select_wrapper(r, w, e,
+                                                ret=([TEST_FILENO], [], []))
+        self.__server._setup_ccsession()
+        self.__server._run_internal()
+        # select should be called only once.
+        self.assertEqual([([TEST_FILENO], [], [])], self.select_params)
+        # check_command should have been called.
+        self.assertTrue(self.__server.mod_ccsession.check_command_param)
+        # module CC session should have been stopped explicitly.
+        self.assertTrue(self.__server.mod_ccsession.stopped)
+
+    def test_select_interrupted(self):
+        """Emulating case select() raises EINTR."""
+        self.select_params = []
+        self.__server._select_fn = \
+            lambda r, w, e: self.select_wrapper(r, w, e,
+                                                ex=select.error(errno.EINTR))
+        self.__server._setup_ccsession()
+        self.__server._run_internal()
+        # EINTR will be ignored and select() will be called again.
+        self.assertEqual([([TEST_FILENO], [], []), ([TEST_FILENO], [], [])],
+                          self.select_params)
+        # check_command() shouldn't have been called
+        self.assertIsNone(self.__server.mod_ccsession.check_command_param)
+        self.assertTrue(self.__server.mod_ccsession.stopped)
+
+    def test_select_other_exception(self):
+        """Emulating case select() raises other select error."""
+        self.select_params = []
+        self.__server._select_fn = \
+            lambda r, w, e: self.select_wrapper(r, w, e,
+                                                ex=select.error(errno.EBADF))
+        self.__server._setup_ccsession()
+        # the exception will be propagated.
+        self.assertRaises(select.error, self.__server._run_internal)
+        self.assertEqual([([TEST_FILENO], [], [])], self.select_params)
+        # in this case module CC session hasn't been stopped explicitly
+        # others will notice it due to connection reset.
+        self.assertFalse(self.__server.mod_ccsession.stopped)
+
 if __name__== "__main__":
     isc.log.init("bind10_server_test")
     isc.log.resetUnitTestRootLogger()