[5247] Corrected issues with assigned-<lease/nas/pd> stats

doc/guide/dhcp4-srv.xml
doc/guide/dhcp6-srv.xml
    Added entries for reclaimed-leases

src/lib/dhcpsrv/alloc_engine.cc

    AllocEngine::reuseExpiredLease(Lease6Ptr&...) - increment
    assigned-<nas/pds> for real allocations

    AllocEngine::extendLease6() - increment assigned-<nas/pds>
    for real allocations if the lease expired

    AllocEngine::renewLease4(const Lease4Ptr&...)
    - set lease state to STATE_DEFAULT for real allocations
    - increment assigned-leases if lease is expired or reclaimed
      for real allocations

    AllocEngine::reuseExpiredLease4(Lease4Ptr&...)
    - increment assigned-leases for real allocations

src/lib/dhcpsrv/cfg_subnets4.cc
    CfgSubnets4::removeStatistics() - added "reclaimed-leases"

src/lib/dhcpsrv/cfg_subnets6.cc
    CfgSubnets4::removeStatistics() - added "reclaimed-leases"

src/lib/dhcpsrv/lease_mgr.cc
    LeaseMgr::recountLeaseStats4()
    LeaseMgr::recountLeaseStats6()
    - added handling of "reclaimed-leases"
    - fixed name of "reclaimed-declined-addresses"

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
    TEST_F(AllocEngine4Test, simpleRenew4)  - new test to verify stats
    on a normal renew, non-expired
    Added EXPECT_TRUE around calls to testStatistics for invocation line numbers
    Added stat checks to several tests

src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
    Added EXPECT_TRUE around calls to testStatistics for invocation line numbers
    Added stat checks to several tests

src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
    Added checks of assigned stats

src/lib/dhcpsrv/tests/alloc_engine_utils.cc
    AllocEngine6Test::initSubnet() - removed artificial stat values

    NakedAllocEngine::addHost() - new method to add a host to the
    current configuration, rather than use staging/commit as the latter

diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml
index 9807138b1ce13764659e682664420ef9c4865b71..c7b2e191951f38957cbf78d1a2988e05c0a034a7 100644 (file)
--- a/doc/guide/dhcp4-srv.xml
+++ b/doc/guide/dhcp4-srv.xml
@@ -3614,7 +3614,25 @@ src/lib/dhcpsrv/cfg_host_operations.cc -->
               separately. This statistic is reset during reconfiguration event.
               </entry>
             </row>
-
+            <row>
+              <entry>reclaimed-leases</entry>
+              <entry>integer</entry>
+              <entry>This statistic shows the number of expired leases that have
+              have been reclaimed since server startup. It increases every time an
+              expired lease undergoes expiration reclamation. This statistic is reset 
+              during reconfiguration event.
+              </entry>
+            </row>
+            <row>
+              <entry>subnet[id].reclaimed-leases</entry>
+              <entry>integer</entry>
+              <entry>This statistic shows the number of expired leases that have
+              and been reclaimed since server startup. It increases every time an expired
+              lease undergoes expiration reclamation. The <emphasis>id</emphasis> is
+              the subnet-id of the subnet. This statistic is exposed for each subnet
+              separately. This statistic is reset during reconfiguration event.
+              </entry>
+            </row>
             <row>
             <entry>declined-addresses</entry>
             <entry>integer</entry>

diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml
index 5fdde731bb986ca909c71e067ef275d1029d9a9c..a61ba959b29aff9f36fbb6bfe608bd1c77b9665e 100644 (file)
--- a/doc/guide/dhcp6-srv.xml
+++ b/doc/guide/dhcp6-srv.xml
@@ -3856,6 +3856,29 @@ If not specified, the default value is:
             </entry>
             </row>
 
+            <row>
+              <entry>reclaimed-leases</entry>
+              <entry>integer</entry>
+              <entry>This statistic shows the number of expired leases that have
+              and been reclaimed since server startup. It increases every time an
+              expired lease undergoes expiration reclamation. This statistic is reset
+              during reconfiguration event. Note it counts both NA and PD reclamations.
+              This statistic is reset during reconfiguration event.
+              </entry>
+            </row>
+
+            <row>
+              <entry>subnet[id].reclaimed-leases</entry>
+              <entry>integer</entry>
+              <entry>This statistic shows the number of expired leases that have
+              and been reclaimed since server startup. It increases every time an expired
+              lease undergoes expiration reclamation. The <emphasis>id</emphasis> is
+              the subnet-id of the subnet. This statistic is exposed for each subnet
+              separately. Note it counts both NA and PD reclamations.
+              This statistic is reset during reconfiguration event.
+              </entry>
+            </row>
+
             <row>
             <entry>declined-addresses</entry>
             <entry>integer</entry>

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 13eb1cc875975f51f062e42dbf52ec7c83f0ac63..95e75012f77e4016a7df0aefdfc3f65ad2162113 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -1093,6 +1093,16 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
     if (!ctx.fake_allocation_) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease6(expired);
+
+        // If the lease is in the current subnet we need to account
+        // for the re-assignment of The lease.
+        if (ctx.subnet_->inPool(ctx.currentIA().type_, expired->addr_)) {
+            StatsMgr::instance().addValue(
+                StatsMgr::generateName("subnet", ctx.subnet_->getID(),
+                                       ctx.currentIA().type_ == Lease::TYPE_NA ?
+                                       "assigned-nas" : "assigned-pds"),
+                static_cast<int64_t>(1));
+        }
     }
 
     // We do nothing for SOLICIT. We'll just update database when
@@ -1387,6 +1397,15 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         if (old_data->expired()) {
             reclaimExpiredLease(old_data, ctx.callout_handle_);
 
+            // If the lease is in the current subnet we need to account
+            // for the re-assignment of The lease.
+            if (ctx.subnet_->inPool(ctx.currentIA().type_, old_data->addr_)) {
+                StatsMgr::instance().addValue(
+                    StatsMgr::generateName("subnet", ctx.subnet_->getID(),
+                                       ctx.currentIA().type_ == Lease::TYPE_NA ?
+                                       "assigned-nas" : "assigned-pds"),
+                    static_cast<int64_t>(1));
+            }
         } else {
             if (!lease->hasIdenticalFqdn(*old_data)) {
                 // We're not reclaiming the lease but since the FQDN has changed
@@ -2656,13 +2675,14 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
         // involves execution of hooks and DNS update.
         if (ctx.old_lease_->expired()) {
             reclaimExpiredLease(ctx.old_lease_, ctx.callout_handle_);
-            lease->state_ = Lease::STATE_DEFAULT;
 
         } else if (!lease->hasIdenticalFqdn(*ctx.old_lease_)) {
             // The lease is not expired but the FQDN information has
             // changed. So, we have to remove the previous DNS entry.
             queueNCR(CHG_REMOVE, ctx.old_lease_);
         }
+
+        lease->state_ = Lease::STATE_DEFAULT;
     }
 
     bool skip = false;
@@ -2710,6 +2730,13 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
     if (!ctx.fake_allocation_ && !skip) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease4(lease);
+
+        // We need to account for the re-assignment of The lease.
+        if (ctx.old_lease_->expired() || ctx.old_lease_->state_ == Lease::STATE_EXPIRED_RECLAIMED) {
+            StatsMgr::instance().addValue(
+                StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-addresses"),
+                static_cast<int64_t>(1));
+        }
     }
     if (skip) {
         // Rollback changes (really useful only for memfile)
@@ -2796,6 +2823,11 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired,
     if (!ctx.fake_allocation_) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease4(expired);
+
+        // We need to account for the re-assignment of The lease.
+        StatsMgr::instance().addValue(
+                StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-addresses"),
+                static_cast<int64_t>(1));
     }
 
     // We do nothing for SOLICIT. We'll just update database when

diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc
index 14f1c15be674cedb13bfc09c9d31ae158318c78f..df70d79517dc5af74608de0f32997d080ae92d49 100644 (file)
--- a/src/lib/dhcpsrv/cfg_subnets4.cc
+++ b/src/lib/dhcpsrv/cfg_subnets4.cc
@@ -250,6 +250,9 @@ CfgSubnets4::removeStatistics() {
 
         stats_mgr.del(StatsMgr::generateName("subnet", subnet_id,
                                              "declined-reclaimed-addresses"));
+
+        stats_mgr.del(StatsMgr::generateName("subnet", subnet_id,
+                                             "reclaimed-leases"));
     }
 }
 

diff --git a/src/lib/dhcpsrv/cfg_subnets6.cc b/src/lib/dhcpsrv/cfg_subnets6.cc
index c2c8c5f16546a1f6375328018209fd36c356ea4a..a795fe4a609d1c334ce5db36e0ddf6d9eb32665d 100644 (file)
--- a/src/lib/dhcpsrv/cfg_subnets6.cc
+++ b/src/lib/dhcpsrv/cfg_subnets6.cc
@@ -201,6 +201,9 @@ CfgSubnets6::removeStatistics() {
 
         stats_mgr.del(StatsMgr::generateName("subnet", subnet_id,
                                              "declined-reclaimed-addresses"));
+
+        stats_mgr.del(StatsMgr::generateName("subnet", subnet_id,
+                                             "reclaimed-leases"));
     }
 }
 

diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc
index e57348655a13fa7263b703f9432a6fe0bf1c72ad..f34f92713d65d2d254dc66dd9f2b25db22c88371 100644 (file)
--- a/src/lib/dhcpsrv/lease_mgr.cc
+++ b/src/lib/dhcpsrv/lease_mgr.cc
@@ -62,7 +62,8 @@ LeaseMgr::recountLeaseStats4() {
     // Zero out the global stats.
     int64_t zero = 0;
     stats_mgr.setValue("declined-addresses", zero);
-    stats_mgr.setValue("declined-reclaimed-addresses", zero);
+    stats_mgr.setValue("reclaimed-declined-addresses", zero);
+    stats_mgr.setValue("reclaimed-leases", zero);
 
     // Clear subnet level stats.  This ensures we don't end up with corner
     // cases that leave stale values in place.
@@ -79,8 +80,13 @@ LeaseMgr::recountLeaseStats4() {
         stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id,
                                                   "declined-addresses"),
                            zero);
+
+        stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id,
+                                                  "reclaimed-declined-addresses"),
+                           zero);
+
         stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id,
-                                                  "declined-reclaimed-addresses"),
+                                                  "reclaimed-leases"),
                            zero);
     }
 
@@ -133,7 +139,8 @@ LeaseMgr::recountLeaseStats6() {
     // clearing it when we clear the rest.
     int64_t zero = 0;
     stats_mgr.setValue("declined-addresses", zero);
-    stats_mgr.setValue("declined-reclaimed-addresses", zero);
+    stats_mgr.setValue("reclaimed-declined-addresses", zero);
+    stats_mgr.setValue("reclaimed-leases", zero);
 
     // Clear subnet level stats.  This ensures we don't end up with corner
     // cases that leave stale values in place.
@@ -153,12 +160,16 @@ LeaseMgr::recountLeaseStats6() {
 
         stats_mgr.setValue(StatsMgr::
                            generateName("subnet", subnet_id,
-                                        "declined-reclaimed-addresses"),
+                                        "reclaimed-declined-addresses"),
                            zero);
 
         stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id,
                                                   "assigned-pds"),
                            zero);
+
+        stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id,
+                                                  "reclaimed-leases"),
+                           zero);
     }
 
     // Get counts per state per subnet. Iterate over the result set

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index 0cf7b896480b2d05654b67a767a8c77dca0830a0..00ae10ea951400e48aa1a988ee512db9a06060a0 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -44,7 +44,6 @@ TEST_F(AllocEngine4Test, constructor) {
     EXPECT_THROW(x->getAllocator(Lease::TYPE_PD), BadValue);
 }
 
-
 // This test checks if the simple IPv4 allocation can succeed
 TEST_F(AllocEngine4Test, simpleAlloc4) {
     boost::scoped_ptr<AllocEngine> engine;
@@ -52,6 +51,9 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
                                                  100, false)));
     ASSERT_TRUE(engine);
 
+    // Assigned addresses should be zero.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
                                     false, true, "somehost.example.com.", false);
     ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234));
@@ -72,6 +74,9 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
 
     // Now check that the lease in LeaseMgr has the same parameters
     detailCompareLease(lease, from_mgr);
+
+    // Assigned addresses should have incremented.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 1, subnet_->getID()));
 }
 
 // This test checks if the fake allocation (for DHCPDISCOVER) can succeed
@@ -81,6 +86,9 @@ TEST_F(AllocEngine4Test, fakeAlloc4) {
                                                  100, false)));
     ASSERT_TRUE(engine);
 
+    // Assigned addresses should be zero.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
                                     IOAddress("0.0.0.0"), false, true,
                                     "host.example.com.", true);
@@ -100,6 +108,9 @@ TEST_F(AllocEngine4Test, fakeAlloc4) {
     // Check that the lease is NOT in LeaseMgr
     Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
     ASSERT_FALSE(from_mgr);
+
+    // Assigned addresses should still be zero.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
 }
 
 
@@ -267,7 +278,45 @@ TEST_F(AllocEngine4Test, allocateLease4Nulls) {
     detailCompareLease(lease, from_mgr);
 }
 
+// This test checks if a returning client can renew an
+// an existing lease and assigned-leases increments accordingly
+TEST_F(AllocEngine4Test, simpleRenew4) {
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
+                                                 100, false)));
+    ASSERT_TRUE(engine);
+
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
 
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
+                                    false, true, "somehost.example.com.", false);
+    ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234));
+
+    Lease4Ptr lease = engine->allocateLease4(ctx);
+
+    // Check that we got a lease and it's sane
+    ASSERT_TRUE(lease);
+    checkLease4(lease);
+
+    // The new lease has been allocated, so the old lease should not exist.
+    EXPECT_FALSE(ctx.old_lease_);
+
+    // We should have incremented assigned-addresses
+    EXPECT_TRUE(testStatistics("assigned-addresses", 1, subnet_->getID()));
+
+    // Do it again, this should amount to the renew of an existing lease
+    Lease4Ptr lease2 = engine->allocateLease4(ctx);
+
+    // Check that we got a lease and it's sane
+    ASSERT_TRUE(lease2);
+    checkLease4(lease2);
+
+    // Lease already existsed, so old_lease should be set.
+    EXPECT_TRUE(ctx.old_lease_);
+
+    // Should NOT have bumped assigned-addresses
+    EXPECT_TRUE(testStatistics("assigned-addresses", 1, subnet_->getID()));
+}
 
 // This test verifies that the allocator picks addresses that belong to the
 // pool
@@ -504,14 +553,18 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
 
     IOAddress addr("192.0.2.105");
 
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
+
     // Just a different hw/client-id for the second client
     uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
     HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
     time_t now = time(NULL) - 500; // Allocated 500 seconds ago
+
     Lease4Ptr lease(new Lease4(addr, hwaddr2, clientid2, sizeof(clientid2),
-                               sizeof(hwaddr2), 495, 100, 200, now,
-                               subnet_->getID()));
+                               495, 100, 200, now, subnet_->getID()));
     // Make a copy of the lease, so as we can compare that with the old lease
     // instance returned by the allocation engine.
     Lease4 original_lease(*lease);
@@ -543,6 +596,13 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     // lease should be equal to the original lease.
     ASSERT_TRUE(ctx.old_lease_);
     EXPECT_TRUE(*ctx.old_lease_ == original_lease);
+
+    // Check that the stats declined stats were modified correctly.  Note, because
+    // added the lease directly, assigned-leases never bumped to one, so when we
+    // reclaime it gets decremented to -1, then on assignment back to 0.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1, subnet_->getID()));
 }
 
 // This test checks if an expired declined lease can be reused when responding
@@ -599,35 +659,23 @@ TEST_F(AllocEngine4Test, discoverReuseDeclinedLease4Stats) {
     pool_ = Pool4Ptr(new Pool4(addr, addr)); // just a single address
     subnet_->addPool(pool_);
     cfg_mgr.getStagingCfg()->getCfgSubnets4()->add(subnet_);
+    cfg_mgr.commit(); // so we will recalc stats
 
     // Now create a declined lease, decline it and rewind its cltt, so it
     // is expired.
     Lease4Ptr declined = generateDeclinedLease("192.0.2.15", 100, -10);
 
-    // Let's fix some global stats...
-    StatsMgr& stats_mgr = StatsMgr::instance();
-    stats_mgr.setValue("declined-addresses", static_cast<int64_t>(1000));
-    stats_mgr.setValue("reclaimed-declined-addresses", static_cast<int64_t>(1000));
-
-    // ...and subnet specific stats as well.
-    string stat1 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "declined-addresses");
-    string stat2 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "reclaimed-declined-addresses");
-    stats_mgr.setValue(stat1, static_cast<int64_t>(1000));
-    stats_mgr.setValue(stat2, static_cast<int64_t>(1000));
-
     // Ask for any address. There's only one address in the pool, so it doesn't
     // matter much.
     Lease4Ptr assigned;
     testReuseLease4(engine, declined, "0.0.0.0", true, SHOULD_PASS, assigned);
 
-    // Check that the stats were not modified
-    testStatistics("declined-addresses", 1000);
-    testStatistics("reclaimed-declined-addresses", 1000);
-
-    testStatistics(stat1, 1000);
-    testStatistics(stat2, 1000);
+    // Check that the stats declined stats were not modified
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("declined-addresses", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 0));
+    EXPECT_TRUE(testStatistics("declined-addresses", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 0, subnet_->getID()));
 }
 
 // This test checks if an expired declined lease can be reused when responding
@@ -682,36 +730,26 @@ TEST_F(AllocEngine4Test, requestReuseDeclinedLease4Stats) {
     pool_ = Pool4Ptr(new Pool4(addr, addr)); // just a single address
     subnet_->addPool(pool_);
     cfg_mgr.getStagingCfg()->getCfgSubnets4()->add(subnet_);
+    cfg_mgr.commit();
 
     // Now create a declined lease, decline it and rewind its cltt, so it
     // is expired.
     Lease4Ptr declined = generateDeclinedLease("192.0.2.15", 100, -10);
 
-    // Let's fix some global stats...
-    StatsMgr& stats_mgr = StatsMgr::instance();
-    stats_mgr.setValue("declined-addresses", static_cast<int64_t>(1000));
-    stats_mgr.setValue("reclaimed-declined-addresses", static_cast<int64_t>(1000));
-
-    // ...and subnet specific stats as well.
-    string stat1 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "declined-addresses");
-    string stat2 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "reclaimed-declined-addresses");
-    stats_mgr.setValue(stat1, static_cast<int64_t>(1000));
-    stats_mgr.setValue(stat2, static_cast<int64_t>(1000));
-
     // Asking specifically for this address
     Lease4Ptr assigned;
     testReuseLease4(engine, declined, "192.0.2.15", false, SHOULD_PASS, assigned);
     // Check that we got it.
     ASSERT_TRUE(assigned);
 
-    // Check that the stats were modified
-    testStatistics("declined-addresses", 999);
-    testStatistics("reclaimed-declined-addresses", 1001);
-
-    testStatistics(stat1, 999);
-    testStatistics(stat2, 1001);
+    // Check that the stats are correct.  Note that assigned-addresses does
+    // not get decremented when a lease is declined, ergo not incremented
+    // when it is reused.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("declined-addresses", -1));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 1));
+    EXPECT_TRUE(testStatistics("declined-addresses", -1, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 1, subnet_->getID()));
 }
 
 // This test checks that the Allocation Engine correctly identifies the

diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index a5d011e63ddd803e677ca3b8b49061a1e1b7704e..47b268db51dea2e780117a3d00c057c9e9c03d12 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -77,7 +77,7 @@ TEST_F(AllocEngine6Test, simpleAlloc6) {
     string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_EQ(1, stat->getInteger().first);
 }
 
 // This test checks if the simple PD allocation (REQUEST) can succeed
@@ -90,7 +90,7 @@ TEST_F(AllocEngine6Test, pdSimpleAlloc6) {
     string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_EQ(1, stat->getInteger().first);
 }
 
 // This test checks if the fake allocation (for SOLICIT) can succeed
@@ -103,7 +103,7 @@ TEST_F(AllocEngine6Test, fakeAlloc6) {
     string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
-    EXPECT_EQ(100, stat->getInteger().first);
+    EXPECT_EQ(0, stat->getInteger().first);
 }
 
 // This test checks if the fake PD allocation (for SOLICIT) can succeed
@@ -115,7 +115,7 @@ TEST_F(AllocEngine6Test, pdFakeAlloc6) {
     string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds");
     ObservationPtr stat = StatsMgr::instance().getObservation(name);
     ASSERT_TRUE(stat);
-    EXPECT_EQ(100, stat->getInteger().first);
+    EXPECT_EQ(0, stat->getInteger().first);
 };
 
 // This test checks if the allocation with a hint that is valid (in range,
@@ -545,6 +545,11 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) {
     // Check that we got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr, lease->addr_);
+
+    // Verify the none of relelvant stats were altered.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
 }
 
 // This test checks if an expired lease can be reused in REQUEST (actual allocation)
