[208-finish-move-logging] Fixed libprocess - need d2 unit tests

diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc
index f56dfe9a2e7002ab4329ba828ec5387b48d05a6e..8938d542bd738a90dbe46c0403de487237598a99 100644 (file)
--- a/src/lib/process/d_controller.cc
+++ b/src/lib/process/d_controller.cc
@@ -413,8 +413,7 @@ DControllerBase::configFromFile() {
         // Temporary storage for logging configuration
         ConfigPtr storage(new ConfigBase());
 
-        // Get 'Logging' element from the config and use it to set up
-        // logging. If there's no such element, we'll just pass NULL.
+        // Configure logging to the tempoary storage.
         Daemon::configureLogger(module_config, storage);
 
         // Let's apply the new logging. We do it early, so we'll be able
@@ -508,30 +507,6 @@ DControllerBase::configWriteHandler(const std::string&,
     size_t size = 0;
     ElementPtr cfg = process_->getCfgMgr()->getContext()->toElement();
 
-    // Logging storage is messed up in CA. During its configuration (see
-    // DControllerBase::configFromFile() it calls Daemon::configureLogger()
-    // that stores the logging info in isc::dhcp::CfgMgr::getStagingCfg().
-    // This is later moved to getCurrentCfg() when the configuration is
-    // commited. All control-agent specific configuration is stored in
-    // a structure accessible by process_->getCfgMgr()->getContext(). Note
-    // logging information is not stored there.
-    //
-    // As a result, we need to extract the CA configuration from one
-    // place and logging from another.
-    if (!cfg->contains("Logging")) {
-        ConfigPtr base_cfg = process_->getCfgMgr()->getContext();
-
-        ConstElementPtr loginfo = base_cfg->toElement();
-        if (loginfo) {
-            // If there was a config stored in dhcp::CfgMgr, try to get Logging info from it.
-            loginfo = loginfo->get("Logging");
-        }
-        if (loginfo) {
-            // If there is some logging information, add it to our config.
-            cfg->set("Logging", loginfo);
-        }
-    }
-
     try {
         size = writeConfigFile(filename, cfg);
     } catch (const isc::Exception& ex) {
@@ -561,13 +536,11 @@ DControllerBase::configTestHandler(const std::string&, ConstElementPtr args) {
     std::string message;
 
     // Command arguments are expected to be:
-    // { "Module": { ... }, "Logging": { ... } }
-    // The Logging component is technically optional. If it's not supplied
-    // logging will revert to default logging.
+    // { "Module": { ... } }
     if (!args) {
         message = "Missing mandatory 'arguments' parameter.";
     } else {
-      module_config = args->get(app_name);
+        module_config = args->get(app_name);
         if (!module_config) {
             message = "Missing mandatory '" + app_name + "' parameter.";
         } else if (module_config->getType() != Element::map) {
@@ -582,6 +555,28 @@ DControllerBase::configTestHandler(const std::string&, ConstElementPtr args) {
         return (result);
     }
 
+    // Check obsolete or unknown (aka unsupported) objects.
+    for (auto obj : args->mapValue()) {
+        const std::string& obj_name = obj.first;
+        if (obj_name == app_name) {
+            continue;
+        }
+        if (obj_name == "Logging") {
+            LOG_WARN(dctl_logger, DCTL_CONFIG_DEPRECATED)
+                .arg("'Logging' defined in top level. This is deprecated."
+                     " Please define it in the '" + app_name + "' scope.");
+            continue;
+        }
+        LOG_WARN(dctl_logger, DCTL_CONFIG_DEPRECATED)
+            .arg("'" + obj_name + "', defining anything in global level besides '"
+                 + app_name + "' is no longer supported.");
+    }
+
+    // Relocate Logging: if there is a global Logging object takes its
+    // loggers entry, move the entry to AppName object and remove
+    // now empty Logging.
+    Daemon::relocateLogging(args, getAppName());
+
     // We are starting the configuration process so we should remove any
     // staging configuration that has been created during previous
     // configuration attempts.
@@ -606,13 +601,11 @@ DControllerBase::configSetHandler(const std::string&, ConstElementPtr args) {
     std::string message;
 
     // Command arguments are expected to be:
-    // { "Module": { ... }, "Logging": { ... } }
-    // The Logging component is technically optional. If it's not supplied
-    // logging will revert to default logging.
+    // { "Module": { ... } }
     if (!args) {
         message = "Missing mandatory 'arguments' parameter.";
     } else {
-      module_config = args->get(app_name);
+        module_config = args->get(app_name);
         if (!module_config) {
             message = "Missing mandatory '" + app_name + "' parameter.";
         } else if (module_config->getType() != Element::map) {
@@ -627,30 +620,62 @@ DControllerBase::configSetHandler(const std::string&, ConstElementPtr args) {
         return (result);
     }
 
-    // We are starting the configuration process so we should remove any
-    // staging configuration that has been created during previous
-    // configuration attempts.
-    // We're not using cfgmgr to store logging information anymore.
-    // isc::dhcp::CfgMgr::instance().rollback();
+    try {
+
+        // Check obsolete or unknown (aka unsupported) objects.
+        for (auto obj : args->mapValue()) {
+            const std::string& obj_name = obj.first;
+            if (obj_name == app_name) {
+                continue;
+            }
+            if (obj_name == "Logging") {
+                LOG_WARN(dctl_logger, DCTL_CONFIG_DEPRECATED)
+                    .arg("'Logging' defined in top level. This is deprecated."
+                         " Please define it in the '" + app_name + "' scope.");
+                continue;
+            }
+            LOG_WARN(dctl_logger, DCTL_CONFIG_DEPRECATED)
+                .arg("'" + obj_name + "', defining anything in global level besides '"
+                     + app_name + "' is no longer supported.");
+        }
 
-    // Temporary storage for logging configuration
-    ConfigPtr storage = process_->getCfgMgr()->getContext();
+        // Relocate Logging: if there is a global Logging object takes its
+        // loggers entry, move the entry to AppName object and remove
+        // now empty Logging.
+        Daemon::relocateLogging(args, getAppName());
 
-    // Get 'Logging' element from the config and use it to set up
-    // logging. If there's no such element, we'll just pass NULL.
-    Daemon::configureLogger(args->get("Logging"), storage);
+        // We are starting the configuration process so we should remove any
+        // staging configuration that has been created during previous
+        // configuration attempts.
+        // We're not using cfgmgr to store logging information anymore.
+        // isc::dhcp::CfgMgr::instance().rollback();
 
-    // Now we check the server proper.
-    ConstElementPtr answer = updateConfig(module_config);
-    int rcode = 0;
-    parseAnswer(rcode, answer);
-    if (!rcode) {
-        // Configuration successful, so apply the logging configuration
-        // to log4cplus.
+        // Temporary storage for logging configuration
+        ConfigPtr storage(new ConfigBase());
+
+        // Configure logging to the tempoary storage.
+        Daemon::configureLogger(module_config, storage);
+
+        // Let's apply the new logging. We do it early, so we'll be able
+        // to print out what exactly is wrong with the new config in
+        // case of problems.
         storage->applyLoggingCfg();
-    }
 
-    return (answer);
+        ConstElementPtr answer = updateConfig(module_config);
+        int rcode = 0;
+        parseAnswer(rcode, answer);
+        // In all cases the right logging configuration is in the context.
+        process_->getCfgMgr()->getContext()->applyLoggingCfg();
+        return (answer);
+    } catch (const std::exception& ex) {
+        // Rollback logging configuration.
+        process_->getCfgMgr()->getContext()->applyLoggingCfg();
+
+        // build an error result
+        ConstElementPtr error = createAnswer(COMMAND_ERROR,
+                 std::string("Configuration parsing failed: ") + ex.what());
+        return (error);
+    }
 }
 
 ConstElementPtr