[#2823] Addressed review comments

diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
index ca1c9d736693d29fd523e737cf47c67f8d70a239..5ba211f5195bfb11f7b74e0968bc7a69ea1ab630 100644 (file)
--- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
+++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc
@@ -221,10 +221,10 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
     // Allocator selection at the global level can affect subnets and shared networks
     // for which the allocator hasn't been specified explicitly. Let's see if the
     // allocator has been specified at the global level.
-    std::string default_allocator;
+    std::string global_allocator;
     auto allocator = external_cfg->getConfiguredGlobal(CfgGlobals::ALLOCATOR);
     if (allocator && (allocator->getType() == Element::string)) {
-        default_allocator = allocator->stringValue();
+       global_allocator = allocator->stringValue();
     }
 
     // If we're fetching the changes from the config backend we also want
@@ -234,10 +234,9 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
     // We're only affected by the allocator change if this is the update from
     // the configuration backend.
     if (cb_update) {
-        std::string current_default_allocator;
         auto allocator = CfgMgr::instance().getCurrentCfg()->getConfiguredGlobal(CfgGlobals::ALLOCATOR);
         if (allocator && (allocator->getType() == Element::string)) {
-            allocator_changed = (default_allocator != allocator->stringValue());
+            allocator_changed = (global_allocator != allocator->stringValue());
         }
     }
 
@@ -268,7 +267,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
         (*network)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr {
              return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals());
         });
-        (*network)->setDefaultAllocatorType(default_allocator);
+        (*network)->setDefaultAllocatorType(global_allocator);
         external_cfg->getCfgSharedNetworks4()->add((*network));
     }
 
@@ -279,7 +278,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
     Subnet4Collection subnets;
     if (allocator_changed || reconfig) {
         // A change of the allocator or the server reconfiguration can affect all
-        // shared networks. Get all subnets.
+        // subnets. Get all subnets.
         subnets = getMgr().getPool()->getAllSubnets4(backend_selector, server_selector);
 
     }  else if (!updated_entries.empty()) {
@@ -300,7 +299,7 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector,
         (*subnet)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr {
             return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals());
         });
-        (*subnet)->setDefaultAllocatorType(default_allocator);
+        (*subnet)->setDefaultAllocatorType(global_allocator);
         external_cfg->getCfgSubnets4()->add((*subnet));
     }
 

diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc
index 4624bf495b038c006673df18fa022eaf1482c802..91b482ebad452fd3fd5ce1d63dec762e7cab88cf 100644 (file)
--- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc
+++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc
@@ -220,17 +220,17 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
     // Allocator selection at the global level can affect subnets and shared networks
     // for which the allocator hasn't been specified explicitly. Let's see if the
     // allocator has been specified at the global level.
-    std::string default_allocator;
+    std::string global_allocator;
     auto allocator = external_cfg->getConfiguredGlobal(CfgGlobals::ALLOCATOR);
     if (allocator && (allocator->getType() == Element::string)) {
-        default_allocator = allocator->stringValue();
+        global_allocator = allocator->stringValue();
     }
 
     // Also, get the PD allocator.
-    std::string default_pd_allocator;
+    std::string global_pd_allocator;
     allocator = external_cfg->getConfiguredGlobal(CfgGlobals::PD_ALLOCATOR);
     if (allocator && (allocator->getType() == Element::string)) {
-        default_pd_allocator = allocator->stringValue();
+        global_pd_allocator = allocator->stringValue();
     }
 
     // If we're fetching the changes from the config backend we also want
@@ -242,7 +242,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
     if (cb_update) {
         auto allocator = CfgMgr::instance().getCurrentCfg()->getConfiguredGlobal(CfgGlobals::ALLOCATOR);
         if (allocator && (allocator->getType() == Element::string)) {
-            allocator_changed = (default_allocator != allocator->stringValue());
+            allocator_changed = (global_allocator != allocator->stringValue());
         }
 
         // The address allocator hasn't changed. So, let's check if the PD allocator
@@ -250,7 +250,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
         if (!allocator_changed) {
             auto allocator = CfgMgr::instance().getCurrentCfg()->getConfiguredGlobal(CfgGlobals::PD_ALLOCATOR);
             if (allocator && (allocator->getType() == Element::string)) {
-                allocator_changed = (default_pd_allocator != allocator->stringValue());
+                allocator_changed = (global_pd_allocator != allocator->stringValue());
             }
         }
     }
@@ -278,8 +278,8 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
         (*network)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr {
             return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals());
         });
-        (*network)->setDefaultAllocatorType(default_allocator);
-        (*network)->setDefaultPdAllocatorType(default_pd_allocator);
+        (*network)->setDefaultAllocatorType(global_allocator);
+        (*network)->setDefaultPdAllocatorType(global_pd_allocator);
         external_cfg->getCfgSharedNetworks6()->add((*network));
     }
 
