Commit 80333c54 authored by Francis Dupont's avatar Francis Dupont
Browse files

[5152] Addressed review comments

parent 882eff53
......@@ -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.
///
......
......@@ -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.
......
......@@ -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);;
......
......@@ -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.
///
......
......@@ -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[])
{
......
......@@ -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.
......
......@@ -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.
///
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment