BIND 10 trac640, updated. 61203697e8fc714835ef504a7e3eda925dfcda42 [640] more comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Feb 8 16:18:23 UTC 2012
The branch, trac640 has been updated
via 61203697e8fc714835ef504a7e3eda925dfcda42 (commit)
via 5da80cb570399e277dd02d6d45bc4efb3f949d61 (commit)
via 5d11f8513af16aca9eb298db3226791f1f4e584a (commit)
from c6aec4cdb46dd8825248437bc9c45d6a54b81121 (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 61203697e8fc714835ef504a7e3eda925dfcda42
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Feb 8 17:18:14 2012 +0100
[640] more comments
commit 5da80cb570399e277dd02d6d45bc4efb3f949d61
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Feb 8 16:27:03 2012 +0100
[640] add test and modify XfroutServer.shutdown() a bit
commit 5d11f8513af16aca9eb298db3226791f1f4e584a
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Feb 8 14:41:49 2012 +0100
[640] initial part of review comments
-----------------------------------------------------------------------
Summary of changes:
src/bin/bind10/bind10_src.py.in | 3 +-
src/bin/cmdctl/cmdctl.py.in | 9 +-
src/bin/cmdctl/tests/cmdctl_test.py | 11 +
src/bin/stats/tests/b10-stats_test.py | 4 +
src/bin/xfrout/tests/xfrout_test.py.in | 25 +++
src/bin/xfrout/xfrout.py.in | 12 +-
src/lib/config/ccsession.cc | 18 +-
src/lib/config/ccsession.h | 4 +-
src/lib/python/isc/config/ccsession.py | 28 ++-
src/lib/python/isc/config/cfgmgr.py | 34 ++--
src/lib/python/isc/config/tests/cfgmgr_test.py | 246 ++++++++++++++--------
src/lib/python/isc/testutils/ccsession_mock.py | 10 +-
tests/lettuce/features/terrain/bind10_control.py | 2 +-
13 files changed, 270 insertions(+), 136 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in
index 805275a..e8bfdef 100755
--- a/src/bin/bind10/bind10_src.py.in
+++ b/src/bin/bind10/bind10_src.py.in
@@ -683,8 +683,7 @@ class BoB:
# is stopping. Since everything will be stopped shortly, this is not
# really necessary, but this is done to reflect that boss is also
# 'just' a module.
- if self.ccs is not None:
- self.ccs.send_stopping()
+ self.ccs.send_stopping()
# try using the BIND 10 request to stop
try:
diff --git a/src/bin/cmdctl/cmdctl.py.in b/src/bin/cmdctl/cmdctl.py.in
index 5baa143..5163c3a 100755
--- a/src/bin/cmdctl/cmdctl.py.in
+++ b/src/bin/cmdctl/cmdctl.py.in
@@ -315,8 +315,13 @@ class CommandControl():
# set it. If it is None, delete the module if it is
# known (and ignore otherwise).
with self._lock:
- if args[1] is None and args[0] in self.modules_spec:
- del self.modules_spec[args[0]]
+ if args[1] is None:
+ if args[0] in self.modules_spec:
+ del self.modules_spec[args[0]]
+ else:
+ answer = ccsession.create_answer(1,
+ 'No such module: ' +
+ args[0])
else:
self.modules_spec[args[0]] = args[1]
diff --git a/src/bin/cmdctl/tests/cmdctl_test.py b/src/bin/cmdctl/tests/cmdctl_test.py
index 22b9004..5fdabb4 100644
--- a/src/bin/cmdctl/tests/cmdctl_test.py
+++ b/src/bin/cmdctl/tests/cmdctl_test.py
@@ -368,6 +368,17 @@ class TestCommandControl(unittest.TestCase):
# Should no longer be present
self.assertFalse("foo" in self.cmdctl.modules_spec)
+ # Don't store 'None' if it wasn't there in the first place!
+ answer = self.cmdctl.command_handler(
+ ccsession.COMMAND_MODULE_SPECIFICATION_UPDATE, [ "foo", None ])
+ rcode, msg = ccsession.parse_answer(answer)
+ self.assertEqual(rcode, 1)
+ self.assertEqual(msg, "No such module: foo")
+
+ # Should still not present
+ self.assertFalse("foo" in self.cmdctl.modules_spec)
+
+
def test_check_config_handler(self):
answer = self.cmdctl.config_handler({'non-exist': 123})
self._check_answer(answer, 1, 'unknown config item: non-exist')
diff --git a/src/bin/stats/tests/b10-stats_test.py b/src/bin/stats/tests/b10-stats_test.py
index 3c8599a..f5a9c48 100644
--- a/src/bin/stats/tests/b10-stats_test.py
+++ b/src/bin/stats/tests/b10-stats_test.py
@@ -31,6 +31,7 @@ import imp
import stats
import isc.cc.session
from test_utils import BaseModules, ThreadingServerManager, MyStats, SignalHandler, send_command, send_shutdown
+from isc.testutils.ccsession_mock import MockModuleCCSession
class TestUtilties(unittest.TestCase):
items = [
@@ -201,7 +202,10 @@ class TestStats(unittest.TestCase):
self.assertEqual(send_command("status", "Stats"),
(0, "Stats is up. (PID " + str(os.getpid()) + ")"))
self.assertTrue(self.stats.running)
+ # Override moduleCCSession so we can check if send_stopping is called
+ self.stats.mccs = MockModuleCCSession()
self.assertEqual(send_shutdown("Stats"), (0, None))
+ self.assertTrue(self.stats.mccs.stopped)
self.assertFalse(self.stats.running)
self.stats_server.shutdown()
diff --git a/src/bin/xfrout/tests/xfrout_test.py.in b/src/bin/xfrout/tests/xfrout_test.py.in
index 396139e..2176126 100644
--- a/src/bin/xfrout/tests/xfrout_test.py.in
+++ b/src/bin/xfrout/tests/xfrout_test.py.in
@@ -19,6 +19,7 @@
import unittest
import os
from isc.testutils.tsigctx_mock import MockTSIGContext
+from isc.testutils.ccsession_mock import MockModuleCCSession
from isc.cc.session import *
import isc.config
from isc.dns import *
@@ -1423,6 +1424,30 @@ class TestInitialization(unittest.TestCase):
xfrout.init_paths()
self.assertEqual(xfrout.UNIX_SOCKET_FILE, "The/Socket/File")
+class MyNotifier():
+ def __init__(self):
+ self.shutdown_called = False
+
+ def shutdown(self):
+ self.shutdown_called = True
+
+class MyXfroutServer(XfroutServer):
+ def __init__(self):
+ self._cc = MockModuleCCSession()
+ self._shutdown_event = threading.Event()
+ self._notifier = MyNotifier()
+ self._unix_socket_server = None
+
+class TestXfroutServer(unittest.TestCase):
+ def setUp(self):
+ self.xfrout_server = MyXfroutServer()
+
+ def test_shutdown(self):
+ self.xfrout_server.shutdown()
+ self.assertTrue(self.xfrout_server._notifier.shutdown_called)
+ self.assertTrue(self.xfrout_server._cc.stopped)
+
+
if __name__== "__main__":
isc.log.resetUnitTestRootLogger()
unittest.main()
diff --git a/src/bin/xfrout/xfrout.py.in b/src/bin/xfrout/xfrout.py.in
index 1540e6b..e23be76 100755
--- a/src/bin/xfrout/xfrout.py.in
+++ b/src/bin/xfrout/xfrout.py.in
@@ -975,12 +975,12 @@ class XfroutServer:
if self._unix_socket_server:
self._unix_socket_server.shutdown()
- # Wait for all threads to terminate
- main_thread = threading.currentThread()
- for th in threading.enumerate():
- if th is main_thread:
- continue
- th.join()
+ # Wait for all threads to terminate
+ main_thread = threading.currentThread()
+ for th in threading.enumerate():
+ if th is main_thread:
+ continue
+ th.join()
def command_handler(self, cmd, args):
if cmd == "shutdown":
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index 63fa4cd..aa34c02 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -490,14 +490,20 @@ ModuleCCSession::ModuleCCSession(
}
ModuleCCSession::~ModuleCCSession() {
+ // Logging itself can possibly throw too
+ // TODO: add exception-free logging calls
try {
- sendStopping();
- } catch (const std::exception& exc) {
- LOG_ERROR(config_logger,
- CONFIG_CCSESSION_STOPPING).arg(exc.what());
+ try {
+ sendStopping();
+ } catch (const std::exception& exc) {
+ LOG_ERROR(config_logger,
+ CONFIG_CCSESSION_STOPPING).arg(exc.what());
+ } catch (...) {
+ LOG_ERROR(config_logger,
+ CONFIG_CCSESSION_STOPPING_UNKNOWN);
+ }
} catch (...) {
- LOG_ERROR(config_logger,
- CONFIG_CCSESSION_STOPPING_UNKNOWN);
+ // Can't really do anything at this point...
}
};
diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h
index d3225d2..059968c 100644
--- a/src/lib/config/ccsession.h
+++ b/src/lib/config/ccsession.h
@@ -195,10 +195,10 @@ public:
///
/// Destructor
///
- /// The desctructor automatically calls sendStopping(), which sends
+ /// The destructor automatically calls sendStopping(), which sends
/// a message to the ConfigManager that this module is stopping
///
- ~ModuleCCSession();
+ virtual ~ModuleCCSession();
/// Start receiving new commands and configuration changes asynchronously.
///
diff --git a/src/lib/python/isc/config/ccsession.py b/src/lib/python/isc/config/ccsession.py
index caff9f3..5da56cf 100644
--- a/src/lib/python/isc/config/ccsession.py
+++ b/src/lib/python/isc/config/ccsession.py
@@ -429,11 +429,15 @@ class UIModuleCCSession(MultiConfigData):
passed must have send_GET and send_POST functions"""
MultiConfigData.__init__(self)
self._conn = conn
- self.request_specifications()
- self.request_current_config()
+ self.update_specs_and_config()
def request_specifications(self):
- """Request the module specifications from b10-cmdctl"""
+ """Clears the current list of specifications, and requests a new
+ list from b10-cmdctl. As other actions may have caused modules
+ to be stopped, or new modules to be added, this is expected to
+ be run after each interaction (at this moment). It is usually
+ also combined with request_current_config(). For that reason,
+ we provide update_specs_and_config() which calls both."""
# this step should be unnecessary but is the current way cmdctl returns stuff
# so changes are needed there to make this clean (we need a command to simply get the
# full specs for everything, including commands etc, not separate gets for that)
@@ -442,18 +446,24 @@ class UIModuleCCSession(MultiConfigData):
for module in specs.keys():
self.set_specification(isc.config.ModuleSpec(specs[module]))
- def update_specs_and_config(self):
- self.request_specifications()
- self.request_current_config()
-
def request_current_config(self):
"""Requests the current configuration from the configuration
- manager through b10-cmdctl, and stores those as CURRENT"""
+ manager through b10-cmdctl, and stores those as CURRENT. This
+ does not modify any local changes, it just updates to the current
+ state of the server itself."""
config = self._conn.send_GET('/config_data')
if 'version' not in config or config['version'] != BIND10_CONFIG_DATA_VERSION:
raise ModuleCCSessionError("Bad config version")
self._set_current_config(config)
+ def update_specs_and_config(self):
+ """Convenience function to both clear and update the known list of
+ module specifications, and update the current configuration on
+ the server side. There are a few cases where the caller might only
+ want to run one of these tasks, but often they are both needed."""
+ self.request_specifications()
+ self.request_current_config()
+
def _add_value_to_list(self, identifier, value, module_spec):
cur_list, status = self.get_value(identifier)
if not cur_list:
@@ -602,7 +612,7 @@ class UIModuleCCSession(MultiConfigData):
# answer is either an empty dict (on success), or one
# containing errors
if answer == {}:
- self.request_current_config()
+ self.update_specs_and_config()
self.clear_local_changes()
elif "error" in answer:
raise ModuleCCSessionError("Error: " + str(answer["error"]) + "\n" + "Configuration not committed")
diff --git a/src/lib/python/isc/config/cfgmgr.py b/src/lib/python/isc/config/cfgmgr.py
index f7ecce9..dd97827 100644
--- a/src/lib/python/isc/config/cfgmgr.py
+++ b/src/lib/python/isc/config/cfgmgr.py
@@ -297,7 +297,7 @@ class ConfigManager:
"""Write the current configuration to the file specificied at init()"""
self.config.write_to_file()
- def _handle_get_module_spec(self, cmd):
+ def __handle_get_module_spec(self, cmd):
"""Private function that handles the 'get_module_spec' command"""
answer = {}
if cmd != None:
@@ -318,7 +318,7 @@ class ConfigManager:
answer = ccsession.create_answer(0, self.get_module_spec())
return answer
- def _handle_get_config_dict(self, cmd):
+ def __handle_get_config_dict(self, cmd):
"""Private function that handles the 'get_config' command
where the command has been checked to be a dict"""
if 'module_name' in cmd and cmd['module_name'] != '':
@@ -332,17 +332,17 @@ class ConfigManager:
else:
return ccsession.create_answer(1, "Bad module_name in get_config command")
- def _handle_get_config(self, cmd):
+ def __handle_get_config(self, cmd):
"""Private function that handles the 'get_config' command"""
if cmd != None:
if type(cmd) == dict:
- return self._handle_get_config_dict(cmd)
+ return self.__handle_get_config_dict(cmd)
else:
return ccsession.create_answer(1, "Bad get_config command, argument not a dict")
else:
return ccsession.create_answer(0, self.config.data)
- def _handle_set_config_module(self, module_name, cmd):
+ def __handle_set_config_module(self, module_name, cmd):
# the answer comes (or does not come) from the relevant module
# so we need a variable to see if we got it
answer = None
@@ -405,7 +405,7 @@ class ConfigManager:
self.config.data = old_data
return answer
- def _handle_set_config_all(self, cmd):
+ def __handle_set_config_all(self, cmd):
old_data = copy.deepcopy(self.config.data)
got_error = False
err_list = []
@@ -413,7 +413,7 @@ class ConfigManager:
# sets, so we simply call set_config_module for each of those
for module in cmd:
if module != "version":
- answer = self._handle_set_config_module(module, cmd[module])
+ answer = self.__handle_set_config_module(module, cmd[module])
if answer == None:
got_error = True
err_list.append("No answer message from " + module)
@@ -432,16 +432,16 @@ class ConfigManager:
self.config.data = old_data
return ccsession.create_answer(1, " ".join(err_list))
- def _handle_set_config(self, cmd):
+ def __handle_set_config(self, cmd):
"""Private function that handles the 'set_config' command"""
answer = None
if cmd == None:
return ccsession.create_answer(1, "Wrong number of arguments")
if len(cmd) == 2:
- answer = self._handle_set_config_module(cmd[0], cmd[1])
+ answer = self.__handle_set_config_module(cmd[0], cmd[1])
elif len(cmd) == 1:
- answer = self._handle_set_config_all(cmd[0])
+ answer = self.__handle_set_config_all(cmd[0])
else:
answer = ccsession.create_answer(1, "Wrong number of arguments")
if not answer:
@@ -449,7 +449,7 @@ class ConfigManager:
return answer
- def _handle_module_spec(self, spec):
+ def __handle_module_spec(self, spec):
"""Private function that handles the 'module_spec' command"""
# todo: validate? (no direct access to spec as
# todo: use ModuleSpec class
@@ -460,7 +460,7 @@ class ConfigManager:
spec.get_full_spec())
return ccsession.create_answer(0)
- def _handle_module_stopping(self, arg):
+ def __handle_module_stopping(self, arg):
"""Private function that handles a 'stopping' command;
The argument is of the form { 'module_name': <name> }.
If the module is known, it is removed from the known list,
@@ -495,19 +495,19 @@ class ConfigManager:
elif cmd == ccsession.COMMAND_GET_STATISTICS_SPEC:
answer = ccsession.create_answer(0, self.get_statistics_spec())
elif cmd == ccsession.COMMAND_GET_MODULE_SPEC:
- answer = self._handle_get_module_spec(arg)
+ answer = self.__handle_get_module_spec(arg)
elif cmd == ccsession.COMMAND_GET_CONFIG:
- answer = self._handle_get_config(arg)
+ answer = self.__handle_get_config(arg)
elif cmd == ccsession.COMMAND_SET_CONFIG:
- answer = self._handle_set_config(arg)
+ answer = self.__handle_set_config(arg)
elif cmd == ccsession.COMMAND_MODULE_STOPPING:
- answer = self._handle_module_stopping(arg)
+ answer = self.__handle_module_stopping(arg)
elif cmd == ccsession.COMMAND_SHUTDOWN:
self.running = False
answer = ccsession.create_answer(0)
elif cmd == ccsession.COMMAND_MODULE_SPEC:
try:
- answer = self._handle_module_spec(isc.config.ModuleSpec(arg))
+ answer = self.__handle_module_spec(isc.config.ModuleSpec(arg))
except isc.config.ModuleSpecError as dde:
answer = ccsession.create_answer(1, "Error in data definition: " + str(dde))
else:
diff --git a/src/lib/python/isc/config/tests/cfgmgr_test.py b/src/lib/python/isc/config/tests/cfgmgr_test.py
index c38d454..0eb0171 100644
--- a/src/lib/python/isc/config/tests/cfgmgr_test.py
+++ b/src/lib/python/isc/config/tests/cfgmgr_test.py
@@ -240,9 +240,6 @@ class TestConfigManager(unittest.TestCase):
def test_read_config(self):
self.assertEqual(self.cm.config.data, {'version': config_data.BIND10_CONFIG_DATA_VERSION})
- self.cm.read_config()
- # due to what get written, the value here is what the last set_config command in test_handle_msg does
- self.assertEqual(self.cm.config.data, {'TestModule': {'test': 125}, 'version': config_data.BIND10_CONFIG_DATA_VERSION})
self.cm.data_path = "/no_such_path"
self.cm.read_config()
self.assertEqual(self.cm.config.data, {'version': config_data.BIND10_CONFIG_DATA_VERSION})
@@ -255,115 +252,174 @@ class TestConfigManager(unittest.TestCase):
answer = self.cm.handle_msg(msg)
self.assertEqual(expected_answer, answer)
- def test_handle_msg(self):
- self._handle_msg_helper({}, { 'result': [ 1, 'Unknown message format: {}']})
- self._handle_msg_helper("", { 'result': [ 1, 'Unknown message format: ']})
- self._handle_msg_helper({ "command": [ "badcommand" ] }, { 'result': [ 1, "Unknown command: badcommand"]})
- self._handle_msg_helper({ "command": [ "get_commands_spec" ] }, { 'result': [ 0, {} ]})
- self._handle_msg_helper({ "command": [ "get_statistics_spec" ] }, { 'result': [ 0, {} ]})
- self._handle_msg_helper({ "command": [ "get_module_spec" ] }, { 'result': [ 0, {} ]})
- self._handle_msg_helper({ "command": [ "get_module_spec", { "module_name": "Spec2" } ] }, { 'result': [ 0, {} ]})
- #self._handle_msg_helper({ "command": [ "get_module_spec", { "module_name": "nosuchmodule" } ] },
- # {'result': [1, 'No specification for module nosuchmodule']})
+ def test_handle_msg_basic_commands(self):
+ # Some basic commands, where not much interaction happens, just
+ # check the result
+ self._handle_msg_helper({},
+ { 'result': [ 1, 'Unknown message format: {}']})
+ self._handle_msg_helper("",
+ { 'result': [ 1, 'Unknown message format: ']})
+ self._handle_msg_helper({ "command": [ "badcommand" ] },
+ { 'result': [ 1, "Unknown command: badcommand"]})
+ self._handle_msg_helper({ "command": [ "get_commands_spec" ] },
+ { 'result': [ 0, {} ]})
+ self._handle_msg_helper({ "command": [ "get_statistics_spec" ] },
+ { 'result': [ 0, {} ]})
+ self._handle_msg_helper({ "command": [ "get_module_spec" ] },
+ { 'result': [ 0, {} ]})
+ self._handle_msg_helper({ "command": [ "get_module_spec",
+ { "module_name": "Spec2" } ] },
+ { 'result': [ 0, {} ]})
self._handle_msg_helper({ "command": [ "get_module_spec", 1 ] },
- {'result': [1, 'Bad get_module_spec command, argument not a dict']})
+ {'result': [1, 'Bad get_module_spec command, '+
+ 'argument not a dict']})
self._handle_msg_helper({ "command": [ "get_module_spec", { } ] },
- {'result': [1, 'Bad module_name in get_module_spec command']})
- self._handle_msg_helper({ "command": [ "get_config" ] }, { 'result': [ 0, { 'version': config_data.BIND10_CONFIG_DATA_VERSION } ]})
- self._handle_msg_helper({ "command": [ "get_config", { "module_name": "nosuchmodule" } ] },
- {'result': [0, { 'version': config_data.BIND10_CONFIG_DATA_VERSION }]})
+ {'result': [1, 'Bad module_name in '+
+ 'get_module_spec command']})
+ self._handle_msg_helper({ "command": [ "get_config" ] },
+ { 'result': [ 0, { 'version':
+ config_data.BIND10_CONFIG_DATA_VERSION }]})
+ self._handle_msg_helper({ "command": [ "get_config",
+ { "module_name": "nosuchmodule" } ] },
+ {'result': [0, { 'version':
+ config_data.BIND10_CONFIG_DATA_VERSION }]})
self._handle_msg_helper({ "command": [ "get_config", 1 ] },
- {'result': [1, 'Bad get_config command, argument not a dict']})
+ {'result': [1, 'Bad get_config command, '+
+ 'argument not a dict']})
self._handle_msg_helper({ "command": [ "get_config", { } ] },
- {'result': [1, 'Bad module_name in get_config command']})
+ {'result': [1, 'Bad module_name in '+
+ 'get_config command']})
self._handle_msg_helper({ "command": [ "set_config" ] },
{'result': [1, 'Wrong number of arguments']})
self._handle_msg_helper({ "command": [ "set_config", [{}]] },
{'result': [0]})
+
self.assertEqual(len(self.fake_session.message_queue), 0)
- # the targets of some of these tests expect specific answers, put
- # those in our fake msgq first.
- my_ok_answer = { 'result': [ 0 ] }
+ def test_handle_msg_module_and_stats_commands(self):
+ self._handle_msg_helper({ "command":
+ ["module_spec", self.spec.get_full_spec()]
+ },
+ {'result': [0]})
+ # There should be a message on the queue about the 'new' Spec2 module
+ # from ConfigManager to Cmdctl, containing its name and full
+ # specification
+ self.assertEqual(ccsession.create_command(
+ ccsession.COMMAND_MODULE_SPECIFICATION_UPDATE,
+ [ self.spec.get_module_name(),
+ self.spec.get_full_spec()]),
+ self.fake_session.get_message("Cmdctl", None))
+
+ self._handle_msg_helper({ "command": [ "module_spec", { 'foo': 1 } ] },
+ {'result': [1, 'Error in data definition: no '+
+ 'module_name in module_spec']})
+
+ self._handle_msg_helper({ "command": [ "get_module_spec" ] },
+ { 'result': [ 0, { self.spec.get_module_name():
+ self.spec.get_full_spec() } ]})
+ self._handle_msg_helper({ "command": [ "get_module_spec",
+ { "module_name" : "Spec2" } ] },
+ { 'result': [ 0, self.spec.get_full_spec() ] })
+ self._handle_msg_helper({ "command": [ "get_commands_spec" ] },
+ { 'result': [ 0, { self.spec.get_module_name():
+ self.spec.get_commands_spec()}]})
+ self._handle_msg_helper({ "command": [ "get_statistics_spec" ] },
+ { 'result': [ 0, { self.spec.get_module_name():
+ self.spec.get_statistics_spec()}]})
+
+ def __test_handle_msg_update_config_helper(self, new_config):
+ # Helper function for the common pattern in
+ # test_handle_msg_update_config; send 'set config', check for
+ # update message, check if config has indeed been updated
+ my_ok_answer = { 'result': [ 0 ] }
# Send the 'ok' that cfgmgr expects back to the fake queue first
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- # then send the command
- self._handle_msg_helper({ "command": [ "set_config", [self.name, { "test": 123 }] ] },
+
+ self._handle_msg_helper({ "command": [ "set_config",
+ [self.name, new_config] ] },
my_ok_answer)
- # The cfgmgr should have eaten the ok message, and sent out an update again
+
+ # The cfgmgr should have eaten the ok message, and sent out an update
+ # message
self.assertEqual(len(self.fake_session.message_queue), 1)
- self.assertEqual({'command': [ 'config_update', {'test': 123}]},
+ self.assertEqual({'command': [ 'config_update', new_config]},
self.fake_session.get_message(self.name, None))
+
+ # Config should have been updated
+ self.assertEqual(self.cm.config.data, {self.name: new_config,
+ 'version': config_data.BIND10_CONFIG_DATA_VERSION})
+
# and the queue should now be empty again
self.assertEqual(len(self.fake_session.message_queue), 0)
- # below are variations of the theme above
- self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self._handle_msg_helper({ "command": [ "set_config", [self.name, { "test": 124 }] ] },
- my_ok_answer)
- self.assertEqual(len(self.fake_session.message_queue), 1)
- self.assertEqual({'command': [ 'config_update', {'test': 124}]},
- self.fake_session.get_message(self.name, None))
- self.assertEqual(len(self.fake_session.message_queue), 0)
+ def test_handle_msg_update_config(self):
+ # Update the configuration and check results a few times
+ # only work the first time
+ self.__test_handle_msg_update_config_helper({ "test": 123 })
+ self.__test_handle_msg_update_config_helper({ "test": 124 })
- # This is the last 'succes' one, the value set here is what test_read_config expects
- self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self._handle_msg_helper({ "command": [ "set_config", [ { self.name: { "test": 125 } }] ] },
- my_ok_answer )
- self.assertEqual(len(self.fake_session.message_queue), 1)
- self.assertEqual({'command': [ 'config_update', {'test': 125}]},
- self.fake_session.get_message(self.name, None))
- self.assertEqual(len(self.fake_session.message_queue), 0)
+ self.__test_handle_msg_update_config_helper({ "test": 125 })
- my_bad_answer = { 'result': [1, "bad_answer"] }
+ self.__test_handle_msg_update_config_helper({ "test": 126 })
+
+ # Now send an error result (i.e. config not accepted)
+ my_bad_answer = { 'result': [1, "bad config"] }
self.fake_session.group_sendmsg(my_bad_answer, "ConfigManager")
- self._handle_msg_helper({ "command": [ "set_config", [ self.name, { "test": 125 }] ] },
+ self._handle_msg_helper({ "command": [ "set_config",
+ [self.name, { "test": 127 }] ] },
my_bad_answer )
self.assertEqual(len(self.fake_session.message_queue), 1)
- self.assertEqual({'command': [ 'config_update', {'test': 125}]},
+ self.assertEqual({'command': [ 'config_update', {'test': 127}]},
self.fake_session.get_message(self.name, None))
+ # Config should not be updated due to the error
+ self.cm.read_config()
+ self.assertEqual(self.cm.config.data, { self.name: {'test': 126},
+ 'version': config_data.BIND10_CONFIG_DATA_VERSION})
+
self.assertEqual(len(self.fake_session.message_queue), 0)
self.fake_session.group_sendmsg(None, 'ConfigManager')
self._handle_msg_helper({ "command": [ "set_config", [ ] ] },
{'result': [1, 'Wrong number of arguments']} )
- self._handle_msg_helper({ "command": [ "set_config", [ self.name, { "test": 125 }] ] },
- { 'result': [1, 'No answer message from TestModule']} )
-
- #self.assertEqual(len(self.fake_session.message_queue), 1)
- #self.assertEqual({'config_update': {'test': 124}},
- # self.fake_session.get_message(self.name, None))
- #self.assertEqual({'version': 1, 'TestModule': {'test': 124}}, self.cm.config.data)
- #
+ self._handle_msg_helper({ "command": [ "set_config",
+ [ self.name, { "test": 128 }]]},
+ { 'result': [1, 'No answer message '+
+ 'from TestModule']} )
+
+ # This command should leave a message to the TestModule to update its
+ # configuration (since the TestModule did not eat it)
+ self.assertEqual(len(self.fake_session.message_queue), 1)
+ self.assertEqual(
+ ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
+ { "test": 128 }),
+ self.fake_session.get_message("TestModule", None))
+
+ # Make sure queue is empty now
+ self.assertEqual(len(self.fake_session.message_queue), 0)
+
+ # Shutdown should result in 'ok' answer
+ self._handle_msg_helper({ "command":
+ ["shutdown"]
+ },
+ {'result': [0]})
+
+ def test_stopping_message(self):
+ # Update the system by announcing this module
self._handle_msg_helper({ "command":
["module_spec", self.spec.get_full_spec()]
},
{'result': [0]})
- self._handle_msg_helper({ "command": [ "module_spec", { 'foo': 1 } ] },
- {'result': [1, 'Error in data definition: no module_name in module_spec']})
- self._handle_msg_helper({ "command": [ "get_module_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_full_spec() } ]})
- self._handle_msg_helper({ "command": [ "get_module_spec",
- { "module_name" : "Spec2" } ] },
- { 'result': [ 0, self.spec.get_full_spec() ] })
- self._handle_msg_helper({ "command": [ "get_commands_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_commands_spec() } ]})
- self._handle_msg_helper({ "command": [ "get_statistics_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_statistics_spec() } ]})
- # re-add this once we have new way to propagate spec changes (1 instead of the current 2 messages)
- #self.assertEqual(len(self.fake_session.message_queue), 2)
- # the name here is actually wrong (and hardcoded), but needed in the current version
- # TODO: fix that
- #self.assertEqual({'specification_update': [ self.name, self.spec ] },
- # self.fake_session.get_message("Cmdctl", None))
- #self.assertEqual({'commands_update': [ self.name, self.commands ] },
- # self.fake_session.get_message("Cmdctl", None))
- # drop the two messages for now
- self.assertEqual(len(self.fake_session.message_queue), 2)
- self.fake_session.get_message("Cmdctl", None)
- self.fake_session.get_message("TestModule", None)
- self.assertEqual(len(self.fake_session.message_queue), 0)
+ # This causes a update to be sent from the ConfigManager to the CmdCtl
+ # channel, containing the new module's name and full specification
+ self.assertEqual(ccsession.create_command(
+ ccsession.COMMAND_MODULE_SPECIFICATION_UPDATE,
+ [ self.spec.get_module_name(),
+ self.spec.get_full_spec()]),
+ self.fake_session.get_message("Cmdctl", None))
# A stopping message should get no response, but should cause another
# message to be sent, if it is a known module
@@ -375,17 +431,20 @@ class TestConfigManager(unittest.TestCase):
['Spec2', None] ] },
self.fake_session.get_message("Cmdctl", None))
- # but not if it is either unknown or not running
+ # but if the 'stopping' module is either unknown or not running,
+ # no followup message should be sent
self._handle_msg_helper({ "command":
[ "stopping",
{ "module_name": "NoSuchModule" } ] },
None)
self.assertEqual(len(self.fake_session.message_queue), 0)
- self._handle_msg_helper({ "command":
- ["shutdown"]
- },
- {'result': [0]})
+ # If the module does not exist, or is not seen as 'running', the
+ # same message should not cause new messages to be sent
+ self._handle_msg_helper({ "command": [ "stopping",
+ { "module_name": "Foo"}] },
+ None)
+ self.assertEqual(len(self.fake_session.message_queue), 0)
def test_set_config_virtual(self):
"""Test that if the module is virtual, we don't send it over the
@@ -404,9 +463,9 @@ class TestConfigManager(unittest.TestCase):
self.cm.set_virtual_module(self.spec, check_test)
# The fake session will throw now if it tries to read a response.
# Handy, we don't need to find a complicated way to check for it.
- result = self.cm._handle_set_config_module(self.spec.
- get_module_name(),
- {'item1': value})
+ result = self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG,
+ [self.spec.get_module_name(), { "item1": value }]))
# Check the correct result is passed and our function was called
# With correct data
self.assertEqual(self.called_with['item1'], value)
@@ -438,19 +497,22 @@ class TestConfigManager(unittest.TestCase):
self.assertEqual({"version": 2}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value1": 123 }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value1": 123 }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value1": 123 }
}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value1": 124 }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value1": 124 }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value1": 124 }
}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value2": True }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value2": True }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value1": 124,
"value2": True
@@ -458,7 +520,8 @@ class TestConfigManager(unittest.TestCase):
}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value3": [ 1, 2, 3 ] }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value3": [ 1, 2, 3 ] }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value1": 124,
"value2": True,
@@ -467,7 +530,8 @@ class TestConfigManager(unittest.TestCase):
}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value2": False }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value2": False }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value1": 124,
"value2": False,
@@ -476,7 +540,8 @@ class TestConfigManager(unittest.TestCase):
}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value1": None }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value1": None }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value2": False,
"value3": [ 1, 2, 3 ]
@@ -484,7 +549,8 @@ class TestConfigManager(unittest.TestCase):
}, self.cm.config.data)
self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
- self.cm._handle_set_config_all({"test": { "value3": [ 1 ] }})
+ self.cm.handle_msg(ccsession.create_command(
+ ccsession.COMMAND_SET_CONFIG, ["test", { "value3": [ 1 ] }]))
self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
"test": { "value2": False,
"value3": [ 1 ]
diff --git a/src/lib/python/isc/testutils/ccsession_mock.py b/src/lib/python/isc/testutils/ccsession_mock.py
index c28d704..5f88678 100644
--- a/src/lib/python/isc/testutils/ccsession_mock.py
+++ b/src/lib/python/isc/testutils/ccsession_mock.py
@@ -14,13 +14,21 @@
# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
class MockModuleCCSession():
+ """Fake ModuleCCSession with a minimal implementation as needed by the
+ tests. Currently this module only stores whether some methods have
+ been called on it (send_stopping(), and close())"""
def __init__(self):
- self.started = False
+ """Will be set to True when send_stopping() is called"""
self.stopped = False
+ """Will be set to True when close() is called"""
self.closed = False
def send_stopping(self):
+ """Fake send_stopping() call. No message is sent, but only stores
+ that this method has been called."""
self.stopped = True
def close(self):
+ """Fake close() call. Nothing is closed, but only stores
+ that this method has been called."""
self.closed = True
diff --git a/tests/lettuce/features/terrain/bind10_control.py b/tests/lettuce/features/terrain/bind10_control.py
index 9210738..7ccd2b3 100644
--- a/tests/lettuce/features/terrain/bind10_control.py
+++ b/tests/lettuce/features/terrain/bind10_control.py
@@ -113,7 +113,7 @@ def have_bind10_running(step, config_file, cmdctl_port, process_name):
step.given(wait_step)
# function to send lines to bindctl, and store the result
-def run_bindctl(commands, cmdctl_port=47805):
+def run_bindctl(commands, cmdctl_port=None):
"""Run bindctl.
Parameters:
commands: a sequence of strings which will be sent.
More information about the bind10-changes
mailing list