From: Francis Dupont Date: Thu, 23 Apr 2020 19:42:55 +0000 (+0200) Subject: [#816] Addressed new comments X-Git-Tag: Kea-1.7.7~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1d31239aa985c1401584720fb2771daf07be8317;p=thirdparty%2Fkea.git [#816] Addressed new comments --- diff --git a/doc/sphinx/api/stat-lease4-get.json b/doc/sphinx/api/stat-lease4-get.json index 7ddef1a2b0..871fbeb52c 100644 --- a/doc/sphinx/api/stat-lease4-get.json +++ b/doc/sphinx/api/stat-lease4-get.json @@ -19,11 +19,12 @@ " \"result-set\": {", " \"columns\": [ \"subnet-id\",", " \"total-addresses\",", + " \"cumulative-assigned-addresses\",", " \"assigned-addresses\",", " \"declined-addresses\" ],", " \"rows\": [", - " [ 10, 256, 111, 0 ],", - " [ 20, 4098, 2034, 4 ]", + " [ 10, 256, 200, 111, 0 ],", + " [ 20, 4098, 5000, 2034, 4 ]", " ],", " \"timestamp\": \"2018-05-04 15:03:37.000000\"", " }", diff --git a/doc/sphinx/api/stat-lease6-get.json b/doc/sphinx/api/stat-lease6-get.json index 9c2a430918..4455f0a2f7 100644 --- a/doc/sphinx/api/stat-lease6-get.json +++ b/doc/sphinx/api/stat-lease6-get.json @@ -20,11 +20,11 @@ " \"text\": \"stat-lease6-get: 2 rows found\",", " \"arguments\": {", " \"result-set\": {", - " \"columns\": [ \"subnet-id\", \"total-nas\", \"assigned-nas\", \"declined-nas\", \"total-pds\", \"assigned-pds\" ],", + " \"columns\": [ \"subnet-id\", \"total-nas\", \"cumulative-assigned-nas\", \"assigned-nas\", \"declined-nas\", \"total-pds\", \"cumulative-assigned-pds\", \"assigned-pds\" ],", " \"rows\": [", - " [ 10, 4096, 2400, 3, 0, 0],", - " [ 20, 0, 0, 0, 1048, 233 ],", - " [ 30, 256, 60, 0, 1048, 15 ]", + " [ 10, 4096, 3000, 2400, 3, 0, 0],", + " [ 20, 0, 0, 0, 1048, 500, 233 ],", + " [ 30, 256, 300, 60, 0, 1048, 15, 15 ]", " ],", " \"timestamp\": \"2018-05-04 15:03:37.000000\"", " }", diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 4f27bcecec..5de2b692ac 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -272,7 +272,7 @@ LeaseMgr::recountLeaseStats6() { stats_mgr.setValue(StatsMgr:: generateName("subnet", row.subnet_id_, "assigned-pds"), - row.state_count_); + row.state_count_); } break; diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc index b1a0e0fc9b..34c316a50f 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc @@ -97,6 +97,11 @@ TEST_F(AllocEngine6Test, simpleAlloc6) { // We should have bumped the assigned counter by 1 EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + glbl_cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); // Reset last allocated address to check that the other client will // be refused the already allocated address and will get the one @@ -110,10 +115,10 @@ TEST_F(AllocEngine6Test, simpleAlloc6) { // We should have bumped the assigned counter by 2 EXPECT_TRUE(testStatistics("assigned-nas", 2, subnet_->getID())); - cumulative += 2; + cumulative += 1; EXPECT_TRUE(testStatistics("cumulative-assigned-nas", cumulative, subnet_->getID())); - glbl_cumulative += 2; + glbl_cumulative += 1; EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); } @@ -903,7 +908,7 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) { // Initialize FQDN data for the lease. initFqdn("myhost.example.com", true, true); - // Verify the none of relevant stats are zero. + // Verify the all of relevant stats are zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); EXPECT_TRUE(testStatistics("reclaimed-leases", 0)); EXPECT_TRUE(testStatistics("reclaimed-leases", 0, subnet_->getID())); @@ -1172,7 +1177,8 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) { Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid, 501, 502, other_subnetid, HWAddrPtr(), 0)); - int64_t other_cumulative = getStatistics("cumulative-assigned-nas", other_subnetid); + int64_t other_cumulative = + getStatistics("cumulative-assigned-nas", other_subnetid); lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago lease->valid_lft_ = 495; // Lease was valid for 495 seconds @@ -1525,12 +1531,22 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestNoHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), false); ASSERT_TRUE(lease); EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText()); // Assigned count should have been incremented by one. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + glbl_cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); } // Checks that a client gets the address reserved (in-pool case) @@ -1553,6 +1569,11 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitValidHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // Let's pretend the client sends hint 2001:db8:1::10. Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), true); ASSERT_TRUE(lease); @@ -1562,6 +1583,9 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitValidHint) { // Assigned count should not have been incremented. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (in-pool case) @@ -1584,6 +1608,11 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestValidHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // Let's pretend the client sends hint 2001:db8:1::10. Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), false); ASSERT_TRUE(lease); @@ -1593,6 +1622,11 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestValidHint) { // Assigned count should have been incremented. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + glbl_cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); } // Checks that a client gets the address reserved (in-pool case) @@ -1615,6 +1649,11 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitMatchingHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // Let's pretend the client sends hint 2001:db8:1::10. Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), true); ASSERT_TRUE(lease); @@ -1624,6 +1663,9 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolSolicitMatchingHint) { // Assigned count should not have been incremented. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (in-pool case) @@ -1646,6 +1688,11 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestMatchingHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // Let's pretend the client sends hint 2001:db8:1::10. Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), false); ASSERT_TRUE(lease); @@ -1655,6 +1702,11 @@ TEST_F(AllocEngine6Test, reservedAddressInPoolRequestMatchingHint) { // Assigned count should have been incremented. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + glbl_cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); } // Checks that a client gets the address reserved (out-of-pool case) @@ -1677,13 +1729,20 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitNoHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + 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())); - + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (out-of-pool case) @@ -1706,12 +1765,20 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestNoHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + 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 EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (in-pool case) @@ -1734,6 +1801,11 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitValidHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // 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); @@ -1743,6 +1815,9 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitValidHint) { // We should not have bumped the address counter EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (out-of-pool case) @@ -1765,6 +1840,11 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestValidHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // 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); @@ -1774,6 +1854,9 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestValidHint) { // We should not have bumped the address counter EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (out-of-pool case) @@ -1796,6 +1879,11 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitMatchingHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // 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); @@ -1805,6 +1893,9 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolSolicitMatchingHint) { // We should not have bumped the address counter EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // Checks that a client gets the address reserved (out-of-pool case) @@ -1827,6 +1918,11 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestMatchingHint) { // Assigned count should be zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + // 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); @@ -1836,6 +1932,9 @@ TEST_F(AllocEngine6Test, reservedAddressOutOfPoolRequestMatchingHint) { // We should not have bumped the address counter EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + EXPECT_EQ(cumulative, + getStatistics("cumulative-assigned-nas", subnet_->getID())); + EXPECT_EQ(glbl_cumulative, getStatistics("cumulative-assigned-nas")); } // In the following situation: @@ -2100,6 +2199,11 @@ TEST_F(AllocEngine6Test, addressRenewal) { // Assigned count should zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + Lease6Collection leases; leases = allocateTest(engine, pool_, IOAddress("::"), false, true); @@ -2107,6 +2211,11 @@ TEST_F(AllocEngine6Test, addressRenewal) { // Assigned count should be one. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + glbl_cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); // This is what the client will send in his renew message. AllocEngine::HintContainer hints; @@ -2126,6 +2235,9 @@ TEST_F(AllocEngine6Test, addressRenewal) { // Assigned count should still be one. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); } // Checks whether an address can be renewed (in-pool reservation) @@ -2139,6 +2251,11 @@ TEST_F(AllocEngine6Test, reservedAddressRenewal) { // Assigned count should zero. EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID())); + // Get the cumulative count of assigned addresses. + int64_t cumulative = getStatistics("cumulative-assigned-nas", + subnet_->getID()); + int64_t glbl_cumulative = getStatistics("cumulative-assigned-nas"); + Lease6Collection leases; leases = allocateTest(engine, pool_, IOAddress("::"), false, true); @@ -2147,6 +2264,11 @@ TEST_F(AllocEngine6Test, reservedAddressRenewal) { // Assigned count should be one. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + glbl_cumulative += 1; + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); // This is what the client will send in his renew message. AllocEngine::HintContainer hints; @@ -2158,6 +2280,9 @@ TEST_F(AllocEngine6Test, reservedAddressRenewal) { // Assigned count should still be one. EXPECT_TRUE(testStatistics("assigned-nas", 1, subnet_->getID())); + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", + cumulative, subnet_->getID())); + EXPECT_TRUE(testStatistics("cumulative-assigned-nas", glbl_cumulative)); } // Checks whether a single host can have more than one reservation. diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index e459ad0389..1f17308089 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -53,7 +53,7 @@ bool testStatistics(const std::string& stat_name, const int64_t exp_value, << "value of the observed statistics '" << name << "' " << "(" << observation->getInteger().first << ") " - << "doesn't match expected value (" << exp_value << ")"; + << "doesn't match expected value (" << exp_value << ")"; } return (observation->getInteger().first == exp_value); } else {