@@ -567,10 +572,12 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     // Let's create an expired lease
     DuidPtr other_duid = DuidPtr(new DUID(vector<uint8_t>(12, 0xff)));
     const uint32_t other_iaid = 3568;
+
     const SubnetID other_subnetid = 999;
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid,
                                501, 502, 503, 504, other_subnetid, HWAddrPtr(),
                                0));
+
     lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago
     lease->valid_lft_ = 495; // Lease was valid for 495 seconds
     lease->fqdn_fwd_ = true;
@@ -578,10 +585,6 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     lease->hostname_ = "myhost.example.com.";
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
-    // By default we pretend our subnet has 100 addresses
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
-    StatsMgr::instance().setValue(name, static_cast<int64_t>(100));
-
     // A client comes along, asking specifically for this address
     AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", false,
                                     Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
@@ -615,12 +618,12 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     // Now check that the lease in LeaseMgr has the same parameters
     detailCompareLease(lease, from_mgr);
 
-    // We should not have bumped the address counter
-    // NOTE: when we start expiring addresses and removing them from
-    // the stats this will no longer be true.
-    ObservationPtr stat = StatsMgr::instance().getObservation(name);
-    ASSERT_TRUE(stat);
-    EXPECT_EQ(100, stat->getInteger().first);
+    // Verify the stats got adjusted correctly
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("assigned-nas", -1, other_subnetid));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-leases", 1, other_subnetid));
 }
 
 // Checks if the lease lifetime is extended when the client sends the
@@ -776,18 +779,15 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestNoHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
-    // By default we pretend our subnet has 100 addresses
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
-    StatsMgr::instance().setValue(name, static_cast<int64_t>(100));
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), false);
     ASSERT_TRUE(lease);
     EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
 
-    // We should have bumped the address counter
-    ObservationPtr stat = StatsMgr::instance().getObservation(name);
-    ASSERT_TRUE(stat);
-    EXPECT_EQ(101, stat->getInteger().first);
+    // Assigned count should have been incremented by one.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (in-pool case)
@@ -807,12 +807,18 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitValidHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), true);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+
+    // Assigned count should not have been incremented.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (in-pool case)
@@ -832,12 +838,18 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestValidHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), false);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+
+    // Assigned count should have been incremented.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (in-pool case)
@@ -857,12 +869,18 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitMatchingHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), true);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+
+    // Assigned count should not have been incremented.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (in-pool case)
@@ -882,12 +900,18 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestMatchingHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), false);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+
+    // Assigned count should have been incremented.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (out-of-pool case)
@@ -907,10 +931,16 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitNoHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), true, false);
     ASSERT_TRUE(lease);
     EXPECT_EQ("2001:db8::abcd", lease->addr_.toText());
 
+    // Assigned count should not have been incremented.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
 }
 
 // Checks that a client gets the address reserved (out-of-pool case)
@@ -930,18 +960,15 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestNoHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
-    // By default we pretend our subnet has 100 addresses
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
-    StatsMgr::instance().setValue(name, static_cast<int64_t>(100));
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), false, false);
     ASSERT_TRUE(lease);
     EXPECT_EQ("2001:db8::abcd", lease->addr_.toText());
 
     // We should not have bumped the address counter
-    ObservationPtr stat = StatsMgr::instance().getObservation(name);
-    ASSERT_TRUE(stat);
-    EXPECT_EQ(100, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (in-pool case)
@@ -961,12 +988,18 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitValidHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), true, false);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8::abcd", lease->addr_.toText());
+
+    // We should not have bumped the address counter
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (out-of-pool case)
@@ -986,12 +1019,18 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestValidHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), false, false);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8::abcd", lease->addr_.toText());
+
+    // We should not have bumped the address counter
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (out-of-pool case)
@@ -1011,12 +1050,18 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitMatchingHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), true, false);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8::abcd", lease->addr_.toText());
+
+    // We should not have bumped the address counter
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // Checks that a client gets the address reserved (out-of-pool case)
@@ -1036,12 +1081,18 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestMatchingHint) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Let's pretend the client sends hint 2001:db8:1::10.
     Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), false, false);
     ASSERT_TRUE(lease);
 
     // The hint should be ignored and the reserved address should be assigned
     EXPECT_EQ("2001:db8::abcd", lease->addr_.toText());
+
+    // We should not have bumped the address counter
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
 }
 
 // In the following situation:
