[kea5574] Checkpoint: dhcpsrv lib first attempt

src/lib/dhcpsrv/cfg_db_access.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2018 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
@@ -38,15 +38,15 @@ CfgDbAccess::getHostDbAccessString() const {
 
 
 void
-CfgDbAccess::createManagers(DatabaseConnection::DbLostCallback db_lost_callback) const {
+CfgDbAccess::createManagers() const {
     // Recreate lease manager.
     LeaseMgrFactory::destroy();
-    LeaseMgrFactory::create(getLeaseDbAccessString(), db_lost_callback);
+    LeaseMgrFactory::create(getLeaseDbAccessString());
 
     // Recreate host data source.
     HostDataSourceFactory::destroy();
     if (!host_db_access_.empty()) {
-        HostMgr::create(getHostDbAccessString(), db_lost_callback);
+        HostMgr::create(getHostDbAccessString());
     }
 }
 

src/lib/dhcpsrv/cfg_db_access.h

@@ -64,11 +64,7 @@ public:
 
     /// @brief Creates instance of lease manager and host data source
     /// according to the configuration specified.
-    ///
+    void createManagers() const;
-    /// @param db_lost_callback function to invoke if connectivity to
-    /// either the lease or host managers, once established, is subsequently
-    /// lost.
-    void createManagers(DatabaseConnection::DbLostCallback db_lost_callback = 0) const;
 
     /// @brief Unparse an access string
     ///

src/lib/dhcpsrv/database_connection.cc

@@ -111,38 +111,49 @@ DatabaseConnection::configuredReadOnly() const {
 ReconnectCtlPtr
 DatabaseConnection::makeReconnectCtl() const {
     ReconnectCtlPtr retry;
+    string name = "unknown";
     unsigned int retries = 0;
     unsigned int interval = 0;
 
     // Assumes that parsing ensurse only valid values are present
+    try {
+        name = getParameter("type");
+    } catch (...) {
+        // Wasn't specified so we'll use default of "unknown".
+    }
+
     std::string parm_str;
     try {
         parm_str = getParameter("max-reconnect-tries");
         retries = boost::lexical_cast<unsigned int>(parm_str);
     } catch (...) {
-        // Wasn't specified so  so we'll use default of 0;
+        // Wasn't specified so we'll use default of 0;
     }
 
     try {
         parm_str = getParameter("reconnect-wait-time");
         interval = boost::lexical_cast<unsigned int>(parm_str);
     } catch (...) {
-        // Wasn't specified so  so we'll use default of 0;
+        // Wasn't specified so we'll use default of 0;
     }
 
-    retry.reset(new ReconnectCtl(retries, interval));
+    retry.reset(new ReconnectCtl(name, retries, interval));
     return (retry);
 }
 
 bool
 DatabaseConnection::invokeDbLostCallback() const {
-    if (db_lost_callback_ != NULL) {
+    if (DatabaseConnection::db_lost_callback) {
         // Invoke the callback, passing in a new instance of ReconnectCtl
-        return (db_lost_callback_)(makeReconnectCtl());
+        return (DatabaseConnection::db_lost_callback)(makeReconnectCtl());
     }
 
     return (false);
 }
 
+
+DatabaseConnection::DbLostCallback
+DatabaseConnection::db_lost_callback = 0;
+
 };
 };

src/lib/dhcpsrv/database_connection.h

@@ -75,11 +75,18 @@ public:
 class ReconnectCtl {
 public:
     /// @brief Constructor
+    /// @param backend_name name of the caller backend.
     /// @param max_retries maximum number of reconnect attempts to make
     /// @param retry_interval amount of time to between reconnect attempts
-    ReconnectCtl(unsigned int max_retries, unsigned int retry_interval)
+    ReconnectCtl(const std::string& backend_name, unsigned int max_retries,
-        : max_retries_(max_retries), retries_left_(max_retries), 
+                 unsigned int retry_interval)
-          retry_interval_(retry_interval) {}
+        : backend_name_(backend_name), max_retries_(max_retries),
+          retries_left_(max_retries), retry_interval_(retry_interval) {}
+
+    /// @brief Returns the name of the caller backend.
+    std::string backendName() const {
+        return (backend_name_);
+    }
 
     /// @brief Decrements the number of retries remaining
     ///
@@ -105,6 +112,9 @@ public:
     }
 
 private:
+    /// @brief Caller backend name.
+    const std::string backend_name_;
+
     /// @brief Maximum number of retry attempts to make
     unsigned int max_retries_;
 
@@ -140,18 +150,12 @@ public:
     /// @brief Database configuration parameter map
     typedef std::map<std::string, std::string> ParameterMap;
 
-    /// @brief Defines a callback prototype for propogating events upward
-    typedef boost::function<bool (ReconnectCtlPtr db_retry)> DbLostCallback;
-
     /// @brief Constructor
     ///
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
-    /// @param db_lost_callback  Optional call back function to invoke if a
+    DatabaseConnection(const ParameterMap& parameters)
-    ///        successfully open connection subsequently fails
+        :parameters_(parameters) {
-    DatabaseConnection(const ParameterMap& parameters,
-        DbLostCallback db_lost_callback = 0)
-        :parameters_(parameters), db_lost_callback_(db_lost_callback) {
     }
 
     /// @brief Destructor
@@ -197,6 +201,9 @@ public:
     /// and set to false.
     bool configuredReadOnly() const;
 
+    /// @brief Defines a callback prototype for propogating events upward
+    typedef boost::function<bool (ReconnectCtlPtr db_retry)> DbLostCallback;
+
     /// @brief Invokes the connection's lost connectivity callback
     ///
     /// This function may be called by derivations when the connectivity
@@ -208,6 +215,10 @@ public:
     /// callback.
     bool invokeDbLostCallback() const;
 
+    /// @brief Optional call back function to invoke if a successfully
+    /// open connection subsequently fails
+    static DbLostCallback db_lost_callback;
+
 private:
 
     /// @brief List of parameters passed in dbconfig
@@ -217,10 +228,6 @@ private:
     /// intended to keep any DHCP-related parameters.
     ParameterMap parameters_;
 
-protected:
-
-    /// @brief Optional function to invoke if the connectivity is lost
-    DbLostCallback db_lost_callback_;
 };
 
 }; // end of isc::dhcp namespace

src/lib/dhcpsrv/host_data_source_factory.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2018 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,8 +45,7 @@ HostDataSourceFactory::getHostDataSourcePtr() {
 }
 
 void
-HostDataSourceFactory::create(const std::string& dbaccess,
+HostDataSourceFactory::create(const std::string& dbaccess) {
-                              DatabaseConnection::DbLostCallback db_lost_callback) {
     // Parse the access string and create a redacted string for logging.
     DatabaseConnection::ParameterMap parameters =
             DatabaseConnection::parse(dbaccess);
@@ -73,8 +72,7 @@ HostDataSourceFactory::create(const std::string& dbaccess,
     if (db_type == "postgresql") {
         LOG_INFO(dhcpsrv_logger, DHCPSRV_PGSQL_HOST_DB)
             .arg(DatabaseConnection::redactedAccessString(parameters));
-        getHostDataSourcePtr().reset(new PgSqlHostDataSource(parameters,
+        getHostDataSourcePtr().reset(new PgSqlHostDataSource(parameters));
-                                     db_lost_callback));
         return;
     }
 #endif
@@ -104,17 +102,5 @@ HostDataSourceFactory::destroy() {
     getHostDataSourcePtr().reset();
 }
 
-#if 0
-BaseHostDataSource&
-HostDataSourceFactory::instance() {
-    BaseHostDataSource* hdsptr = getHostDataSourcePtr().get();
-    if (hdsptr == NULL) {
-        isc_throw(NoHostDataSourceManager,
-                "no current host data source instance is available");
-    }
-    return (*hdsptr);
-}
-#endif
-
 }  // namespace dhcp
 }  // namespace isc

src/lib/dhcpsrv/host_data_source_factory.h

@@ -59,15 +59,11 @@ public:
     ///        -end specific, although must include the "type" keyword which
     ///        gives the backend in use.
     ///
-    /// @param db_lost_callback function to invoke if connectivity to host
-    /// data source is lost.
-    ///
     /// @throw isc::InvalidParameter dbaccess string does not contain the "type"
     ///        keyword.
     /// @throw isc::dhcp::InvalidType The "type" keyword in dbaccess does not
     ///        identify a supported backend.
-    static void create(const std::string& dbaccess,
+    static void create(const std::string& dbaccess);
-                       DatabaseConnection::DbLostCallback db_lost_callback = 0);
 
     /// @brief Destroy host data source
     ///

src/lib/dhcpsrv/host_mgr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2018 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
@@ -37,23 +37,22 @@ HostMgr::getHostMgrPtr() {
 }
 
 void
-HostMgr::create(const std::string& access,
+HostMgr::create(const std::string& access) {
-                DatabaseConnection::DbLostCallback db_lost_callback) {
     getHostMgrPtr().reset(new HostMgr());
 
     if (!access.empty()) {
         // If the user specified parameters, let's pass them to the create
         // method. It will destroy any prior instances and will create
         // the new one.
-        HostDataSourceFactory::create(access, db_lost_callback);
+        HostDataSourceFactory::create(access);
     } else {
         // Ok, no parameters were specified. We should destroy the existing
         // instance.
         HostDataSourceFactory::destroy();
     }
 
-    // Now store the host data source pointer. It may be NULL. That's ok as
+    // Now store the host data source pointer. It may be null. That's ok as
-    // NULL value indicates that there's no host data source configured.
+    // null value indicates that there's no host data source configured.
     getHostMgrPtr()->alternate_source_ =
         HostDataSourceFactory::getHostDataSourcePtr();
 }

src/lib/dhcpsrv/host_mgr.h

@@ -69,11 +69,7 @@ public:
     /// However, the "type" parameter will be common and it will specify which
     /// data source is to be used. Currently, no parameters are supported
     /// and the parameter is ignored.
-    ///
+    static void create(const std::string& access = "");
-    /// @param db_lost_callback function to invoke if connectivity to
-    /// the host database is lost.
-    static void create(const std::string& access = "",
-                       DatabaseConnection::DbLostCallback db_lost_callback = 0);
 
     /// @brief Returns a sole instance of the @c HostMgr.
     ///

src/lib/dhcpsrv/lease_mgr_factory.cc

@@ -41,8 +41,7 @@ LeaseMgrFactory::getLeaseMgrPtr() {
 }
 
 void
-LeaseMgrFactory::create(const std::string& dbaccess,
+LeaseMgrFactory::create(const std::string& dbaccess) {
-                        DatabaseConnection::DbLostCallback db_lost_callback) {
     const std::string type = "type";
 
     // Parse the access string and create a redacted string for logging.
@@ -68,7 +67,7 @@ LeaseMgrFactory::create(const std::string& dbaccess,
 #ifdef HAVE_PGSQL
     if (parameters[type] == string("postgresql")) {
         LOG_INFO(dhcpsrv_logger, DHCPSRV_PGSQL_DB).arg(redacted);
-        getLeaseMgrPtr().reset(new PgSqlLeaseMgr(parameters, db_lost_callback));
+        getLeaseMgrPtr().reset(new PgSqlLeaseMgr(parameters));
         return;
     }
 #endif

src/lib/dhcpsrv/lease_mgr_factory.h

@@ -63,15 +63,11 @@ public:
     ///        -end specific, although must include the "type" keyword which
     ///        gives the backend in use.
     ///
-    /// @param db_lost_callback function to invoke if connectivity to lease
-    /// database is lost.
-    ///
     /// @throw isc::InvalidParameter dbaccess string does not contain the "type"
     ///        keyword.
     /// @throw isc::dhcp::InvalidType The "type" keyword in dbaccess does not
     ///        identify a supported backend.
-    static void create(const std::string& dbaccess, 
+    static void create(const std::string& dbaccess);
-                       DatabaseConnection::DbLostCallback db_lost_callback = 0);
 
     /// @brief Destroy lease manager
     ///

src/lib/dhcpsrv/pgsql_connection.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2018 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
@@ -304,11 +304,6 @@ public:
         : DatabaseConnection(parameters) {
     }
 
-    PgSqlConnection(const ParameterMap& parameters, 
-                    DbLostCallback db_lost_callback)
-        : DatabaseConnection(parameters, db_lost_callback) {
-    }
-
     /// @brief Destructor
     virtual ~PgSqlConnection();
 

src/lib/dhcpsrv/pgsql_host_data_source.cc

@@ -1281,8 +1281,7 @@ public:
     ///
     /// This constructor opens database connection and initializes prepared
     /// statements used in the queries.
-    PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters,
+    PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters);
-                            DatabaseConnection::DbLostCallback db_lost_callback);
 
     /// @brief Destructor.
     ~PgSqlHostDataSourceImpl();
@@ -1697,15 +1696,14 @@ TaggedStatementArray tagged_statements = { {
 }; // end anonymous namespace
 
 PgSqlHostDataSourceImpl::
-PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters,
+PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters)
-                        DatabaseConnection::DbLostCallback db_lost_callback)
     : host_exchange_(new PgSqlHostWithOptionsExchange(PgSqlHostWithOptionsExchange::DHCP4_ONLY)),
       host_ipv6_exchange_(new PgSqlHostIPv6Exchange(PgSqlHostWithOptionsExchange::DHCP6_ONLY)),
       host_ipv46_exchange_(new PgSqlHostIPv6Exchange(PgSqlHostWithOptionsExchange::
                                                      DHCP4_AND_DHCP6)),
       host_ipv6_reservation_exchange_(new PgSqlIPv6ReservationExchange()),
       host_option_exchange_(new PgSqlOptionExchange()),
-      conn_(parameters, db_lost_callback),
+      conn_(parameters),
       is_readonly_(false) {
 
     // Open the database.
@@ -1922,9 +1920,8 @@ PgSqlHostDataSourceImpl::checkReadOnly() const {
 /*********** PgSqlHostDataSource *********************/
 
 PgSqlHostDataSource::
-PgSqlHostDataSource(const PgSqlConnection::ParameterMap& parameters,
+PgSqlHostDataSource(const PgSqlConnection::ParameterMap& parameters)
-                    DatabaseConnection::DbLostCallback db_lost_callback)
+    : impl_(new PgSqlHostDataSourceImpl(parameters)) {
-    : impl_(new PgSqlHostDataSourceImpl(parameters, db_lost_callback)) {
 }
 
 PgSqlHostDataSource::~PgSqlHostDataSource() {

src/lib/dhcpsrv/pgsql_host_data_source.h

@@ -53,15 +53,11 @@ public:
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
     ///
-    /// @param db_lost_callback function to invoke if connectivity to
-    /// to host database is lost.
-    ///
     /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given
     /// @throw isc::dhcp::DbOpenError Error opening the database
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
-    PgSqlHostDataSource(const DatabaseConnection::ParameterMap& parameters,
+   PgSqlHostDataSource(const DatabaseConnection::ParameterMap& parameters);
-                        DatabaseConnection::DbLostCallback db_lost_callback = 0);
 
     /// @brief Virtual destructor.
     /// Frees database resources and closes the database connection through

src/lib/dhcpsrv/pgsql_lease_mgr.cc

@@ -887,10 +887,9 @@ protected:
     bool fetch_type_;
 };
 
-PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters,
+PgSqlLeaseMgr::PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters)
-    DatabaseConnection::DbLostCallback db_lost_callback)
     : LeaseMgr(), exchange4_(new PgSqlLease4Exchange()),
-    exchange6_(new PgSqlLease6Exchange()), conn_(parameters, db_lost_callback) {
+    exchange6_(new PgSqlLease6Exchange()), conn_(parameters) {
     conn_.openDatabase();
     int i = 0;
     for( ; tagged_statements[i].text != NULL ; ++i) {

src/lib/dhcpsrv/pgsql_lease_mgr.h

@@ -51,15 +51,11 @@ public:
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
     ///
-    /// @param db_lost_callback function to invoke if connectivity to
-    /// to lease database is lost.
-    ///
     /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given
     /// @throw isc::dhcp::DbOpenError Error opening the database
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
-    PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters,
+    PgSqlLeaseMgr(const DatabaseConnection::ParameterMap& parameters);
-                  DatabaseConnection::DbLostCallback db_lost_callback = 0);
 
     /// @brief Destructor (closes database)
     virtual ~PgSqlLeaseMgr();

src/lib/dhcpsrv/tests/database_connection_unittest.cc

@@ -38,8 +38,6 @@ public:
     ReconnectCtlPtr db_reconnect_ctl_;
 };
 
-
-
 /// @brief getParameter test
 ///
 /// This test checks if the LeaseMgr can be instantiated and that it
@@ -63,6 +61,7 @@ TEST(DatabaseConnectionTest, getParameter) {
 /// DbLostCallback.
 TEST_F(DatabaseConnectionCallbackTest, NoDbLostCallback) {
     DatabaseConnection::ParameterMap pmap;
+    pmap[std::string("type")] = std::string("test");
     pmap[std::string("max-reconnect-tries")] = std::string("3");
     pmap[std::string("reconnect-wait-time")] = std::string("60");
     DatabaseConnection datasrc(pmap);
@@ -73,8 +72,7 @@ TEST_F(DatabaseConnectionCallbackTest, NoDbLostCallback) {
     EXPECT_FALSE(db_reconnect_ctl_);
 }
 
-
+/// @brief dbLostCallback
-/// @brief NoDbLostCallback
 ///
 /// This test verifies that DatabaseConnection::invokeDbLostCallback
 /// safely invokes the registered DbLostCallback.  It also tests
@@ -83,12 +81,15 @@ TEST_F(DatabaseConnectionCallbackTest, dbLostCallback) {
     /// Create a Database configuration that includes the reconnect
     /// control parameters.
     DatabaseConnection::ParameterMap pmap;
+    pmap[std::string("type")] = std::string("test");
     pmap[std::string("max-reconnect-tries")] = std::string("3");
     pmap[std::string("reconnect-wait-time")] = std::string("60");
 
-    /// Create the connection with a DbLostCallback.
+    /// Install the callback.
-    DatabaseConnection datasrc(pmap, boost::bind(&DatabaseConnectionCallbackTest
+    DatabaseConnection::db_lost_callback =
-                                                 ::dbLostCallback, this, _1));
+        boost::bind(&DatabaseConnectionCallbackTest::dbLostCallback, this, _1);
+    /// Create the connection..
+    DatabaseConnection datasrc(pmap);
 
     /// We should be able to invoke the callback and glean
     /// the correct reconnect contorl parameters from it.
@@ -96,6 +97,7 @@ TEST_F(DatabaseConnectionCallbackTest, dbLostCallback) {
     ASSERT_NO_THROW(ret = datasrc.invokeDbLostCallback());
     EXPECT_TRUE(ret);
     ASSERT_TRUE(db_reconnect_ctl_);
+    ASSERT_EQ("test", db_reconnect_ctl_->backendName());
     ASSERT_EQ(3, db_reconnect_ctl_->maxRetries());
     ASSERT_EQ(3, db_reconnect_ctl_->retriesLeft());
     EXPECT_EQ(60, db_reconnect_ctl_->retryInterval());
@@ -114,7 +116,6 @@ TEST_F(DatabaseConnectionCallbackTest, dbLostCallback) {
     EXPECT_EQ(3, db_reconnect_ctl_->maxRetries());
 }
 
-
 // This test checks that a database access string can be parsed correctly.
 TEST(DatabaseConnectionTest, parse) {
 

src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2018 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
@@ -247,9 +247,10 @@ TEST(PgSqlHostDataSource, NoCallbackOnOpenFail) {
     createPgSQLSchema();
 
     callback_called = false;
+    DatabaseConnection::db_lost_callback = db_lost_callback;
     EXPECT_THROW(HostDataSourceFactory::create(connectionString(
-        PGSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD),
+        PGSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD)),
-        db_lost_callback), DbOpenError);
+                 DbOpenError);
 
     EXPECT_FALSE(callback_called);
     destroyPgSQLSchema();

src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc

@@ -216,9 +216,9 @@ TEST(PgSqlOpenTest, NoCallbackOnOpenFail) {
     createPgSQLSchema();
 
     callback_called = false;
+    DatabaseConnection::db_lost_callback = db_lost_callback;
     EXPECT_THROW(LeaseMgrFactory::create(connectionString(
-        PGSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD),
+        PGSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD)),
-        db_lost_callback),
         DbOpenError);
     EXPECT_FALSE(callback_called);