Skip to content

GitLab

Kea
Kea
Commit f0d600fb authored by Francis Dupont's avatar Francis Dupont
Browse files
Options

[128-netconf-use-libprocess] Addressed comments (remove code option)

parent 53142665
Show whitespace changes
Inline Side-by-side
Showing with 28 additions and 397 deletions
src/bin/agent/ca_messages.mes
View file @ f0d600fb
......@@ -30,7 +30,7 @@ attempt. Details are provided. Additional details may be available
in earlier log entries, possibly on lower levels.
% CTRL_AGENT_FAILED application experienced a fatal error: %1
This is a debug message issued when the Control Agent application
This is a fatal error message issued when the Control Agent application
encounters an unrecoverable error from within the event loop.
% CTRL_AGENT_HTTP_SERVICE_STARTED HTTP service bound to address %1:%2
......
src/bin/agent/tests/ca_controller_unittests.cc
View file @ f0d600fb
......@@ -40,8 +40,9 @@ const char* valid_agent_config =
" }"
"}";
/// @brief test fixture class for testing CtrlAgentController class. This
/// class derives from DControllerTest and wraps CtrlAgentController. Much
/// @brief test fixture class for testing CtrlAgentController class.
///
/// This class derives from DControllerTest and wraps CtrlAgentController. Much
/// of the underlying functionality is in the DControllerBase class which
/// has extensive set of unit tests that are independent from the Control
/// Agent.
......@@ -167,8 +168,8 @@ public:
// Verifies that the controller singleton gets created and that the
// basic derivation from the base class is intact.
TEST_F(CtrlAgentControllerTest, basicInstanceTesting) {
// Verify the we can the singleton instance can be fetched and that
// it is the correct type.
// Verify the singleton instance can be fetched and that
// it has the correct type.
DControllerBasePtr& controller = DControllerTest::getController();
ASSERT_TRUE(controller);
ASSERT_NO_THROW(boost::dynamic_pointer_cast<CtrlAgentController>(controller));
......
src/bin/agent/tests/ca_process_unittests.cc
View file @ f0d600fb
......@@ -69,7 +69,7 @@ TEST_F(CtrlAgentProcessTest, shutdown) {
// shutdown invocation. (Note IntervalTimer setup is in milliseconds).
IntervalTimer timer(*getIoService());
timer.setup(boost::bind(&CtrlAgentProcessTest::genShutdownCallback, this),
2 * 1000);
200);
// Record start time, and invoke run().
ptime start = microsec_clock::universal_time();
......@@ -82,8 +82,8 @@ TEST_F(CtrlAgentProcessTest, shutdown) {
// timer duration. This demonstrates that the shutdown was driven
// by an io_service event and callback.
time_duration elapsed = stop - start;
EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
elapsed.total_milliseconds() <= 2200);
EXPECT_TRUE(elapsed.total_milliseconds() >= 100 &&
elapsed.total_milliseconds() <= 400);
}
......
src/bin/d2/tests/d2_controller_unittests.cc
View file @ f0d600fb
......@@ -24,9 +24,10 @@ using namespace boost::posix_time;
namespace isc {
namespace d2 {
/// @brief Test fixture class for testing D2Controller class. This class
/// derives from DControllerTest and wraps a D2Controller. Much of the
/// underlying functionality is in the DControllerBase class which has an
/// @brief Test fixture class for testing D2Controller class.
///
/// This class derives from DControllerTest and wraps a D2Controller. Much of
/// the underlying functionality is in the DControllerBase class which has an
/// extensive set of unit tests that are independent of DHCP-DDNS.
/// @TODO Currently These tests are relatively light and duplicate some of
/// the testing done on the base class. These tests are sufficient to ensure
......@@ -82,8 +83,8 @@ public:
/// Verifies that the controller singleton gets created and that the
/// basic derivation from the base class is intact.
TEST_F(D2ControllerTest, basicInstanceTesting) {
// Verify the we can the singleton instance can be fetched and that
// it is the correct type.
// Verify the singleton instance can be fetched and that
// it has the correct type.
DControllerBasePtr& controller = DControllerTest::getController();
ASSERT_TRUE(controller);
ASSERT_NO_THROW(boost::dynamic_pointer_cast<D2Controller>(controller));
......
src/bin/netconf/Makefile.am
View file @ f0d600fb
......@@ -70,16 +70,11 @@ kea_netconf_LDADD += $(top_builddir)/src/lib/http/libkea-http.la
kea_netconf_LDADD += $(top_builddir)/src/lib/process/libkea-process.la
kea_netconf_LDADD += $(top_builddir)/src/lib/cfgrpt/libcfgrpt.la
kea_netconf_LDADD += $(top_builddir)/src/lib/yang/libkea-yang.la
kea_netconf_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
kea_netconf_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la
kea_netconf_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
kea_netconf_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
kea_netconf_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la
kea_netconf_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
kea_netconf_LDADD += $(top_builddir)/src/lib/dns/libkea-dns++.la
kea_netconf_LDADD += $(top_builddir)/src/lib/cryptolink/libkea-cryptolink.la
kea_netconf_LDADD += $(top_builddir)/src/lib/log/libkea-log.la
kea_netconf_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
kea_netconf_LDADD += $(top_builddir)/src/lib/util/threads/libkea-threads.la
kea_netconf_LDADD += $(top_builddir)/src/lib/util/libkea-util.la
kea_netconf_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
......
src/bin/netconf/netconf_cfg_mgr.h
View file @ f0d600fb
......@@ -34,13 +34,6 @@ public:
/// @brief Default constructor
NetconfCfgContext();
/// @brief Creates a clone of this context object.
///
/// @return A pointer to the new clone.
virtual process::ConfigPtr clone() {
return (process::ConfigPtr(new NetconfCfgContext(*this)));
}
/// @brief Returns non-const reference to configured hooks libraries.
///
/// @return non-const reference to configured hooks libraries.
......@@ -71,7 +64,6 @@ private:
/// @brief Private copy constructor
///
/// It is private to forbid anyone outside of this class to make copies.
/// The only legal way to copy a context is to call @ref clone().
///
/// @param orig the original context to copy from
NetconfCfgContext(const NetconfCfgContext& orig);
......
src/bin/netconf/netconf_controller.cc
View file @ f0d600fb
......@@ -8,9 +8,6 @@
#include <netconf/netconf_controller.h>
#include <netconf/netconf_process.h>
#ifdef notyet
#include <netconf/parser_context.h>
#endif
using namespace isc::process;
......@@ -45,13 +42,8 @@ NetconfController::createProcess() {
isc::data::ConstElementPtr
NetconfController::parseFile(const std::string& name) {
#ifdef notyet
ParserContext parser;
return (parser.parseFile(name, ParserContext::PARSER_NETCONF));
#else
isc_throw(NotImplemented, "NetconfController::parseFile("
<< name << ")");
#endif
}
NetconfController::NetconfController()
......
src/bin/netconf/netconf_messages.mes
View file @ f0d600fb
......@@ -17,7 +17,7 @@ attempt. Details are provided. Additional details may be available
in earlier log entries, possibly on lower levels.
% NETCONF_FAILED application experienced a fatal error: %1
This is a debug message issued when the Netconf application
This is a fatal error message issued when the Netconf application
encounters an unrecoverable error from within the event loop.
% NETCONF_RUN_EXIT application is exiting the event loop
......
src/bin/netconf/netconf_process.cc
View file @ f0d600fb
......@@ -80,28 +80,8 @@ NetconfProcess::shutdown(isc::data::ConstElementPtr /*args*/) {
isc::data::ConstElementPtr
NetconfProcess::configure(isc::data::ConstElementPtr config_set,
bool check_only) {
// System reconfiguration often poses an interesting issue whereby the
// configuration parsing is successful, but an attempt to use a new
// configuration is not. This will leave us in the inconsistent state
// when the configuration is in fact only partially applied and the
// system's ability to operate is impaired. The use of C++ lambda is
// a way to resolve this problem by injecting the code to the
// simpleParseConfig which performs an attempt to open new instance
// of the listener (if required). The lambda code will throw an
// exception if it fails and cause the simpleParseConfig to rollback
// configuration changes and report an error.
ConstElementPtr answer = getCfgMgr()->simpleParseConfig(config_set,
check_only,
[this]() {
ConfigPtr base_ctx = getCfgMgr()->getContext();
NetconfCfgContextPtr
ctx = boost::dynamic_pointer_cast<NetconfCfgContext>(base_ctx);
if (!ctx) {
isc_throw(Unexpected, "Internal logic error: bad context type");
}
});
ConstElementPtr answer =
getCfgMgr()->simpleParseConfig(config_set, check_only);
int rcode = 0;
config::parseAnswer(rcode, answer);
return (answer);
......
src/bin/netconf/parser_context.cc
View file @ f0d600fb
......@@ -111,7 +111,6 @@ ParserContext::require(const std::string& name,
}
}
void
ParserContext::enter(const LexerContext& ctx)
{
......
src/bin/netconf/simple_parser.cc
View file @ f0d600fb
......@@ -64,7 +64,6 @@ NetconfSimpleParser::parse(const NetconfCfgContextPtr& ctx,
}
// Finally, let's get the hook libs!
using namespace isc::hooks;
HooksConfig& libraries = ctx->getHooksConfig();
ConstElementPtr hooks = config->get("hooks-libraries");
......
src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc
View file @ f0d600fb
......@@ -6,9 +6,6 @@
#include <config.h>
#include <netconf/netconf_cfg_mgr.h>
#ifdef notyet
#include <netconf/parser_context.h>
#endif
#include <exceptions/exceptions.h>
#include <process/testutils/d_test_stubs.h>
#include <process/d_cfg_mgr.h>
......@@ -55,30 +52,6 @@ TEST(NetconfCfgMgr, getContext) {
ASSERT_TRUE(ctx);
}
// Tests if copied context retains all parameters.
TEST(NetconfCfgMgr, contextCopy) {
NetconfCfgContext ctx;
HooksConfig& libs = ctx.getHooksConfig();
string exp_name("testlib1.so");
ConstElementPtr exp_param(new StringElement("myparam"));
libs.add(exp_name, exp_param);
// Make a copy.
ConfigPtr copy_base(ctx.clone());
NetconfCfgContextPtr copy = boost::dynamic_pointer_cast<NetconfCfgContext>(copy_base);
ASSERT_TRUE(copy);
// Check hook libs
const HookLibsCollection& libs2 = copy->getHooksConfig().get();
ASSERT_EQ(1, libs2.size());
EXPECT_EQ(exp_name, libs2[0].first);
ASSERT_TRUE(libs2[0].second);
EXPECT_EQ(exp_param->str(), libs2[0].second->str());
}
// Tests if the context can store and retrieve hook libs information.
TEST(NetconfCfgMgr, contextHookParams) {
NetconfCfgContext ctx;
......@@ -94,205 +67,7 @@ TEST(NetconfCfgMgr, contextHookParams) {
const HooksConfig& stored_libs = ctx.getHooksConfig();
EXPECT_EQ(3, stored_libs.get().size());
// @todo add a == operator to HooksConfig
EXPECT_EQ(libs.get(), stored_libs.get());
}
#ifdef notyet
/// Netconf configurations used in tests.
const char* NETCONF_CONFIGS[] = {
// configuration 0: empty (nothing specified)
"{ }",
// Configuration 1: http parameters only (no control sockets, not hooks)
"{ \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001\n"
"}",
// Configuration 2: http and 1 socket
"{\n"
" \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001,\n"
" \"control-sockets\": {\n"
" \"dhcp4\": {\n"
" \"socket-name\": \"/tmp/socket-v4\"\n"
" }\n"
" }\n"
"}",
// Configuration 3: http and all 3 sockets
"{\n"
" \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001,\n"
" \"control-sockets\": {\n"
" \"dhcp4\": {\n"
" \"socket-name\": \"/tmp/socket-v4\"\n"
" },\n"
" \"dhcp6\": {\n"
" \"socket-name\": \"/tmp/socket-v6\"\n"
" },\n"
" \"d2\": {\n"
" \"socket-name\": \"/tmp/socket-d2\"\n"
" }\n"
" }\n"
"}",
// Configuration 4: http, 1 socket and hooks
// CA is able to load hook libraries that augment its operation.
// The primary functionality is the ability to add new commands.
"{\n"
" \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001,\n"
" \"control-sockets\": {\n"
" \"dhcp4\": {\n"
" \"socket-name\": \"/tmp/socket-v4\"\n"
" }\n"
" },\n"
" \"hooks-libraries\": ["
" {"
" \"library\": \"%LIBRARY%\","
" \"parameters\": {\n"
" \"param1\": \"foo\"\n"
" }\n"
" }\n"
" ]\n"
"}",
// Configuration 5: http and 1 socket (d2 only)
"{\n"
" \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001,\n"
" \"control-sockets\": {\n"
" \"d2\": {\n"
" \"socket-name\": \"/tmp/socket-d2\"\n"
" }\n"
" }\n"
"}",
// Configuration 6: http and 1 socket (dhcp6 only)
"{\n"
" \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001,\n"
" \"control-sockets\": {\n"
" \"dhcp6\": {\n"
" \"socket-name\": \"/tmp/socket-v6\"\n"
" }\n"
" }\n"
"}",
// Configuration 7: http and 2 sockets with user contexts and comments
"{\n"
" \"user-context\": { \"comment\": \"Indirect comment\" },\n"
" \"http-host\": \"betelgeuse\",\n"
" \"http-port\": 8001,\n"
" \"control-sockets\": {\n"
" \"dhcp4\": {\n"
" \"comment\": \"dhcp4 socket\",\n"
" \"socket-name\": \"/tmp/socket-v4\"\n"
" },\n"
" \"dhcp6\": {\n"
" \"socket-name\": \"/tmp/socket-v6\",\n"
" \"user-context\": { \"version\": 1 }\n"
" }\n"
" }\n"
"}"
};
/// @brief Class used for testing CfgMgr
class NetconfParserTest : public isc::process::ConfigParseTest {
public:
/// @brief Tries to load input text as a configuration
///
/// @param config text containing input configuration
/// @param expected_answer expected result of configuration (0 = success)
void configParse(const char* config, int expected_answer) {
isc::netconf::ParserContext parser;
ConstElementPtr json = parser.parseString(config, ParserContext::PARSER_SUB_NETCONF);
EXPECT_NO_THROW(answer_ = cfg_mgr_.parse(json, false));
EXPECT_TRUE(checkAnswer(expected_answer));
}
/// @brief Replaces %LIBRARY% with specified library name
///
/// @param config input config text (should contain "%LIBRARY%" string)
/// @param lib_name %LIBRARY% will be replaced with that name
/// @return configuration text with library name replaced
string pathReplacer(const char* config, const char* lib_name) {
string txt(config);
txt.replace(txt.find("%LIBRARY%"), strlen("%LIBRARY%"), string(lib_name));
return (txt);
}
/// Configuration Manager (used in tests)
NakedNetconfCfgMgr cfg_mgr_;
};
// This test verifies if an empty config is handled properly. In practice such
// a config makes little sense, but perhaps it's ok for a default deployment.
// Sadly, our bison parser requires at last one parameter to be present.
// Until we determine whether we want the empty config to be allowed or not,
// this test remains disabled.
TEST_F(NetconfParserTest, DISABLED_configParseEmpty) {
configParse(NETCONF_CONFIGS[0], 0);
}
// This test checks that the config file with hook library specified can be
// loaded. This one is a bit tricky, because the parser sanity checks the lib
// name. In particular, it checks if such a library exists. Therefore we
// can't use NETCONF_CONFIGS[4] as is, but need to run it through path replacer.
TEST_F(NetconfParserTest, configParseHooks) {
// Create the configuration with proper lib path.
string cfg = pathReplacer(NETCONF_CONFIGS[4], BASIC_CALLOUT_LIBRARY);
// The configuration should be successful.
configParse(cfg.c_str(), 0);
// The context now should have the library specified.
NetconfCfgContextPtr ctx = cfg_mgr_.getNetconfCfgContext();
const HookLibsCollection libs = ctx->getHooksConfig().get();
ASSERT_EQ(1, libs.size());
EXPECT_EQ(string(BASIC_CALLOUT_LIBRARY), libs[0].first);
ASSERT_TRUE(libs[0].second);
EXPECT_EQ("{ \"param1\": \"foo\" }", libs[0].second->str());
}
// This test checks comments.
TEST_F(NetconfParserTest, comments) {
configParse(NETCONF_CONFIGS[7], 0);
NetconfCfgContextPtr netconf_ctx = cfg_mgr_.getNetconfCfgContext();
ASSERT_TRUE(netconf_ctx);
// Check global user context.
ConstElementPtr ctx = netconf_ctx->getContext();
ASSERT_TRUE(ctx);
ASSERT_EQ(1, ctx->size());
ASSERT_TRUE(ctx->get("comment"));
EXPECT_EQ("\"Indirect comment\"", ctx->get("comment")->str());
// There is a DHCP4 control socket.
ConstElementPtr socket4 = netconf_ctx->getControlSocketInfo("dhcp4");
ASSERT_TRUE(socket4);
// Check DHCP4 control socket user context.
ConstElementPtr ctx4 = socket4->get("user-context");
ASSERT_TRUE(ctx4);
ASSERT_EQ(1, ctx4->size());
ASSERT_TRUE(ctx4->get("comment"));
EXPECT_EQ("\"dhcp4 socket\"", ctx4->get("comment")->str());
// There is a DHCP6 control socket.
ConstElementPtr socket6 = netconf_ctx->getControlSocketInfo("dhcp6");
ASSERT_TRUE(socket6);
// Check DHCP6 control socket user context.
ConstElementPtr ctx6 = socket6->get("user-context");
ASSERT_TRUE(ctx6);
ASSERT_EQ(1, ctx6->size());
ASSERT_TRUE(ctx6->get("version"));
EXPECT_EQ("1", ctx6->get("version")->str());
}
#endif
}; // end of anonymous namespace
src/bin/netconf/tests/netconf_controller_unittests.cc
View file @ f0d600fb
......@@ -21,25 +21,9 @@ using namespace boost::posix_time;
namespace {
#ifdef notyet
/// @brief Valid Netconf Config used in tests.
const char* valid_netconf_config =
"{"
" \"control-sockets\": {"
" \"dhcp4\": {"
" \"socket-type\": \"unix\","
" \"socket-name\": \"/first/dhcp4/socket\""
" },"
" \"dhcp6\": {"
" \"socket-type\": \"unix\","
" \"socket-name\": \"/first/dhcp6/socket\""
" }"
" }"
"}";
#endif
/// @brief test fixture class for testing NetconfController class. This
/// class derives from DControllerTest and wraps NetconfController. Much
/// @brief test fixture class for testing NetconfController class.
///
/// This class derives from DControllerTest and wraps NetconfController. Much
/// of the underlying functionality is in the DControllerBase class which
/// has extensive set of unit tests that are independent from Netconf.
class NetconfControllerTest : public DControllerTest {
......@@ -72,55 +56,14 @@ public:
}
return (p);
}
/// @brief Compares the status in the given parse result to a given value.
///
/// @param answer Element set containing an integer response and string
/// comment.
/// @param exp_status is an integer against which to compare the status.
/// @param exp_txt is expected text (not checked if "")
///
void checkAnswer(isc::data::ConstElementPtr answer,
int exp_status,
string exp_txt = "") {
// Get rid of the outer list.
ASSERT_TRUE(answer);
ASSERT_EQ(Element::list, answer->getType());
ASSERT_LE(1, answer->size());
answer = answer->get(0);
int rcode = 0;
isc::data::ConstElementPtr comment;
comment = isc::config::parseAnswer(rcode, answer);
if (rcode != exp_status) {
ADD_FAILURE() << "Expected status code " << exp_status
<< " but received " << rcode << ", comment: "
<< (comment ? comment->str() : "(none)");
}
// Ok, parseAnswer interface is weird. If there are no arguments,
// it returns content of text. But if there is an argument,
// it returns the argument and it's not possible to retrieve
// "text" (i.e. comment).
if (comment->getType() != Element::string) {
comment = answer->get("text");
}
if (!exp_txt.empty()) {
EXPECT_EQ(exp_txt, comment->stringValue());
}
}
};
// Basic Controller instantiation testing.
// Verifies that the controller singleton gets created and that the
// basic derivation from the base class is intact.
TEST_F(NetconfControllerTest, basicInstanceTesting) {
// Verify the we can the singleton instance can be fetched and that
// it is the correct type.
// Verify the singleton instance can be fetched and that
// it has the correct type.
DControllerBasePtr& controller = DControllerTest::getController();
ASSERT_TRUE(controller);
ASSERT_NO_THROW(boost::dynamic_pointer_cast<NetconfController>(controller));
......@@ -176,50 +119,4 @@ TEST_F(NetconfControllerTest, initProcessTesting) {
EXPECT_TRUE(checkProcess());
}
#ifdef notyet
// Tests launch and normal shutdown (stand alone mode).
// This creates an interval timer to generate a normal shutdown and then
// launches with a valid, stand-alone command line and no simulated errors.
TEST_F(NetconfControllerTest, launchNormalShutdown) {
// Write valid_netconf_config and then run launch() for 1000 ms.
time_duration elapsed_time;
runWithConfig(valid_netconf_config, 1000, elapsed_time);
// Give a generous margin to accommodate slower test environs.
EXPECT_TRUE(elapsed_time.total_milliseconds() >= 800 &&
elapsed_time.total_milliseconds() <= 1300);
}
// Tests that the SIGINT triggers a normal shutdown.
TEST_F(NetconfControllerTest, sigintShutdown) {
// Setup to raise SIGHUP in 1 ms.
TimedSignal sighup(*getIOService(), SIGINT, 1);
// Write valid_netconf_config and then run launch() for a maximum
// of 1000 ms.
time_duration elapsed_time;
runWithConfig(valid_netconf_config, 1000, elapsed_time);
// Signaled shutdown should make our elapsed time much smaller than
// the maximum run time. Give generous margin to accommodate slow
// test environs.
EXPECT_TRUE(elapsed_time.total_milliseconds() < 300);
}
// Tests that the SIGTERM triggers a normal shutdown.
TEST_F(NetconfControllerTest, sigtermShutdown) {
// Setup to raise SIGHUP in 1 ms.
TimedSignal sighup(*getIOService(), SIGTERM, 1);
// Write valid_netconf_config and then run launch() for a maximum of 1 s.
time_duration elapsed_time;
runWithConfig(valid_netconf_config, 1000, elapsed_time);
// Signaled shutdown should make our elapsed time much smaller than
// the maximum run time. Give generous margin to accommodate slow
// test environs.
EXPECT_TRUE(elapsed_time.total_milliseconds() < 300);
}
#endif
}
src/bin/netconf/tests/netconf_process_unittests.cc
View file @ f0d600fb
......@@ -67,7 +67,7 @@ TEST_F(NetconfProcessTest, shutdown) {
// shutdown invocation. (Note IntervalTimer setup is in milliseconds).
IntervalTimer timer(*getIoService());
timer.setup(boost::bind(&NetconfProcessTest::genShutdownCallback, this),
2 * 1000);
200);
// Record start time, and invoke run().
ptime start = microsec_clock::universal_time();
......@@ -80,8 +80,8 @@ TEST_F(NetconfProcessTest, shutdown) {
// timer duration. This demonstrates that the shutdown was driven
// by an io_service event and callback.
time_duration elapsed = stop - start;
EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
elapsed.total_milliseconds() <= 2200);
EXPECT_TRUE(elapsed.total_milliseconds() >= 100 &&
elapsed.total_milliseconds() <= 400);
}
}
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