[#153,!82] Changes after review:

 - removed #if 0
 - cleaned up messages

diff --git a/src/bin/netconf/netconf.cc b/src/bin/netconf/netconf.cc
index 28732ea751e1c9e691918bc579564b02e97d683c..cfaef37653cc66b3a302088738a0eec43dd9dcb2 100644 (file)
--- 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 c8d58d73824543f068181942e5f98a3be29f9481..cf93db239958f824ae94097e30d3b06bcd6a8b7f 100644 (file)
--- 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 8bd02a108d44c586169c81c1973dcc25b70e5631..15923d3c6ce019db60b415b7a7ae887d5b72a704 100644 (file)
--- 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 57a3f33cfb24248a356447c724a8aad59ecfacc7..b2ef36943e4ad5d0992a8325664fae0614abd928 100644 (file)
--- 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<Thread> 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());