[2637] Fix the DHCP configuration handling.

In particular, the new configuration is merged with the existing
configuration to trigger the full reconfiguration of the server. This is
to satisfy dependencies between various configuration values.

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -47,7 +47,22 @@ namespace dhcp {
 ControlledDhcpv4Srv* ControlledDhcpv4Srv::server_ = NULL;
 
 ConstElementPtr
-ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr) {
+ControlledDhcpv4Srv::dhcp4StubConfigHandler(ConstElementPtr) {
+    // This configuration handler is intended to be used only
+    // when the initial configuration comes in. To receive this
+    // configuration a pointer to this handler must be passed
+    // using ModuleCCSession constructor. This constructor will
+    // invoke the handler and will store the configuration for
+    // the configuration session when the handler returns success.
+    // Since this configuration is partial we just pretend to
+    // parse it and always return success. The function that
+    // initiates the session must get the configuration on its
+    // own using getFullConfig.
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+ConstElementPtr
+ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr new_config) {
     if (!server_ || !server_->config_session_) {
         // That should never happen as we install config_handler
         // after we instantiate the server.
@@ -56,6 +71,7 @@ ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr) {
                                       " server is during startup/shutdown phase.");
         return (answer);
     }
+
     // The configuration passed to this handler function is partial.
     // In other words, it just includes the values being modified.
     // In the same time, there are dependencies between various
@@ -65,15 +81,26 @@ ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr) {
     // removes that definition is triggered while a relevant option value
     // may remain configured. This eventually results in the DHCP server
     // configuration being in the inconsistent state.
-    // In order to work around this problem we always get the full
-    // configuration for the server. This triggers all parsers to reset
-    // the configuration data and check the dependencies between values
-    // being set. Thus, the configuration passed to this function
-    // is ignored.
+    // In order to work around this problem we need to merge the new
+    // configuration with the existing (full) configuration.
+
+    // Let's create a new object that will hold the merged configuration.
+    boost::shared_ptr<MapElement> merged_config(new MapElement());
+    // Let's get the existing configuration.
     ConstElementPtr full_config = server_->config_session_->getFullConfig();
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_UPDATE)
-              .arg(full_config->str());
+    // The full_config and merged_config should be always non-NULL
+    // but to provide some level of exception safety we check that they
+    // really are (in case we go out of memory).
+    if (full_config && merged_config) {
+        merged_config->setValue(full_config->mapValue());
+
+        // Merge an existing and new configuration.
+        isc::data::merge(merged_config, new_config);
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_UPDATE)
+            .arg(full_config->str());
+    }
 
+    // Configure the server.
     return (configureDhcp4Server(*server_, full_config));
 }
 
@@ -125,8 +152,15 @@ void ControlledDhcpv4Srv::establishSession() {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_CCSESSION_STARTING)
               .arg(specfile);
     cc_session_ = new Session(io_service_.get_io_service());
+    // Create a session with the dummy configuration handler.
+    // Dumy configuration handler is internally invoked by the
+    // constructor and on success the constructor updates
+    // the current session with the configuration that had been
+    // commited in the previous session. If we did not install
+    // the dummy handler, the previous configuration would have
+    // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,
-                                          NULL,
+                                          dhcp4StubConfigHandler,
                                           dhcp4CommandHandler, false);
     config_session_->start();
 

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -94,6 +94,27 @@ protected:
     static isc::data::ConstElementPtr
     dhcp4ConfigHandler(isc::data::ConstElementPtr new_config);
 
