BIND 10 trac1451, updated. ae488cc48d35ca3a96c42ba6a7fd1d33c4b2a1c7 [1451] addressed review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Dec 16 17:21:55 UTC 2011
The branch, trac1451 has been updated
via ae488cc48d35ca3a96c42ba6a7fd1d33c4b2a1c7 (commit)
from a0a07a1b5ee1442f1bcbbb455d83bd8f3c229813 (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 ae488cc48d35ca3a96c42ba6a7fd1d33c4b2a1c7
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Dec 16 18:21:45 2011 +0100
[1451] addressed review comments
-----------------------------------------------------------------------
Summary of changes:
src/bin/ddns/b10-ddns.8 | 2 +-
src/bin/ddns/b10-ddns.xml | 4 +-
src/bin/ddns/ddns.py.in | 40 ++++++++++++------
src/bin/ddns/ddns.spec | 6 +-
src/bin/ddns/ddns_messages.mes | 28 ++++++++++--
src/bin/ddns/tests/ddns_test.in | 27 ------------
src/bin/ddns/tests/ddns_test.py | 85 +++++++++++++++++++++++++++++++++++++++
7 files changed, 140 insertions(+), 52 deletions(-)
delete mode 100644 src/bin/ddns/tests/ddns_test.in
-----------------------------------------------------------------------
diff --git a/src/bin/ddns/b10-ddns.8 b/src/bin/ddns/b10-ddns.8
index d1e05d9..3e2e102 100644
--- a/src/bin/ddns/b10-ddns.8
+++ b/src/bin/ddns/b10-ddns.8
@@ -68,7 +68,7 @@ The configurable settings are:
.PP
\fIzones\fR
-The zones option is a named set of zones that can be updated with DDNS\&. Each entry has one element called update_acls, which itself is a list of ACLs that define update permissions\&. By default this is empty; DDNS must be explicitely enabled per zone\&.
+The zones option is a named set of zones that can be updated with DDNS\&. Each entry has one element called update_acl, which is is a list of access control rules that define update permissions\&. By default this is empty; DDNS must be explicitely enabled per zone\&.
.PP
The module commands are:
.PP
diff --git a/src/bin/ddns/b10-ddns.xml b/src/bin/ddns/b10-ddns.xml
index 73bdbc2..d38b5f1 100644
--- a/src/bin/ddns/b10-ddns.xml
+++ b/src/bin/ddns/b10-ddns.xml
@@ -106,8 +106,8 @@
<para>
<varname>zones</varname>
The zones option is a named set of zones that can be updated with
- DDNS. Each entry has one element called update_acls, which itself
- is a list of ACLs that define update permissions.
+ DDNS. Each entry has one element called update_acl, which is
+ is a list of access control rules that define update permissions.
By default this is empty; DDNS must be explicitely enabled per zone.
</para>
diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in
index d296fdf..3527979 100755
--- a/src/bin/ddns/ddns.py.in
+++ b/src/bin/ddns/ddns.py.in
@@ -63,15 +63,6 @@ class DDNSSession:
def __init__(self):
'''Initialize a DDNS Session'''
- self._handle()
-
- def _handle(self):
- '''
- Handle a DDNS update.
- This should be called from the initializer, and should contain the
- logic for doing the checks, handling the update, and responding with
- the result.
- '''
pass
class DDNSServer:
@@ -120,6 +111,7 @@ class DDNSServer:
so the main loop in run() stops.
'''
self._shutdown = True
+ logger.info(DDNS_SHUTDOWN)
def run(self):
'''
@@ -128,7 +120,13 @@ class DDNSServer:
'''
logger.info(DDNS_RUNNING)
while not self._shutdown:
+ # We do not catch any exceptions here right now, but this would
+ # be a good place to catch any exceptions that b10-ddns can
+ # recover from. We currently have no exception hierarchy to
+ # make such a distinction easily, but once we do, this would
+ # be the place to catch.
self._cc.check_command(False)
+ logger.info(DDNS_STOPPED)
def create_signal_handler(ddns_server):
'''
@@ -142,8 +140,7 @@ def create_signal_handler(ddns_server):
here, the actual signal is not checked and the server is simply shut
down.
'''
- if ddns_server is not None:
- ddns_server.shutdown()
+ ddns_server.shutdown()
return signal_handler
def set_signal_handler(signal_handler):
@@ -160,7 +157,17 @@ def set_cmd_options(parser):
parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
help="display more about what is going on")
-if '__main__' == __name__:
+def main(ddns_server=None):
+ '''
+ The main function.
+ Parameters:
+ ddns_server: If None (default), a DDNSServer object is initialized.
+ If specified, the given DDNSServer will be used. This is
+ mainly used for testing.
+ cc_session: If None (default), a new ModuleCCSession will be set up.
+ If specified, the given session will be used. This is
+ mainly used for testing.
+ '''
try:
parser = OptionParser()
set_cmd_options(parser)
@@ -168,8 +175,8 @@ if '__main__' == __name__:
if options.verbose:
print("[b10-ddns] Warning: -v verbose option is ignored at this point.")
- ddns_server = DDNSServer()
- logger.debug(10, DDNS_STOPPED_BY_KEYBOARD)
+ if ddns_server is None:
+ ddns_server = DDNSServer()
set_signal_handler(create_signal_handler(ddns_server))
ddns_server.run()
except KeyboardInterrupt:
@@ -182,3 +189,8 @@ if '__main__' == __name__:
logger.error(DDNS_CONFIG_ERROR, str(e))
except SessionTimeout as e:
logger.error(DDNS_CC_SESSION_TIMEOUT_ERROR)
+ except Exception as e:
+ logger.error(DDNS_UNCAUGHT_EXCEPTION, type(e).__name__, str(e))
+
+if '__main__' == __name__:
+ main()
diff --git a/src/bin/ddns/ddns.spec b/src/bin/ddns/ddns.spec
index 03899e4..07cd2a9 100644
--- a/src/bin/ddns/ddns.spec
+++ b/src/bin/ddns/ddns.spec
@@ -12,12 +12,12 @@
"item_type": "map",
"item_optional": true,
"item_default": {
- "update_acls": [{"action": "ACCEPT", "from": "127.0.0.1"},
- {"action": "ACCEPT", "from": "::1"}]
+ "update_acl": [{"action": "ACCEPT", "from": "127.0.0.1"},
+ {"action": "ACCEPT", "from": "::1"}]
},
"map_item_spec": [
{
- "item_name": "update_acls",
+ "item_name": "update_acl",
"item_type": "list",
"item_optional": false,
"list_item_spec": {
diff --git a/src/bin/ddns/ddns_messages.mes b/src/bin/ddns/ddns_messages.mes
index 286ae7f..36c6ed1 100644
--- a/src/bin/ddns/ddns_messages.mes
+++ b/src/bin/ddns/ddns_messages.mes
@@ -15,9 +15,13 @@
# No namespace declaration - these constants go in the global namespace
# of the ddns messages python module.
+# When you add a message to this file, it is a good idea to run
+# <topsrcdir>/tools/reorder_message_file.py to make sure the
+# messages are in the correct order.
+
% DDNS_CC_SESSION_ERROR error reading from cc channel: %1
There was a problem reading from the command and control channel. The
-most likely cause is that the msgq daemon is not running.
+most likely cause is that the msgq process is not running.
% DDNS_CC_SESSION_TIMEOUT_ERROR timeout waiting for cc response
There was a problem reading a response from another module over the
@@ -36,13 +40,27 @@ failed to start at initialization. A detailed error message from the module
will also be displayed.
% DDNS_RECEIVED_SHUTDOWN_COMMAND shutdown command received
-The ddns daemon received a shutdown command from the command channel
+The ddns process received a shutdown command from the command channel
and will now shut down.
% DDNS_RUNNING ddns server is running and listening for updates
-The ddns daemon has successfully started and is now ready to receive commands
+The ddns process has successfully started and is now ready to receive commands
and updates.
+% DDNS_SHUTDOWN ddns server shutting down
+The ddns process is shutting down. It will no longer listen for new commands
+or updates. Any command or update that is being addressed at this moment will
+be completed, after which the process will exit.
+
+% DDNS_STOPPED ddns server has stopped
+The ddns process has successfully stopped and is no longer listening for or
+handling commands or updates, and will now exit.
+
% DDNS_STOPPED_BY_KEYBOARD keyboard interrupt, shutting down
-There was a keyboard interrupt signal to stop the ddns daemon. The
-daemon will now shut down.
+There was a keyboard interrupt signal to stop the ddns process. The
+process will now shut down.
+
+% DDNS_UNCAUGHT_EXCEPTION uncaught exception of type %1: %2
+The b10-ddns process encountered an uncaught exception and will now shut
+down. This is indicative of a programming error and should not happen under
+normal circumstances. The exception type and message are printed.
diff --git a/src/bin/ddns/tests/ddns_test.in b/src/bin/ddns/tests/ddns_test.in
deleted file mode 100644
index e61c924..0000000
--- a/src/bin/ddns/tests/ddns_test.in
+++ /dev/null
@@ -1,27 +0,0 @@
-#! /bin/sh
-
-# Copyright (C) 2011 Internet Systems Consortium.
-#
-# 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.
-
-PYTHON_EXEC=${PYTHON_EXEC:- at PYTHON@}
-export PYTHON_EXEC
-
-TEST_PATH=@abs_top_srcdir@/src/bin/ddns/tests
-PYTHONPATH=@abs_top_srcdir@/src/bin/ddns:@abs_top_srcdir@/src/lib/python
-export PYTHONPATH
-
-cd ${TEST_PATH}
-exec ${PYTHON_EXEC} -O ddns_test.py $*
-
diff --git a/src/bin/ddns/tests/ddns_test.py b/src/bin/ddns/tests/ddns_test.py
index e18a990..0ebfe79 100755
--- a/src/bin/ddns/tests/ddns_test.py
+++ b/src/bin/ddns/tests/ddns_test.py
@@ -32,6 +32,32 @@ class MyCCSession(isc.config.ConfigData):
'''Called by DDNSServer initialization, but not used in tests'''
self._started = True
+class MyDDNSServer():
+ '''Fake DDNS server used to test the main() function'''
+ def __init__(self):
+ self.reset()
+
+ def run(self):
+ '''
+ Fake the run() method of the DDNS server. This will set
+ self._run_called to True.
+ If self._exception is not None, this is raised as an exception
+ '''
+ self.run_called = True
+ if self._exception is not None:
+ self.exception_raised = True
+ raise self._exception
+
+ def set_exception(self, exception):
+ '''Set an exception to be raised when run() is called'''
+ self._exception = exception
+
+ def reset(self):
+ '''(Re)set to initial values'''
+ self.run_called = False
+ self.exception_raised = False
+ self._exception = None
+
class TestDDNSServer(unittest.TestCase):
def setUp(self):
cc_session = MyCCSession()
@@ -67,6 +93,65 @@ class TestDDNSServer(unittest.TestCase):
signal_handler(None, None)
self.assertTrue(self.ddns_server._shutdown)
+class TestMain(unittest.TestCase):
+ def setUp(self):
+ self._server = MyDDNSServer()
+
+ def test_main(self):
+ self.assertFalse(self._server.run_called)
+ ddns.main(self._server)
+ self.assertTrue(self._server.run_called)
+
+ def test_exceptions(self):
+ '''
+ Test whether exceptions are caught in main()
+ These exceptions should not bubble up.
+ '''
+ self._server.set_exception(KeyboardInterrupt())
+ self.assertFalse(self._server.exception_raised)
+ ddns.main(self._server)
+ self.assertTrue(self._server.exception_raised)
+
+ # Should technically not be necessary, but reset server to be sure
+ self._server.reset()
+ self.assertFalse(self._server.exception_raised)
+ self._server.set_exception(isc.cc.SessionError("error"))
+ ddns.main(self._server)
+ self.assertTrue(self._server.exception_raised)
+
+ self._server.reset()
+ self.assertFalse(self._server.exception_raised)
+ self._server.set_exception(isc.config.ModuleCCSessionError("error"))
+ ddns.main(self._server)
+ self.assertTrue(self._server.exception_raised)
+
+ self._server.reset()
+ self.assertFalse(self._server.exception_raised)
+ self._server.set_exception(ddns.DDNSConfigError("error"))
+ ddns.main(self._server)
+ self.assertTrue(self._server.exception_raised)
+
+ self._server.reset()
+ self.assertFalse(self._server.exception_raised)
+ self._server.set_exception(isc.cc.SessionTimeout("error"))
+ ddns.main(self._server)
+ self.assertTrue(self._server.exception_raised)
+
+ self._server.reset()
+ self.assertFalse(self._server.exception_raised)
+ self._server.set_exception(Exception("error"))
+ ddns.main(self._server)
+ self.assertTrue(self._server.exception_raised)
+
+ # Add one that is not a subclass of Exception, and hence not
+ # caught. Misuse BaseException for that.
+ self._server.reset()
+ self.assertFalse(self._server.exception_raised)
+ self._server.set_exception(BaseException("error"))
+ self.assertRaises(BaseException, ddns.main, self._server)
+ self.assertTrue(self._server.exception_raised)
+
+
if __name__== "__main__":
isc.log.resetUnitTestRootLogger()
unittest.main()
More information about the bind10-changes
mailing list