From: Francis Dupont Date: Wed, 3 Oct 2018 22:22:46 +0000 (+0200) Subject: [128-netconf-use-libprocess] Addressed last comments X-Git-Tag: 128-netconf-config_base~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e6e082e5635ecacbb2a39b9428c71695b234f42a;p=thirdparty%2Fkea.git [128-netconf-use-libprocess] Addressed last comments --- diff --git a/src/bin/agent/ca_cfg_mgr.cc b/src/bin/agent/ca_cfg_mgr.cc index 9086457e02..20c193bd10 100644 --- a/src/bin/agent/ca_cfg_mgr.cc +++ b/src/bin/agent/ca_cfg_mgr.cc @@ -12,6 +12,7 @@ #include #include +using namespace isc::config; using namespace isc::dhcp; using namespace isc::process; using namespace isc::data; @@ -86,10 +87,10 @@ CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) { parser.parse(ctx, cfg, check_only); } catch (const isc::Exception& ex) { excuse = ex.what(); - answer = isc::config::createAnswer(2, excuse); + answer = createAnswer(CONTROL_RESULT_ERROR, excuse); } catch (...) { excuse = "undefined configuration parsing error"; - answer = isc::config::createAnswer(2, excuse); + answer = createAnswer(CONTROL_RESULT_ERROR, excuse); } // At this stage the answer was created only in case of exception. @@ -103,9 +104,11 @@ CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) { } if (check_only) { - answer = isc::config::createAnswer(0, "Configuration check successful"); + answer = createAnswer(CONTROL_RESULT_SUCCESS, + "Configuration check successful"); } else { - answer = isc::config::createAnswer(0, "Configuration applied successfully."); + answer = createAnswer(CONTROL_RESULT_SUCCESS, + "Configuration applied successfully."); } return (answer); diff --git a/src/bin/agent/tests/ca_unittests.cc b/src/bin/agent/tests/ca_unittests.cc index ab55fe8f34..85b494c082 100644 --- a/src/bin/agent/tests/ca_unittests.cc +++ b/src/bin/agent/tests/ca_unittests.cc @@ -15,7 +15,7 @@ main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); // See the documentation of the KEA_* environment variables in - // src/lib/log/README for info on how to tweak logging + // the developer's guide for info on how to tweak logging isc::log::initLogger(); // Override --localstatedir value for PID files diff --git a/src/bin/d2/d2_cfg_mgr.cc b/src/bin/d2/d2_cfg_mgr.cc index 9c172ce098..cb163d1fbc 100644 --- a/src/bin/d2/d2_cfg_mgr.cc +++ b/src/bin/d2/d2_cfg_mgr.cc @@ -15,6 +15,7 @@ #include using namespace isc::asiolink; +using namespace isc::config; using namespace isc::data; using namespace isc::process; @@ -269,10 +270,10 @@ D2CfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) { parser.parse(ctx, cfg, check_only); } catch (const isc::Exception& ex) { excuse = ex.what(); - answer = isc::config::createAnswer(2, excuse); + answer = createAnswer(CONTROL_RESULT_ERROR, excuse); } catch (...) { excuse = "undefined configuration parsing error"; - answer = isc::config::createAnswer(2, excuse); + answer = createAnswer(CONTROL_RESULT_ERROR, excuse); } // At this stage the answer was created only in case of exception. @@ -286,9 +287,11 @@ D2CfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) { } if (check_only) { - answer = isc::config::createAnswer(0, "Configuration check successful"); + answer = createAnswer(CONTROL_RESULT_SUCCESS, + "Configuration check successful"); } else { - answer = isc::config::createAnswer(0, "Configuration applied successfully."); + answer = createAnswer(CONTROL_RESULT_SUCCESS, + "Configuration applied successfully."); } return (answer); diff --git a/src/bin/d2/tests/d2_controller_unittests.cc b/src/bin/d2/tests/d2_controller_unittests.cc index 4ed618813d..5ee602cab7 100644 --- a/src/bin/d2/tests/d2_controller_unittests.cc +++ b/src/bin/d2/tests/d2_controller_unittests.cc @@ -188,12 +188,12 @@ TEST_F(D2ControllerTest, configUpdateTests) { config_set = isc::data::Element::fromJSON(config); answer = updateConfig(config_set); isc::config::parseAnswer(rcode, answer); - EXPECT_EQ(2, rcode); + EXPECT_EQ(1, rcode); // Use an invalid configuration to verify checking error return. answer = checkConfig(config_set); isc::config::parseAnswer(rcode, answer); - EXPECT_EQ(2, rcode); + EXPECT_EQ(1, rcode); } // Tests that the original configuration is retained after a SIGHUP triggered diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index 35be2b0d97..f5eae674a7 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -213,7 +213,7 @@ TEST_F(D2ProcessTest, configure) { // Create an invalid configuration set from text config. ASSERT_TRUE(fromJSON("{ \"ip-address\": \"950 Charter St.\" } ")); answer = configure(config_set_, false); - ASSERT_TRUE(checkAnswer(answer, 2)); + ASSERT_TRUE(checkAnswer(answer, 1)); EXPECT_FALSE(getReconfQueueFlag()); EXPECT_EQ(D2QueueMgr::RUNNING, queue_mgr->getMgrState()); } diff --git a/src/bin/netconf/netconf_cfg_mgr.cc b/src/bin/netconf/netconf_cfg_mgr.cc index cfee2a759f..7b61123336 100644 --- a/src/bin/netconf/netconf_cfg_mgr.cc +++ b/src/bin/netconf/netconf_cfg_mgr.cc @@ -12,6 +12,7 @@ #include #include +using namespace isc::config; using namespace isc::dhcp; using namespace isc::process; using namespace isc::data; @@ -19,15 +20,15 @@ using namespace isc::data; namespace isc { namespace netconf { -NetconfCfgContext::NetconfCfgContext() { +NetconfConfig::NetconfConfig() { } -NetconfCfgContext::NetconfCfgContext(const NetconfCfgContext& orig) +NetconfConfig::NetconfConfig(const NetconfConfig& orig) : ConfigBase(), hooks_config_(orig.hooks_config_) { } NetconfCfgMgr::NetconfCfgMgr() - : DCfgMgrBase(ConfigPtr(new NetconfCfgContext())) { + : DCfgMgrBase(ConfigPtr(new NetconfConfig())) { } NetconfCfgMgr::~NetconfCfgMgr() { @@ -36,7 +37,7 @@ NetconfCfgMgr::~NetconfCfgMgr() { std::string NetconfCfgMgr::getConfigSummary(const uint32_t /*selection*/) { - NetconfCfgContextPtr ctx = getNetconfCfgContext(); + NetconfConfigPtr ctx = getNetconfConfig(); std::ostringstream s; @@ -52,7 +53,7 @@ NetconfCfgMgr::getConfigSummary(const uint32_t /*selection*/) { ConfigPtr NetconfCfgMgr::createNewContext() { - return (ConfigPtr(new NetconfCfgContext())); + return (ConfigPtr(new NetconfConfig())); } isc::data::ConstElementPtr @@ -63,7 +64,7 @@ NetconfCfgMgr::parse(isc::data::ConstElementPtr config_set, isc_throw(DhcpConfigError, "Mandatory config parameter not provided"); } - NetconfCfgContextPtr ctx = getNetconfCfgContext(); + NetconfConfigPtr ctx = getNetconfConfig(); // Set the defaults ElementPtr cfg = boost::const_pointer_cast(config_set); @@ -78,10 +79,10 @@ NetconfCfgMgr::parse(isc::data::ConstElementPtr config_set, parser.parse(ctx, cfg, check_only); } catch (const isc::Exception& ex) { excuse = ex.what(); - answer = isc::config::createAnswer(2, excuse); + answer = createAnswer(CONTROL_RESULT_ERROR, excuse); } catch (...) { excuse = "undefined configuration parsing error"; - answer = isc::config::createAnswer(2, excuse); + answer = createAnswer(CONTROL_RESULT_ERROR, excuse); } // At this stage the answer was created only in case of exception. @@ -95,16 +96,18 @@ NetconfCfgMgr::parse(isc::data::ConstElementPtr config_set, } if (check_only) { - answer = isc::config::createAnswer(0, "Configuration check successful"); + answer = createAnswer(CONTROL_RESULT_SUCCESS, + "Configuration check successful"); } else { - answer = isc::config::createAnswer(0, "Configuration applied successfully."); + answer = createAnswer(CONTROL_RESULT_SUCCESS, + "Configuration applied successfully."); } return (answer); } ElementPtr -NetconfCfgContext::toElement() const { +NetconfConfig::toElement() const { ElementPtr netconf = Element::createMap(); // Set user-context contextToElement(netconf); diff --git a/src/bin/netconf/netconf_cfg_mgr.h b/src/bin/netconf/netconf_cfg_mgr.h index ca3e4d0cac..0a56d0c820 100644 --- a/src/bin/netconf/netconf_cfg_mgr.h +++ b/src/bin/netconf/netconf_cfg_mgr.h @@ -17,9 +17,9 @@ namespace isc { namespace netconf { -class NetconfCfgContext; +class NetconfConfig; /// @brief Pointer to a configuration context. -typedef boost::shared_ptr NetconfCfgContextPtr; +typedef boost::shared_ptr NetconfConfigPtr; /// @brief Netconf Configuration Context. /// @@ -28,11 +28,11 @@ typedef boost::shared_ptr NetconfCfgContextPtr; /// and any other Netconf specific information that needs to be accessible /// during configuration parsing as well as to the application as a whole. /// It is derived from the context base class, ConfigBase. -class NetconfCfgContext : public process::ConfigBase { +class NetconfConfig : public process::ConfigBase { public: /// @brief Default constructor - NetconfCfgContext(); + NetconfConfig(); /// @brief Returns non-const reference to configured hooks libraries. /// @@ -66,12 +66,12 @@ private: /// It is private to forbid anyone outside of this class to make copies. /// /// @param orig the original context to copy from - NetconfCfgContext(const NetconfCfgContext& orig); + NetconfConfig(const NetconfConfig& orig); /// @brief Private assignment operator to avoid potential for slicing. /// /// @param rhs Context to be assigned. - NetconfCfgContext& operator=(const NetconfCfgContext& rhs); + NetconfConfig& operator=(const NetconfConfig& rhs); /// @brief Configured hooks libraries. isc::hooks::HooksConfig hooks_config_; @@ -94,8 +94,8 @@ public: /// context. /// /// @return returns a pointer to the configuration context. - NetconfCfgContextPtr getNetconfCfgContext() { - return (boost::dynamic_pointer_cast(getContext())); + NetconfConfigPtr getNetconfConfig() { + return (boost::dynamic_pointer_cast(getContext())); } /// @brief Returns configuration summary in the textual format. @@ -117,11 +117,11 @@ protected: virtual isc::data::ConstElementPtr parse(isc::data::ConstElementPtr config, bool check_only); - /// @brief Creates a new, blank NetconfCfgContext context. + /// @brief Creates a new, blank NetconfConfig context. /// /// /// This method is used at the beginning of configuration process to - /// create a fresh, empty copy of a NetconfCfgContext. This new context + /// create a fresh, empty copy of a NetconfConfig. This new context /// will be populated during the configuration process and will replace the /// existing context provided the configuration process completes without /// error. diff --git a/src/bin/netconf/simple_parser.cc b/src/bin/netconf/simple_parser.cc index d3a3ffd64a..bee5dd8b1a 100644 --- a/src/bin/netconf/simple_parser.cc +++ b/src/bin/netconf/simple_parser.cc @@ -53,7 +53,7 @@ size_t NetconfSimpleParser::setAllDefaults(const isc::data::ElementPtr& global) } void -NetconfSimpleParser::parse(const NetconfCfgContextPtr& ctx, +NetconfSimpleParser::parse(const NetconfConfigPtr& ctx, const isc::data::ConstElementPtr& config, bool check_only) { diff --git a/src/bin/netconf/simple_parser.h b/src/bin/netconf/simple_parser.h index f2422e6528..a0216fea7d 100644 --- a/src/bin/netconf/simple_parser.h +++ b/src/bin/netconf/simple_parser.h @@ -37,7 +37,7 @@ public: /// @param check_only - if true the configuration is verified only, not applied /// /// @throw ConfigError if any issues are encountered. - void parse(const NetconfCfgContextPtr& ctx, + void parse(const NetconfConfigPtr& ctx, const isc::data::ConstElementPtr& config, bool check_only); diff --git a/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc b/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc index 8a38c924bf..038ae2f61b 100644 --- a/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc +++ b/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc @@ -35,8 +35,8 @@ TEST(NetconfCfgMgr, construction) { ASSERT_NO_THROW(cfg_mgr.reset(new NetconfCfgMgr())); // Verify that the context can be retrieved and is not null. - NetconfCfgContextPtr context; - ASSERT_NO_THROW(context = cfg_mgr->getNetconfCfgContext()); + NetconfConfigPtr context; + ASSERT_NO_THROW(context = cfg_mgr->getNetconfConfig()); EXPECT_TRUE(context); // Verify that the manager can be destructed without error. @@ -47,14 +47,14 @@ TEST(NetconfCfgMgr, construction) { TEST(NetconfCfgMgr, getContext) { NetconfCfgMgr cfg_mgr; - NetconfCfgContextPtr ctx; - ASSERT_NO_THROW(ctx = cfg_mgr.getNetconfCfgContext()); + NetconfConfigPtr ctx; + ASSERT_NO_THROW(ctx = cfg_mgr.getNetconfConfig()); ASSERT_TRUE(ctx); } // Tests if the context can store and retrieve hook libs information. TEST(NetconfCfgMgr, contextHookParams) { - NetconfCfgContext ctx; + NetconfConfig ctx; // By default there should be no hooks. HooksConfig& libs = ctx.getHooksConfig(); diff --git a/src/bin/netconf/tests/netconf_controller_unittests.cc b/src/bin/netconf/tests/netconf_controller_unittests.cc index 2ba3563627..d984b8f3fc 100644 --- a/src/bin/netconf/tests/netconf_controller_unittests.cc +++ b/src/bin/netconf/tests/netconf_controller_unittests.cc @@ -49,10 +49,10 @@ public: } /// @brief Returns a pointer to the configuration context. - NetconfCfgContextPtr getNetconfCfgContext() { - NetconfCfgContextPtr p; + NetconfConfigPtr getNetconfConfig() { + NetconfConfigPtr p; if (getNetconfCfgMgr()) { - p = getNetconfCfgMgr()->getNetconfCfgContext(); + p = getNetconfCfgMgr()->getNetconfConfig(); } return (p); } diff --git a/src/bin/netconf/tests/netconf_process_unittests.cc b/src/bin/netconf/tests/netconf_process_unittests.cc index 0fc73cec79..e75b38323f 100644 --- a/src/bin/netconf/tests/netconf_process_unittests.cc +++ b/src/bin/netconf/tests/netconf_process_unittests.cc @@ -29,7 +29,7 @@ public: NetconfProcessTest() : NetconfProcess("netconf-test", IOServicePtr(new isc::asiolink::IOService())) { - NetconfCfgContextPtr ctx = getNetconfCfgMgr()->getNetconfCfgContext(); + NetconfConfigPtr ctx = getNetconfCfgMgr()->getNetconfConfig(); } /// @brief Destructor diff --git a/src/bin/netconf/tests/run_unittests.cc b/src/bin/netconf/tests/run_unittests.cc index 77dbb04902..3d8595f0e5 100644 --- a/src/bin/netconf/tests/run_unittests.cc +++ b/src/bin/netconf/tests/run_unittests.cc @@ -15,7 +15,7 @@ main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv); // See the documentation of the KEA_* environment variables in - // src/lib/log/README for info on how to tweak logging + // the developer guide for info on how to tweak logging isc::log::initLogger(); // Override --localstatedir value for PID files diff --git a/src/bin/shell/tests/shell_process_tests.sh.in b/src/bin/shell/tests/shell_process_tests.sh.in index 5171ec0cdb..8e33c7f0cd 100644 --- a/src/bin/shell/tests/shell_process_tests.sh.in +++ b/src/bin/shell/tests/shell_process_tests.sh.in @@ -190,5 +190,5 @@ shell_command_test "shell.no-map-config-test" "config-test" \ "[ { \"result\": 1, \"text\": \"'Control-agent' parameter expected to be a map.\" } ]" \ "\"Control-agent\": [ ]" shell_command_test "shell.bad-value-config-test" "config-test" \ - "[ { \"result\": 2, \"text\": \"out of range value (80000) specified for parameter 'http-port' (:1:76) You did not include \\\"service\\\" parameter in the command, which indicates that Kea Control Agent should process this command rather than forward it to one or more DHCP servers. If you aimed to send this command to one of the DHCP servers you should include the \\\"service\\\" parameter in your request, e.g. \\\"service\\\": [ \\\"dhcp4\\\" ] to forward the command to the DHCPv4 server, or \\\"service\\\": [ \\\"dhcp4\\\", \\\"dhcp6\\\" ] to forward it to both DHCPv4 and DHCPv6 servers etc.\" } ]" \ + "[ { \"result\": 1, \"text\": \"out of range value (80000) specified for parameter 'http-port' (:1:76)\" } ]" \ "\"Control-agent\": { \"http-port\": 80000 }"