@@ -1054,15 +1105,15 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestMatchingHint) {
 TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedThis) {
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Client gets an address
     Lease6Ptr lease1 = simpleAlloc6Test(pool_, IOAddress("::"), false);
     ASSERT_TRUE(lease1);
 
     // We should have bumped the address counter
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
-    ObservationPtr stat = StatsMgr::instance().getObservation(name);
-    ASSERT_TRUE(stat);
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 
     // Just check that if the client requests again, it will get the same
     // address.
@@ -1071,7 +1122,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedThis) {
     detailCompareLease(lease1, lease2);
 
     // We should not have bumped the address counter again
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 
     // Now admin creates a reservation for this client. This is in-pool
     // reservation, as the pool is 2001:db8:1::10 - 2001:db8:1::20.
@@ -1101,9 +1152,9 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedThis) {
     // Now check that the lease in LeaseMgr has the same parameters
     detailCompareLease(lease3, from_mgr);
 
-    // Lastly check to see that the address counter is still 101 we should have
+    // Lastly check to see that the address counter is still 1, we should have
     // have decremented it on the implied release and incremented it on the reserved
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 // In the following situation:
 // - client X is assigned an address A
@@ -1114,15 +1165,15 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedThis) {
 TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedOther) {
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
 
+    // Assigned count should be zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     // Client gets an address
     Lease6Ptr lease1 = simpleAlloc6Test(pool_, IOAddress("::"), false);
     ASSERT_TRUE(lease1);
 
     // We should have bumped the address counter
-    string name = StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas");
-    ObservationPtr stat = StatsMgr::instance().getObservation(name);
-    ASSERT_TRUE(stat);
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 
     // Just check that if the client requests again, it will get the same
     // address.
@@ -1131,7 +1182,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedOther) {
     detailCompareLease(lease1, lease2);
 
     // We should not have bumped the address counter again
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 
     // Now admin creates a reservation for this client. Let's use the
     // address client X just received. Let's generate a host, but don't add it
@@ -1143,8 +1194,7 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedOther) {
     host->setIdentifier(&other_duid[0], other_duid.size(), Host::IDENT_DUID);
 
     // Ok, now add it to the HostMgr
-    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
-    CfgMgr::instance().commit();
+    addHost(host);
 
     // Just check that this time the client will get a different lease.
     Lease6Ptr lease3 = simpleAlloc6Test(pool_, lease1->addr_, false);
@@ -1167,9 +1217,9 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolReassignedOther) {
     // Now check that the lease in LeaseMgr has the same parameters
     detailCompareLease(lease3, from_mgr);
 
-    // Lastly check to see that the address counter is still 101 we should have
+    // Lastly check to see that the address counter is still 1 we should have
     // have decremented it on the implied release and incremented it on the reserved
-    EXPECT_EQ(101, stat->getInteger().first);
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 
 // Checks that a reserved address for client A is not assigned when
@@ -1267,11 +1317,17 @@ TEST_F(AllocEngine6Test, allocateLeasesInvalidData) {
 TEST_F(AllocEngine6Test, addressRenewal) {
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100);
 
+    // Assigned count should zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     Lease6Collection leases;
 
     leases = allocateTest(engine, pool_, IOAddress("::"), false, true);
     ASSERT_EQ(1, leases.size());
 
+    // Assigned count should be one. 
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
+
     // This is what the client will send in his renew message.
     AllocEngine::HintContainer hints;
     hints.push_back(make_pair(leases[0]->addr_, 128));
@@ -1287,6 +1343,9 @@ TEST_F(AllocEngine6Test, addressRenewal) {
     EXPECT_EQ(leases[0]->type_, renewed[0]->type_);
     EXPECT_EQ(leases[0]->preferred_lft_, renewed[0]->preferred_lft_);
     EXPECT_EQ(leases[0]->valid_lft_, renewed[0]->valid_lft_);
+
+    // Assigned count should still be one.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 
 // Checks whether an address can be renewed (in-pool reservation)
@@ -1297,12 +1356,18 @@ TEST_F(AllocEngine6Test, reservedAddressRenewal) {
 
     AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100);
 
+    // Assigned count should zero.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+
     Lease6Collection leases;
 
     leases = allocateTest(engine, pool_, IOAddress("::"), false, true);
     ASSERT_EQ(1, leases.size());
     ASSERT_EQ("2001:db8:1::1c", leases[0]->addr_.toText());
 
+    // Assigned count should be one.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
+
     // This is what the client will send in his renew message.
     AllocEngine::HintContainer hints;
     hints.push_back(make_pair(leases[0]->addr_, 128));
@@ -1310,6 +1375,9 @@ TEST_F(AllocEngine6Test, reservedAddressRenewal) {
     Lease6Collection renewed = renewTest(engine, pool_, hints, true);
     ASSERT_EQ(1, renewed.size());
     ASSERT_EQ("2001:db8:1::1c", leases[0]->addr_.toText());
+
+    // Assigned count should still be one.
+    EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID()));
 }
 
 // Checks whether a single host can have more than one reservation.
@@ -1775,30 +1843,17 @@ TEST_F(AllocEngine6Test, solicitReuseDeclinedLease6Stats) {
     // is expired.
     Lease6Ptr declined = generateDeclinedLease(addr_txt, 100, -10);
 
-    // Let's fix some global stats...
-    StatsMgr& stats_mgr = StatsMgr::instance();
-    stats_mgr.setValue("declined-addresses", static_cast<int64_t>(1000));
-    stats_mgr.setValue("reclaimed-declined-addresses", static_cast<int64_t>(1000));
-
-    // ...and subnet specific stats as well.
-    string stat1 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "declined-addresses");
-    string stat2 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "reclaimed-declined-addresses");
-    stats_mgr.setValue(stat1, static_cast<int64_t>(1000));
-    stats_mgr.setValue(stat2, static_cast<int64_t>(1000));
-
     // Ask for any address. There's only one address in the pool, so it doesn't
     // matter much.
     Lease6Ptr assigned;
     testReuseLease6(engine, declined, "::", true, SHOULD_PASS, assigned);
 
     // Check that the stats were not modified