+    /// @brief A dummy configuration handler that always returns success.
+    ///
+    /// This configuration handler does not perform configuration
+    /// parsing and always returns success. A dummy hanlder should
+    /// be installed using \ref isc::config::ModuleCCSession ctor
+    /// to get the initial configuration. This initial configuration
+    /// comprises values for only those elements that were modified
+    /// the previous session. The \ref dhcp4ConfigHandler can't be
+    /// used to parse the initial configuration because it needs the
+    /// full configuration to satisfy dependencies between the
+    /// various configuration values. Installing the dummy handler
+    /// that guarantees to return success causes initial configuration
+    /// to be stored for the session being created and that it can
+    /// be later accessed with \ref isc::ConfigData::getFullConfig.
+    ///
+    /// @param new_config new configuration.
+    ///
+    /// @return success configuration status.
+    static isc::data::ConstElementPtr
+    dhcp4StubConfigHandler(isc::data::ConstElementPtr new_config);
+
     /// @brief A callback for handling incoming commands.
     ///
     /// @param command textual representation of the command

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -46,7 +46,22 @@ namespace dhcp {
 ControlledDhcpv6Srv* ControlledDhcpv6Srv::server_ = NULL;
 
 ConstElementPtr
-ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr) {
+ControlledDhcpv6Srv::dhcp6StubConfigHandler(ConstElementPtr) {
+    // This configuration handler is intended to be used only
+    // when the initial configuration comes in. To receive this
+    // configuration a pointer to this handler must be passed
+    // using ModuleCCSession constructor. This constructor will
+    // invoke the handler and will store the configuration for
+    // the configuration session when the handler returns success.
+    // Since this configuration is partial we just pretend to
+    // parse it and always return success. The function that
+    // initiates the session must get the configuration on its
+    // own using getFullConfig.
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+ConstElementPtr
+ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr new_config) {
 
     if (!server_ || !server_->config_session_) {
         // That should never happen as we install config_handler
@@ -66,16 +81,27 @@ ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr) {
     // removes that definition is triggered while a relevant option value
     // may remain configured. This eventually results in the DHCP server
     // configuration being in the inconsistent state.
-    // In order to work around this problem we always get the full
-    // configuration for the server. This triggers all parsers to reset
-    // the configuration data and check the dependencies between values
-    // being set. Thus, the configuration passed to this function
-    // is ignored.
+    // In order to work around this problem we need to merge the new
+    // configuration with the existing (full) configuration.
+
+    // Let's create a new object that will hold the merged configuration.
+    boost::shared_ptr<MapElement> merged_config(new MapElement());
+    // Let's get the existing configuration.
     ConstElementPtr full_config = server_->config_session_->getFullConfig();
-    LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_UPDATE)
-              .arg(full_config->str());
+    // The full_config and merged_config should be always non-NULL
+    // but to provide some level of exception safety we check that they
+    // really are (in case we go out of memory).
+    if (full_config && merged_config) {
+        merged_config->setValue(full_config->mapValue());
+
+        // Merge an existing and new configuration.
+        isc::data::merge(merged_config, new_config);
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_UPDATE)
+            .arg(merged_config->str());
+    }
 
-    return (configureDhcp6Server(*server_, full_config));
+    // Configure the server.
+    return (configureDhcp6Server(*server_, merged_config));
 }
 
 ConstElementPtr
@@ -126,18 +152,26 @@ void ControlledDhcpv6Srv::establishSession() {
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_CCSESSION_STARTING)
               .arg(specfile);
     cc_session_ = new Session(io_service_.get_io_service());
+    // Create a session with the dummy configuration handler.
+    // Dumy configuration handler is internally invoked by the
+    // constructor and on success the constructor updates
+    // the current session with the configuration that had been
+    // commited in the previous session. If we did not install
+    // the dummy handler, the previous configuration would have
+    // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,
-                                          NULL,
+                                          dhcp6StubConfigHandler,
                                           dhcp6CommandHandler, false);
     config_session_->start();
 
-    // We initially create ModuleCCSession() without configHandler, as
-    // the session module is too eager to send partial configuration.
-    // We want to get the full configuration, so we explicitly call
-    // getFullConfig() and then pass it to our configHandler.
+    // The constructor already pulled the configuration that had
+    // been created in the previous session thanks to the dummy
+    // handler. We can switch to the handler that will be
+    // parsing future changes to the configuration.
     config_session_->setConfigHandler(dhcp6ConfigHandler);
 
     try {
+        // Pull the full configuration out from the session.
         configureDhcp6Server(*this, config_session_->getFullConfig());
     } catch (const DhcpConfigError& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(ex.what());

src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -94,6 +94,27 @@ protected:
     static isc::data::ConstElementPtr
     dhcp6ConfigHandler(isc::data::ConstElementPtr new_config);
 
+    /// @brief A dummy configuration handler that always returns success.
+    ///
+    /// This configuration handler does not perform configuration
+    /// parsing and always returns success. A dummy hanlder should
+    /// be installed using \ref isc::config::ModuleCCSession ctor
+    /// to get the initial configuration. This initial configuration
+    /// comprises values for only those elements that were modified
+    /// the previous session. The \ref dhcp6ConfigHandler can't be
+    /// used to parse the initial configuration because it needs the
+    /// full configuration to satisfy dependencies between the
+    /// various configuration values. Installing the dummy handler
+    /// that guarantees to return success causes initial configuration
+    /// to be stored for the session being created and that it can
+    /// be later accessed with \ref isc::ConfigData::getFullConfig.
+    ///
+    /// @param new_config new configuration.
+    ///
+    /// @return success configuration status.
+    static isc::data::ConstElementPtr
+    dhcp6StubConfigHandler(isc::data::ConstElementPtr new_config);
+
     /// @brief A callback for handling incoming commands.
     ///
     /// @param command textual representation of the command