[#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 e74f473dd0f8524c26cd3ace7b15a715a48a6f2e..04ef1fe615af14fa0d5c33b351e0061e7d43b476 100644 (file)
--- 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 8ff43ed601421f954664025b6e0f582eaabd5f9a..71bce5eb40706bf81f524d40663eb21aa5aec200 100644 (file)
--- 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 7928e532bf186635ecbe5182e850fa00eb198944..ff7bbba5ce58ff51e64e32f9c097b3dc856773d4 100644 (file)
--- 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 ab26ec7d691f3bcb6cf685826afaa51ddaae4086..fe94b556bce307972f3401b023fd78658db4676b 100644 (file)
--- 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 102e521cc8f86148620d507c925fce1819faf087..12cd2bbd5f1df2b195cfcd9a679e2a5b4eed85eb 100644 (file)
--- 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 93a6c8b7d8f6d3aa67b33ad2bfd813487068f001..2cfa28a5e069033de5492c075d215e306638b83f 100644 (file)
--- 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 6127afd4bd7a1058cdce0e3b4db5909c697d6899..0d3049d241da06c8cbd7b5a3196862c384b4202f 100644 (file)
--- 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 e02b64310fa9048e772fd07803e3097dab6a926c..7e95545ba5d63ddb571c44caf1be75e86d24b2fe 100644 (file)
--- 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 86b22b0c2688a8979f27b36a7f97213b373e8fed..f82cd999072872cf672895f567928cdf1582851b 100644 (file)
--- 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 a06e9b788f332d020580b1b1aa7550dfb7a5fd1b..d457f04f829d8432ae1921bcaad61d91df0e26af 100644 (file)
--- 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 <andrei.pavel@qualitance.com>
@@ -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 c856a25ac97c3b07e0651096071339a28f9ba7ac..a97197976ce426677095111f8fe46106cd0dd8df 100644 (file)
--- 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 <andrei.pavel@qualitance.com>
@@ -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 22ce7306aaff0e514cfe947139215df74f8e1d59..a29e1b272487a1e693227a548fc1c69de7b4233a 100644 (file)
--- 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 adba8f904e1d25429a1f854696caf251fb3655be..c21d8c47863e932667a3bdd291be907e3bc54949 100644 (file)
--- 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 13a8b6320e5c1f3e17d7b7528e3524bc617423b9..5f92dc37ad451595a1d883a7d61f0748d473cf39 100644 (file)
--- 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 23abb1d7492255a8528b58c943e7e66c7305713a..2975cb1cd8717068cb381b7162070d1e0899fc30 100644 (file)
--- 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 7d7e9a85904c2531c8714a4572a8cd796a659e7a..f82196a9d454fff3a9fce917138ad2a45c5e629b 100644 (file)
--- 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 fc5bce3146f2f1ef0b3dcf98b49b7c04869f3b2f..d4f19269a61973a9f6f94381ef9a2575f4c7c22b 100644 (file)
--- 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:
 ///
-///  <TEST_ADMIN_SCRIPTS_DIR>/mysql/dhcpdb_drop.sh
+///  <TEST_ADMIN_SCRIPTS_DIR>/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 0ce2c5d735911346f4abbb45260426d62071811b..8aa1c26ae047b4bf3434ef2e601b159580dde270 100644 (file)
--- 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 3b5901089bbf0a07e38c2cf93f5bcdf11fbeb94b..b2ac8b9dd2190c7c8cb68d0033d32f4458090160 100644 (file)
--- 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