Commit 52bd9f4d authored by Marcin Siodelski's avatar Marcin Siodelski

[3671] Addressed review comments.

parent fbce7dd4
......@@ -80,7 +80,6 @@ libkea_dhcpsrv_la_SOURCES += dhcpsrv_log.cc dhcpsrv_log.h
libkea_dhcpsrv_la_SOURCES += host.cc host.h
libkea_dhcpsrv_la_SOURCES += host_container.h
libkea_dhcpsrv_la_SOURCES += host_mgr.cc host_mgr.h
libkea_dhcpsrv_la_SOURCES += inmemory_lease_storage.h
libkea_dhcpsrv_la_SOURCES += key_from_key.h
libkea_dhcpsrv_la_SOURCES += lease.cc lease.h
libkea_dhcpsrv_la_SOURCES += lease_file_loader.h
......@@ -89,6 +88,7 @@ libkea_dhcpsrv_la_SOURCES += lease_mgr_factory.cc lease_mgr_factory.h
libkea_dhcpsrv_la_SOURCES += logging.cc logging.h
libkea_dhcpsrv_la_SOURCES += logging_info.cc logging_info.h
libkea_dhcpsrv_la_SOURCES += memfile_lease_mgr.cc memfile_lease_mgr.h
libkea_dhcpsrv_la_SOURCES += memfile_lease_storage.h
if HAVE_MYSQL
libkea_dhcpsrv_la_SOURCES += mysql_lease_mgr.cc mysql_lease_mgr.h
......
# Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
# Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
#
# Permission to use, copy, modify, and/or distribute this software for any
# purpose with or without fee is hereby granted, provided that the above
......
......@@ -16,7 +16,7 @@
#define LEASE_FILE_LOADER_H
#include <dhcpsrv/dhcpsrv_log.h>
#include <dhcpsrv/inmemory_lease_storage.h>
#include <dhcpsrv/memfile_lease_storage.h>
#include <util/csv_file.h>
#include <boost/shared_ptr.hpp>
......@@ -62,15 +62,19 @@ public:
/// removes an existing lease from the container.
///
/// @param lease_file A reference to the @c CSVLeaseFile4 or
/// @c CSVLeaseFile6 object representing the lease file. The
/// lease file must be opened and the internal file pointer should
/// be set to the beginning of the file.
/// @c CSVLeaseFile6 object representing the lease file. The file
/// doesn't need to be open because the method re-opens the file.
/// @param storage A reference to the container to which leases
/// should be inserted.
/// @param max_errors Maximum number of corrupted leases in the
/// lease file. The method will skip corrupted leases but after
/// exceeding the specified number of errors it will throw an
/// exception.
/// @param close_file_on_exit A boolean flag which indicates if
/// the file should be closed after it has been successfully parsed.
/// One case when the file is not opened is when the server starts
/// up, reads the leases in the file and then leaves the file open
/// for writing future lease updates.
/// @tparam LeaseObjectType A @c Lease4 or @c Lease6.
/// @tparam LeaseFileType A @c CSVLeaseFile4 or @c CSVLeaseFile6.
/// @tparam StorageType A @c Lease4Storage or @c Lease6Storage.
......@@ -80,11 +84,17 @@ public:
template<typename LeaseObjectType, typename LeaseFileType,
typename StorageType>
static void load(LeaseFileType& lease_file, StorageType& storage,
const uint32_t max_errors = 0xFFFFFFFF) {
const uint32_t max_errors = 0xFFFFFFFF,
const bool close_file_on_exit = true) {
LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LEASE_FILE_LOAD)
.arg(lease_file.getFilename());
// Reopen the file, as we don't know whether the file is open
// and we also don't know its current state.
lease_file.close();
lease_file.open();
boost::shared_ptr<LeaseObjectType> lease;
// Track the number of corrupted leases.
uint32_t errcnt = 0;
......@@ -95,7 +105,14 @@ public:
// until the whole file is parsed, even if errors occur.
// Otherwise, check if we have exceeded the maximum number
// of errors and throw an exception if we have.
if ((max_errors < 0xFFFFFFFF) && (++errcnt > max_errors)) {
if (++errcnt > max_errors) {
// If we break parsing the CSV file because of too many
// errors, it doesn't make sense to keep the file open.
// This is because the caller wouldn't know where we
// stopped parsing and where the internal file pointer
// is. So, there are probably no cases when the caller
// would continue to use the open file.
lease_file.close();
isc_throw(util::CSVFileError, "exceeded maximum number of"
" failures " << max_errors << " to read a lease"
" from the lease file "
......@@ -138,6 +155,10 @@ public:
}
}
if (close_file_on_exit) {
lease_file.close();
}
}
};
......
......@@ -482,25 +482,56 @@ loadLeasesFromFiles(const std::string& filename,
storage.clear();
for (int i = 2; i >= 0; --i) {
bool completed_exists = false;
// Initialize the name of the lease file to parse. For the
// first two loops we're going to append .2 and .1 to the
// lease file name.
// lease file name, unless the file with the .completed
// postfix exists.
std::ostringstream s;
s << filename;
if (i > 0) {
s << "." << i;
// If this is the first file to load leases from, we should check
// if the file with the .completed prefix exists. If it exists
// we are going to skip loading leases from the files with
// postfixes .2 and .1.
if (i == 2) {
s << ".completed";
lease_file.reset(new LeaseFileType(s.str()));
// If the .completed file exists, reduce the value of i to
// skip loop for the file with postfix .1. We will only
// load leases from the .completed file and the lease
// file without postfix (for i = 0).
if (lease_file->exists()) {
--i;
completed_exists = true;
}
}
lease_file.reset(new LeaseFileType(s.str()));
// If .completed file doesn't exist, load the files with postfixes
// .1 and .2.
if (!completed_exists) {
std::ostringstream s2;
s2 << filename;
if (i > 0) {
s2 << "." << i;
}
lease_file.reset(new LeaseFileType(s2.str()));
}
// Don't open the file if it doesn't exist and it is not the
// primary lease file - not ending with .1 or .2. Those files
// are optional and we don't want to create them if they don't
// exist.
// primary lease file - not ending with .1 or .2 or .completed.
// Those files are optional and we don't want to create them if
// they don't exist.
if (i == 0 || lease_file->exists()) {
// If the file doesn't exist it will be created as an empty
// file (with no leases).
lease_file->open();
// file (with no leases). The last parameter is set to false
// when the primary lease file is parsed. This is to
// indicate that the lease file should remain open after
// parsing. The backend will use this file to append future
// lease updates.
LeaseFileLoader::load<LeaseObjectType>(*lease_file, storage,
MAX_LEASE_ERRORS);
MAX_LEASE_ERRORS, (i != 0));
}
}
}
......@@ -18,7 +18,7 @@
#include <dhcp/hwaddr.h>
#include <dhcpsrv/csv_lease_file4.h>
#include <dhcpsrv/csv_lease_file6.h>
#include <dhcpsrv/inmemory_lease_storage.h>
#include <dhcpsrv/memfile_lease_storage.h>
#include <dhcpsrv/lease_mgr.h>
#include <boost/shared_ptr.hpp>
......@@ -353,9 +353,13 @@ private:
///
/// This method loads DHCPv4 or DHCPv6 leases from lease files in the
/// following order:
/// - leases from the <filename>.2
/// - leases from the <filename>.1
/// - leases from the <filename>
/// - If the <filename>.completed doesn't exist:
/// - leases from the <filename>.2
/// - leases from the <filename>.1
/// - leases from the <filename>
/// - else
/// - leases from the <filename>.completed
/// - leases from the <filename>
///
/// If any of the files doesn't exist the method proceeds to reading
/// leases from the subsequent file. If the <filename> doesn't exist
......@@ -366,9 +370,9 @@ private:
/// end of file. The server will append lease entries to this file as
/// a result of processing new messages from the clients.
///
/// The <filename>.2 and <filename>.1 are the products of the lease
/// file cleanups (LFC). See: http://kea.isc.org/wiki/LFCDesign for
/// details.
/// The <filename>.2, <filename>.1 and <filename>.completed are the
/// products of the lease file cleanups (LFC).
/// See: http://kea.isc.org/wiki/LFCDesign for details.
///
/// @param filename Name of the lease file.
/// @param lease_file An object representing a lease file to which
......
......@@ -16,7 +16,7 @@
#include <asiolink/io_address.h>
#include <dhcpsrv/csv_lease_file4.h>
#include <dhcpsrv/csv_lease_file6.h>
#include <dhcpsrv/inmemory_lease_storage.h>
#include <dhcpsrv/memfile_lease_storage.h>
#include <dhcpsrv/lease_file_loader.h>
#include <dhcpsrv/tests/lease_file_io.h>
#include <boost/scoped_ptr.hpp>
......@@ -146,7 +146,7 @@ TEST_F(LeaseFileLoaderTest, load4) {
}
// This test verifies that the lease with a valid lifetime of 0
// is removed from the storage. The valid lifetime of 0 set set
// is removed from the storage. The valid lifetime of 0 is set
// for the released leases.
TEST_F(LeaseFileLoaderTest, load4LeaseRemove) {
// Create lease file in which one of the entries for 192.0.2.1
......@@ -264,7 +264,8 @@ TEST_F(LeaseFileLoaderTest, load6LeaseRemove) {
}
// This test verifies that the exception is thrown when the specific
// number of errors occurs during reading of the lease file.
// number of errors in the test data occur during reading of the lease
// file.
TEST_F(LeaseFileLoaderTest, loadMaxErrors) {
// Create a lease file for which there is a number of invalid
// entries.
......@@ -308,6 +309,32 @@ TEST_F(LeaseFileLoaderTest, loadMaxErrors) {
EXPECT_EQ(100, lease->cltt_);
}
// This test verifies that the lease with a valid lifetime set to 0 is
// not loaded if there are no previous entries for this lease in the
// lease file.
TEST_F(LeaseFileLoaderTest, loadLeaseWithZeroLifetime) {
// Create lease file. The second lease has a valid lifetime of 0.
io_.writeFile("address,hwaddr,client_id,valid_lifetime,expire,subnet_id,"
"fqdn_fwd,fqdn_rev,hostname\n"
"192.0.2.1,06:07:08:09:0a:bc,,200,200,8,1,1,,\n"
"192.0.2.3,06:07:08:09:0a:bd,,0,200,8,1,1,,\n");
boost::scoped_ptr<CSVLeaseFile4> lf(new CSVLeaseFile4(filename_));
ASSERT_NO_THROW(lf->open());
// Set the error count to 0 to make sure that lease with a zero
// lifetime doesn't cause an error.
Lease4Storage storage;
ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 0));
// The first lease should be present.
Lease4Ptr lease = getLease<Lease4Ptr>("192.0.2.1", storage);
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
// The lease with a valid lifetime of 0 should not be loaded.
EXPECT_FALSE(getLease<Lease4Ptr>("192.0.2.3", storage));
}
} // end of anonymous namespace
......@@ -538,6 +538,56 @@ TEST_F(MemfileLeaseMgrTest, load4MultipleLeaseFiles) {
EXPECT_EQ(200, lease->cltt_);
}
// This test checks that the lease database backend loads the file with
// the .completed postfix instead of files with postfixes .1 and .2 if
// the file with .completed postfix exists.
TEST_F(MemfileLeaseMgrTest, load4CompletedFile) {
LeaseFileIO io2("leasefile4_0.csv.2");
io2.writeFile("address,hwaddr,client_id,valid_lifetime,expire,subnet_id,"
"fqdn_fwd,fqdn_rev,hostname\n"
"192.0.2.2,02:02:02:02:02:02,,200,200,8,1,1,,\n"
"192.0.2.11,bb:bb:bb:bb:bb:bb,,200,200,8,1,1,,\n");
LeaseFileIO io1("leasefile4_0.csv.1");
io1.writeFile("address,hwaddr,client_id,valid_lifetime,expire,subnet_id,"
"fqdn_fwd,fqdn_rev,hostname\n"
"192.0.2.1,01:01:01:01:01:01,,200,200,8,1,1,,\n"
"192.0.2.11,bb:bb:bb:bb:bb:bb,,200,400,8,1,1,,\n"
"192.0.2.12,cc:cc:cc:cc:cc:cc,,200,200,8,1,1,,\n");
LeaseFileIO io("leasefile4_0.csv");
io.writeFile("address,hwaddr,client_id,valid_lifetime,expire,subnet_id,"
"fqdn_fwd,fqdn_rev,hostname\n"
"192.0.2.10,0a:0a:0a:0a:0a:0a,,200,200,8,1,1,,\n"
"192.0.2.12,cc:cc:cc:cc:cc:cc,,200,400,8,1,1,,\n");
LeaseFileIO ioc("leasefile4_0.csv.completed");
ioc.writeFile("address,hwaddr,client_id,valid_lifetime,expire,subnet_id,"
"fqdn_fwd,fqdn_rev,hostname\n"
"192.0.2.13,ff:ff:ff:ff:ff:ff,,200,200,8,1,1,,\n");
startBackend(V4);
// We expect that this file only holds leases that belong to the
// lease file or to the file with .completed postfix.
Lease4Ptr lease = lmptr_->getLease4(IOAddress("192.0.2.10"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
lease = lmptr_->getLease4(IOAddress("192.0.2.12"));
ASSERT_TRUE(lease);
EXPECT_EQ(200, lease->cltt_);
// This lease is in the .completed file.
lease = lmptr_->getLease4(IOAddress("192.0.2.13"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
// Leases from the .1 and .2 files should not be loaded.
EXPECT_FALSE(lmptr_->getLease4(IOAddress("192.0.2.11")));
EXPECT_FALSE(lmptr_->getLease4(IOAddress("192.0.2.1")));
}
// This test checks that the backend reads DHCPv6 lease data from multiple
// files.
TEST_F(MemfileLeaseMgrTest, load6MultipleLeaseFiles) {
......@@ -600,4 +650,154 @@ TEST_F(MemfileLeaseMgrTest, load6MultipleLeaseFiles) {
EXPECT_EQ(0, lease->cltt_);
}
// This test checks that the backend reads DHCPv6 lease data from the
// leasefile without the postfix and the file with a .1 postfix when
// the file with the .2 postfix is missing.
TEST_F(MemfileLeaseMgrTest, load6MultipleNoSecondFile) {
LeaseFileIO io1("leasefile6_0.csv.1");
io1.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::3,03:03:03:03:03:03:03:03:03:03:03:03:03,"
"200,200,8,100,0,7,0,1,1,,\n"
"2001:db8:1::2,02:02:02:02:02:02:02:02:02:02:02:02:02,"
"300,800,8,100,0,7,0,1,1,,\n"
"2001:db8:1::4,04:04:04:04:04:04:04:04:04:04:04:04:04,"
"200,200,8,100,0,7,0,1,1,,\n");
LeaseFileIO io("leasefile6_0.csv");
io.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::4,04:04:04:04:04:04:04:04:04:04:04:04:04,"
"400,1000,8,100,0,7,0,1,1,,\n"
"2001:db8:1::5,05:05:05:05:05:05:05:05:05:05:05:05:05,"
"200,200,8,100,0,7,0,1,1,,\n");
startBackend(V6);
// Check that leases from the leasefile6_0 and leasefile6_0.1 have
// been loaded.
Lease6Ptr lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::2"));
ASSERT_TRUE(lease);
EXPECT_EQ(500, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::3"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::4"));
ASSERT_TRUE(lease);
EXPECT_EQ(600, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::5"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
// Make sure that a lease which is not in those files is not loaded.
EXPECT_FALSE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1")));
}
// This test checks that the backend reads DHCPv6 lease data from the
// leasefile without the postfix and the file with a .2 postfix when
// the file with the .1 postfix is missing.
TEST_F(MemfileLeaseMgrTest, load6MultipleNoFirstFile) {
LeaseFileIO io2("leasefile6_0.csv.2");
io2.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::1,01:01:01:01:01:01:01:01:01:01:01:01:01,"
"200,200,8,100,0,7,0,1,1,,\n"
"2001:db8:1::2,02:02:02:02:02:02:02:02:02:02:02:02:02,"
"200,200,8,100,0,7,0,1,1,,\n");
LeaseFileIO io("leasefile6_0.csv");
io.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::4,04:04:04:04:04:04:04:04:04:04:04:04:04,"
"400,1000,8,100,0,7,0,1,1,,\n"
"2001:db8:1::5,05:05:05:05:05:05:05:05:05:05:05:05:05,"
"200,200,8,100,0,7,0,1,1,,\n");
startBackend(V6);
// Verify that leases which belong to the leasefile6_0.csv and
// leasefile6_0.2 are loaded.
Lease6Ptr lease = lmptr_->getLease6(Lease::TYPE_NA,
IOAddress("2001:db8:1::1"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::2"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::4"));
ASSERT_TRUE(lease);
EXPECT_EQ(600, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::5"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
// A lease which doesn't belong to these files should not be loaded.
EXPECT_FALSE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::3")));
}
// This test checks that the lease database backend loads the file with
// the .completed postfix instead of files with postfixes .1 and .2 if
// the file with .completed postfix exists.
TEST_F(MemfileLeaseMgrTest, load6CompletedFile) {
LeaseFileIO io2("leasefile6_0.csv.2");
io2.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::1,01:01:01:01:01:01:01:01:01:01:01:01:01,"
"200,200,8,100,0,7,0,1,1,,\n"
"2001:db8:1::2,02:02:02:02:02:02:02:02:02:02:02:02:02,"
"200,200,8,100,0,7,0,1,1,,\n");
LeaseFileIO io1("leasefile6_0.csv.1");
io1.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::3,03:03:03:03:03:03:03:03:03:03:03:03:03,"
"200,200,8,100,0,7,0,1,1,,\n"
"2001:db8:1::2,02:02:02:02:02:02:02:02:02:02:02:02:02,"
"300,800,8,100,0,7,0,1,1,,\n"
"2001:db8:1::4,04:04:04:04:04:04:04:04:04:04:04:04:04,"
"200,200,8,100,0,7,0,1,1,,\n");
LeaseFileIO io("leasefile6_0.csv");
io.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::4,04:04:04:04:04:04:04:04:04:04:04:04:04,"
"400,1000,8,100,0,7,0,1,1,,\n"
"2001:db8:1::5,05:05:05:05:05:05:05:05:05:05:05:05:05,"
"200,200,8,100,0,7,0,1,1,,\n");
LeaseFileIO ioc("leasefile6_0.csv.completed");
ioc.writeFile("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
"lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
"2001:db8:1::125,ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff,"
"400,1000,8,100,0,7,0,1,1,,\n");
startBackend(V6);
// We expect that this file only holds leases that belong to the
// lease file or to the file with .completed postfix.
Lease6Ptr lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::4"));
ASSERT_TRUE(lease);
EXPECT_EQ(600, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::5"));
ASSERT_TRUE(lease);
EXPECT_EQ(0, lease->cltt_);
lease = lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::125"));
ASSERT_TRUE(lease);
EXPECT_EQ(600, lease->cltt_);
// Leases from the .1 and .2 files should not be loaded.
EXPECT_FALSE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1")));
EXPECT_FALSE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::2")));
EXPECT_FALSE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::3")));
}
}; // end of anonymous namespace
......@@ -99,13 +99,10 @@ CSVFile::close() {
bool
CSVFile::exists() const {
std::ifstream fs(filename_);
if (fs.good()) {
fs.close();
return (true);
}
std::ifstream fs(filename_.c_str());
const bool file_exists = fs.good();
fs.close();
return (false);
return (file_exists);
}
void
......
......@@ -320,7 +320,7 @@ public:
/// @brief Closes the CSV file.
void close();
/// @brief Checks if the CSV file exists.
/// @brief Checks if the CSV file exists and can be opened for reading.
///
/// This method doesn't check if the existing file has a correct file
/// format.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment