From: Marcin Siodelski Date: Tue, 1 Mar 2016 08:33:06 +0000 (+0100) Subject: [3696] Addressed review comments. X-Git-Tag: trac4315_base~1^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=537f48410d4bbf990e6fa9afe6db8fd45020a180;p=thirdparty%2Fkea.git [3696] Addressed review comments. Addressed a couple of minor issues and added unit tests for switching between various lease database backends. --- diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 3f55111078..da62148523 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 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 diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 09bd30b511..eb50915566 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -24,6 +25,7 @@ #include #include +#include #include #include @@ -35,6 +37,7 @@ using namespace isc::asiolink; using namespace isc::config; using namespace isc::data; using namespace isc::dhcp; +using namespace isc::dhcp::test; using namespace isc::hooks; namespace { @@ -43,6 +46,7 @@ class NakedControlledDhcpv4Srv: public ControlledDhcpv4Srv { // "Naked" DHCPv4 server, exposes internal fields public: NakedControlledDhcpv4Srv():ControlledDhcpv4Srv(0) { } + using ControlledDhcpv4Srv::signal_handler_; }; /// @brief test class for Kea configuration backend @@ -423,4 +427,147 @@ TEST_F(JSONFileBackendTest, defaultLeaseDbBackend) { EXPECT_NO_THROW(static_cast(LeaseMgrFactory::instance())); } +// Starting tests which require MySQL backend availability. Those tests +// will not be executed if Kea has been compiled without the +// --with-dhcp-mysql. +#ifdef HAVE_MYSQL + +/// @brief Test fixture class for the tests utilizing MySQL database +/// backend. +class JSONFileBackendMySQLTest : public JSONFileBackendTest { +public: + + /// @brief Constructor. + /// + /// Recreates MySQL schema for a test. + JSONFileBackendMySQLTest() : JSONFileBackendTest() { + destroyMySQLSchema(); + createMySQLSchema(); + } + + /// @brief Destructor. + /// + /// Destroys MySQL schema. + virtual ~JSONFileBackendMySQLTest() { + destroyMySQLSchema(); + } + + /// @brief Creates server configuration with specified backend type. + /// + /// @param backend Backend type or empty string to indicate that the + /// backend configuration should not be placed in the resulting + /// JSON configuration. + /// + /// @return Server configuration. + std::string createConfiguration(const std::string& backend) const; + + /// @brief Test reconfiguration with a backend change. + /// + /// If any of the parameters is an empty string it indicates that the + /// created configuration should exclude backend configuration. + /// + /// @param backend_first Type of a backend to be used initially. + /// @param backend_second Type of a backend to be used after + /// reconfiguration. + void testBackendReconfiguration(const std::string& backend_first, + const std::string& backend_second); +}; + +std::string +JSONFileBackendMySQLTest::createConfiguration(const std::string& backend) const { + // This is basic server configuration which excludes lease database + // backend specification. The default Memfile backend should be + // initialized in this case. + std::ostringstream config; + config << + "{ \"Dhcp4\": {" + "\"interfaces-config\": {" + " \"interfaces\": [ ]" + "},"; + + // For non-empty lease backend type we have to add a backend configuration + // section. + if (!backend.empty()) { + config << + "\"lease-database\": {" + " \"type\": \"" << backend << "\""; + + // SQL backends require database credentials. + if (backend != "memfile") { + config << + "," + " \"name\": \"keatest\"," + " \"user\": \"keatest\"," + " \"password\": \"keatest\""; + } + config << "},"; + } + + // Append the rest of the configuration. + config << + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "\"subnet4\": [ ]," + "\"valid-lifetime\": 4000 }" + "}"; + + return (config.str()); +} + +void +JSONFileBackendMySQLTest:: +testBackendReconfiguration(const std::string& backend_first, + const std::string& backend_second) { + // This is basic server configuration which excludes lease database + // backend specification. The default Memfile backend should be + // initialized in this case. + writeFile(createConfiguration(backend_first)); + + // Create an instance of the server and intialize it. + boost::scoped_ptr srv; + ASSERT_NO_THROW(srv.reset(new NakedControlledDhcpv4Srv())); + srv->setConfigFile(TEST_FILE); + ASSERT_NO_THROW(srv->init(TEST_FILE)); + + // The backend should have been created and its type should be + // correct. + ASSERT_NO_THROW(static_cast(LeaseMgrFactory::instance())); + EXPECT_EQ(backend_first.empty() ? "memfile" : backend_first, + LeaseMgrFactory::instance().getType()); + + // New configuration modifies the lease database backend type to MYSQL. + writeFile(createConfiguration(backend_second)); + + // Explicitly calling signal handler for SIGHUP to trigger server + // reconfiguration. + srv->signal_handler_(SIGHUP); + + // The backend should have been created and its type should be + // correct. + ASSERT_NO_THROW(static_cast(LeaseMgrFactory::instance())); + EXPECT_EQ(backend_second.empty() ? "memfile" : backend_second, + LeaseMgrFactory::instance().getType()); +} + + +// This test verifies that backend specification can be added on +// server reconfiguration. +TEST_F(JSONFileBackendMySQLTest, reconfigureBackendUndefinedToMySQL) { + testBackendReconfiguration("", "mysql"); +} + +// This test verifies that when backend specification is removed the +// default backend is used. +TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMySQLToUndefined) { + testBackendReconfiguration("mysql", ""); +} + +// This test verifies that backend type can be changed from Memfile +// to MySQL. +TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMemfileToMySQL) { + testBackendReconfiguration("memfile", "mysql"); +} + +#endif + } // End of anonymous namespace diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index f0f682fe65..8618c1dba3 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 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 diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index e957235a94..c4680fcf8c 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -35,6 +36,7 @@ using namespace isc::asiolink; using namespace isc::config; using namespace isc::data; using namespace isc::dhcp; +using namespace isc::dhcp::test; using namespace isc::hooks; namespace { @@ -43,6 +45,7 @@ class NakedControlledDhcpv6Srv: public ControlledDhcpv6Srv { // "Naked" DHCPv6 server, exposes internal fields public: NakedControlledDhcpv6Srv():ControlledDhcpv6Srv(0) { } + using ControlledDhcpv6Srv::signal_handler_; }; @@ -406,4 +409,148 @@ TEST_F(JSONFileBackendTest, defaultLeaseDbBackend) { EXPECT_NO_THROW(static_cast(LeaseMgrFactory::instance())); } +// Starting tests which require MySQL backend availability. Those tests +// will not be executed if Kea has been compiled without the +// --with-dhcp-mysql. +#ifdef HAVE_MYSQL + +/// @brief Test fixture class for the tests utilizing MySQL database +/// backend. +class JSONFileBackendMySQLTest : public JSONFileBackendTest { +public: + + /// @brief Constructor. + /// + /// Recreates MySQL schema for a test. + JSONFileBackendMySQLTest() : JSONFileBackendTest() { + destroyMySQLSchema(); + createMySQLSchema(); + } + + /// @brief Destructor. + /// + /// Destroys MySQL schema. + virtual ~JSONFileBackendMySQLTest() { + destroyMySQLSchema(); + } + + /// @brief Creates server configuration with specified backend type. + /// + /// @param backend Backend type or empty string to indicate that the + /// backend configuration should not be placed in the resulting + /// JSON configuration. + /// + /// @return Server configuration. + std::string createConfiguration(const std::string& backend) const; + + /// @brief Test reconfiguration with a backend change. + /// + /// If any of the parameters is an empty string it indicates that the + /// created configuration should exclude backend configuration. + /// + /// @param backend_first Type of a backend to be used initially. + /// @param backend_second Type of a backend to be used after + /// reconfiguration. + void testBackendReconfiguration(const std::string& backend_first, + const std::string& backend_second); +}; + +std::string +JSONFileBackendMySQLTest::createConfiguration(const std::string& backend) const { + // This is basic server configuration which excludes lease database + // backend specification. The default Memfile backend should be + // initialized in this case. + std::ostringstream config; + config << + "{ \"Dhcp6\": {" + "\"interfaces-config\": {" + " \"interfaces\": [ ]" + "},"; + + // For non-empty lease backend type we have to add a backend configuration + // section. + if (!backend.empty()) { + config << + "\"lease-database\": {" + " \"type\": \"" << backend << "\""; + + // SQL backends require database credentials. + if (backend != "memfile") { + config << + "," + " \"name\": \"keatest\"," + " \"user\": \"keatest\"," + " \"password\": \"keatest\""; + } + config << "},"; + } + + // Append the rest of the configuration. + config << + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "\"subnet6\": [ ]," + "\"preferred-lifetime\": 3000, " + "\"valid-lifetime\": 4000 }" + "}"; + + return (config.str()); +} + +void +JSONFileBackendMySQLTest:: +testBackendReconfiguration(const std::string& backend_first, + const std::string& backend_second) { + // This is basic server configuration which excludes lease database + // backend specification. The default Memfile backend should be + // initialized in this case. + writeFile(TEST_FILE, createConfiguration(backend_first)); + + // Create an instance of the server and intialize it. + boost::scoped_ptr srv; + ASSERT_NO_THROW(srv.reset(new NakedControlledDhcpv6Srv())); + srv->setConfigFile(TEST_FILE); + ASSERT_NO_THROW(srv->init(TEST_FILE)); + + // The backend should have been created and its type should be + // correct. + ASSERT_NO_THROW(static_cast(LeaseMgrFactory::instance())); + EXPECT_EQ(backend_first.empty() ? "memfile" : backend_first, + LeaseMgrFactory::instance().getType()); + + // New configuration modifies the lease database backend type to MYSQL. + writeFile(TEST_FILE, createConfiguration(backend_second)); + + // Explicitly calling signal handler for SIGHUP to trigger server + // reconfiguration. + srv->signal_handler_(SIGHUP); + + // The backend should have been created and its type should be + // correct. + ASSERT_NO_THROW(static_cast(LeaseMgrFactory::instance())); + EXPECT_EQ(backend_second.empty() ? "memfile" : backend_second, + LeaseMgrFactory::instance().getType()); +} + + +// This test verifies that backend specification can be added on +// server reconfiguration. +TEST_F(JSONFileBackendMySQLTest, reconfigureBackendUndefinedToMySQL) { + testBackendReconfiguration("", "mysql"); +} + +// This test verifies that when backend specification is removed the +// default backend is used. +TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMySQLToUndefined) { + testBackendReconfiguration("mysql", ""); +} + +// This test verifies that backend type can be changed from Memfile +// to MySQL. +TEST_F(JSONFileBackendMySQLTest, reconfigureBackendMemfileToMySQL) { + testBackendReconfiguration("memfile", "mysql"); +} + +#endif + } // End of anonymous namespace diff --git a/src/lib/dhcpsrv/cfg_db_access.h b/src/lib/dhcpsrv/cfg_db_access.h index 2de904da1c..f3efd864f5 100644 --- a/src/lib/dhcpsrv/cfg_db_access.h +++ b/src/lib/dhcpsrv/cfg_db_access.h @@ -26,6 +26,9 @@ public: /// @brief Sets parameters which will be appended to the database /// access strings. + /// + /// @param appended_parameters String holding collection of parameters + /// in the following format: "parameter0=value0 parameter1=value1 ...". void setAppendedParameters(const std::string& appended_parameters) { appended_parameters_ = appended_parameters; } diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 76e52f9663..4695d53a5c 100755 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -777,7 +777,7 @@ Memfile_LeaseMgr::deleteExpiredReclaimedLeases(const uint32_t secs, std::string Memfile_LeaseMgr::getDescription() const { - return (std::string("In Memory Database")); + return (std::string("In memory database with leases stored in a CSV file.")); } void diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 45c639388a..ba0bc33cf2 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -279,7 +279,7 @@ public: return (cfg_duid_); } - /// @brief Returns pointer to the objec holding configuration of the + /// @brief Returns pointer to the object holding configuration of the /// lease and host database connection parameters. CfgDbAccessPtr getCfgDbAccess() { return (cfg_db_access_); diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 3b70183687..86890aa6d4 100755 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -105,7 +105,6 @@ libdhcpsrv_unittests_SOURCES += generic_host_data_source_unittest.cc generic_hos libdhcpsrv_unittests_SOURCES += memfile_lease_mgr_unittest.cc libdhcpsrv_unittests_SOURCES += dhcp_parsers_unittest.cc if HAVE_MYSQL -libdhcpsrv_unittests_SOURCES += mysql_schema.cc mysql_schema.h libdhcpsrv_unittests_SOURCES += mysql_lease_mgr_unittest.cc libdhcpsrv_unittests_SOURCES += mysql_host_data_source_unittest.cc endif @@ -116,7 +115,6 @@ if HAVE_PGSQL libdhcpsrv_unittests_SOURCES += pgsql_lease_mgr_unittest.cc endif libdhcpsrv_unittests_SOURCES += pool_unittest.cc -libdhcpsrv_unittests_SOURCES += schema_mysql_copy.h libdhcpsrv_unittests_SOURCES += schema_pgsql_copy.h libdhcpsrv_unittests_SOURCES += srv_config_unittest.cc libdhcpsrv_unittests_SOURCES += subnet_unittest.cc diff --git a/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc b/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc index b7d17274c1..49d3a378ff 100644 --- a/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include using namespace isc; diff --git a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc index 21931a9ab4..972b6e5d53 100644 --- a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index ed29cef6d5..7568f6d022 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index e4f2a82b94..be43adaa70 100755 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 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 @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/lib/dhcpsrv/testutils/Makefile.am b/src/lib/dhcpsrv/testutils/Makefile.am index f420fb4302..97ffcba62c 100644 --- a/src/lib/dhcpsrv/testutils/Makefile.am +++ b/src/lib/dhcpsrv/testutils/Makefile.am @@ -16,9 +16,20 @@ if HAVE_GTEST noinst_LTLIBRARIES = libdhcpsrvtest.la libdhcpsrvtest_la_SOURCES = config_result_check.cc config_result_check.h +if HAVE_MYSQL +libdhcpsrvtest_la_SOURCES += mysql_schema.cc mysql_schema.h +libdhcpsrvtest_la_SOURCES += schema_mysql_copy.h +endif + libdhcpsrvtest_la_CXXFLAGS = $(AM_CXXFLAGS) -libdhcpsrvtest_la_CPPFLAGS = $(AM_CPPFLAGS) -libdhcpsrvtest_la_LDFLAGS = $(AM_LDFLAGS) +libdhcpsrvtest_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) +if HAVE_MYSQL +libdhcpsrvtest_la_CPPFLAGS += $(MYSQL_CPPFLAGS) +endif +libdhcpsrvtest_la_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) +if HAVE_MYSQL +libdhcpsrvtest_la_LDFLAGS += $(MYSQL_LIBS) +endif libdhcpsrvtest_la_LIBADD = $(top_builddir)/src/lib/cc/libkea-cc.la libdhcpsrvtest_la_LIBADD += $(top_builddir)/src/lib/log/libkea-log.la diff --git a/src/lib/dhcpsrv/tests/mysql_schema.cc b/src/lib/dhcpsrv/testutils/mysql_schema.cc similarity index 100% rename from src/lib/dhcpsrv/tests/mysql_schema.cc rename to src/lib/dhcpsrv/testutils/mysql_schema.cc diff --git a/src/lib/dhcpsrv/tests/mysql_schema.h b/src/lib/dhcpsrv/testutils/mysql_schema.h similarity index 100% rename from src/lib/dhcpsrv/tests/mysql_schema.h rename to src/lib/dhcpsrv/testutils/mysql_schema.h diff --git a/src/lib/dhcpsrv/tests/schema_mysql_copy.h b/src/lib/dhcpsrv/testutils/schema_mysql_copy.h similarity index 100% rename from src/lib/dhcpsrv/tests/schema_mysql_copy.h rename to src/lib/dhcpsrv/testutils/schema_mysql_copy.h