[#103,!289] Addressed review comments.

diff --git a/src/lib/dhcp/option_space_container.h b/src/lib/dhcp/option_space_container.h
index f480db058524f423dd94d3e95eb0ebbd284d761c..5700ccdc8426a818debf6159a1188dd01cbc9a33 100644 (file)
--- a/src/lib/dhcp/option_space_container.h
+++ b/src/lib/dhcp/option_space_container.h
@@ -99,8 +99,10 @@ public:
     /// or option definitions having the same id (typically id of 0).
     /// When configuration backend is in use it sets the unique ids
     /// from the database. In cases when the configuration backend is
-    /// not used, the ids default to 0.
-
+    /// not used, the ids default to 0. Passing the id of 0 would
+    /// result in deleting all options or option definitions that were
+    /// not added via the database.
+    ///
     /// @param id Identifier of the items to be deleted.
     ///
     /// @return Number of deleted options or option definitions.

diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
index 20385bb7f660fa775fe2b7dc6e0e10153af779f2..dccdc22cfd1e562413d8d5870ff42f7e53971bc1 100644 (file)
--- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
+++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
@@ -85,7 +85,21 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
             range = index.equal_range(boost::make_tuple("dhcp4_subnet",
                                                         AuditEntry::ModificationType::DELETE));
             for (auto entry = range.first; entry != range.second; ++entry) {
-                cfg->getCfgSubnets4()->del((*entry)->getObjectId());
+                // If the deleted subnet belongs to a shared network and the
+                // shared network is not being removed, we need to detach the
+                // subnet from the shared network.
+                auto subnet = cfg->getCfgSubnets4()->getBySubnetId((*entry)->getObjectId());
+                if (subnet) {
+                    // Check if the subnet belongs to a shared network.
+                    SharedNetwork4Ptr network;
+                    subnet->getSharedNetwork(network);
+                    if (network) {
+                        // Detach the subnet from the shared network.
+                        network->del(subnet->getID());
+                    }
+                    // Actually delete the subnet from the configuration.
+                    cfg->getCfgSubnets4()->del((*entry)->getObjectId());
+                }
             }
 
         } catch (...) {

diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h
index 9de6ae513b40a286b0c2b3489debaaedfe238723..22c57227dc7630f2b775d4bd01f001d6dfcd0d3c 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option.h
+++ b/src/lib/dhcpsrv/cfg_option.h
@@ -541,6 +541,8 @@ public:
     /// having the same id (typically id of 0). When configuration backend
     /// is in use it sets the unique ids from the database. In cases when
     /// the configuration backend is not used, the ids default to 0.
+    /// Passing the id of 0 would result in deleting all options that were
+    /// not added via the database.
     ///
     /// Both regular and vendor specific options are deleted with this
     /// method.

diff --git a/src/lib/dhcpsrv/cfg_option_def.h b/src/lib/dhcpsrv/cfg_option_def.h
index 80f95e70e9a9f914f98dd8c586992e087390de8e..3c58b77aafa4c40cbfb2b806f147a3aa2bd42710 100644 (file)
--- a/src/lib/dhcpsrv/cfg_option_def.h
+++ b/src/lib/dhcpsrv/cfg_option_def.h
@@ -119,7 +119,9 @@ public:
     /// definitions having the same id (typically id of 0). When
     /// configuration backend is in use it sets the unique ids from the
     /// database. In cases when the configuration backend is not used,
-    /// the ids default to 0.
+    /// the ids default to 0. Passing the id of 0 would result in
+    /// deleting all option definitions that were not added via the
+    /// database.
     ///
     /// @param id Identifier of the option definitions to be deleted.
     ///

diff --git a/src/lib/dhcpsrv/cfg_shared_networks.h b/src/lib/dhcpsrv/cfg_shared_networks.h
index 7d7bb993060314ce69dd98fe8072888882813002..f97b4dbd6949ba64d531e3c90c1e679466875227 100644 (file)
--- a/src/lib/dhcpsrv/cfg_shared_networks.h
+++ b/src/lib/dhcpsrv/cfg_shared_networks.h
@@ -75,7 +75,8 @@ public:
     /// networks having the same id (typically id of 0). When configuration
     /// backend is in use it sets the unique ids from the database.
     /// In cases when the configuration backend is not used, the ids
-    /// default to 0.
+    /// default to 0. Passing the id of 0 would result in deleting all
+    /// shared networks that were not added via the database.
     ///
     /// @param id Identifier of the shared networks to be deleted.
     ///

diff --git a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
index 1fe5dad512fe12edf67ba390923fe8698fcac64f..7dad1b7f3527220a590c7fb578cd076fe781bb04 100644 (file)
--- a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc
@@ -285,6 +285,7 @@ public:
         // Insert subnets into the database.
         Subnet4Ptr subnet(new Subnet4(IOAddress("192.0.3.0"), 26, 1, 2, 3, SubnetID(1)));
         subnet->setModificationTime(getTimestamp("dhcp4_subnet"));
+        subnet->setSharedNetworkName("one");
         mgr.getPool()->createUpdateSubnet4(BackendSelector::UNSPEC(), ServerSelector::ALL(),
                                            subnet);
 
@@ -577,6 +578,14 @@ public:
             if (deleteConfigElement("dhcp4_subnet", 1)) {
                 EXPECT_FALSE(found_subnet);
 
+                // If the subnet has been deleted, make sure that
+                // it was detached from the shared network it belonged
+                // to, if the shared network still exists.
+                auto found_network = srv_cfg->getCfgSharedNetworks4()->getByName("one");
+                if (found_network) {
+                    EXPECT_TRUE(found_network->getAllSubnets()->empty());
+                }
+
             } else {
                 EXPECT_TRUE(found_subnet);
             }
@@ -730,6 +739,7 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplySharedNetworkNotFetched) {
 // This test verifies that only a subnet is merged into the current
 // configuration.
 TEST_F(CBControlDHCPv4Test, databaseConfigApplySubnet) {
+    addCreateAuditEntry("dhcp4_shared_network");
     addCreateAuditEntry("dhcp4_subnet");
     testDatabaseConfigApply(getTimestamp(-5));
 }

diff --git a/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc
index 5c9a9075c9c2a6920bbd6102f7899c3259777136..48f945aa89b1e2956894937ab5bfc74a426cf953 100644 (file)
--- a/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc
@@ -61,6 +61,8 @@ TEST(CfgOptionDefTest, getAllThenDelete) {
         option_name << "option-" << code;
         OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
                                                      "uint16"));
+        // We're setting id of 123 for all option definitions in this
+        // code range.
         def->setId(123);
 
         // Add option definition to "isc" option space.
@@ -76,7 +78,8 @@ TEST(CfgOptionDefTest, getAllThenDelete) {
         option_name << "option-" << code;
         OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
                                                      "uint16"));
-
+        // We're setting id of 234 for all option definitions in this
+        // code range.
         def->setId(234);
 
         ASSERT_NO_THROW(cfg.add(def, "abcde"));