diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 9807138b1c..c7b2e19195 100644 --- 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. - + + reclaimed-leases + integer + 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. + + + + subnet[id].reclaimed-leases + integer + 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 id is + the subnet-id of the subnet. This statistic is exposed for each subnet + separately. This statistic is reset during reconfiguration event. + + declined-addresses integer diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 5fdde731bb..a61ba959b2 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -3856,6 +3856,29 @@ If not specified, the default value is: + + reclaimed-leases + integer + 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. + + + + + subnet[id].reclaimed-leases + integer + 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 id 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. + + + declined-addresses integer diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 13eb1cc875..95e75012f7 100644 --- 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(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(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(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(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 14f1c15be6..df70d79517 100644 --- 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 c2c8c5f165..a795fe4a60 100644 --- 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 e57348655a..f34f92713d 100644 --- 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, - "declined-reclaimed-addresses"), + "reclaimed-declined-addresses"), + zero); + + stats_mgr.setValue(StatsMgr::generateName("subnet", subnet_id, + "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 0cf7b89648..00ae10ea95 100644 --- 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 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 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(1000)); - stats_mgr.setValue("reclaimed-declined-addresses", static_cast(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(1000)); - stats_mgr.setValue(stat2, static_cast(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(1000)); - stats_mgr.setValue("reclaimed-declined-addresses", static_cast(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(1000)); - stats_mgr.setValue(stat2, static_cast(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 a5d011e63d..47b268db51 100644 --- 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(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(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(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(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(1000)); - stats_mgr.setValue("reclaimed-declined-addresses", static_cast(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(1000)); - stats_mgr.setValue(stat2, static_cast(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(1000)); - stats_mgr.setValue("reclaimed-declined-addresses", static_cast(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(1000)); - stats_mgr.setValue(stat2, static_cast(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 94b4c3827c..ffb812cb3d 100644 --- 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 a38fb8970b..ee380ad9dc 100644 --- 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(100)); - StatsMgr::instance().setValue( - StatsMgr::generateName("subnet", subnet_->getID(), "assigned-pds"), - static_cast(100)); } void diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index 73631cd4f2..63d3a63ccd 100644 --- 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(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 6f255caf78..b3d46c69e0 100644 --- 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.