From e03cf770b34dfaa962cdb888917d7a46b7001b4f Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Fri, 23 Oct 2015 04:23:45 +0200 Subject: [PATCH] [3978] Addressed comments --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 28 ++++- src/bin/dhcp4/ctrl_dhcp4_srv.h | 12 +- .../dhcp4/tests/ctrl_dhcp4_srv_unittest.cc | 98 ++++++++++++++--- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 28 ++++- src/bin/dhcp6/ctrl_dhcp6_srv.h | 12 +- .../dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 104 +++++++++++++++--- 6 files changed, 232 insertions(+), 50 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 1fb22746fd..f024381593 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -72,12 +72,28 @@ ControlledDhcpv4Srv::commandConfigReloadHandler(const string&, } ConstElementPtr -ControlledDhcpv4Srv::commandLeasesReclaimHandler(const string&, ConstElementPtr) { - - - server_->alloc_engine_->reclaimExpiredLeases4(0, 0, true); - ConstElementPtr answer = isc::config::createAnswer(0, - "Leases successfully reclaimed."); +ControlledDhcpv4Srv::commandLeasesReclaimHandler(const string& command, + ConstElementPtr args) { + int status_code = 1; + string message; + + // args must be { "remove": } + if (!args) { + message = "Missing mandatory 'remove' parameter."; + } else { + ConstElementPtr remove_name = args->get("remove"); + if (!remove_name) { + message = "Missing mandatory 'remove' parameter."; + } else if (remove_name->getType() != Element::boolean) { + message = "'remove' parameter expected to be a boolean."; + } else { + bool remove_lease = remove_name->boolValue(); + server_->alloc_engine_->reclaimExpiredLeases4(0, 0, remove_lease); + status_code = 0; + message = "Reclamation of expired leases is complete."; + } + } + ConstElementPtr answer = isc::config::createAnswer(status_code, message); return (answer); } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 6bfcdc1da2..d1abf7e450 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -152,15 +152,19 @@ private: isc::data::ConstElementPtr args); - /// @brief handler for processing 'leases-reclaim' command + /// @brief Handler for processing 'leases-reclaim' command /// /// This handler processes leases-reclaim command, which triggers - /// the leases reclamation immediately + /// the leases reclamation immediately. + /// No limit for processing time or number of processed leases applies. /// /// @param command (parameter ignored) - /// @param args (parameter ignored) + /// @param args arguments map { "remove": } + /// if true a lease is removed when it is reclaimed, + /// if false its state is changed to "expired-reclaimed". /// - /// @return status of the command + /// @return status of the command (should be success unless args + /// was not a Bool Element). isc::data::ConstElementPtr commandLeasesReclaimHandler(const std::string& command, isc::data::ConstElementPtr args); diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 961d399ad3..9cf01e236e 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -344,33 +344,101 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlChannelShutdown) { EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutting down.\" }",response); } -// Thist test verifies that the DHCP server immediately reclaims expired +// This test verifies that the DHCP server immediately reclaims expired // leases on leases-reclaim command TEST_F(CtrlChannelDhcpv4SrvTest, controlLeasesReclaim) { createUnixChannelServer(); - // Create an expired lease. The lease is expired by 40 seconds ago + // Create expired leases. Leases are expired by 40 seconds ago // (valid lifetime = 60, cltt = now - 100). - HWAddrPtr hwaddr_expired(new HWAddr(HWAddr::fromText("00:01:02:03:04:05"))); - Lease4Ptr lease_expired(new Lease4(IOAddress("10.0.0.1"), hwaddr_expired, - ClientIdPtr(), 60, 10, 20, - time(NULL) - 100, SubnetID(1))); - - // Add lease to the database. + HWAddrPtr hwaddr0(new HWAddr(HWAddr::fromText("00:01:02:03:04:05"))); + Lease4Ptr lease0(new Lease4(IOAddress("10.0.0.1"), hwaddr0, + ClientIdPtr(), 60, 10, 20, + time(NULL) - 100, SubnetID(1))); + HWAddrPtr hwaddr1(new HWAddr(HWAddr::fromText("01:02:03:04:05:06"))); + Lease4Ptr lease1(new Lease4(IOAddress("10.0.0.2"), hwaddr1, + ClientIdPtr(), 60, 10, 20, + time(NULL) - 100, SubnetID(1))); + + // Add leases to the database. LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); - ASSERT_NO_THROW(lease_mgr.addLease(lease_expired)); + ASSERT_NO_THROW(lease_mgr.addLease(lease0)); + ASSERT_NO_THROW(lease_mgr.addLease(lease1)); - // Make sure it has been added. + // Make sure they have been added. ASSERT_TRUE(lease_mgr.getLease4(IOAddress("10.0.0.1"))); + ASSERT_TRUE(lease_mgr.getLease4(IOAddress("10.0.0.2"))); - // Send the command. + // No arguments std::string response; sendUnixCommand("{ \"command\": \"leases-reclaim\" }", response); - EXPECT_EQ("{ \"result\": 0, \"text\": \"Leases successfully reclaimed.\" }", response); + EXPECT_EQ("{ \"result\": 1, \"text\": " + "\"Missing mandatory 'remove' parameter.\" }", response); + + // Bad argument name + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"reclaim\": true } }", response); + EXPECT_EQ("{ \"result\": 1, \"text\": " + "\"Missing mandatory 'remove' parameter.\" }", response); + + // Bad remove argument type + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"remove\": \"bogus\" } }", response); + EXPECT_EQ("{ \"result\": 1, \"text\": " + "\"'remove' parameter expected to be a boolean.\" }", response); + + // Send the command + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"remove\": false } }", response); + EXPECT_EQ("{ \"result\": 0, \"text\": " + "\"Reclamation of expired leases is complete.\" }", response); + + // Leases should be reclaimed, but not removed + ASSERT_NO_THROW(lease0 = lease_mgr.getLease4(IOAddress("10.0.0.1"))); + ASSERT_NO_THROW(lease1 = lease_mgr.getLease4(IOAddress("10.0.0.2"))); + ASSERT_TRUE(lease0); + ASSERT_TRUE(lease1); + EXPECT_TRUE(lease0->stateExpiredReclaimed()); + EXPECT_TRUE(lease1->stateExpiredReclaimed()); +} + +// Thist test verifies that the DHCP server immediately removed expired +// leases on leases-reclaim command with remove = true +TEST_F(CtrlChannelDhcpv4SrvTest, controlLeasesReclaimRemove) { + createUnixChannelServer(); + + // Create expired leases. Leases are expired by 40 seconds ago + // (valid lifetime = 60, cltt = now - 100). + HWAddrPtr hwaddr0(new HWAddr(HWAddr::fromText("00:01:02:03:04:05"))); + Lease4Ptr lease0(new Lease4(IOAddress("10.0.0.1"), hwaddr0, + ClientIdPtr(), 60, 10, 20, + time(NULL) - 100, SubnetID(1))); + HWAddrPtr hwaddr1(new HWAddr(HWAddr::fromText("01:02:03:04:05:06"))); + Lease4Ptr lease1(new Lease4(IOAddress("10.0.0.2"), hwaddr1, + ClientIdPtr(), 60, 10, 20, + time(NULL) - 100, SubnetID(1))); + + // Add leases to the database. + LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + ASSERT_NO_THROW(lease_mgr.addLease(lease0)); + ASSERT_NO_THROW(lease_mgr.addLease(lease1)); + + // Make sure they have been added. + ASSERT_TRUE(lease_mgr.getLease4(IOAddress("10.0.0.1"))); + ASSERT_TRUE(lease_mgr.getLease4(IOAddress("10.0.0.2"))); + + // Send the command + std::string response; + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"remove\": true } }", response); + EXPECT_EQ("{ \"result\": 0, \"text\": " + "\"Reclamation of expired leases is complete.\" }", response); - // Verify that the lease in the database has been processed as expected. - ASSERT_NO_THROW(lease_expired = lease_mgr.getLease4(IOAddress("10.0.0.1"))); - EXPECT_FALSE(lease_expired); + // Leases should have been removed. + ASSERT_NO_THROW(lease0 = lease_mgr.getLease4(IOAddress("10.0.0.1"))); + ASSERT_NO_THROW(lease1 = lease_mgr.getLease4(IOAddress("10.0.0.2"))); + EXPECT_FALSE(lease0); + EXPECT_FALSE(lease1); } // Tests that the server properly responds to statistics commands. Note this diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 8762809705..b1fa5e4cfc 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -70,12 +70,28 @@ ControlledDhcpv6Srv::commandConfigReloadHandler(const string&, ConstElementPtr a } ConstElementPtr -ControlledDhcpv6Srv::commandLeasesReclaimHandler(const string&, ConstElementPtr) { - - - server_->alloc_engine_->reclaimExpiredLeases6(0, 0, true); - ConstElementPtr answer = isc::config::createAnswer(0, - "Leases successfully reclaimed."); +ControlledDhcpv6Srv::commandLeasesReclaimHandler(const string& command, + ConstElementPtr args) { + int status_code = 1; + string message; + + // args must be { "remove": } + if (!args) { + message = "Missing mandatory 'remove' parameter."; + } else { + ConstElementPtr remove_name = args->get("remove"); + if (!remove_name) { + message = "Missing mandatory 'remove' parameter."; + } else if (remove_name->getType() != Element::boolean) { + message = "'remove' parameter expected to be a boolean."; + } else { + bool remove_lease = remove_name->boolValue(); + server_->alloc_engine_->reclaimExpiredLeases6(0, 0, remove_lease); + status_code = 0; + message = "Reclamation of expired leases is complete."; + } + } + ConstElementPtr answer = isc::config::createAnswer(status_code, message); return (answer); } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 43b42b6459..cc58155192 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -151,15 +151,19 @@ private: commandConfigReloadHandler(const std::string& command, isc::data::ConstElementPtr args); - /// @brief handler for processing 'leases-reclaim' command + /// @brief Handler for processing 'leases-reclaim' command /// /// This handler processes leases-reclaim command, which triggers - /// the leases reclamation immediately + /// the leases reclamation immediately. + /// No limit for processing time or number of processed leases applies. /// /// @param command (parameter ignored) - /// @param args (parameter ignored) + /// @param args arguments map { "remove": } + /// if true a lease is removed when it is reclaimed, + /// if false its state is changed to "expired-reclaimed". /// - /// @return status of the command + /// @return status of the command (should be success unless args + /// was not a Bool Element). isc::data::ConstElementPtr commandLeasesReclaimHandler(const std::string& command, isc::data::ConstElementPtr args); diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 38683a9699..967bdb2067 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -419,35 +419,109 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlChannelShutdown) { EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutting down.\" }",response); } -// Thist test verifies that the DHCP server immediately reclaims expired +// This test verifies that the DHCP server immediately reclaims expired // leases on leases-reclaim command TEST_F(CtrlChannelDhcpv6SrvTest, controlLeasesReclaim) { createUnixChannelServer(); - // Create an expired lease. The lease is expired by 40 seconds ago + // Create expired leases. Leases are expired by 40 seconds ago // (valid lifetime = 60, cltt = now - 100). - DuidPtr duid_expired(new DUID(DUID::fromText("00:01:02:03:04:05:06").getDuid())); - Lease6Ptr lease_expired(new Lease6(Lease::TYPE_NA, IOAddress("3000::1"), - duid_expired, 1, 50, 60, 10, 20, SubnetID(1))); - lease_expired->cltt_ = time(NULL) - 100; - - // Add lease to the database. + DuidPtr duid0(new DUID(DUID::fromText("00:01:02:03:04:05:06").getDuid())); + Lease6Ptr lease0(new Lease6(Lease::TYPE_NA, IOAddress("3000::1"), + duid0, 1, 50, 60, 10, 20, SubnetID(1))); + lease0->cltt_ = time(NULL) - 100; + DuidPtr duid1(new DUID(DUID::fromText("01:02:03:04:05:06:07").getDuid())); + Lease6Ptr lease1(new Lease6(Lease::TYPE_NA, IOAddress("3000::2"), + duid1, 1, 50, 60, 10, 20, SubnetID(1))); + lease1->cltt_ = time(NULL) - 100; + + // Add leases to the database. LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); - ASSERT_NO_THROW(lease_mgr.addLease(lease_expired)); + ASSERT_NO_THROW(lease_mgr.addLease(lease0)); + ASSERT_NO_THROW(lease_mgr.addLease(lease1)); - // Make sure it has been added. + // Make sure they have been added. ASSERT_TRUE(lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1"))); + ASSERT_TRUE(lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::2"))); - // Send the command. + // No arguments std::string response; sendUnixCommand("{ \"command\": \"leases-reclaim\" }", response); - EXPECT_EQ("{ \"result\": 0, \"text\": \"Leases successfully reclaimed.\" }", response); + EXPECT_EQ("{ \"result\": 1, \"text\": " + "\"Missing mandatory 'remove' parameter.\" }", response); + + // Bad argument name + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"reclaim\": true } }", response); + EXPECT_EQ("{ \"result\": 1, \"text\": " + "\"Missing mandatory 'remove' parameter.\" }", response); + + // Bad remove argument type + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"remove\": \"bogus\" } }", response); + EXPECT_EQ("{ \"result\": 1, \"text\": " + "\"'remove' parameter expected to be a boolean.\" }", response); + + // Send the command + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"remove\": false } }", response); + EXPECT_EQ("{ \"result\": 0, \"text\": " + "\"Reclamation of expired leases is complete.\" }", response); + + // Leases should be reclaimed, but not removed + ASSERT_NO_THROW( + lease0 = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1")) + ); + ASSERT_NO_THROW( + lease1 = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::2")) + ); + ASSERT_TRUE(lease0); + ASSERT_TRUE(lease1); + EXPECT_TRUE(lease0->stateExpiredReclaimed()); + EXPECT_TRUE(lease1->stateExpiredReclaimed()); +} + +// This test verifies that the DHCP server immediately reclaims expired +// leases on leases-reclaim command with remove = true +TEST_F(CtrlChannelDhcpv6SrvTest, controlLeasesReclaimRemove) { + createUnixChannelServer(); - // Verify that the lease in the database has been processed as expected. + // Create expired leases. Leases are expired by 40 seconds ago + // (valid lifetime = 60, cltt = now - 100). + DuidPtr duid0(new DUID(DUID::fromText("00:01:02:03:04:05:06").getDuid())); + Lease6Ptr lease0(new Lease6(Lease::TYPE_NA, IOAddress("3000::1"), + duid0, 1, 50, 60, 10, 20, SubnetID(1))); + lease0->cltt_ = time(NULL) - 100; + DuidPtr duid1(new DUID(DUID::fromText("01:02:03:04:05:06:07").getDuid())); + Lease6Ptr lease1(new Lease6(Lease::TYPE_NA, IOAddress("3000::2"), + duid1, 1, 50, 60, 10, 20, SubnetID(1))); + lease1->cltt_ = time(NULL) - 100; + + // Add leases to the database. + LeaseMgr& lease_mgr = LeaseMgrFactory().instance(); + ASSERT_NO_THROW(lease_mgr.addLease(lease0)); + ASSERT_NO_THROW(lease_mgr.addLease(lease1)); + + // Make sure they have been added. + ASSERT_TRUE(lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1"))); + ASSERT_TRUE(lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::2"))); + + // Send the command + std::string response; + sendUnixCommand("{ \"command\": \"leases-reclaim\", " + "\"arguments\": { \"remove\": true } }", response); + EXPECT_EQ("{ \"result\": 0, \"text\": " + "\"Reclamation of expired leases is complete.\" }", response); + + // Leases should have been removed. + ASSERT_NO_THROW( + lease0 = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1")) + ); ASSERT_NO_THROW( - lease_expired = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::1")) + lease1 = lease_mgr.getLease6(Lease::TYPE_NA, IOAddress("3000::2")) ); - EXPECT_FALSE(lease_expired); + ASSERT_FALSE(lease0); + ASSERT_FALSE(lease1); } // Tests that the server properly responds to statistics commands. Note this -- 2.47.2