From afb5415ab2cd3688e652b214d77c66c679a36b7f Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Tue, 30 Oct 2018 00:30:01 +0100 Subject: [PATCH] [#153,!82] Changes after review: - removed #if 0 - cleaned up messages --- src/bin/netconf/netconf.cc | 25 ++++++++++++++++++---- src/bin/netconf/netconf.h | 14 +++++++++++- src/bin/netconf/netconf_messages.mes | 6 +++--- src/bin/netconf/tests/netconf_unittests.cc | 16 ++++++-------- 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/bin/netconf/netconf.cc b/src/bin/netconf/netconf.cc index 28732ea751..cfaef37653 100644 --- a/src/bin/netconf/netconf.cc +++ b/src/bin/netconf/netconf.cc @@ -40,6 +40,9 @@ public: /// @brief Module change callback. /// + /// This callback is called by sysrepo when there is a change to + /// configuration data. + /// /// @param sess The running datastore session. /// @param module_name The module name. /// @param event The event. @@ -115,6 +118,8 @@ NetconfAgent::init(NetconfCfgMgrPtr cfg_mgr) { if (NetconfProcess::global_shut_down_flag) { return; } + + // Retrieve configuration from existing running DHCP daemons. keaConfig(pair); if (NetconfProcess::global_shut_down_flag) { return; @@ -123,10 +128,13 @@ NetconfAgent::init(NetconfCfgMgrPtr cfg_mgr) { if (NetconfProcess::global_shut_down_flag) { return; } + + // Initialize sysrepo interface. initSysrepo(); if (NetconfProcess::global_shut_down_flag) { return; } + for (auto pair : *servers) { if (NetconfProcess::global_shut_down_flag) { return; @@ -205,7 +213,7 @@ NetconfAgent::keaConfig(const CfgServersMapPair& service_pair) { if (!config) { LOG_ERROR(netconf_logger, NETCONF_GET_CONFIG_FAILED) .arg(service_pair.first) - .arg("configGet returned an empty configuration"); + .arg("config-get returned an empty configuration"); return; } LOG_INFO(netconf_logger, NETCONF_BOOT_UPDATE_COMPLETE) @@ -237,19 +245,26 @@ NetconfAgent::initSysrepo() { void NetconfAgent::yangConfig(const CfgServersMapPair& service_pair) { + // If we're shutting down, or the boot-update flag is not set or the model + // associated with it is not specified. if (NetconfProcess::global_shut_down_flag || !service_pair.second->getBootUpdate() || service_pair.second->getModel().empty()) { return; } + + // First we need a way to reach the actual servers. CfgControlSocketPtr ctrl_sock = service_pair.second->getCfgControlSocket(); if (!ctrl_sock) { return; } + LOG_DEBUG(netconf_logger, NETCONF_DBG_TRACE, NETCONF_SET_CONFIG) .arg(service_pair.first); ConstElementPtr config; try { + + // Retrieve configuration from Sysrepo. TranslatorConfig tc(startup_sess_, service_pair.second->getModel()); config = tc.getConfig(); if (!config) { @@ -269,7 +284,7 @@ NetconfAgent::yangConfig(const CfgServersMapPair& service_pair) { } } catch (const std::exception& ex) { ostringstream msg; - msg << "YANG getConfig for " << service_pair.first + msg << "YANG config-get for " << service_pair.first << " failed with " << ex.what(); LOG_ERROR(netconf_logger, NETCONF_SET_CONFIG_FAILED) .arg(service_pair.first) @@ -383,7 +398,7 @@ NetconfAgent::validate(S_Session sess, const CfgServersMapPair& service_pair) { } } catch (const std::exception& ex) { ostringstream msg; - msg << "YANG getConfig for " << service_pair.first + msg << "YANG config-get for " << service_pair.first << " failed with " << ex.what(); LOG_ERROR(netconf_logger, NETCONF_VALIDATE_CONFIG_FAILED) .arg(service_pair.first) @@ -468,7 +483,7 @@ NetconfAgent::update(S_Session sess, const CfgServersMapPair& service_pair) { } } catch (const std::exception& ex) { ostringstream msg; - msg << "YANG getConfig for " << service_pair.first + msg << "YANG config-get " << service_pair.first << " failed with " << ex.what(); LOG_ERROR(netconf_logger, NETCONF_UPDATE_CONFIG_FAILED) .arg(service_pair.first) @@ -507,6 +522,8 @@ NetconfAgent::update(S_Session sess, const CfgServersMapPair& service_pair) { .arg(msg.str()); return (SR_ERR_VALIDATION_FAILED); } + + // rcode == CONTROL_RESULT_SUCCESS, unless the docs say otherwise :) if (rcode != CONTROL_RESULT_SUCCESS) { stringstream msg; msg << "configSet returned " << answerToText(answer); diff --git a/src/bin/netconf/netconf.h b/src/bin/netconf/netconf.h index c8d58d7382..cf93db2399 100644 --- a/src/bin/netconf/netconf.h +++ b/src/bin/netconf/netconf.h @@ -94,10 +94,22 @@ public: protected: /// @brief Get and display Kea server configuration. /// + /// Retrieves current configuration via control socket (unix or http) + /// from a running Kea server. If boot-update is set to false, this + /// operation is a no-op. + /// /// @param service_pair The service name and configuration pair. void keaConfig(const CfgServersMapPair& service_pair); - /// @brief Kea server configuration from YANG datastore. + /// @brief Retrieve Kea server configuration from YANG datastore and + /// applies it to servers. + /// + /// This method retrieves the configuation from sysrepo first, then + /// established control socket connection to Kea servers (currently + /// dhcp4 and/or dhcp6) and then attempts to send configuration + /// using config-set. + /// + /// If boot-update is set to false, this operation is a no-op. /// /// @param service_pair The service name and configuration pair. void yangConfig(const CfgServersMapPair& service_pair); diff --git a/src/bin/netconf/netconf_messages.mes b/src/bin/netconf/netconf_messages.mes index 8bd02a108d..15923d3c6c 100644 --- a/src/bin/netconf/netconf_messages.mes +++ b/src/bin/netconf/netconf_messages.mes @@ -50,11 +50,11 @@ are printed. % NETCONF_LOG_CHANGE_FAIL Netconf configuration change logging failed: %1 The warning message indicates that the configuration change logging -encountered a not expected condition. +encountered an unexpected condition. Details of it will be logged. % NETCONF_RUN_EXIT application is exiting the event loop This is a debug message issued when the Netconf application exits its -event loop. +event loop. This is a normal step during kea-netconf shutdown. % NETCONF_SET_CONFIG setting configuration to %1 server This debug message indicates that Netconf is trying to set the @@ -64,7 +64,7 @@ configuration to a Kea server. This debug message indicates that Netconf set the configuration to a Kea server. The server name and the applied configuration are printed. -% NETCONF_SET_CONFIG_FAILED setting configuration to %1 server: %2 +% NETCONF_SET_CONFIG_FAILED setting configuration to %1 server failed: %2 The error message indicates that Netconf got an error setting the configuration to a Kea server. The name of the server and the error are printed. Make sure that the server is up and running, has appropriate diff --git a/src/bin/netconf/tests/netconf_unittests.cc b/src/bin/netconf/tests/netconf_unittests.cc index 57a3f33cfb..b2ef36943e 100644 --- a/src/bin/netconf/tests/netconf_unittests.cc +++ b/src/bin/netconf/tests/netconf_unittests.cc @@ -41,7 +41,7 @@ namespace { const string TEST_SOCKET = "test-socket"; /// @brief Test timeout in ms. -const long TEST_TIMEOUT = 10000; +//const long TEST_TIMEOUT = 10000; /// @brief Type definition for the pointer to Thread objects. typedef boost::shared_ptr ThreadPtr; @@ -451,9 +451,8 @@ TEST_F(NetconfAgentLogTest, logChanges) { usleep(1000); } // Enable this for debugging. -#if 0 - logCheckVerbose(true); -#endif + // logCheckVerbose(true); + EXPECT_TRUE(checkFile()); } @@ -564,9 +563,8 @@ TEST_F(NetconfAgentLogTest, logChanges2) { usleep(1000); } // Enable this for debugging. -#if 0 - logCheckVerbose(true); -#endif + // logCheckVerbose(true); + EXPECT_TRUE(checkFile()); } @@ -634,9 +632,7 @@ TEST_F(NetconfAgentTest, keaConfig) { ConstElementPtr pruned = prune(expected, request); EXPECT_TRUE(expected->equals(*pruned)); // Alternative showing more for debugging... -#if 0 - EXPECT_EQ(prettyPrint(expected), prettyPrint(pruned)); -#endif + // EXPECT_EQ(prettyPrint(expected), prettyPrint(pruned)); // Check response. ASSERT_EQ(1, responses_.size()); -- 2.47.2