-    testStatistics("declined-addresses", 1000);
-    testStatistics("reclaimed-declined-addresses", 1000);
-
-    testStatistics(stat1, 1000);
-    testStatistics(stat2, 1000);
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("declined-addresses", 0));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 0));
+    EXPECT_TRUE(testStatistics("declined-addresses", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 0, subnet_->getID()));
 }
 
 // This test checks if statistics are not updated when expired declined lease
@@ -1819,30 +1874,18 @@ TEST_F(AllocEngine6Test, requestReuseDeclinedLease6Stats) {
     // is expired.
     Lease6Ptr declined = generateDeclinedLease(addr_txt, 100, -10);
 
-    // Let's fix some global stats...
-    StatsMgr& stats_mgr = StatsMgr::instance();
-    stats_mgr.setValue("declined-addresses", static_cast<int64_t>(1000));
-    stats_mgr.setValue("reclaimed-declined-addresses", static_cast<int64_t>(1000));
-
-    // ...and subnet specific stats as well.
-    string stat1 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "declined-addresses");
-    string stat2 = StatsMgr::generateName("subnet", subnet_->getID(),
-                                          "reclaimed-declined-addresses");
-    stats_mgr.setValue(stat1, static_cast<int64_t>(1000));
-    stats_mgr.setValue(stat2, static_cast<int64_t>(1000));
-
     // Ask for any address. There's only one address in the pool, so it doesn't
     // matter much.
     Lease6Ptr assigned;
     testReuseLease6(engine, declined, "::", false, SHOULD_PASS, assigned);
 
-    // Check that the stats were not modified
-    testStatistics("declined-addresses", 999);
-    testStatistics("reclaimed-declined-addresses", 1001);
+    // Check that the stats were modified as expected.
+    EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("declined-addresses", -1));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 1));
+    EXPECT_TRUE(testStatistics("declined-addresses", -1, subnet_->getID()));
+    EXPECT_TRUE(testStatistics("reclaimed-declined-addresses", 1, subnet_->getID()));
 
-    testStatistics(stat1, 999);
-    testStatistics(stat2, 1001);
 }
 
 }; // namespace test

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
index 94b4c3827c464bb8c83e720a78fa6bc14d0de982..ffb812cb3d881b65cba14bfca1ce5c69b30625ce 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc
@@ -1437,13 +1437,12 @@ ExpirationAllocEngine6Test::testReclaimReusedLeases(const uint16_t msg_type,
     // initially reclaimed.
     if (use_reclaimed || (msg_type == DHCPV6_SOLICIT)) {
         EXPECT_TRUE(testStatistics("reclaimed-leases", 0));
-
     } else {
         EXPECT_TRUE(testStatistics("reclaimed-leases", TEST_LEASES_NUM));
+        EXPECT_TRUE(testStatistics("assigned-nas", TEST_LEASES_NUM, subnet->getID()));
         // Leases should have been updated in the lease database and their
         // state should not be 'expired-reclaimed' anymore.
         EXPECT_TRUE(testLeases(&leaseNotReclaimed, &allLeaseIndexes));
-
     }
 
 }
