[#3025] simplify logic in ensureSchemaVersion

... and allow kea-admin path to be overridden in tests.

12 files changed, 57 insertions, 45 deletions

diff --git a/src/lib/database/database_connection.h b/src/lib/database/database_connection.h
index f85d8a0b56..5b46e40783 100644
--- a/src/lib/database/database_connection.h
+++ b/src/lib/database/database_connection.h
@@ -94,7 +94,7 @@ public:
 };
 
 /// @brief Thrown when an initialization of the schema failed.
-class SchemaInitializationFailed: public Exception {
+class SchemaInitializationFailed : public Exception {
 public:
     SchemaInitializationFailed(const char* file, size_t line, const char* what) :
         isc::Exception(file, line, what) {}
diff --git a/src/lib/database/db_log.cc b/src/lib/database/db_log.cc
index 3fb45e4c1e..fca1a18be7 100644
--- a/src/lib/database/db_log.cc
+++ b/src/lib/database/db_log.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2018-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2024 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/lib/database/db_log.h b/src/lib/database/db_log.h
index 3207d51a16..f4b13be019 100644
--- a/src/lib/database/db_log.h
+++ b/src/lib/database/db_log.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2018-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2024 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/lib/database/db_messages.mes b/src/lib/database/db_messages.mes
index 3f4c09ea82..c9c7e7ea68 100644
--- a/src/lib/database/db_messages.mes
+++ b/src/lib/database/db_messages.mes
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2023 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2024 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/lib/mysql/mysql_connection.cc b/src/lib/mysql/mysql_connection.cc
index 73e465ab79..593865ec45 100644
--- a/src/lib/mysql/mysql_connection.cc
+++ b/src/lib/mysql/mysql_connection.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2024 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
@@ -28,6 +28,8 @@ using namespace std;
 namespace isc {
 namespace db {
 
+std::string MySqlConnection::KEA_ADMIN_ = KEA_ADMIN;
+
 int MySqlHolder::atexit_ = [] {
     return atexit([] { mysql_library_end(); });
 }();
@@ -372,7 +374,7 @@ MySqlConnection::getVersion(const ParameterMap& parameters,
 void
 MySqlConnection::ensureSchemaVersion(const ParameterMap& parameters,
                                      const DbCallback& cb,
-                                     string timer_name) {
+                                     const string& timer_name) {
     // retry-on-startup?
     bool const retry(parameters.count("retry-on-startup") &&
                      parameters.at("retry-on-startup") == "true");
@@ -380,20 +382,11 @@ MySqlConnection::ensureSchemaVersion(const ParameterMap& parameters,
     IOServiceAccessorPtr ac(new IOServiceAccessor(&DatabaseConnection::getIOService));
     pair<uint32_t, uint32_t> schema_version;
     try {
-        // Attempt to get version without retrying or other sophistries. This
-        // provides the most accurate view of the status of the database and the
-        // most flexibility to reacting to errors.
-        schema_version = getVersion(parameters);
+        schema_version = getVersion(parameters, ac, cb, retry ? timer_name : string());
     } catch (DbOpenError const& exception) {
-        // Could not establish a connection. Best thing is to wait for the
-        // database server to come up. Establish he mechanism of retrying.
-        if (retry && !timer_name.empty()) {
-            MySqlConnection conn(parameters, ac, cb);
-            conn.makeReconnectCtl(timer_name);
-            conn.openDatabase();
-        } else {
-            rethrow_exception(current_exception());
-        }
+        throw;
+    } catch (DbOpenErrorWithRetry const& exception) {
+        throw;
     } catch (exception const& exception) {
         // This failure may occur for a variety of reasons. We are looking at
         // initializing schema as the only potential mitigation. We could narrow
@@ -428,13 +421,19 @@ MySqlConnection::initializeSchema(const ParameterMap& parameters) {
         return;
     }
 
+    if (!isc::util::file::isFile(KEA_ADMIN_)) {
+        // It can happen for kea-admin to not exist, especially with
+        // packages that install it in a separate package.
+        return;
+    }
+
     // Convert parameters.
     vector<string> kea_admin_parameters(toKeaAdminParameters(parameters));
     ProcessEnvVars const vars;
     kea_admin_parameters.insert(kea_admin_parameters.begin(), "db-init");
 
     // Run.
-    ProcessSpawn kea_admin(KEA_ADMIN, kea_admin_parameters, vars,
+    ProcessSpawn kea_admin(KEA_ADMIN_, kea_admin_parameters, vars,
                            /* inherit_env = */ true);
     DB_LOG_INFO(MYSQL_INITIALIZE_SCHEMA).arg(kea_admin.getCommandLine());
     pid_t const pid(kea_admin.spawn());
diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h
index 1ad11ccc7f..5a2f81a089 100644
--- a/src/lib/mysql/mysql_connection.h
+++ b/src/lib/mysql/mysql_connection.h
@@ -296,7 +296,7 @@ public:
     static void
     ensureSchemaVersion(const ParameterMap& parameters,
                         const DbCallback& cb = DbCallback(),
-                        std::string timer_name = std::string());
+                        const std::string& timer_name = std::string());
 
     /// @brief Initialize schema.
     ///
@@ -826,6 +826,8 @@ public:
 
     /// @brief TLS flag (true when TLS was required, false otherwise).
     bool tls_;
+
+    static std::string KEA_ADMIN_;
 };
 
 } // end of isc::db namespace
diff --git a/src/lib/mysql/tests/Makefile.am b/src/lib/mysql/tests/Makefile.am
index ce71f4a660..bf6ed1bd83 100644
--- a/src/lib/mysql/tests/Makefile.am
+++ b/src/lib/mysql/tests/Makefile.am
@@ -2,6 +2,7 @@ SUBDIRS = .
 
 AM_CPPFLAGS  =
 AM_CPPFLAGS += -DABS_TOP_BUILDDIR="\"$(abs_top_builddir)\""
+AM_CPPFLAGS += -DKEA_ADMIN=\"${abs_top_builddir}/src/bin/admin/kea-admin\"
 AM_CPPFLAGS += -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES) $(MYSQL_CPPFLAGS)
 
diff --git a/src/lib/mysql/tests/mysql_connection_unittest.cc b/src/lib/mysql/tests/mysql_connection_unittest.cc
index f62ea84edf..69a325a89f 100644
--- a/src/lib/mysql/tests/mysql_connection_unittest.cc
+++ b/src/lib/mysql/tests/mysql_connection_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2018-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2018-2024 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
@@ -78,6 +78,8 @@ public:
     MySqlConnectionTest(bool const primary_key = false)
         : conn_(DatabaseConnection::parse(validMySQLConnectionString())) {
 
+        MySqlConnection::KEA_ADMIN_ = KEA_ADMIN;
+
         try {
             // Open new connection.
             conn_.openDatabase();
@@ -946,7 +948,7 @@ TEST_F(MySqlConnectionTest, ensureSchemaVersion) {
     EXPECT_EQ(MYSQL_SCHEMA_VERSION_MINOR, version.second);
 }
 
-/// @brief Check ensureSchemaVersion when schema is not created.
+/// @brief Check initializeSchema when schema is not created.
 TEST_F(MySqlConnectionTest, initializeSchemaNoSchema) {
     pair<uint32_t, uint32_t> version;
     auto const parameters(DatabaseConnection::parse(validMySQLConnectionString()));
@@ -965,7 +967,7 @@ TEST_F(MySqlConnectionTest, initializeSchemaNoSchema) {
     EXPECT_EQ(MYSQL_SCHEMA_VERSION_MINOR, version.second);
 }
 
-/// @brief Check ensureSchemaVersion when schema is created.
+/// @brief Check initializeSchema when schema is created.
 TEST_F(MySqlConnectionTest, initializeSchema) {
     pair<uint32_t, uint32_t> version;
     auto const parameters(DatabaseConnection::parse(validMySQLConnectionString()));
@@ -983,7 +985,7 @@ TEST_F(MySqlConnectionTest, initializeSchema) {
     EXPECT_EQ(MYSQL_SCHEMA_VERSION_MINOR, version.second);
 }
 
-/// @brief Check ensureSchemaVersion when schema is created.
+/// @brief Check toKeaAdminParameters.
 TEST_F(MySqlConnectionTest, toKeaAdminParameters) {
     auto parameters(DatabaseConnection::parse(validMySQLConnectionString()));
     vector<string> kea_admin_parameters(MySqlConnection::toKeaAdminParameters(parameters));
diff --git a/src/lib/pgsql/pgsql_connection.cc b/src/lib/pgsql/pgsql_connection.cc
index 640eccc3eb..accef17ed5 100644
--- a/src/lib/pgsql/pgsql_connection.cc
+++ b/src/lib/pgsql/pgsql_connection.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2016-2023 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2024 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,6 +12,7 @@
 #include <database/db_exceptions.h>
 #include <database/db_log.h>
 #include <pgsql/pgsql_connection.h>
+#include <util/file_utilities.h>
 
 #include <exception>
 
@@ -39,6 +40,8 @@ using namespace std;
 namespace isc {
 namespace db {
 
+std::string PgSqlConnection::KEA_ADMIN_ = KEA_ADMIN;
+
 // Default connection timeout
 
 /// @todo: migrate this default timeout to src/bin/dhcpX/simple_parserX.cc
@@ -173,7 +176,7 @@ PgSqlConnection::getVersion(const ParameterMap& parameters,
 void
 PgSqlConnection::ensureSchemaVersion(const ParameterMap& parameters,
                                      const DbCallback& cb,
-                                     string timer_name) {
+                                     const string& timer_name) {
     // retry-on-startup?
     bool const retry(parameters.count("retry-on-startup") &&
                      parameters.at("retry-on-startup") == "true");
@@ -184,17 +187,11 @@ PgSqlConnection::ensureSchemaVersion(const ParameterMap& parameters,
         // Attempt to get version without retrying or other sophistries. This
         // provides the most accurate view of the status of the database and the
         // most flexibility to reacting to errors.
-        schema_version = getVersion(parameters);
+        schema_version = getVersion(parameters, ac, cb, retry ? timer_name : string());
     } catch (DbOpenError const& exception) {
-        // Could not establish a connection. Best thing is to wait for the
-        // database server to come up. Establish he mechanism of retrying.
-        if (retry && !timer_name.empty()) {
-            PgSqlConnection conn(parameters, ac, cb);
-            conn.makeReconnectCtl(timer_name);
-            conn.openDatabaseInternal(false);
-        } else {
-            rethrow_exception(current_exception());
-        }
+        throw;
+    } catch (DbOpenErrorWithRetry const& exception) {
+        throw;
     } catch (exception const& exception) {
         // This failure may occur for a variety of reasons. We are looking at
         // initializing schema as the only potential mitigation. We could narrow
@@ -229,6 +226,12 @@ PgSqlConnection::initializeSchema(const ParameterMap& parameters) {
         return;
     }
 
+    if (!isc::util::file::isFile(KEA_ADMIN_)) {
+        // It can happen for kea-admin to not exist, especially with
+        // packages that install it in a separate package.
+        return;
+    }
+
     // Convert parameters.
     auto const tupl(toKeaAdminParameters(parameters));
     vector<string> kea_admin_parameters(get<0>(tupl));
@@ -236,7 +239,7 @@ PgSqlConnection::initializeSchema(const ParameterMap& parameters) {
     kea_admin_parameters.insert(kea_admin_parameters.begin(), "db-init");
 
     // Run.
-    ProcessSpawn kea_admin(KEA_ADMIN, kea_admin_parameters, vars,
+    ProcessSpawn kea_admin(KEA_ADMIN_, kea_admin_parameters, vars,
                            /* inherit_env = */ true);
     DB_LOG_INFO(PGSQL_INITIALIZE_SCHEMA).arg(kea_admin.getCommandLine());
     pid_t const pid(kea_admin.spawn());
diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h
index 1e763ac02b..961823c946 100644
--- a/src/lib/pgsql/pgsql_connection.h
+++ b/src/lib/pgsql/pgsql_connection.h
@@ -226,9 +226,9 @@ public:
     /// @brief Destructor
     virtual ~PgSqlConnection();
 
-    /// @brief Convert MySQL library parameters to kea-admin parameters.
+    /// @brief Convert PostgreSQL library parameters to kea-admin parameters.
     ///
-    /// @param params input MySQL parameters
+    /// @param params input PostgreSQL parameters
     ///
     /// @return tuple of (vector of kea-admin parameters, vector of PostgreSQL
     /// environment variables)
@@ -267,7 +267,7 @@ public:
     static void
     ensureSchemaVersion(const ParameterMap& parameters,
                         const DbCallback& cb = DbCallback(),
-                        std::string timer_name = std::string());
+                        const std::string& timer_name = std::string());
 
     /// @brief Initialize schema.
     ///
@@ -625,6 +625,8 @@ public:
     /// ongoing transaction. We do not want to start new transactions when one
     /// is already in progress.
     int transaction_ref_count_;
+
+    static std::string KEA_ADMIN_;
 };
 
 /// @brief Defines a pointer to a PgSqlConnection
diff --git a/src/lib/pgsql/tests/Makefile.am b/src/lib/pgsql/tests/Makefile.am
index 969e01d0e7..ac4cfc77c9 100644
--- a/src/lib/pgsql/tests/Makefile.am
+++ b/src/lib/pgsql/tests/Makefile.am
@@ -2,6 +2,7 @@ SUBDIRS = .
 
 AM_CPPFLAGS  =
 AM_CPPFLAGS += -DABS_TOP_BUILDDIR="\"$(abs_top_builddir)\""
+AM_CPPFLAGS += -DKEA_ADMIN=\"${abs_top_builddir}/src/bin/admin/kea-admin\"
 AM_CPPFLAGS += -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES) $(PGSQL_CPPFLAGS)
 
diff --git a/src/lib/pgsql/tests/pgsql_connection_unittest.cc b/src/lib/pgsql/tests/pgsql_connection_unittest.cc
index 3bed93e85d..e1660e1e51 100644
--- a/src/lib/pgsql/tests/pgsql_connection_unittest.cc
+++ b/src/lib/pgsql/tests/pgsql_connection_unittest.cc
@@ -144,7 +144,9 @@ public:
     typedef std::vector<TestRow> TestRowSet;
 
     /// @brief Constructor.
-    PgSqlConnectionTest() : PgSqlBasicsTest() {};
+    PgSqlConnectionTest() : PgSqlBasicsTest() {
+        PgSqlConnection::KEA_ADMIN_ = KEA_ADMIN;
+    };
 
     /// @brief Destructor.
     virtual ~PgSqlConnectionTest() {
@@ -687,7 +689,7 @@ TEST_F(PgSqlConnectionTest, ensureSchemaVersion) {
     EXPECT_EQ(PGSQL_SCHEMA_VERSION_MINOR, version.second);
 }
 
-/// @brief Check ensureSchemaVersion when schema is not created.
+/// @brief Check initializeSchema when schema is not created.
 TEST_F(PgSqlConnectionTest, initializeSchemaNoSchema) {
     pair<uint32_t, uint32_t> version;
     auto const parameters(DatabaseConnection::parse(validPgSQLConnectionString()));
@@ -708,7 +710,7 @@ TEST_F(PgSqlConnectionTest, initializeSchemaNoSchema) {
     EXPECT_EQ(PGSQL_SCHEMA_VERSION_MINOR, version.second);
 }
 
-/// @brief Check ensureSchemaVersion when schema is created.
+/// @brief Check initializeSchema when schema is created.
 TEST_F(PgSqlConnectionTest, initializeSchema) {
     pair<uint32_t, uint32_t> version;
     auto const parameters(DatabaseConnection::parse(validPgSQLConnectionString()));
@@ -726,7 +728,7 @@ TEST_F(PgSqlConnectionTest, initializeSchema) {
     EXPECT_EQ(PGSQL_SCHEMA_VERSION_MINOR, version.second);
 }
 
-/// @brief Check ensureSchemaVersion when schema is created.
+/// @brief Check toKeaAdminParameters.
 TEST_F(PgSqlConnectionTest, toKeaAdminParameters) {
     auto parameters(DatabaseConnection::parse(validPgSQLConnectionString()));
     auto tupl(PgSqlConnection::toKeaAdminParameters(parameters));