Skip to content

GitLab

Commit 5785c87a authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[3360] Memfile now opens v4 or v6 lease file, not both.

parent 26ddda67
Hide whitespace changes
Inline Side-by-side
src/bin/dhcp4/config_parser.cc
View file @ 5785c87a
......@@ -407,7 +407,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) {
parser = new StringParser(config_id,
globalContext()->string_values_);
} else if (config_id.compare("lease-database") == 0) {
parser = new DbAccessParser(config_id);
parser = new DbAccessParser(config_id, *globalContext());
} else if (config_id.compare("hooks-libraries") == 0) {
parser = new HooksLibrariesParser(config_id);
} else if (config_id.compare("echo-client-id") == 0) {
......
src/bin/dhcp4/dhcp4.spec
View file @ 5785c87a
......@@ -191,6 +191,18 @@
"item_type": "string",
"item_optional": true,
"item_default": ""
},
{
"item_name": "persist",
"item_type": "boolean",
"item_optional": true,
"item_default": true
},
{
"item_name": "leasefile",
"item_type": "string",
"item_optional": true,
"item_default": ""
}
]
},
......
src/bin/dhcp6/config_parser.cc
View file @ 5785c87a
// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2012-2014 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
......@@ -637,7 +637,7 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id) {
parser = new StringParser(config_id,
globalContext()->string_values_);
} else if (config_id.compare("lease-database") == 0) {
parser = new DbAccessParser(config_id);
parser = new DbAccessParser(config_id, *globalContext());
} else if (config_id.compare("hooks-libraries") == 0) {
parser = new HooksLibrariesParser(config_id);
} else if (config_id.compare("dhcp-ddns") == 0) {
......
src/bin/dhcp6/dhcp6.spec
View file @ 5785c87a
......@@ -185,6 +185,18 @@
"item_type": "string",
"item_optional": true,
"item_default": ""
},
{
"item_name": "persist",
"item_type": "boolean",
"item_optional": true,
"item_default": true
},
{
"item_name": "leasefile",
"item_type": "string",
"item_optional": true,
"item_default": ""
}
]
},
......
src/lib/dhcpsrv/dbaccess_parser.cc
View file @ 5785c87a
// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2012-2014 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
......@@ -12,6 +12,7 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
#include <dhcp/option.h>
#include <dhcpsrv/dbaccess_parser.h>
#include <dhcpsrv/dhcpsrv_log.h>
#include <dhcpsrv/lease_mgr_factory.h>
......@@ -30,11 +31,10 @@ namespace dhcp {
// Factory function to build the parser
DbAccessParser::DbAccessParser(const std::string& param_name) : values_()
DbAccessParser::DbAccessParser(const std::string&, const ParserContext& ctx)
: values_(), ctx_(ctx)
{
if (param_name != "lease-database") {
LOG_WARN(dhcpsrv_logger, DHCPSRV_UNEXPECTED_NAME).arg(param_name);
}
ctx_ = ctx;
}
// Parse the configuration and check that the various keywords are consistent.
......@@ -43,19 +43,25 @@ DbAccessParser::build(isc::data::ConstElementPtr config_value) {
// To cope with incremental updates, the strategy is:
// 1. Take a copy of the stored keyword/value pairs.
// 2. Update the copy with the passed keywords.
// 3. Perform validation checks on the updated keyword/value pairs.
// 4. If all is OK, update the stored keyword/value pairs.
// 2. Inject the universe parameter.
// 3. Update the copy with the passed keywords.
// 4. Perform validation checks on the updated keyword/value pairs.
// 5. If all is OK, update the stored keyword/value pairs.
// 1. Take a copy of the stored keyword/value pairs.
std::map<string, string> values_copy = values_;
// 2. Update the copy with the passed keywords.
// 2. Inject the parameter which defines whether we are configuring
// DHCPv4 or DHCPv6. Some database backends (e.g. Memfile make
// use of it).
values_copy["universe"] = ctx_.universe_ == Option::V4 ? "4" : "6";
// 3. Update the copy with the passed keywords.
BOOST_FOREACH(ConfigPair param, config_value->mapValue()) {
values_copy[param.first] = param.second->stringValue();
}
// 3. Perform validation checks on the updated set of keyword/values.
// 4. Perform validation checks on the updated set of keyword/values.
//
// a. Check if the "type" keyword exists and thrown an exception if not.
StringPairMap::const_iterator type_ptr = values_copy.find("type");
......@@ -71,7 +77,7 @@ DbAccessParser::build(isc::data::ConstElementPtr config_value) {
isc_throw(BadValue, "unknown backend database type: " << dbtype);
}
// 4. If all is OK, update the stored keyword/value pairs. We do this by
// 5. If all is OK, update the stored keyword/value pairs. We do this by
// swapping contents - values_copy is destroyed immediately after the
// operation (when the method exits), so we are not interested in its new
// value.
......
src/lib/dhcpsrv/dbaccess_parser.h
View file @ 5785c87a
// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2012-2014 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
......@@ -17,6 +17,7 @@
#include <cc/data.h>
#include <dhcpsrv/dhcp_config_parser.h>
#include <dhcpsrv/dhcp_parsers.h>
#include <exceptions/exceptions.h>
#include <string>
......@@ -54,7 +55,8 @@ public:
///
/// @param param_name Name of the parameter under which the database
/// access details are held.
DbAccessParser(const std::string& param_name);
/// @param ctx Parser context.
DbAccessParser(const std::string& param_name, const ParserContext& ctx);
/// The destructor.
virtual ~DbAccessParser()
......@@ -96,11 +98,13 @@ public:
///
/// @param param_name Name of the parameter used to access the
/// configuration.
/// @param ctx Parser context.
///
/// @return Pointer to a DbAccessParser. The caller is responsible for
/// destroying the parser after use.
static DhcpConfigParser* factory(const std::string& param_name) {
return (new DbAccessParser(param_name));
static DhcpConfigParser* factory(const std::string& param_name,
const ParserContext& ctx) {
return (new DbAccessParser(param_name, ctx));
}
protected:
......@@ -124,7 +128,10 @@ protected:
std::string getDbAccessString() const;
private:
std::map<std::string, std::string> values_; ///< Stored parameter values
ParserContext ctx_; ///< Parser context
};
}; // namespace dhcp
......
src/lib/dhcpsrv/memfile_lease_mgr.cc
View file @ 5785c87a
......@@ -23,18 +23,22 @@ using namespace isc::dhcp;
Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
: LeaseMgr(parameters) {
// Get the lease files locations and open for IO.
std::string file4 = initLeaseFilePath(V4);
if (!file4.empty()) {
lease_file4_.reset(new CSVLeaseFile4(file4));
lease_file4_->open();
load4();
}
std::string file6 = initLeaseFilePath(V6);
if (!file6.empty()) {
lease_file6_.reset(new CSVLeaseFile6(file6));
lease_file6_->open();
load6();
// Check the universe and use v4 file or v6 file.
std::string universe = getParameter("universe");
if (universe == "4") {
std::string file4 = initLeaseFilePath(V4);
if (!file4.empty()) {
lease_file4_.reset(new CSVLeaseFile4(file4));
lease_file4_->open();
load4();
}
} else {
std::string file6 = initLeaseFilePath(V6);
if (!file6.empty()) {
lease_file6_.reset(new CSVLeaseFile6(file6));
lease_file6_->open();
load6();
}
}
// If lease persistence have been disabled for both v4 and v6,
......@@ -424,28 +428,26 @@ Memfile_LeaseMgr::persistLeases(Universe u) const {
std::string
Memfile_LeaseMgr::initLeaseFilePath(Universe u) {
std::string persist_val;
std::string persist_str = (u == V4 ? "persist4" : "persist6");
try {
persist_val = getParameter(persist_str);
persist_val = getParameter("persist");
} catch (const Exception& ex) {
// If parameter persist hasn't been specified, we use a default value
// 'yes'.
persist_val = "yes";
persist_val = "true";
}
// If persist_val is 'no' we will not store leases to disk, so let's
// If persist_val is 'false' we will not store leases to disk, so let's
// return empty file name.
if (persist_val == "no") {
if (persist_val == "false") {
return ("");
} else if (persist_val != "yes") {
isc_throw(isc::BadValue, "invalid value '" << persist_str
<< "=" << persist_val << "'");
} else if (persist_val != "true") {
isc_throw(isc::BadValue, "invalid value 'persist="
<< persist_val << "'");
}
std::string param_name = (u == V4 ? "leasefile4" : "leasefile6");
std::string lease_file;
try {
lease_file = getParameter(param_name);
lease_file = getParameter("leasefile");
} catch (const Exception& ex) {
lease_file = getDefaultLeaseFilePath(u);
}
......
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
View file @ 5785c87a
......@@ -101,7 +101,7 @@ public:
initFqdn("", false, false);
factory_.create("type=memfile persist4=no persist6=no");
factory_.create("type=memfile universe=6 persist=false");
}
/// @brief Configures a subnet and adds one pool to it.
......@@ -424,7 +424,7 @@ public:
subnet_->addPool(pool_);
cfg_mgr.addSubnet4(subnet_);
factory_.create("type=memfile persist4=no persist6=no");
factory_.create("type=memfile universe=4 persist=false");
}
/// @brief checks if Lease4 matches expected configuration
......
src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc
View file @ 5785c87a
......@@ -15,6 +15,7 @@
#include <config.h>
#include <dhcpsrv/dbaccess_parser.h>
#include <dhcpsrv/dhcp_parsers.h>
#include <dhcpsrv/lease_mgr_factory.h>
#include <config/ccsession.h>
#include <gtest/gtest.h>
......@@ -114,14 +115,18 @@ public:
/// @param dbaccess set of database access parameters to check
/// @param keyval Array of "const char*" strings in the order keyword,
/// value, keyword, value ... A NULL entry terminates the list.
/// @param u Universe (V4 or V6).
void checkAccessString(const char* trace_string,
const DbAccessParser::StringPairMap& parameters,
const char* keyval[]) {
const char* keyval[],
Option::Universe u = Option::V4) {
SCOPED_TRACE(trace_string);
// Construct a map of keyword value pairs.
std::map<string, string> expected;
size_t expected_count = 0;
expected["universe"] = (u == Option::V4 ? "4" : "6");
// The universe is always injected by the parser itself.
size_t expected_count = 1;
for (size_t i = 0; keyval[i] != NULL; i += 2) {
// Get the value. This should not be NULL
ASSERT_NE(static_cast<const char*>(NULL), keyval[i + 1]) <<
......@@ -134,7 +139,7 @@ public:
}
// Check no duplicates in the test set of reference keywords.
ASSERT_EQ(expected_count, expected.size()) <<
ASSERT_EQ(expected_count, expected.size()) <<
"Supplied reference keyword/value list contains duplicate keywords";
// The passed parameter map should have the same number of entries as
......@@ -169,8 +174,8 @@ public:
/// @brief Constructor
///
/// @brief Keyword/value collection of ddatabase access parameters
TestDbAccessParser(const std::string& param_name)
: DbAccessParser(param_name)
TestDbAccessParser(const std::string& param_name, const ParserContext& ctx)
: DbAccessParser(param_name, ctx)
{}
/// @brief Destructor
......@@ -211,7 +216,7 @@ TEST_F(DbAccessParserTest, validTypeMemfile) {
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_NO_THROW(parser.build(json_elements));
checkAccessString("Valid memfile", parser.getDbAccessParameters(), config);
}
......@@ -227,11 +232,49 @@ TEST_F(DbAccessParserTest, emptyKeyword) {
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_NO_THROW(parser.build(json_elements));
checkAccessString("Valid memfile", parser.getDbAccessParameters(), config);
}
// Check that the parser works with more complex configuration when
// lease file path is specified for DHCPv4.
TEST_F(DbAccessParserTest, persistV4Memfile) {
const char* config[] = {"type", "memfile",
"persist", "true",
"leasefile", "/opt/bind10/var/kea-leases4.csv",
NULL};
string json_config = toJson(config);
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_NO_THROW(parser.build(json_elements));
checkAccessString("Valid memfile", parser.getDbAccessParameters(),
config);
}
// Check that the parser works with more complex configuration when
// lease file path is specified for DHCPv6.
TEST_F(DbAccessParserTest, persistV6Memfile) {
const char* config[] = {"type", "memfile",
"persist", "true",
"leasefile", "/opt/bind10/var/kea-leases6.csv",
NULL};
string json_config = toJson(config);
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database", ParserContext(Option::V6));
EXPECT_NO_THROW(parser.build(json_elements));
checkAccessString("Valid memfile", parser.getDbAccessParameters(),
config, Option::V6);
}
// Check that the parser works with a valid MySQL configuration
TEST_F(DbAccessParserTest, validTypeMysql) {
const char* config[] = {"type", "mysql",
......@@ -245,7 +288,7 @@ TEST_F(DbAccessParserTest, validTypeMysql) {
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_NO_THROW(parser.build(json_elements));
checkAccessString("Valid mysql", parser.getDbAccessParameters(), config);
}
......@@ -262,7 +305,7 @@ TEST_F(DbAccessParserTest, missingTypeKeyword) {
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_THROW(parser.build(json_elements), TypeKeywordMissing);
}
......@@ -271,7 +314,8 @@ TEST_F(DbAccessParserTest, factory) {
// Check that the parser is built through the factory.
boost::scoped_ptr<DhcpConfigParser> parser(
DbAccessParser::factory("lease-database"));
DbAccessParser::factory("lease-database", ParserContext(Option::V4))
);
EXPECT_TRUE(parser);
DbAccessParser* dbap = dynamic_cast<DbAccessParser*>(parser.get());
EXPECT_NE(static_cast<DbAccessParser*>(NULL), dbap);
......@@ -321,7 +365,7 @@ TEST_F(DbAccessParserTest, incrementalChanges) {
"name", "keatest",
NULL};
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
// First configuration string should cause a representation of that string
// to be held.
......@@ -385,7 +429,7 @@ TEST_F(DbAccessParserTest, getDbAccessString) {
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_NO_THROW(parser.build(json_elements));
// Get the database access string
......@@ -394,8 +438,7 @@ TEST_F(DbAccessParserTest, getDbAccessString) {
// String should be either "type=mysql name=keatest" or
// "name=keatest type=mysql". The "host" entry is null, so should not be
// output.
EXPECT_TRUE((dbaccess == "type=mysql name=keatest") ||
(dbaccess == "name=keatest type=mysql"));
EXPECT_EQ(dbaccess, "name=keatest type=mysql universe=4");
}
// Check that the "commit" function actually opens the database. We will
......@@ -410,18 +453,17 @@ TEST_F(DbAccessParserTest, commit) {
}, isc::dhcp::NoLeaseManager);
// Set up the parser to open the memfile database.
const char* config[] = {"type", "memfile", "persist4", "no",
"persist6", "no", NULL};
const char* config[] = {"type", "memfile", "persist", "false", NULL};
string json_config = toJson(config);
ConstElementPtr json_elements = Element::fromJSON(json_config);
EXPECT_TRUE(json_elements);
TestDbAccessParser parser("lease-database");
TestDbAccessParser parser("lease-database", ParserContext(Option::V4));
EXPECT_NO_THROW(parser.build(json_elements));
// Ensure that the access string is as expected.
EXPECT_EQ(std::string("persist4=no persist6=no type=memfile"),
EXPECT_EQ("persist=false type=memfile universe=4",
parser.getDbAccessString());
// Committal of the parser changes should open the database.
......
src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
View file @ 5785c87a
......@@ -766,7 +766,7 @@ GenericLeaseMgrTest::testBasicLease6() {
lmptr_->commit();
// Reopen the database to ensure that they actually got stored.
reopen();
reopen(V6);
Lease6Ptr l_returned = lmptr_->getLease6(leasetype6_[1], ioaddress6_[1]);
ASSERT_TRUE(l_returned);
......@@ -795,7 +795,7 @@ GenericLeaseMgrTest::testBasicLease6() {
ASSERT_TRUE(l_returned);
detailCompareLease(leases[2], l_returned);
reopen();
reopen(V6);
// The deleted lease should be still gone after we re-read leases from
// persistent storage.
......@@ -814,7 +814,7 @@ GenericLeaseMgrTest::testBasicLease6() {
ASSERT_NO_THROW(lmptr_->updateLease6(leases[2]));
reopen();
reopen(V6);
// The lease should be now updated in the storage.
l_returned = lmptr_->getLease6(leasetype6_[2], ioaddress6_[2]);
......
src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h
View file @ 5785c87a
......@@ -30,6 +30,12 @@ namespace test {
class GenericLeaseMgrTest : public ::testing::Test {
public:
/// @brief Universe (V4 or V6).
enum Universe {
V4,
V6
};
/// @brief Default constructor.
GenericLeaseMgrTest();
......@@ -40,7 +46,9 @@ public:
///
/// Closes the database and re-opens it. It must be implemented
/// in derived classes.
virtual void reopen() = 0;
///
/// @param u Universe (V4 or V6), required by some backends.
virtual void reopen(Universe u = V4) = 0;
/// @brief Initialize Lease4 Fields
///
......
src/lib/dhcpsrv/tests/lease_mgr_unittest.cc
View file @ 5785c87a
......@@ -253,7 +253,7 @@ public:
///
/// No-op implementation. We need to provide concrete implementation,
/// as this is a pure virtual method in GenericLeaseMgrTest.
virtual void reopen() {
virtual void reopen(Universe) {
}
};
......
src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
View file @ 5785c87a
......@@ -50,21 +50,18 @@ public:
// Make sure there are no dangling files after previous tests.
io4_.removeFile();
io6_.removeFile();
try {
LeaseMgrFactory::create(getConfigString());
} catch (...) {
std::cerr << "*** ERROR: unable to create instance of the Memfile\n"
" lease database backend.\n";
throw;
}
lmptr_ = &(LeaseMgrFactory::instance());
}
virtual void reopen() {
/// @brief Reopens the connection to the backend.
///
/// This function is called by unit tests to force reconnection of the
/// backend to check that the leases are stored and can be retrieved
/// from the storage.
///
/// @param u Universe (V4 or V6)
virtual void reopen(Universe u) {
LeaseMgrFactory::destroy();
LeaseMgrFactory::create(getConfigString());
lmptr_ = &(LeaseMgrFactory::instance());
startBackend(u);
}
/// @brief destructor
......@@ -92,14 +89,32 @@ public:
/// backend and use leasefile4_0.csv and leasefile6_0.csv files as
/// storage for leases.
///
/// @param no_universe Indicates whether universe parameter should be
/// included (false), or not (true).
///
/// @return Configuration string for @c LeaseMgrFactory.
static std::string getConfigString() {
static std::string getConfigString(Universe u) {
std::ostringstream s;
s << "type=memfile leasefile4=" << getLeaseFilePath("leasefile4_0.csv")
<< " leasefile6=" << getLeaseFilePath("leasefile6_0.csv");
s << "type=memfile " << (u == V4 ? "universe=4 " : "universe=6 ")
<< "leasefile="
<< getLeaseFilePath(u == V4 ? "leasefile4_0.csv" : "leasefile6_0.csv");
return (s.str());
}
/// @brief Creates instance of the backend.
///
/// @param u Universe (v4 or V6).
void startBackend(Universe u) {
try {
LeaseMgrFactory::create(getConfigString(u));
} catch (...) {
std::cerr << "*** ERROR: unable to create instance of the Memfile\n"
" lease database backend.\n";
throw;
}
lmptr_ = &(LeaseMgrFactory::instance());
}
/// @brief Object providing access to v4 lease IO.
LeaseFileIO io4_;
......@@ -111,37 +126,27 @@ public:
// This test checks if the LeaseMgr can be instantiated and that it
// parses parameters string properly.
TEST_F(MemfileLeaseMgrTest, constructor) {
LeaseMgr::ParameterMap pmap;
pmap["persist4"] = "no";
pmap["persist6"] = "no";
pmap["universe"] = "4";
pmap["persist"] = "false";
boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr;
EXPECT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)));
pmap["persist4"] = "yes";
pmap["persist6"] = "yes";
pmap["leasefile4"] = getLeaseFilePath("leasefile4_1.csv");
pmap["leasefile6"] = getLeaseFilePath("leasefile6_1.csv");
pmap["persist"] = "true";
pmap["leasefile"] = getLeaseFilePath("leasefile4_1.csv");
EXPECT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)));
// Expecting that persist parameter is yes or no. Everything other than
// that is wrong.
pmap["persist4"] = "bogus";
pmap["persist6"] = "yes";
pmap["leasefile4"] = getLeaseFilePath("leasefile4_1.csv");
pmap["leasefile6"] = getLeaseFilePath("leasefile6_1.csv");