Merge branch 'trac1010'

src/bin/xfrout/xfrout.py.in

@@ -559,7 +559,7 @@ class XfroutServer:
         #self._log = None
         self._listen_sock_file = UNIX_SOCKET_FILE
         self._shutdown_event = threading.Event()
-        self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler)
+        self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler, None, True)
         self._config_data = self._cc.get_full_config()
         self._cc.start()
         self._cc.add_remote_config(AUTH_SPECFILE_LOCATION);

src/lib/config/ccsession.cc

@@ -247,7 +247,9 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
 } // end anonymous namespace
 
 void
-my_logconfig_handler(const std::string&n, ConstElementPtr new_config, const ConfigData& config_data) {
+default_logconfig_handler(const std::string& module_name,
+                          ConstElementPtr new_config,
+                          const ConfigData& config_data) {
     config_data.getModuleSpec().validateConfig(new_config, true);
 
     std::vector<isc::log::LoggerSpecification> specs;
@@ -353,7 +355,7 @@ ModuleCCSession::ModuleCCSession(
 
     // Keep track of logging settings automatically
     if (handle_logging) {
-        addRemoteConfig("Logging", my_logconfig_handler, false);
+        addRemoteConfig("Logging", default_logconfig_handler, false);
     }
 
     if (start_immediately) {

src/lib/config/ccsession.h

@@ -354,6 +354,25 @@ private:
     ModuleSpec fetchRemoteSpec(const std::string& module, bool is_filename);
 };
 
+/// \brief Default handler for logging config updates
+///
+/// When CCSession is initialized with handle_logging set to true,
+/// this callback will be used to update the logger when a configuration
+/// change comes in.
+///
+/// This function updates the (global) loggers by initializing a
+/// LoggerManager and passing the settings as specified in the given
+/// configuration update.
+///
+/// \param module_name The name of the module
+/// \param new_config The modified configuration values
+/// \param config_data The full config data for the (remote) logging
+///                    module.
+void
+default_logconfig_handler(const std::string& module_name,
+                          isc::data::ConstElementPtr new_config,
+                          const ConfigData& config_data);
+
 }
 }
 #endif // __CCSESSION_H

src/lib/python/isc/config/ccsession.py

@@ -39,6 +39,10 @@
 from isc.cc import Session
 from isc.config.config_data import ConfigData, MultiConfigData, BIND10_CONFIG_DATA_VERSION
 import isc
+from isc.util.file import path_search
+import bind10_config
+from isc.log import log_config_update
+import json
 
 class ModuleCCSessionError(Exception): pass
 
@@ -116,6 +120,18 @@ def create_command(command_name, params = None):
     msg = { 'command': cmd }
     return msg
 
+def default_logconfig_handler(new_config, config_data):
+    errors = []
+
+    if config_data.get_module_spec().validate_config(False, new_config, errors):
+        isc.log.log_config_update(json.dumps(new_config),
+            json.dumps(config_data.get_module_spec().get_full_spec()))
+    else:
+        # no logging here yet, TODO: log these errors
+        print("Error in logging configuration, ignoring config update: ")
+        for err in errors:
+            print(err)
+
 class ModuleCCSession(ConfigData):
     """This class maintains a connection to the command channel, as
        well as configuration options for modules. The module provides
@@ -126,7 +142,7 @@ class ModuleCCSession(ConfigData):
        callbacks are called when 'check_command' is called on the
        ModuleCCSession"""
        
-    def __init__(self, spec_file_name, config_handler, command_handler, cc_session = None):
+    def __init__(self, spec_file_name, config_handler, command_handler, cc_session = None, handle_logging_config = False):
         """Initialize a ModuleCCSession. This does *NOT* send the
            specification and request the configuration yet. Use start()
            for that once the ModuleCCSession has been initialized.
@@ -149,6 +165,11 @@ class ModuleCCSession(ConfigData):
         self._session.group_subscribe(self._module_name, "*")
 
         self._remote_module_configs = {}
+        self._remote_module_callbacks = {}
+
+        if handle_logging_config:
+            self.add_remote_config(path_search('logging.spec', bind10_config.PLUGIN_PATHS),
+                                   default_logconfig_handler)
 
     def __del__(self):
         # If the CC Session obejct has been closed, it returns
@@ -218,6 +239,9 @@ class ModuleCCSession(ConfigData):
                             newc = self._remote_module_configs[module_name].get_local_config()
                             isc.cc.data.merge(newc, new_config)
                             self._remote_module_configs[module_name].set_local_config(newc)
+                            if self._remote_module_callbacks[module_name] != None:
+                                self._remote_module_callbacks[module_name](new_config,
+                                                                           self._remote_module_configs[module_name])
                         # For other modules, we're not supposed to answer
                         return
 
@@ -260,7 +284,7 @@ class ModuleCCSession(ConfigData):
            and return an answer created with create_answer()"""
         self._command_handler = command_handler
 
-    def add_remote_config(self, spec_file_name):
+    def add_remote_config(self, spec_file_name, config_update_callback = None):
         """Gives access to the configuration of a different module.
            These remote module options can at this moment only be
            accessed through get_remote_config_value(). This function
@@ -289,9 +313,12 @@ class ModuleCCSession(ConfigData):
             if rcode == 0:
                 if value != None and module_spec.validate_config(False, value):
                     module_cfg.set_local_config(value);
+                    if config_update_callback is not None:
+                        config_update_callback(value, module_cfg)
 
         # all done, add it
         self._remote_module_configs[module_name] = module_cfg
+        self._remote_module_callbacks[module_name] = config_update_callback
         return module_name
         
     def remove_remote_config(self, module_name):
@@ -299,6 +326,7 @@ class ModuleCCSession(ConfigData):
         if module_name in self._remote_module_configs:
             self._session.group_unsubscribe(module_name)
             del self._remote_module_configs[module_name]
+            del self._remote_module_callbacks[module_name]
 
     def get_remote_config_value(self, module_name, identifier):
         """Returns the current setting for the given identifier at the

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

@@ -14,6 +14,7 @@ endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \
+	B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
 	CONFIG_TESTDATA_PATH=$(abs_top_srcdir)/src/lib/config/tests/testdata \
 	CONFIG_WR_TESTDATA_PATH=$(abs_top_builddir)/src/lib/config/tests/testdata \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \

src/lib/python/isc/config/tests/ccsession_test.py

@@ -22,6 +22,7 @@ import os
 from isc.config.ccsession import *
 from isc.config.config_data import BIND10_CONFIG_DATA_VERSION
 from unittest_fakesession import FakeModuleCCSession, WouldBlockForever
+import bind10_config
 
 class TestHelperFunctions(unittest.TestCase):
     def test_parse_answer(self):
@@ -604,7 +605,43 @@ class TestModuleCCSession(unittest.TestCase):
         self.assertEqual(len(fake_session.message_queue), 1)
         mccs.check_command()
         self.assertEqual(len(fake_session.message_queue), 0)
-        
+
+    def test_logconfig_handler(self):
+        # test whether default_logconfig_handler reacts nicely to
+        # bad data. We assume the actual logger output is tested
+        # elsewhere
+        self.assertRaises(TypeError, default_logconfig_handler);
+        self.assertRaises(TypeError, default_logconfig_handler, 1);
+
+        spec = isc.config.module_spec_from_file(
+            path_search('logging.spec', bind10_config.PLUGIN_PATHS))
+        config_data = ConfigData(spec)
+
+        self.assertRaises(TypeError, default_logconfig_handler, 1, config_data)
+
+        default_logconfig_handler({}, config_data)
+
+        # Wrong data should not raise, but simply not be accepted
+        # This would log a lot of errors, so we may want to suppress that later
+        default_logconfig_handler({ "bad_data": "indeed" }, config_data)
+        default_logconfig_handler({ "bad_data": 1}, config_data)
+        default_logconfig_handler({ "bad_data": 1123 }, config_data)
+        default_logconfig_handler({ "bad_data": True }, config_data)
+        default_logconfig_handler({ "bad_data": False }, config_data)
+        default_logconfig_handler({ "bad_data": 1.1 }, config_data)
+        default_logconfig_handler({ "bad_data": [] }, config_data)
+        default_logconfig_handler({ "bad_data": [[],[],[[1, 3, False, "foo" ]]] },
+                                  config_data)
+        default_logconfig_handler({ "bad_data": [ 1, 2, { "b": { "c": "d" } } ] },
+                                  config_data)
+
+        # Try a correct config
+        log_conf = {"loggers":
+                       [{"name": "b10-xfrout", "output_options":
+                           [{"output": "/tmp/bind10.log",
+                                       "destination": "file",
+                                       "flush": True}]}]}
+        default_logconfig_handler(log_conf, config_data)
 
 class fakeData:
     def decode(self):

src/lib/python/isc/log/Makefile.am

@@ -15,6 +15,9 @@ log_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
 log_la_LDFLAGS = $(PYTHON_LDFLAGS)
 log_la_LDFLAGS += -module
 log_la_LIBADD = $(top_builddir)/src/lib/log/liblog.la
+log_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la
+log_la_LIBADD += $(top_builddir)/src/lib/config/libcfgclient.la
+log_la_LIBADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 log_la_LIBADD += $(PYTHON_LIB)
 
 # This is not installed, it helps locate the module during tests

src/lib/python/isc/log/__init__.py

@@ -23,7 +23,14 @@
 # Should we look there? Or define something in bind10_config?
 
 import os
+import sys
+
 cwd = os.getcwd()
-pos = cwd.rfind('/src/')
-import sys; sys.path.insert(0, cwd[:pos] + '/src/lib/python/isc/log/.libs')
+base = os.path.split(cwd)[0]
+
+for base in sys.path:
+    loglibdir = os.path.join(base, 'isc/log/.libs')
+    if os.path.exists(loglibdir):
+        sys.path.append(loglibdir)
+
 from log import *

src/lib/python/isc/log/log.cc

@@ -22,6 +22,8 @@
 #include <log/logger_manager.h>
 #include <log/logger.h>
 
+#include <config/ccsession.h>
+
 #include <string>
 #include <boost/bind.hpp>
 
@@ -49,6 +51,14 @@ namespace {
 // NULL and will use the global dictionary.
 MessageDictionary* testDictionary = NULL;
 
+// To propagate python exceptions trough our code
+// This exception is used to signal to the calling function that a
+// proper Python Exception has already been set, and the caller
+// should now return NULL.
+// Since it is only used internally, and should not pass any
+// information itself, is is not derived from std::exception
+class InternalError {};
+
 PyObject*
 setTestDictionary(PyObject*, PyObject* args) {
     PyObject* enableO;
@@ -177,6 +187,47 @@ init(PyObject*, PyObject* args) {
     Py_RETURN_NONE;
 }
 
+PyObject*
+logConfigUpdate(PyObject*, PyObject* args) {
+    // we have no wrappers for ElementPtr and ConfigData,
+    // So we expect JSON strings and convert them.
+    // The new_config object is assumed to have been validated.
+
+    const char* new_config_json;
+    const char* mod_spec_json;
+    if (!PyArg_ParseTuple(args, "ss",
+                          &new_config_json, &mod_spec_json)) {
+        return (NULL);
+    }
+
+    try {
+        isc::data::ConstElementPtr new_config =
+            isc::data::Element::fromJSON(new_config_json);
+        isc::data::ConstElementPtr mod_spec_e =
+            isc::data::Element::fromJSON(mod_spec_json);
+        isc::config::ModuleSpec mod_spec(mod_spec_e);
+        isc::config::ConfigData config_data(mod_spec);
+        isc::config::default_logconfig_handler("logging", new_config,
+                                               config_data);
+
+        Py_RETURN_NONE;
+    } catch (const isc::data::JSONError& je) {
+        std::string error_msg = std::string("JSON format error: ") + je.what();
+        PyErr_SetString(PyExc_TypeError, error_msg.c_str());
+    } catch (const isc::data::TypeError& de) {
+        PyErr_SetString(PyExc_TypeError, "argument 1 of log_config_update "
+                                         "is not a map of config data");
+    } catch (const isc::config::ModuleSpecError& mse) {
+        PyErr_SetString(PyExc_TypeError, "argument 2 of log_config_update "
+                                         "is not a correct module specification");
+    } catch (const std::exception& e) {
+        PyErr_SetString(PyExc_RuntimeError, e.what());
+    } catch (...) {
+        PyErr_SetString(PyExc_RuntimeError, "Unknown C++ exception");
+    }
+    return (NULL);
+}
+
 PyMethodDef methods[] = {
     {"set_test_dictionary", setTestDictionary, METH_VARARGS,
         "Set or unset testing mode for message dictionary. In testing, "
@@ -198,6 +249,19 @@ PyMethodDef methods[] = {
         "logging severity (one of 'DEBUG', 'INFO', 'WARN', 'ERROR' or "
         "'FATAL'), a debug level (integer in the range 0-99) and a file name "
         "of a dictionary with message text translations."},
+    {"log_config_update", logConfigUpdate, METH_VARARGS,
+        "Update logger settings. This method is automatically used when "
+        "ModuleCCSession is initialized with handle_logging_config set "
+        "to True. When called, the first argument is the new logging "
+        "configuration (in JSON format). The second argument is "
+        "the raw specification (as returned from "
+        "ConfigData.get_module_spec().get_full_spec(), and converted to "
+        "JSON format).\n"
+        "Raises a TypeError if either argument is not a (correct) JSON "
+        "string, or if the spec is not a correct spec.\n"
+        "If this call succeeds, the global logger settings have "
+        "been updated."
+    },
     {NULL, NULL, 0, NULL}
 };
 

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

@@ -23,5 +23,6 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/log/python/.libs \
+	B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/log/tests/log_test.py

@@ -16,6 +16,9 @@
 # This tests it can be loaded, nothing more yet
 import isc.log
 import unittest
+import json
+import bind10_config
+from isc.config.ccsession import path_search
 
 class LogDict(unittest.TestCase):
     def setUp(self):
@@ -52,6 +55,33 @@ class Manager(unittest.TestCase):
         # ignore errors like missing file?
         isc.log.init("root", "INFO", 0, "/no/such/file");
 
+    def test_log_config_update(self):
+        log_spec = json.dumps(isc.config.module_spec_from_file(path_search('logging.spec', bind10_config.PLUGIN_PATHS)).get_full_spec())
+
+        self.assertRaises(TypeError, isc.log.log_config_update)
+        self.assertRaises(TypeError, isc.log.log_config_update, 1)
+        self.assertRaises(TypeError, isc.log.log_config_update, 1, 1)
+        self.assertRaises(TypeError, isc.log.log_config_update, 1, 1, 1)
+
+        self.assertRaises(TypeError, isc.log.log_config_update, 1, log_spec)
+        self.assertRaises(TypeError, isc.log.log_config_update, [], log_spec)
+        self.assertRaises(TypeError, isc.log.log_config_update, "foo", log_spec)
+        self.assertRaises(TypeError, isc.log.log_config_update, "{ '", log_spec)
+
+        # empty should pass
+        isc.log.log_config_update("{}", log_spec)
+
+        # bad spec
+        self.assertRaises(TypeError, isc.log.log_config_update, "{}", json.dumps({"foo": "bar"}))
+
+        # Try a correct one
+        log_conf = json.dumps({"loggers":
+                                [{"name": "b10-xfrout", "output_options":
+                                    [{"output": "/tmp/bind10.log",
+                                       "destination": "file",
+                                       "flush": True}]}]})
+        isc.log.log_config_update(log_conf, log_spec)
+
 class Logger(unittest.TestCase):
     def tearDown(self):
         isc.log.reset()