From: Thomas Markwalder Date: Mon, 11 Mar 2019 17:50:00 +0000 (-0400) Subject: [#526,!269] Addressed more review comments X-Git-Tag: Kea-1.6.0-beta~391^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9bc1a0edf92f301d598dce37838e240b65280e71;p=thirdparty%2Fkea.git [#526,!269] Addressed more review comments src/lib/cql/testutils/cql_schema.* Modified to use common softWipeEnabled(), which defaults to true src/lib/database/testutils/schema.* bool softWipeEnabled() - new function that checks env varible to determine if DB data wiping is enabled (default is true/enabled) src/lib/mysql/testutils/mysql_schema.* createMySQLSchema() destroyMySQLSchema()- now softWipeEnabled() to allow data wiping to be turned on/off src/share/database/scripts/mysql/wipe_data.sh.in Added better error messaging several files: Removed extraneous calls to destroyMySQLSchema, added comments --- diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index e74f473dd0..04ef1fe615 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -2200,7 +2200,7 @@ public: /// /// Recreates MySQL schema for a test. DORAMySQLTest() : DORATest() { - db::test::destroyMySQLSchema(); + // Ensure we have the proper schema with no transient data. db::test::createMySQLSchema(); } @@ -2208,6 +2208,7 @@ public: /// /// Destroys MySQL schema. virtual ~DORAMySQLTest() { + // If data wipe enabled, delete transient data otherwise destroy the schema. db::test::destroyMySQLSchema(); } }; diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 8ff43ed601..71bce5eb40 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2019 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 @@ -662,7 +662,7 @@ public: /// /// Recreates MySQL schema for a test. JSONFileBackendMySQLTest() : JSONFileBackendTest() { - destroyMySQLSchema(); + // Ensure we have the proper schema with no transient data. createMySQLSchema(); } @@ -670,6 +670,7 @@ public: /// /// Destroys MySQL schema. virtual ~JSONFileBackendMySQLTest() { + // If data wipe enabled, delete transient data otherwise destroy the schema. destroyMySQLSchema(); } diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index 7928e532bf..ff7bbba5ce 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2019 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 @@ -649,7 +649,7 @@ public: /// /// Recreates MySQL schema for a test. JSONFileBackendMySQLTest() : JSONFileBackendTest() { - destroyMySQLSchema(); + // Ensure we have the proper schema with no transient data. createMySQLSchema(); } @@ -657,6 +657,7 @@ public: /// /// Destroys MySQL schema. virtual ~JSONFileBackendMySQLTest() { + // If data wipe enabled, delete transient data otherwise destroy the schema destroyMySQLSchema(); } diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc index ab26ec7d69..fe94b556bc 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_mgr_unittest.cc @@ -29,7 +29,7 @@ public: // Recreate a fresh mgr. ConfigBackendDHCPv4Mgr::create(); - // Recreate database schema. + // Ensure we have the proper schema with no transient data. createMySQLSchema(); } @@ -37,6 +37,8 @@ public: virtual ~MySqlConfigBackendDHCPv4MgrTest() { // Destroy the mgr. ConfigBackendDHCPv4Mgr::destroy(); + + // If data wipe enabled, delete transient data otherwise destroy the schema. destroyMySQLSchema(); } }; diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc index 102e521cc8..12cd2bbd5f 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc @@ -47,7 +47,7 @@ public: /// @brief Constructor. MySqlConfigBackendDHCPv4Test() : test_subnets_(), test_networks_(), timestamps_(), audit_entries_() { - // Recreate database schema. + // Ensure we have the proper schema with no transient data. createMySQLSchema(); try { @@ -76,6 +76,7 @@ public: /// @brief Destructor. virtual ~MySqlConfigBackendDHCPv4Test() { cbptr_.reset(); + // If data wipe enabled, delete transient data otherwise destroy the schema. destroyMySQLSchema(); } diff --git a/src/lib/cql/testutils/cql_schema.cc b/src/lib/cql/testutils/cql_schema.cc index 93a6c8b7d8..2cfa28a5e0 100644 --- a/src/lib/cql/testutils/cql_schema.cc +++ b/src/lib/cql/testutils/cql_schema.cc @@ -31,16 +31,6 @@ validCqlConnectionString() { VALID_PASSWORD)); } -bool -softWipeEnabled() { - const char* const env = getenv("KEA_TEST_CASSANDRA_WIPE"); - if (env && (std::string(env) == std::string("soft"))) { - return (true); - } - - return (false); -} - void destroyCqlSchema(bool force_wipe, bool show_err) { if (force_wipe || !softWipeEnabled()) { diff --git a/src/lib/cql/testutils/cql_schema.h b/src/lib/cql/testutils/cql_schema.h index 6127afd4bd..0d3049d241 100644 --- a/src/lib/cql/testutils/cql_schema.h +++ b/src/lib/cql/testutils/cql_schema.h @@ -65,30 +65,6 @@ void createCqlSchema(bool force_wipe, bool show_err = false); /// @throw Unexpected when the script returns an error. void runCqlScript(const std::string& path, const std::string& script_name, bool show_err); - -/// @brief Returns status if the soft-wipe is enabled or not. -/// -/// In some deployments (In case of Tomek's dev system) Cassandra tests take -/// a very long time to execute. This was traced back to slow table/indexes -/// creation/deletion. With full wipe and recreation of all structures, it -/// took over 60 seconds for each test to execute. To avoid this problem, a -/// feature called soft-wipe has been implemented. If enabled, it does not -/// remove the structures, just the data from essential tables. To enable -/// it set KEA_TEST_CASSANDRA_WIPE environment variable to 'soft'. Make sure -/// that the database schema is set up properly before running in soft-wipe -/// mode. -/// -/// For example to use soft-wipe mode, you can: -/// -/// $ cqlsh -u keatest -p keatest -k keatest -/// -f src/share/database/scripts/cql/dhcpdb_create.cql -/// $ export KEA_TEST_CASSANDRA_WIPE=soft -/// $ cd src/lib/dhcpsrv -/// $ make -j9 -/// $ tests/libdhcpsrv_unittests --gtest_filter=CqlLeaseMgrTest.* -/// -/// @return true if soft-wipe is enabled, false otherwise -bool softWipeEnabled(); }; }; }; diff --git a/src/lib/database/testutils/schema.cc b/src/lib/database/testutils/schema.cc index e02b64310f..7e95545ba5 100644 --- a/src/lib/database/testutils/schema.cc +++ b/src/lib/database/testutils/schema.cc @@ -89,6 +89,16 @@ string connectionString(const char* type, const char* name, const char* host, return (result); } +bool +softWipeEnabled() { + const char* const wipe_only = getenv("KEA_TEST_DB_WIPE_DATA_ONLY"); + if (wipe_only && (std::string(wipe_only) == std::string("false"))) { + return (false); + } + + return (true); +} + }; }; }; diff --git a/src/lib/database/testutils/schema.h b/src/lib/database/testutils/schema.h index 86b22b0c26..f82cd99907 100644 --- a/src/lib/database/testutils/schema.h +++ b/src/lib/database/testutils/schema.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2019 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 @@ -45,6 +45,13 @@ std::string connectionString(const char* type, const char* name = NULL, const char* host = NULL, const char* user = NULL, const char* password = NULL, const char* timeout = NULL, const char* readonly_db = NULL); + +/// @brief Determines if wiping only the data between tests is enabled +/// +/// @return true if the environment variable, KEA_TEST_DB_WIPE_ONLY is +/// defined as "true" or if it is not present. +bool softWipeEnabled(); + }; }; }; diff --git a/src/lib/dhcpsrv/benchmarks/mysql_host_data_source_benchmark.cc b/src/lib/dhcpsrv/benchmarks/mysql_host_data_source_benchmark.cc index a06e9b788f..d457f04f82 100644 --- a/src/lib/dhcpsrv/benchmarks/mysql_host_data_source_benchmark.cc +++ b/src/lib/dhcpsrv/benchmarks/mysql_host_data_source_benchmark.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC") // Copyright (C) 2017 Deutsche Telekom AG. // // Authors: Andrei Pavel @@ -38,7 +38,7 @@ public: /// /// It cleans up schema and recreates tables, then instantiates HostMgr void SetUp(::benchmark::State const&) override { - destroyMySQLSchema(false); + // Ensure we have the proper schema with no transient data. createMySQLSchema(false); try { HostDataSourceFactory::destroy(); @@ -60,6 +60,7 @@ public: << endl; } HostDataSourceFactory::destroy(); + // If data wipe enabled, delete transient data otherwise destroy the schema. destroyMySQLSchema(false); } }; diff --git a/src/lib/dhcpsrv/benchmarks/mysql_lease_mgr_benchmark.cc b/src/lib/dhcpsrv/benchmarks/mysql_lease_mgr_benchmark.cc index c856a25ac9..a97197976c 100644 --- a/src/lib/dhcpsrv/benchmarks/mysql_lease_mgr_benchmark.cc +++ b/src/lib/dhcpsrv/benchmarks/mysql_lease_mgr_benchmark.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2019 Internet Systems Consortium, Inc. ("ISC") // Copyright (C) 2017 Deutsche Telekom AG. // // Authors: Andrei Pavel @@ -36,7 +36,7 @@ public: /// /// It cleans up schema and recreates tables, then instantiates LeaseMgr void SetUp(::benchmark::State const&) override { - destroyMySQLSchema(false); + // Ensure we have the proper schema with no transient data. createMySQLSchema(false); try { LeaseMgrFactory::destroy(); @@ -58,6 +58,7 @@ public: << endl; } LeaseMgrFactory::destroy(); + // If data wipe enabled, delete transient data otherwise destroy the schema. destroyMySQLSchema(false); } }; diff --git a/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc b/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc index 22ce7306aa..a29e1b2724 100644 --- a/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_db_access_unittest.cc @@ -147,12 +147,13 @@ public: /// @brief Constructor. CfgMySQLDbAccessTest() { - // Ensure schema is the correct one. + // Ensure we have the proper schema with no transient data. createMySQLSchema(); } /// @brief Destructor. virtual ~CfgMySQLDbAccessTest() { + // If data wipe enabled, delete transient data otherwise destroy the schema destroyMySQLSchema(); LeaseMgrFactory::destroy(); } diff --git a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc index adba8f904e..c21d8c4786 100644 --- a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc @@ -971,7 +971,7 @@ void MySQLHostMgrTest::SetUp() { HostMgrTest::SetUp(); - // Ensure schema is the correct one. + // Ensure we have the proper schema with no transient data. db::test::createMySQLSchema(); // Connect to the database @@ -991,6 +991,8 @@ void MySQLHostMgrTest::TearDown() { HostMgr::instance().getHostDataSource()->rollback(); HostMgr::delBackend("mysql"); + + // If data wipe enabled, delete transient data otherwise destroy the schema db::test::destroyMySQLSchema(); } 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 13a8b6320e..5f92dc37ad 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -41,7 +41,7 @@ class MySqlHostDataSourceTest : public GenericHostDataSourceTest { public: /// @brief Clears the database and opens connection to it. void initializeTest() { - // Ensure schema is the correct one. + // Ensure we have the proper schema with no transient data. createMySQLSchema(); // Connect to the database @@ -69,6 +69,8 @@ public: } HostMgr::delAllBackends(); hdsptr_.reset(); + + // If data wipe enabled, delete transient data otherwise destroy the schema destroyMySQLSchema(); } diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index 23abb1d749..2975cb1cd8 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc @@ -43,7 +43,7 @@ class MySqlLeaseMgrTest : public GenericLeaseMgrTest { public: /// @brief Clears the database and opens connection to it. void initializeTest() { - // Ensure schema is the correct one. + // Ensure we have the proper schema with no transient data. createMySQLSchema(); // Connect to the database @@ -69,6 +69,7 @@ public: // Rollback may fail if backend is in read only mode. That's ok. } LeaseMgrFactory::destroy(); + // If data wipe enabled, delete transient data otherwise destroy the schema destroyMySQLSchema(); } diff --git a/src/lib/mysql/testutils/mysql_schema.cc b/src/lib/mysql/testutils/mysql_schema.cc index 7d7e9a8590..f82196a9d4 100644 --- a/src/lib/mysql/testutils/mysql_schema.cc +++ b/src/lib/mysql/testutils/mysql_schema.cc @@ -32,14 +32,14 @@ validMySQLConnectionString() { void destroyMySQLSchema(bool show_err, bool force) { // If force is true or wipeData() fails, destory the schema. - if (force || wipeData(show_err)) { + if (force || (!softWipeEnabled()) || wipeData(show_err)) { runMySQLScript(DATABASE_SCRIPTS_DIR, "mysql/dhcpdb_drop.mysql", show_err); } } void createMySQLSchema(bool show_err, bool force) { // If force is true or wipeData() fails, recreate the schema. - if (force || wipeData(show_err)) { + if (force || (!softWipeEnabled()) || wipeData(show_err)) { destroyMySQLSchema(show_err, true); runMySQLScript(DATABASE_SCRIPTS_DIR, "mysql/dhcpdb_create.mysql", show_err); } diff --git a/src/lib/mysql/testutils/mysql_schema.h b/src/lib/mysql/testutils/mysql_schema.h index fc5bce3146..d4f19269a6 100644 --- a/src/lib/mysql/testutils/mysql_schema.h +++ b/src/lib/mysql/testutils/mysql_schema.h @@ -28,7 +28,7 @@ std::string validMySQLConnectionString(); /// database /or destroys the database itself by submitting the /// SQL script: /// -/// /mysql/dhcpdb_drop.sh +/// /mysql/dhcpdb_drop.mysql /// /// If wipeData() is called and fails, it will destroy /// the schema. If the schema destruction fails, the diff --git a/src/share/database/scripts/mysql/.gitignore b/src/share/database/scripts/mysql/.gitignore index 0ce2c5d735..8aa1c26ae0 100644 --- a/src/share/database/scripts/mysql/.gitignore +++ b/src/share/database/scripts/mysql/.gitignore @@ -8,3 +8,4 @@ /upgrade_5.2_to_6.0.sh /upgrade_6.0_to_7.0.sh /upgrade_7.0_to_8.0.sh +/wipe_data.sh diff --git a/src/share/database/scripts/mysql/wipe_data.sh.in b/src/share/database/scripts/mysql/wipe_data.sh.in index 3b5901089b..b2ac8b9dd2 100644 --- a/src/share/database/scripts/mysql/wipe_data.sh.in +++ b/src/share/database/scripts/mysql/wipe_data.sh.in @@ -28,8 +28,13 @@ shift; # If the existing schema doesn't match, the fail VERSION=`mysql_version "$@"` +if [ "$VERSION" = "" ]; then + printf "Cannot wipe data, schema version could not be detected.\n" + exit 1 +fi + if [ "$VERSION" != "$exp_version" ]; then - printf "Reported version is $VERSION is wrong, expected $exp_version.\n" + printf "Cannot wipe data, wrong schema version. Expected $exp_version, found version $VERSION.\n" exit 1 fi