[3669] Refuse to load leases from a file when the LFC is in progress.

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -17,6 +17,7 @@
 #include <dhcpsrv/lease_file_loader.h>
 #include <dhcpsrv/memfile_lease_mgr.h>
 #include <exceptions/exceptions.h>
+#include <util/pid_file.h>
 #include <util/process_spawn.h>
 #include <cstdio>
 #include <iostream>
@@ -675,6 +676,18 @@ template<typename LeaseObjectType, typename LeaseFileType, typename StorageType>
 void Memfile_LeaseMgr::loadLeasesFromFiles(const std::string& filename,
                                            boost::shared_ptr<LeaseFileType>& lease_file,
                                            StorageType& storage) {
+    // Check if the instance of the LFC is running right now. If it is
+    // running, we refuse to load leases as the LFC may be writing to the
+    // lease files right now. When the user retries server configuration
+    // it should go through.
+    /// @todo Consider applying a timeout for an LFC and retry when this
+    /// timeout elapses.
+    PIDFile pid_file(appendSuffix(filename, FILE_PID));
+    if (pid_file.check()) {
+        isc_throw(DbOpenError, "unable to load leases from files while the "
+                  "lease file cleanup is in progress");
+    }
+
     storage.clear();
 
     // Load the leasefile.completed, if exists.

src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -344,6 +344,7 @@ public:
     /// - Previous File: ".2"
     /// - LFC Output File: ".output"
     /// - LFC Finish File: ".completed"
+    /// - LFC PID File: ".pid"
     ///
     /// See http://kea.isc.org/wiki/LFCDesign for details.
     ///
@@ -439,6 +440,28 @@ private:
     /// argument to this function.
     std::string initLeaseFilePath(Universe u);
 
+    /// @brief Performs a lease file cleanup for DHCPv4 or DHCPv6.
+    ///
+    /// This method performs all the actions necessary to prepare for the
+    /// execution of the LFC and if these actions are sucessful, it executes
+    /// the @c kea-lfc application as a background process to process (cleanup)
+    /// the lease files.
+    ///
+    /// For the design and the terminology used in this description refer to
+    /// the http://kea.isc.org/wiki/LFCDesign.
+    ///
+    /// If the method finds that the Lease File Copy exists it simply runs
+    /// the @c kea-lfc application.
+    ///
+    /// If the Lease File Copy doesn't exist it moves the Current Lease File
+    /// to Lease File Copy, and then recreates the Current Lease File without
+    /// any lease entries. If the file has been successfully moved, it runs
+    /// the @c kea-lfc application.
+    ///
+    /// @param lease_file A pointer to the object representing the Current
+    /// Lease File (DHCPv4 or DHCPv6 lease file).
+    ///
+    /// @tparam LeaseFileType One of @c CSVLeaseFile4 or @c CSVLeaseFile6.
     template<typename LeaseFileType>
     void leaseFileCleanup(boost::shared_ptr<LeaseFileType>& lease_file);
 
@@ -467,6 +490,21 @@ private:
     /// products of the lease file cleanups (LFC).
     /// See: http://kea.isc.org/wiki/LFCDesign for details.
     ///
+    /// @note: When the server starts up or is reconfigured it will try to
+    /// read leases from the lease files using this method. It is possible
+    /// that the Lease File Cleanup is performed upon the lease files to
+    /// be read by this method. This may result in conflicts between the
+    /// server process and the LFC. To prevent it, the method checks if the
+    /// instance of the @c kea-lfc is running (using the PID file) before it
+    /// tries to load leases from the lease files. If it finds that there
+    /// is an LFC in progress, it throws an exception which will result
+    /// in the server refuse to start or reconfigure. When the administrator
+    /// retries starting up or reconfiguring the server it will most likely
+    /// be successful as the LFC should be complete by that time.
+    ///
+    /// @todo Consider implementing delaying the lease files loading when
+    /// the LFC is in progress by the specified amount of time.
+    ///
     /// @param filename Name of the lease file.
     /// @param lease_file An object representing a lease file to which
     /// the server will store lease updates.
@@ -476,6 +514,7 @@ private:
     /// @tparam StorageType @c Lease4Storage or @c Lease6Storage.
     ///
     /// @throw CSVFileError when parsing any of the lease files fails.
+    /// @throw DbOpenError when it is found that the LFC is in progress.
     template<typename LeaseObjectType, typename LeaseFileType,
              typename StorageType>
     void loadLeasesFromFiles(const std::string& filename,

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -23,6 +23,7 @@
 #include <dhcpsrv/tests/lease_file_io.h>
 #include <dhcpsrv/tests/test_utils.h>
 #include <dhcpsrv/tests/generic_lease_mgr_unittest.h>
+#include <util/pid_file.h>
 #include <gtest/gtest.h>
 
 #include <boost/bind.hpp>
@@ -39,6 +40,7 @@ using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
+using namespace isc::util;
 
 namespace {
 
@@ -992,6 +994,33 @@ TEST_F(MemfileLeaseMgrTest, load4CompletedFile) {
     EXPECT_FALSE(lmptr_->getLease4(IOAddress("192.0.2.1")));
 }
 
+// This test checks that backend constructor refuses to load leases from the
+// lease files if the LFC is in progress.
+TEST_F(MemfileLeaseMgrTest, load4LFCInProgress) {
+    // Create the backend configuration.
+    LeaseMgr::ParameterMap pmap;
+    pmap["type"] = "memfile";
+    pmap["universe"] = "4";
+    pmap["name"] = getLeaseFilePath("leasefile4_0.csv");
+    pmap["lfc-interval"] = "1";
+
+    // Create a pid file holding the PID of the current process. Choosing the
+    // pid of the current process guarantees that when the backend starts up
+    // the process is alive.
+    PIDFile pid_file(Memfile_LeaseMgr::appendSuffix(pmap["name"], Memfile_LeaseMgr::FILE_PID));
+    pid_file.write();
+
+    // There is a pid file and the process which pid is in the file is
+    // running, so the backend should refuse to start.
+    boost::scoped_ptr<NakedMemfileLeaseMgr> lease_mgr;
+    ASSERT_THROW(lease_mgr.reset(new NakedMemfileLeaseMgr(pmap)),
+                 DbOpenError);
+
+    // Remove the pid file, and retry. The bakckend should be created.
+    pid_file.deleteFile();
+    ASSERT_NO_THROW(lease_mgr.reset(new NakedMemfileLeaseMgr(pmap)));
+}
+
 // This test checks that the backend reads DHCPv6 lease data from multiple
 // files.
 TEST_F(MemfileLeaseMgrTest, load6MultipleLeaseFiles) {
@@ -1204,4 +1233,31 @@ TEST_F(MemfileLeaseMgrTest, load6CompletedFile) {
     EXPECT_FALSE(lmptr_->getLease6(Lease::TYPE_NA, IOAddress("2001:db8:1::3")));
 }
 
+// This test checks that backend constructor refuses to load leases from the
+// lease files if the LFC is in progress.
+TEST_F(MemfileLeaseMgrTest, load6LFCInProgress) {
+    // Create the backend configuration.
+    LeaseMgr::ParameterMap pmap;
+    pmap["type"] = "memfile";
+    pmap["universe"] = "6";
+    pmap["name"] = getLeaseFilePath("leasefile6_0.csv");
+    pmap["lfc-interval"] = "1";
+
+    // Create a pid file holding the PID of the current process. Choosing the
+    // pid of the current process guarantees that when the backend starts up
+    // the process is alive.
+    PIDFile pid_file(Memfile_LeaseMgr::appendSuffix(pmap["name"], Memfile_LeaseMgr::FILE_PID));
+    pid_file.write();
+
+    // There is a pid file and the process which pid is in the file is
+    // running, so the backend should refuse to start.
+    boost::scoped_ptr<NakedMemfileLeaseMgr> lease_mgr;
+    ASSERT_THROW(lease_mgr.reset(new NakedMemfileLeaseMgr(pmap)),
+                 DbOpenError);
+
+    // Remove the pid file, and retry. The bakckend should be created.
+    pid_file.deleteFile();
+    ASSERT_NO_THROW(lease_mgr.reset(new NakedMemfileLeaseMgr(pmap)));
+}
+
 }; // end of anonymous namespace