@@ -290,7 +290,7 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
     Subnet6Collection subnets;
     if (allocator_changed || reconfig) {
         // A change of the allocator or the server reconfiguration can affect all
-        // shared networks. Get all subnets.
+        // subnets. Get all subnets.
         subnets = getMgr().getPool()->getAllSubnets6(backend_selector, server_selector);
 
     } else if (!updated_entries.empty()) {
@@ -311,8 +311,8 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector
         (*subnet)->setFetchGlobalsFn([] () -> ConstCfgGlobalsPtr {
             return (CfgMgr::instance().getCurrentCfg()->getConfiguredGlobals());
         });
-        (*subnet)->setDefaultAllocatorType(default_allocator);
-        (*subnet)->setDefaultPdAllocatorType(default_pd_allocator);
+        (*subnet)->setDefaultAllocatorType(global_allocator);
+        (*subnet)->setDefaultPdAllocatorType(global_pd_allocator);
         external_cfg->getCfgSubnets6()->add((*subnet));
     }
 

diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h
index 3eda94f6a36fe1ea439122f93f4495b27d813bd6..9d2a8995e10afc0af1fafd3fb8f7c3772143f8c3 100644 (file)
--- a/src/lib/dhcpsrv/network.h
+++ b/src/lib/dhcpsrv/network.h
@@ -1237,6 +1237,9 @@ protected:
     util::Optional<std::string> allocator_type_;
 
     /// @brief Default allocator type.
+    ///
+    /// This value is not configurable by the user. It is used by the configuration
+    /// backend internally.
     util::Optional<std::string> default_allocator_type_;
 
     /// @brief Pointer to another network that this network belongs to.

diff --git a/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc b/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc
index 33ae382901a6319a92e622c62ecfb41f70ff71dc..350106f11b0f8b7dd2e28d1f19ed925a8b1b0238 100644 (file)
--- a/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc
+++ b/src/lib/dhcpsrv/tests/flq_allocator_unittest.cc
@@ -361,8 +361,11 @@ public:
 
 // Test that the allocator returns the correct type.
 TEST_F(FreeLeaseQueueAllocatorTest6, getType) {
-    FreeLeaseQueueAllocator alloc(Lease::TYPE_NA, subnet_);
-    EXPECT_EQ("flq", alloc.getType());
+    FreeLeaseQueueAllocator allocNA(Lease::TYPE_NA, subnet_);
+    EXPECT_EQ("flq", allocNA.getType());
+
+    FreeLeaseQueueAllocator allocPD(Lease::TYPE_PD, subnet_);
+    EXPECT_EQ("flq", allocPD.getType());
 }
 
 // Test populating free DHCPv6 address leases to the queue.

diff --git a/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc b/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc
index 3d28607e455a839b91adc8276fbfc01eaccf73ed..bde1805a112eafe0a049e1f9c53869c88d4329f9 100644 (file)
--- a/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc
+++ b/src/lib/dhcpsrv/tests/iterative_allocator_unittest.cc
@@ -119,8 +119,11 @@ using IterativeAllocatorTest6 = AllocEngine6Test;
 
 // Test that the allocator returns the correct type.
 TEST_F(IterativeAllocatorTest6, getType) {
-    IterativeAllocator alloc(Lease::TYPE_NA, subnet_);
-    EXPECT_EQ("iterative", alloc.getType());
+    IterativeAllocator allocNA(Lease::TYPE_NA, subnet_);
+    EXPECT_EQ("iterative", allocNA.getType());
+
+    IterativeAllocator allocPD(Lease::TYPE_PD, subnet_);
+    EXPECT_EQ("iterative", allocPD.getType());
 }
 
 // This test verifies that the allocator picks addresses that belong to the

diff --git a/src/lib/dhcpsrv/tests/random_allocator_unittest.cc b/src/lib/dhcpsrv/tests/random_allocator_unittest.cc
index 55fec78d2f1276b00955c873f74b57ea6eeb5567..e8009eed3a6ae3a86eadb3809b281c60752f4dee 100644 (file)
--- a/src/lib/dhcpsrv/tests/random_allocator_unittest.cc
+++ b/src/lib/dhcpsrv/tests/random_allocator_unittest.cc
@@ -186,8 +186,11 @@ using RandomAllocatorTest6 = AllocEngine6Test;
 
 // Test that the allocator returns the correct type.
 TEST_F(RandomAllocatorTest6, getType) {
-    RandomAllocator alloc(Lease::TYPE_NA, subnet_);
-    EXPECT_EQ("random", alloc.getType());
+    RandomAllocator allocNA(Lease::TYPE_NA, subnet_);
+    EXPECT_EQ("random", allocNA.getType());
+
+    RandomAllocator allocPD(Lease::TYPE_PD, subnet_);
+    EXPECT_EQ("random", allocPD.getType());
 }
 
 // Test allocating IPv6 addresses when a subnet has a single pool.