From: Francis Dupont Date: Thu, 16 Mar 2017 13:16:33 +0000 (+0100) Subject: [5152] Addressed review comments X-Git-Tag: trac102b_base~3^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=80333c548a23c2f6bc19981b0c0be05f05101b91;p=thirdparty%2Fkea.git [5152] Addressed review comments --- diff --git a/src/bin/agent/ca_process.h b/src/bin/agent/ca_process.h index a6a119bfe4..69de600021 100644 --- a/src/bin/agent/ca_process.h +++ b/src/bin/agent/ca_process.h @@ -84,7 +84,8 @@ public: /// of an integer status value (0 means successful, non-zero means failure), /// and a string explanation of the outcome. virtual isc::data::ConstElementPtr - configure(isc::data::ConstElementPtr config_set, bool check_only); + configure(isc::data::ConstElementPtr config_set, + bool check_only = false); /// @brief Processes the given command. /// diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index 736e8df2fd..9e6649692d 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -37,7 +37,7 @@ to shutdown and has met the required criteria to exit. This is a debug message issued when the DHCP-DDNS application command method has been invoked. -% DHCP_DDNS_CONFIGURE configuration update received: %1 +% DHCP_DDNS_CONFIGURE configuration %1 received: %2 This is a debug message issued when the DHCP-DDNS application configure method has been invoked. diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index a786cd3d9f..4070ff4856 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -192,8 +192,9 @@ D2Process::shutdown(isc::data::ConstElementPtr args) { isc::data::ConstElementPtr D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) { - LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, - DHCP_DDNS_CONFIGURE).arg(config_set->str()); + LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, DHCP_DDNS_CONFIGURE) + .arg(check_only ? "check" : "update") + .arg(config_set->str()); isc::data::ConstElementPtr answer; answer = getCfgMgr()->parseConfig(config_set, check_only);; diff --git a/src/bin/d2/d2_process.h b/src/bin/d2/d2_process.h index b053083bb4..14a5127389 100644 --- a/src/bin/d2/d2_process.h +++ b/src/bin/d2/d2_process.h @@ -158,7 +158,8 @@ public: /// of an integer status value (0 means successful, non-zero means failure), /// and a string explanation of the outcome. virtual isc::data::ConstElementPtr - configure(isc::data::ConstElementPtr config_set, bool check_only); + configure(isc::data::ConstElementPtr config_set, + bool check_only = false); /// @brief Processes the given command. /// diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index b56b4a8bfe..28cbf8d427 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -37,7 +37,7 @@ DControllerBasePtr DControllerBase::controller_; // Note that the constructor instantiates the controller's primary IOService. DControllerBase::DControllerBase(const char* app_name, const char* bin_name) : app_name_(app_name), bin_name_(bin_name), - verbose_(false), spec_file_name_(""), + verbose_(false), check_only_(false), spec_file_name_(""), io_service_(new isc::asiolink::IOService()), io_signal_queue_() { } @@ -74,60 +74,10 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) { setProcName(bin_name_); - if (!test_mode && check_only_) { - try { - // We need to initialize logging, in case any error - // messages are to be printed. - // This is just a test, so we don't care about lockfile. - setenv("KEA_LOCKFILE_DIR", "none", 0); - isc::dhcp::CfgMgr::instance().setDefaultLoggerName(bin_name_); - isc::dhcp::CfgMgr::instance().setVerbose(verbose_); - Daemon::loggerInit(bin_name_.c_str(), verbose_); - - // Check the syntax first. - std::string config_file = getConfigFile(); - if (config_file.empty()) { - // Basic sanity check: file name must not be empty. - isc_throw(InvalidUsage, - "JSON configuration file not specified"); - } - isc::data::ConstElementPtr whole_config = parseFile(config_file); - if (!whole_config) { - // No fallback to fromJSONFile - isc_throw(InvalidUsage, "No configuration found"); - } - if (verbose_) { - std::cerr << "Syntax check OK" << std::endl; - } - - // Check the logic next. - isc::data::ConstElementPtr module_config; - module_config = whole_config->get(getAppName()); - if (!module_config) { - isc_throw(InvalidUsage, "Config file " << config_file << - " does not include '" << getAppName() << "' entry"); - } - - // Get an application process object. - initProcess(); - - isc::data::ConstElementPtr answer; - answer = checkConfig(module_config); - int rcode = 0; - answer = isc::config::parseAnswer(rcode, answer); - if (rcode != 0) { - isc_throw(InvalidUsage, "Error encountered: " - << answer->stringValue()); - } - } catch (const VersionMessage&) { - throw; - } catch (const InvalidUsage&) { - throw; - } catch (const std::exception& ex) { - isc_throw(InvalidUsage, "Syntax check failed with: " << ex.what()); - } + if (isCheckOnly()) { + checkConfigOnly(); return; - } + } // It is important that we set a default logger name because this name // will be used when the user doesn't provide the logging configuration @@ -202,6 +152,61 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) { .arg(app_name_).arg(getpid()).arg(VERSION); } +void +DControllerBase::checkConfigOnly() { + try { + // We need to initialize logging, in case any error + // messages are to be printed. + // This is just a test, so we don't care about lockfile. + setenv("KEA_LOCKFILE_DIR", "none", 0); + isc::dhcp::CfgMgr::instance().setDefaultLoggerName(bin_name_); + isc::dhcp::CfgMgr::instance().setVerbose(verbose_); + Daemon::loggerInit(bin_name_.c_str(), verbose_); + + // Check the syntax first. + std::string config_file = getConfigFile(); + if (config_file.empty()) { + // Basic sanity check: file name must not be empty. + isc_throw(InvalidUsage, "JSON configuration file not specified"); + } + isc::data::ConstElementPtr whole_config = parseFile(config_file); + if (!whole_config) { + // No fallback to fromJSONFile + isc_throw(InvalidUsage, "No configuration found"); + } + if (verbose_) { + std::cerr << "Syntax check OK" << std::endl; + } + + // Check the logic next. + isc::data::ConstElementPtr module_config; + module_config = whole_config->get(getAppName()); + if (!module_config) { + isc_throw(InvalidUsage, "Config file " << config_file << + " does not include '" << getAppName() << "' entry"); + } + + // Get an application process object. + initProcess(); + + isc::data::ConstElementPtr answer; + answer = checkConfig(module_config); + int rcode = 0; + answer = isc::config::parseAnswer(rcode, answer); + if (rcode != 0) { + isc_throw(InvalidUsage, "Error encountered: " + << answer->stringValue()); + } + } catch (const VersionMessage&) { + throw; + } catch (const InvalidUsage&) { + throw; + } catch (const std::exception& ex) { + isc_throw(InvalidUsage, "Syntax check failed with: " << ex.what()); + } + return; +} + void DControllerBase::parseArgs(int argc, char* argv[]) { diff --git a/src/lib/process/d_controller.h b/src/lib/process/d_controller.h index 6b4a3c2507..edd92c70e0 100644 --- a/src/lib/process/d_controller.h +++ b/src/lib/process/d_controller.h @@ -326,6 +326,13 @@ protected: return (""); } + /// @brief Check the configuration + /// + /// Called by @c launch() when @c check_only_ mode is enabled + /// @throw VersionMessage when successful but a message should be displayed + /// @throw InvalidUsage when an error was detected + void checkConfigOnly(); + /// @brief Application-level signal processing method. /// /// This method is the last step in processing a OS signal occurrence. It @@ -367,6 +374,8 @@ protected: /// @brief Method for enabling or disabling check only mode. /// + /// @todo this method and @c setVerbose are currently not used. + /// /// @param value is the new value to assign the flag. void setCheckOnly(bool value) { check_only_ = value; @@ -567,7 +576,8 @@ private: /// @brief Indicates if the verbose logging mode is enabled. bool verbose_; - /// @brief Indicates if the check only mode is enabled. + /// @brief Indicates if the check only mode for the configuration + /// is enabled (usually specified by the command line -t argument). bool check_only_; /// @brief The absolute file name of the JSON spec file. diff --git a/src/lib/process/d_process.h b/src/lib/process/d_process.h index 7de31b2ce3..ea920e4275 100644 --- a/src/lib/process/d_process.h +++ b/src/lib/process/d_process.h @@ -102,7 +102,7 @@ public: /// /// @throw DProcessBaseError if an operational error is encountered. virtual isc::data::ConstElementPtr - shutdown(isc::data::ConstElementPtr args) = 0; + shutdown(isc::data::ConstElementPtr args) = 0; /// @brief Processes the given configuration. /// @@ -118,7 +118,8 @@ public: /// of an integer status value (0 means successful, non-zero means failure), /// and a string explanation of the outcome. virtual isc::data::ConstElementPtr - configure(isc::data::ConstElementPtr config_set, bool check_only) = 0; + configure(isc::data::ConstElementPtr config_set, + bool check_only = false) = 0; /// @brief Processes the given command. ///