@@ -2099,6 +2098,7 @@ ExpirationAllocEngine4Test::testReclaimReusedLeases(const uint8_t msg_type,
     } else if (msg_type == DHCPREQUEST) {
         // Re-allocation of expired leases should result in reclamations.
         EXPECT_TRUE(testStatistics("reclaimed-leases", TEST_LEASES_NUM));
+        EXPECT_TRUE(testStatistics("assigned-addresses", TEST_LEASES_NUM, subnet->getID()));
         // Leases should have been updated in the lease database and their
         // state should not be 'expired-reclaimed' anymore.
         EXPECT_TRUE(testLeases(&leaseNotReclaimed, &allLeaseIndexes));

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc
index a38fb8970b7a161d04082c0de47f16c5415866ed..ee380ad9dcdb6588efde7101ad0cc2373e4b8a73 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc
@@ -39,20 +39,23 @@ namespace isc {
 namespace dhcp {
 namespace test {
 
-bool testStatistics(const std::string& stat_name, const int64_t exp_value) {
+bool testStatistics(const std::string& stat_name, const int64_t exp_value,
+                    const SubnetID subnet_id) {
     try {
-        ObservationPtr observation = StatsMgr::instance().getObservation(stat_name);
+        std::string name = (!subnet_id ? stat_name : 
+                            StatsMgr::generateName("subnet", subnet_id, stat_name));
+        ObservationPtr observation = StatsMgr::instance().getObservation(name);
         if (observation) {
             if (observation->getInteger().first != exp_value) {
                 ADD_FAILURE()
                     << "value of the observed statistics '"
-                    << stat_name << "' " << "("
+                    << name << "' " << "("
                     << observation->getInteger().first << ") "
                     <<  "doesn't match expected value (" << exp_value << ")";
             }
             return (observation->getInteger().first == exp_value);
         } else {
-            ADD_FAILURE() << "Expected statistic " << stat_name
+            ADD_FAILURE() << "Expected statistic " << name
                           << " not found.";
         }
 
@@ -136,7 +139,6 @@ AllocEngine6Test::AllocEngine6Test() {
     // hardcoded anywhere.
     const uint8_t mac[] = { 0, 1, 22, 33, 44, 55};
     hwaddr_ = HWAddrPtr(new HWAddr(mac, sizeof(mac), HTYPE_FDDI));
-
     // Initialize a subnet and short address pool.
     initSubnet(IOAddress("2001:db8:1::"),
                IOAddress("2001:db8:1::10"),
@@ -145,7 +147,6 @@ AllocEngine6Test::AllocEngine6Test() {
                64, 80);
 
     initFqdn("", false, false);
-
 }
 
 void
@@ -170,14 +171,6 @@ AllocEngine6Test::initSubnet(const asiolink::IOAddress& subnet,
 
     cfg_mgr.getStagingCfg()->getCfgSubnets6()->add(subnet_);
     cfg_mgr.commit();
-
-    // By default we pretend our subnet has 100 addresses and prefixes allocated.
-    StatsMgr::instance().setValue(
-        StatsMgr::generateName("subnet", subnet_->getID(), "assigned-nas"),
-        static_cast<int64_t>(100));
-    StatsMgr::instance().setValue(
-        StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds"),
-        static_cast<int64_t>(100));
 }
 
 void

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
index 73631cd4f20758c94fb8d42fa2486a870d2b3d9e..63d3a63ccd925f2b39ff02b5dbcf9821f2291e1f 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h
+++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h
@@ -1,7 +1,6 @@
 // Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this
+// This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 #ifndef LIBDHCPSRV_ALLOC_ENGINE_UTILS_H
@@ -39,10 +38,12 @@ namespace test {
 ///
 /// @param stat_name Statistic name.
 /// @param exp_value Expected value.
+/// @param subnet_id subnet_id of the desired subnet, if not zero
 ///
 /// @return true if the statistic manager holds a particular value,
 /// false otherwise.
-bool testStatistics(const std::string& stat_name, const int64_t exp_value);
+bool testStatistics(const std::string& stat_name, const int64_t exp_value,
+                    const SubnetID subnet_id = 0);
 
 /// @brief Allocation engine with some internal methods exposed
 class NakedAllocEngine : public AllocEngine {
@@ -360,12 +361,26 @@ public:
         host->addReservation(resv);
 
         if (add_to_host_mgr) {
-            CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
-            CfgMgr::instance().commit();
+            addHost(host);
         }
+
         return (host);
     }
 
+    /// @brief Add a host reservation to the curent configuration
+    ///
+    /// Adds the given host reservation to the current configuration by
+    /// casting it to non-const.  We do it this way rather than adding it to
+    /// staging and then committing as that wipes out the current configuration
+    /// such as subnets.
+    ///
+    /// @param host host reservation to add
+    void
+    addHost(HostPtr& host) {
+        SrvConfigPtr cfg = boost::const_pointer_cast<SrvConfig>(CfgMgr::instance().getCurrentCfg());
+        cfg->getCfgHosts()->add(host);
+    }
+
     /// @brief Utility function that creates a host reservation (hwaddr)
     ///
     /// @param add_to_host_mgr true if the reservation should be added

diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
index 6f255caf78f55f60ee3f5b5793d394649d8a0e17..b3d46c69e0a62d103c5ad2137a2205f6086e00d3 100644 (file)
--- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
@@ -2404,7 +2404,7 @@ void
 GenericLeaseMgrTest::checkLeaseStats(const StatValMapList& expectedStats) {
     // Global accumulators
     int64_t declined_addresses = 0;
-    int64_t declined_reclaimed_addresses = 0;
+    int64_t reclaimed_declined_addresses = 0;
 
     // Iterate over all stats for each subnet
     for (int subnet_idx = 0; subnet_idx < expectedStats.size(); ++subnet_idx) {
@@ -2417,15 +2417,15 @@ GenericLeaseMgrTest::checkLeaseStats(const StatValMapList& expectedStats) {
             // Add the value to globals as needed.
             if (expectedStat.first == "declined-addresses") {
                 declined_addresses += expectedStat.second;
-            } else if (expectedStat.first == "declined-reclaimed-addresses") {
-                declined_reclaimed_addresses += expectedStat.second;
+            } else if (expectedStat.first == "reclaimed-declined-addresses") {
+                reclaimed_declined_addresses += expectedStat.second;
             }
         }
     }
 
     // Verify the globals.
     checkStat("declined-addresses", declined_addresses);
-    checkStat("declined-reclaimed-addresses", declined_reclaimed_addresses);
+    checkStat("reclaimed-declined-addresses", reclaimed_declined_addresses);
 }
 
 void
@@ -2501,7 +2501,8 @@ GenericLeaseMgrTest::testRecountLeaseStats4() {
         expectedStats[i]["total-addresses"] = 256;
         expectedStats[i]["assigned-addresses"] = 0;
         expectedStats[i]["declined-addresses"] = 0;
-        expectedStats[i]["declined-reclaimed-addresses"] = 0;
+        expectedStats[i]["reclaimed-declined-addresses"] = 0;
+        expectedStats[i]["reclaimed-leases"] = 0;
     }
 
     // Make sure stats are as expected.
@@ -2604,8 +2605,9 @@ GenericLeaseMgrTest::testRecountLeaseStats6() {
     for (int i = 0; i < num_subnets; ++i) {
         expectedStats[i]["assigned-nas"] = 0;
         expectedStats[i]["declined-addresses"] = 0;
-        expectedStats[i]["declined-reclaimed-addresses"] = 0;
+        expectedStats[i]["reclaimed-declined-addresses"] = 0;
         expectedStats[i]["assigned-pds"] = 0;
+        expectedStats[i]["reclaimed-leases"] = 0;
     }
 
     // Make sure stats are as expected.