[#3017] fix failed to open socket on reconfig

2 files changed, 14 insertions, 48 deletions

diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc
index 20cb04df51..811192c41f 100644
--- a/src/bin/dhcp4/json_config_parser.cc
+++ b/src/bin/dhcp4/json_config_parser.cc
@@ -326,11 +326,8 @@ void configureCommandChannel() {
 /// configuration is valid, or specifying details about the error otherwise.
 ///
 /// @param config_set the configuration being processed
-/// @param check_only whether the processing is only for testing the
-///     configuration, in which case some configuration elements, such as
-///     interfaces, might be left uncommitted or unprocessed
 isc::data::ConstElementPtr
-processDhcp4Config(isc::data::ConstElementPtr config_set, bool const check_only) {
+processDhcp4Config(isc::data::ConstElementPtr config_set) {
     // Before starting any subnet operations, let's reset the subnet-id counter,
     // so newly recreated configuration starts with first subnet-id equal 1.
     Subnet::resetSubnetID();
@@ -451,20 +448,6 @@ processDhcp4Config(isc::data::ConstElementPtr config_set, bool const check_only)
             parser.parse(hr_identifiers);
         }
 
-        // Interfaces are parsed one more time with test_mode=false in the
-        // caller context. Presumably, this would make it possible to create
-        // IfacesConfigParser with test_mode=false here, since if the result of
-        // processDhcp4Config is a success, the interfaces get applied there.
-        // However test_mode influences redetection which can influence the
-        // result of processDhcp4Config, so let's keep test_mode=check_only.
-        ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config");
-        if (ifaces_config) {
-            parameter_name = "interfaces-config";
-            IfacesConfigParser parser(AF_INET, check_only);
-            CfgIfacePtr cfg_iface = srv_config->getCfgIface();
-            parser.parse(cfg_iface, ifaces_config);
-        }
-
         ConstElementPtr sanity_checks = mutable_cfg->get("sanity-checks");
         if (sanity_checks) {
             parameter_name = "sanity-checks";
@@ -776,7 +759,7 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set,
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_START)
         .arg(server.redactConfig(config_set)->str());
 
-    auto answer = processDhcp4Config(config_set, check_only);
+    auto answer = processDhcp4Config(config_set);
 
     int status_code = CONTROL_RESULT_SUCCESS;
     isc::config::parseAnswer(status_code, answer);
@@ -823,9 +806,6 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set,
                 }
             }
         } else {
-            string parameter_name;
-            ElementPtr mutable_cfg;
-
             // disable multi-threading (it will be applied by new configuration)
             // this must be done in order to properly handle MT to ST transition
             // when 'multi-threading' structure is missing from new config and
@@ -839,9 +819,12 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set,
             TimerMgr::instance()->unregisterTimers();
             server.discardPackets();
             server.getCBControl()->reset();
+        }
 
+        if (status_code == CONTROL_RESULT_SUCCESS) {
+            string parameter_name;
+            ElementPtr mutable_cfg;
             try {
-
                 // Get the staging configuration.
                 srv_config = CfgMgr::instance().getStagingCfg();
 
@@ -853,7 +836,7 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set,
                 ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config");
                 if (ifaces_config) {
                     parameter_name = "interfaces-config";
-                    IfacesConfigParser parser(AF_INET, false);
+                    IfacesConfigParser parser(AF_INET, check_only);
                     CfgIfacePtr cfg_iface = srv_config->getCfgIface();
                     cfg_iface->reset();
                     parser.parse(cfg_iface, ifaces_config);
diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc
index 0b897e4ae5..dbd4291e85 100644
--- a/src/bin/dhcp6/json_config_parser.cc
+++ b/src/bin/dhcp6/json_config_parser.cc
@@ -429,11 +429,8 @@ void configureCommandChannel() {
 /// configuration is valid, or specifying details about the error otherwise.
 ///
 /// @param config_set the configuration being processed
-/// @param check_only whether the processing is only for testing the
-///     configuration, in which case some configuration elements, such as
-///     interfaces, might be left uncommitted or unprocessed
 isc::data::ConstElementPtr
-processDhcp6Config(isc::data::ConstElementPtr config_set, bool const check_only) {
+processDhcp6Config(isc::data::ConstElementPtr config_set) {
     // Before starting any subnet operations, let's reset the subnet-id counter,
     // so newly recreated configuration starts with first subnet-id equal 1.
     Subnet::resetSubnetID();
@@ -577,20 +574,6 @@ processDhcp6Config(isc::data::ConstElementPtr config_set, bool const check_only)
             parser.parse(cfg, server_id);
         }
 
-        // Interfaces are parsed one more time with test_mode=false in the
-        // caller context. Presumably, this would make it possible to create
-        // IfacesConfigParser with test_mode=false here, since if the result of
-        // processDhcp6Config is a success, the interfaces get applied there.
-        // However test_mode influences redetection which can influence the
-        // result of processDhcp6Config, so let's keep test_mode=check_only.
-        ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config");
-        if (ifaces_config) {
-            parameter_name = "interfaces-config";
-            IfacesConfigParser parser(AF_INET6, check_only);
-            CfgIfacePtr cfg_iface = srv_config->getCfgIface();
-            parser.parse(cfg_iface, ifaces_config);
-        }
-
         ConstElementPtr sanity_checks = mutable_cfg->get("sanity-checks");
         if (sanity_checks) {
             parameter_name = "sanity-checks";
@@ -901,7 +884,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set,
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_START)
         .arg(server.redactConfig(config_set)->str());
 
-    auto answer = processDhcp6Config(config_set, check_only);
+    auto answer = processDhcp6Config(config_set);
 
     int status_code = CONTROL_RESULT_SUCCESS;
     isc::config::parseAnswer(status_code, answer);
@@ -948,9 +931,6 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set,
                 }
             }
         } else {
-            string parameter_name;
-            ElementPtr mutable_cfg;
-
             // disable multi-threading (it will be applied by new configuration)
             // this must be done in order to properly handle MT to ST transition
             // when 'multi-threading' structure is missing from new config and
@@ -964,9 +944,12 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set,
             TimerMgr::instance()->unregisterTimers();
             server.discardPackets();
             server.getCBControl()->reset();
+        }
 
+        if (status_code == CONTROL_RESULT_SUCCESS) {
+            string parameter_name;
+            ElementPtr mutable_cfg;
             try {
-
                 // Get the staging configuration.
                 srv_config = CfgMgr::instance().getStagingCfg();
 
@@ -978,7 +961,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set,
                 ConstElementPtr ifaces_config = mutable_cfg->get("interfaces-config");
                 if (ifaces_config) {
                     parameter_name = "interfaces-config";
-                    IfacesConfigParser parser(AF_INET6, false);
+                    IfacesConfigParser parser(AF_INET6, check_only);
                     CfgIfacePtr cfg_iface = srv_config->getCfgIface();
                     cfg_iface->reset();
                     parser.parse(cfg_iface, ifaces_config);