Browse Source

[trac3667] Updates per review comments

Some general cleanup

Update the log messages

Send the log output to syslog rather than the console
Shawn Routhier 10 years ago
parent
commit
b04a332cc9

+ 38 - 14
src/bin/lfc/lfc_controller.cc

@@ -21,6 +21,8 @@
 #include <dhcpsrv/memfile_lease_storage.h>
 #include <dhcpsrv/memfile_lease_storage.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_file_loader.h>
 #include <dhcpsrv/lease_file_loader.h>
+#include <log/logger_manager.h>
+#include <log/logger_name.h>
 #include <config.h>
 #include <config.h>
 
 
 #include <iostream>
 #include <iostream>
@@ -61,11 +63,31 @@ void
 LFCController::launch(int argc, char* argv[], const bool test_mode) {
 LFCController::launch(int argc, char* argv[], const bool test_mode) {
     bool do_rotate = true;
     bool do_rotate = true;
 
 
-    // If we aren't running in test mode initialize the logging
-    // system with the defaults.
-    if (!test_mode)
+    // If we are running in test mode use the environment variables
+    // else use our defaults
+    if (test_mode) {
+        initLogger();
+    }
+    else {
+        OutputOption option;
+        LoggerManager manager;
+
         initLogger(lfc_app_name_, INFO, 0, NULL, false);
         initLogger(lfc_app_name_, INFO, 0, NULL, false);
 
 
+        // Prepare the objects to define the logging specification
+        LoggerSpecification spec(getRootLoggerName(),
+                                 keaLoggerSeverity(INFO),
+                                 keaLoggerDbglevel(0));
+
+        // We simply want the default syslog option
+        option.destination = OutputOption::DEST_SYSLOG;
+
+        // ... and set the destination
+        spec.addOutputOption(option);
+
+        manager.process(spec);
+    }
+
     try {
     try {
         parseArgs(argc, argv);
         parseArgs(argc, argv);
     } catch (const InvalidUsage& ex) {
     } catch (const InvalidUsage& ex) {
@@ -79,7 +101,7 @@ LFCController::launch(int argc, char* argv[], const bool test_mode) {
         setDefaultLoggingOutput(verbose_);
         setDefaultLoggingOutput(verbose_);
     }
     }
 
 
-    LOG_WARN(lfc_logger, LFC_START_MESSAGE);
+    LOG_INFO(lfc_logger, LFC_START);
 
 
     // verify we are the only instance
     // verify we are the only instance
     PIDFile pid_file(pid_file_);
     PIDFile pid_file(pid_file_);
@@ -87,14 +109,14 @@ LFCController::launch(int argc, char* argv[], const bool test_mode) {
     try {
     try {
         if (pid_file.check()) {
         if (pid_file.check()) {
             // Already running instance, bail out
             // Already running instance, bail out
-            LOG_FATAL(lfc_logger, LFC_RUNNING_MESSAGE);
+            LOG_FATAL(lfc_logger, LFC_RUNNING);
             return;
             return;
         }
         }
 
 
         // create the pid file for this instance
         // create the pid file for this instance
         pid_file.write();
         pid_file.write();
     } catch (const PIDFileError& pid_ex) {
     } catch (const PIDFileError& pid_ex) {
-        LOG_FATAL(lfc_logger, LFC_FAILURE_MESSAGE).arg(pid_ex.what());
+        LOG_FATAL(lfc_logger, LFC_FAIL_PID_CREATE).arg(pid_ex.what());
         return;
         return;
     }
     }
 
 
@@ -103,7 +125,9 @@ LFCController::launch(int argc, char* argv[], const bool test_mode) {
     // all we care about is if it exists so that's okay
     // all we care about is if it exists so that's okay
     CSVFile lf_finish(getFinishFile());
     CSVFile lf_finish(getFinishFile());
     if (!lf_finish.exists()) {
     if (!lf_finish.exists()) {
-        LOG_INFO(lfc_logger, LFC_PROCESSING_MESSAGE);
+        LOG_INFO(lfc_logger, LFC_PROCESSING)
+          .arg(previous_file_)
+          .arg(copy_file_);
 
 
         try {
         try {
             if (getProtocolVersion() == 4) {
             if (getProtocolVersion() == 4) {
@@ -114,7 +138,7 @@ LFCController::launch(int argc, char* argv[], const bool test_mode) {
         } catch (const isc::Exception& proc_ex) {
         } catch (const isc::Exception& proc_ex) {
             // We don't want to do the cleanup but do want to get rid of the pid
             // We don't want to do the cleanup but do want to get rid of the pid
             do_rotate = false;
             do_rotate = false;
-            LOG_FATAL(lfc_logger, LFC_FAILURE_MESSAGE).arg(proc_ex.what());
+            LOG_FATAL(lfc_logger, LFC_FAIL_PROCESS).arg(proc_ex.what());
         }
         }
     }
     }
 
 
@@ -123,12 +147,12 @@ LFCController::launch(int argc, char* argv[], const bool test_mode) {
     // we don't want to return after the catch as we
     // we don't want to return after the catch as we
     // still need to cleanup the pid file
     // still need to cleanup the pid file
     if (do_rotate) {
     if (do_rotate) {
-        LOG_INFO(lfc_logger, LFC_ROTATING_MESSAGE);
+        LOG_INFO(lfc_logger, LFC_ROTATING);
 
 
         try {
         try {
             fileRotate();
             fileRotate();
         } catch (const RunTimeFail& run_ex) {
         } catch (const RunTimeFail& run_ex) {
-          LOG_FATAL(lfc_logger, LFC_FAILURE_MESSAGE).arg(run_ex.what());
+          LOG_FATAL(lfc_logger, LFC_FAIL_ROTATE).arg(run_ex.what());
         }
         }
     }
     }
 
 
@@ -136,10 +160,10 @@ LFCController::launch(int argc, char* argv[], const bool test_mode) {
     try {
     try {
         pid_file.deleteFile();
         pid_file.deleteFile();
     } catch (const PIDFileError& pid_ex) {
     } catch (const PIDFileError& pid_ex) {
-          LOG_FATAL(lfc_logger, LFC_FAILURE_MESSAGE).arg(pid_ex.what());
+          LOG_FATAL(lfc_logger, LFC_FAIL_PID_DEL).arg(pid_ex.what());
     }
     }
 
 
-    LOG_WARN(lfc_logger, LFC_COMPLETE_MESSAGE);
+    LOG_INFO(lfc_logger, LFC_TERMINATE);
 }
 }
 
 
 void
 void
@@ -346,12 +370,12 @@ LFCController::processLeases() const {
     LeaseFileLoader::write<LeaseObjectType>(lf_output, storage);
     LeaseFileLoader::write<LeaseObjectType>(lf_output, storage);
 
 
     // If desired log the stats
     // If desired log the stats
-    LOG_INFO(lfc_logger, LFC_READ_MESSAGE)
+    LOG_INFO(lfc_logger, LFC_READ_STATS)
       .arg(lf_prev.getReadLeases() + lf_copy.getReadLeases())
       .arg(lf_prev.getReadLeases() + lf_copy.getReadLeases())
       .arg(lf_prev.getReads() + lf_copy.getReads())
       .arg(lf_prev.getReads() + lf_copy.getReads())
       .arg(lf_prev.getReadErrs() + lf_copy.getReadErrs());
       .arg(lf_prev.getReadErrs() + lf_copy.getReadErrs());
 
 
-    LOG_INFO(lfc_logger, LFC_WRITE_MESSAGE)
+    LOG_INFO(lfc_logger, LFC_WRITE_STATS)
       .arg(lf_output.getWriteLeases())
       .arg(lf_output.getWriteLeases())
       .arg(lf_output.getWrites())
       .arg(lf_output.getWrites())
       .arg(lf_output.getWriteErrs());
       .arg(lf_output.getWriteErrs());

+ 21 - 9
src/bin/lfc/lfc_messages.mes

@@ -13,35 +13,47 @@
 # PERFORMANCE OF THIS SOFTWARE.
 # PERFORMANCE OF THIS SOFTWARE.
 
 
 $NAMESPACE isc::lfc
 $NAMESPACE isc::lfc
-% LFC_START_MESSAGE Starting lease file cleanup
+% LFC_START Starting lease file cleanup
 This message is issued as the LFC process starts.
 This message is issued as the LFC process starts.
 
 
-% LFC_COMPLETE_MESSAGE LFC complete
+% LFC_TERMINATE LFC finished processing
 This message is issued when the LFC process completes.  It does not
 This message is issued when the LFC process completes.  It does not
 indicate that the process was successful only that it has finished.
 indicate that the process was successful only that it has finished.
 
 
-% LFC_RUNNING_MESSAGE LFC instance already running
+% LFC_RUNNING LFC instance already running
 This message is issued if LFC detects that a previous copy of LFC
 This message is issued if LFC detects that a previous copy of LFC
 may still be running via the PID check.
 may still be running via the PID check.
 
 
-% LFC_PROCESSING_MESSAGE LFC Processing files
+% LFC_PROCESSING Previous file: %1, copy file: %2
 This message is issued just before LFC starts processing the 
 This message is issued just before LFC starts processing the 
 lease files.
 lease files.
 
 
-% LFC_ROTATING_MESSAGE LFC rotating files
+% LFC_ROTATING LFC rotating files
 This message is issued just before LFC starts rotating the
 This message is issued just before LFC starts rotating the
 lease files - removing the old and replacing them with the new.
 lease files - removing the old and replacing them with the new.
 
 
-% LFC_FAILURE_MESSAGE : %1
+% LFC_FAIL_PID_CREATE : %1
 This message is issued if LFC detected a failure when trying
 This message is issued if LFC detected a failure when trying
-to manipulate the files.  It includes a more specifc error string.
+to create the PID file.  It includes a more specifc error string.
 
 
-% LFC_READ_MESSAGE Leases: %1, attempts: %2, errors: %3.
+% LFC_FAIL_PROCESS : %1
+This message is issued if LFC detected a failure when trying
+to process the files.  It includes a more specifc error string.
+
+% LFC_FAIL_ROTATE : %1
+This message is issued if LFC detected a failure when trying
+to rotate the files.  It includes a more specifc error string.
+
+% LFC_FAIL_PID_DEL : %1
+This message is issued if LFC detected a failure when trying
+to delete the PID file.  It includes a more specifc error string.
+
+% LFC_READ_STATS Leases: %1, attempts: %2, errors: %3.
 This message prints out the number of leases that were read, the
 This message prints out the number of leases that were read, the
 number of attempts to read leases and the number of errors
 number of attempts to read leases and the number of errors
 encountered while reading.
 encountered while reading.
 
 
-% LFC_WRITE_MESSAGE Leases: %1, attempts: %2, errors: %3.
+% LFC_WRITE_STATS Leases: %1, attempts: %2, errors: %3.
 This message prints out the number of leases that were written, the
 This message prints out the number of leases that were written, the
 number of attempts to write leases and the number of errors
 number of attempts to write leases and the number of errors
 encountered while writing.
 encountered while writing.

+ 5 - 3
src/lib/dhcpsrv/csv_lease_file4.h

@@ -51,9 +51,11 @@ public:
 
 
     /// @brief Opens a lease file.
     /// @brief Opens a lease file.
     ///
     ///
-    /// This function is used to clear the statistics while
-    /// calling the base class open.  It doesn't throw any
-    /// exceptions of its own but the base class may.
+    /// This function calls the base class open to do the
+    /// work of opening a file.  It is used to clear any
+    /// statistics associated with any previous use of the file
+    /// While it doesn't throw any exceptions of its own
+    /// the base class may do so.
     void open();
     void open();
 
 
     /// @brief Appends the lease record to the CSV file.
     /// @brief Appends the lease record to the CSV file.

+ 5 - 3
src/lib/dhcpsrv/csv_lease_file6.h

@@ -50,9 +50,11 @@ public:
 
 
     /// @brief Opens a lease file.
     /// @brief Opens a lease file.
     ///
     ///
-    /// This function is used to clear the statistics while
-    /// calling the base class open.  It doesn't throw any
-    /// exceptions of its own but the base class may.
+    /// This function calls the base class open to do the
+    /// work of opening a file.  It is used to clear any
+    /// statistics associated with any previous use of the file
+    /// While it doesn't throw any exceptions of its own
+    /// the base class may do so.
     void open();
     void open();
 
 
     /// @brief Appends the lease record to the CSV file.
     /// @brief Appends the lease record to the CSV file.

+ 12 - 15
src/lib/dhcpsrv/lease_stats.h

@@ -37,32 +37,32 @@ public:
     }
     }
 
 
     /// @brief Gets the number of attempts to read a lease
     /// @brief Gets the number of attempts to read a lease
-    uint32_t getReads() {
+    uint32_t getReads() const {
         return (reads_);
         return (reads_);
     }
     }
 
 
     /// @brief Gets the number of leases read
     /// @brief Gets the number of leases read
-    uint32_t getReadLeases() {
+    uint32_t getReadLeases() const {
         return (read_leases_);
         return (read_leases_);
     }
     }
 
 
     /// @brief Gets the number of errors when reading leases
     /// @brief Gets the number of errors when reading leases
-    uint32_t getReadErrs() {
+    uint32_t getReadErrs() const {
         return (read_errs_);
         return (read_errs_);
     }
     }
 
 
     /// @brief Gets the number of attempts to write a lease
     /// @brief Gets the number of attempts to write a lease
-    uint32_t getWrites() {
+    uint32_t getWrites() const {
         return (writes_);
         return (writes_);
     }
     }
 
 
     /// @brief Gets the number of leases written
     /// @brief Gets the number of leases written
-    uint32_t getWriteLeases() {
+    uint32_t getWriteLeases() const {
         return (write_leases_);
         return (write_leases_);
     }
     }
 
 
     /// @brief Gets the number of errors when writting leases
     /// @brief Gets the number of errors when writting leases
-    uint32_t getWriteErrs() {
+    uint32_t getWriteErrs() const {
         return (write_errs_);
         return (write_errs_);
     }
     }
 
 
@@ -77,26 +77,23 @@ public:
     }
     }
 
 
 protected:
 protected:
-    /// @brief Number of attempts to read a lease (calls to next), reset to 0 on open
+    /// @brief Number of attempts to read a lease
     uint32_t reads_;
     uint32_t reads_;
 
 
-    /// @brief Number of leases read, reset to 0 on open
+    /// @brief Number of leases read
     uint32_t read_leases_;
     uint32_t read_leases_;
 
 
-    /// @brief Number of errors when reading, reset to 0 on open
+    /// @brief Number of errors when reading
     uint32_t read_errs_;
     uint32_t read_errs_;
 
 
-    /// @brief Number of attempts to write a lease, reset to 0 on open
+    /// @brief Number of attempts to write a lease
     uint32_t writes_;
     uint32_t writes_;
 
 
-    /// @brief Number of lease written, reset to 0 on open
+    /// @brief Number of lease written
     uint32_t write_leases_;
     uint32_t write_leases_;
 
 
-    /// @brief Number of errors when writing , reset to 0 on open
+    /// @brief Number of errors when writing
     uint32_t write_errs_;
     uint32_t write_errs_;
-
-private:
-
 };
 };
 
 
 } // namespace isc::dhcp
 } // namespace isc::dhcp

+ 41 - 6
src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc

@@ -58,16 +58,27 @@ public:
     void writeSampleFile() const;
     void writeSampleFile() const;
 
 
     /// @brief Checks the stats for the file
     /// @brief Checks the stats for the file
+    ///
+    /// This method is passed a leasefile and the values for the statistics it
+    /// should have for comparison.
+    ///
+    /// @param lease_file A reference to the file we are using
+    /// @param reads the number of attempted reads
+    /// @param read_leases the number of valid leases read
+    /// @param read_errs the number of errors while reading leases
+    /// @param writes the number of attempted writes
+    /// @param write_leases the number of leases successfully written
+    /// @param write_errs the number of errors while writing
     void checkStats(CSVLeaseFile4& lease_file,
     void checkStats(CSVLeaseFile4& lease_file,
                     uint32_t reads, uint32_t read_leases,
                     uint32_t reads, uint32_t read_leases,
                     uint32_t read_errs, uint32_t writes,
                     uint32_t read_errs, uint32_t writes,
                     uint32_t write_leases, uint32_t write_errs) const {
                     uint32_t write_leases, uint32_t write_errs) const {
-        EXPECT_EQ(lease_file.getReads(), reads);
-        EXPECT_EQ(lease_file.getReadLeases(), read_leases);
-        EXPECT_EQ(lease_file.getReadErrs(), read_errs);
-        EXPECT_EQ(lease_file.getWrites(), writes);
-        EXPECT_EQ(lease_file.getWriteLeases(), write_leases);
-        EXPECT_EQ(lease_file.getWriteErrs(), write_errs);
+        EXPECT_EQ(reads, lease_file.getReads());
+        EXPECT_EQ(read_leases, lease_file.getReadLeases());
+        EXPECT_EQ(read_errs, lease_file.getReadErrs());
+        EXPECT_EQ(writes, lease_file.getWrites());
+        EXPECT_EQ(write_leases, lease_file.getWriteLeases());
+        EXPECT_EQ(write_errs, lease_file.getWriteErrs());
     }
     }
 
 
     /// @brief Name of the test lease file.
     /// @brief Name of the test lease file.
@@ -118,10 +129,15 @@ TEST_F(CSVLeaseFile4Test, parse) {
     ASSERT_NO_THROW(lf->open());
     ASSERT_NO_THROW(lf->open());
 
 
     // Verify the counters are cleared
     // Verify the counters are cleared
+    {
+    SCOPED_TRACE("Check stats are empty");
     checkStats(*lf, 0, 0, 0, 0, 0, 0);
     checkStats(*lf, 0, 0, 0, 0, 0, 0);
+    }
 
 
     Lease4Ptr lease;
     Lease4Ptr lease;
     // Reading first read should be successful.
     // Reading first read should be successful.
+    {
+    SCOPED_TRACE("First lease valid");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     ASSERT_TRUE(lease);
     ASSERT_TRUE(lease);
     checkStats(*lf, 1, 1, 0, 0, 0, 0);
     checkStats(*lf, 1, 1, 0, 0, 0, 0);
@@ -137,13 +153,19 @@ TEST_F(CSVLeaseFile4Test, parse) {
     EXPECT_TRUE(lease->fqdn_fwd_);
     EXPECT_TRUE(lease->fqdn_fwd_);
     EXPECT_TRUE(lease->fqdn_rev_);
     EXPECT_TRUE(lease->fqdn_rev_);
     EXPECT_EQ("host.example.com", lease->hostname_);
     EXPECT_EQ("host.example.com", lease->hostname_);
+    }
 
 
     // Second lease is malformed - HW address is empty.
     // Second lease is malformed - HW address is empty.
+    {
+    SCOPED_TRACE("Second lease malformed");
     EXPECT_FALSE(lf->next(lease));
     EXPECT_FALSE(lf->next(lease));
     checkStats(*lf, 2, 1, 1, 0, 0, 0);
     checkStats(*lf, 2, 1, 1, 0, 0, 0);
+    }
 
 
     // Even though parsing previous lease failed, reading the next lease should be
     // Even though parsing previous lease failed, reading the next lease should be
     // successful.
     // successful.
+    {
+    SCOPED_TRACE("Third lease valid");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     ASSERT_TRUE(lease);
     ASSERT_TRUE(lease);
     checkStats(*lf, 3, 2, 1, 0, 0, 0);
     checkStats(*lf, 3, 2, 1, 0, 0, 0);
@@ -160,17 +182,24 @@ TEST_F(CSVLeaseFile4Test, parse) {
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_rev_);
     EXPECT_FALSE(lease->fqdn_rev_);
     EXPECT_TRUE(lease->hostname_.empty());
     EXPECT_TRUE(lease->hostname_.empty());
+    }
 
 
     // There are no more leases. Reading should cause no error, but the returned
     // There are no more leases. Reading should cause no error, but the returned
     // lease pointer should be NULL.
     // lease pointer should be NULL.
+    {
+    SCOPED_TRACE("Fifth read empty");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     EXPECT_FALSE(lease);
     EXPECT_FALSE(lease);
     checkStats(*lf, 4, 2, 1, 0, 0, 0);
     checkStats(*lf, 4, 2, 1, 0, 0, 0);
+    }
 
 
     // We should be able to do it again.
     // We should be able to do it again.
+    {
+    SCOPED_TRACE("Sixth read empty");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     EXPECT_FALSE(lease);
     EXPECT_FALSE(lease);
     checkStats(*lf, 5, 2, 1, 0, 0, 0);
     checkStats(*lf, 5, 2, 1, 0, 0, 0);
+    }
 }
 }
 
 
 // This test checks creation of the lease file and writing leases.
 // This test checks creation of the lease file and writing leases.
@@ -188,16 +217,22 @@ TEST_F(CSVLeaseFile4Test, recreate) {
                                NULL, 0,
                                NULL, 0,
                                200, 50, 80, 0, 8, true, true,
                                200, 50, 80, 0, 8, true, true,
                                "host.example.com"));
                                "host.example.com"));
+    {
+    SCOPED_TRACE("First write");
     ASSERT_NO_THROW(lf->append(*lease));
     ASSERT_NO_THROW(lf->append(*lease));
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
+    }
 
 
     // Create second lease, with non-NULL client id.
     // Create second lease, with non-NULL client id.
     lease.reset(new Lease4(IOAddress("192.0.3.10"),
     lease.reset(new Lease4(IOAddress("192.0.3.10"),
                            hwaddr1_,
                            hwaddr1_,
                            CLIENTID0, sizeof(CLIENTID0),
                            CLIENTID0, sizeof(CLIENTID0),
                            100, 60, 90, 0, 7));
                            100, 60, 90, 0, 7));
+    {
+    SCOPED_TRACE("Second write");
     ASSERT_NO_THROW(lf->append(*lease));
     ASSERT_NO_THROW(lf->append(*lease));
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
+    }
 
 
     // Close the lease file.
     // Close the lease file.
     lf->close();
     lf->close();

+ 50 - 6
src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc

@@ -64,16 +64,27 @@ public:
     void writeSampleFile() const;
     void writeSampleFile() const;
 
 
     /// @brief Checks the stats for the file
     /// @brief Checks the stats for the file
+    ///
+    /// This method is passed a leasefile and the values for the statistics it
+    /// should have for comparison.
+    ///
+    /// @param lease_file A reference to the file we are using
+    /// @param reads the number of attempted reads
+    /// @param read_leases the number of valid leases read
+    /// @param read_errs the number of errors while reading leases
+    /// @param writes the number of attempted writes
+    /// @param write_leases the number of leases successfully written
+    /// @param write_errs the number of errors while writing
     void checkStats(CSVLeaseFile6& lease_file,
     void checkStats(CSVLeaseFile6& lease_file,
                     uint32_t reads, uint32_t read_leases,
                     uint32_t reads, uint32_t read_leases,
                     uint32_t read_errs, uint32_t writes,
                     uint32_t read_errs, uint32_t writes,
                     uint32_t write_leases, uint32_t write_errs) const {
                     uint32_t write_leases, uint32_t write_errs) const {
-        EXPECT_EQ(lease_file.getReads(), reads);
-        EXPECT_EQ(lease_file.getReadLeases(), read_leases);
-        EXPECT_EQ(lease_file.getReadErrs(), read_errs);
-        EXPECT_EQ(lease_file.getWrites(), writes);
-        EXPECT_EQ(lease_file.getWriteLeases(), write_leases);
-        EXPECT_EQ(lease_file.getWriteErrs(), write_errs);
+        EXPECT_EQ(reads, lease_file.getReads());
+        EXPECT_EQ(read_leases, lease_file.getReadLeases());
+        EXPECT_EQ(read_errs, lease_file.getReadErrs());
+        EXPECT_EQ(writes, lease_file.getWrites());
+        EXPECT_EQ(write_leases, lease_file.getWriteLeases());
+        EXPECT_EQ(write_errs, lease_file.getWriteErrs());
     }
     }
 
 
     /// @brief Name of the test lease file.
     /// @brief Name of the test lease file.
@@ -119,10 +130,15 @@ TEST_F(CSVLeaseFile6Test, parse) {
     ASSERT_NO_THROW(lf->open());
     ASSERT_NO_THROW(lf->open());
 
 
     // Verify the counters are cleared
     // Verify the counters are cleared
+    {
+    SCOPED_TRACE("Check stats are empty");
     checkStats(*lf, 0, 0, 0, 0, 0, 0);
     checkStats(*lf, 0, 0, 0, 0, 0, 0);
+    }
 
 
     Lease6Ptr lease;
     Lease6Ptr lease;
     // Reading first read should be successful.
     // Reading first read should be successful.
+    {
+    SCOPED_TRACE("First lease valid");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     ASSERT_TRUE(lease);
     ASSERT_TRUE(lease);
     checkStats(*lf, 1, 1, 0, 0, 0, 0);
     checkStats(*lf, 1, 1, 0, 0, 0, 0);
@@ -141,13 +157,19 @@ TEST_F(CSVLeaseFile6Test, parse) {
     EXPECT_TRUE(lease->fqdn_fwd_);
     EXPECT_TRUE(lease->fqdn_fwd_);
     EXPECT_TRUE(lease->fqdn_rev_);
     EXPECT_TRUE(lease->fqdn_rev_);
     EXPECT_EQ("host.example.com", lease->hostname_);
     EXPECT_EQ("host.example.com", lease->hostname_);
+    }
 
 
     // Second lease is malformed - DUID is empty.
     // Second lease is malformed - DUID is empty.
+    {
+    SCOPED_TRACE("Second lease malformed");
     EXPECT_FALSE(lf->next(lease));
     EXPECT_FALSE(lf->next(lease));
     checkStats(*lf, 2, 1, 1, 0, 0, 0);
     checkStats(*lf, 2, 1, 1, 0, 0, 0);
+    }
 
 
     // Even, parsing previous lease failed, reading the next lease should be
     // Even, parsing previous lease failed, reading the next lease should be
     // successful.
     // successful.
+    {
+    SCOPED_TRACE("Third lease valid");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     ASSERT_TRUE(lease);
     ASSERT_TRUE(lease);
     checkStats(*lf, 3, 2, 1, 0, 0, 0);
     checkStats(*lf, 3, 2, 1, 0, 0, 0);
@@ -166,8 +188,11 @@ TEST_F(CSVLeaseFile6Test, parse) {
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_rev_);
     EXPECT_FALSE(lease->fqdn_rev_);
     EXPECT_TRUE(lease->hostname_.empty());
     EXPECT_TRUE(lease->hostname_.empty());
+    }
 
 
     // Reading the fourth lease should be successful.
     // Reading the fourth lease should be successful.
+    {
+    SCOPED_TRACE("Fourth lease valid");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     ASSERT_TRUE(lease);
     ASSERT_TRUE(lease);
     checkStats(*lf, 4, 3, 1, 0, 0, 0);
     checkStats(*lf, 4, 3, 1, 0, 0, 0);
@@ -186,17 +211,24 @@ TEST_F(CSVLeaseFile6Test, parse) {
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_fwd_);
     EXPECT_FALSE(lease->fqdn_rev_);
     EXPECT_FALSE(lease->fqdn_rev_);
     EXPECT_TRUE(lease->hostname_.empty());
     EXPECT_TRUE(lease->hostname_.empty());
+    }
 
 
     // There are no more leases. Reading should cause no error, but the returned
     // There are no more leases. Reading should cause no error, but the returned
     // lease pointer should be NULL.
     // lease pointer should be NULL.
+    {
+    SCOPED_TRACE("Fifth read empty");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     EXPECT_FALSE(lease);
     EXPECT_FALSE(lease);
     checkStats(*lf, 5, 3, 1, 0, 0, 0);
     checkStats(*lf, 5, 3, 1, 0, 0, 0);
+    }
 
 
     // We should be able to do it again.
     // We should be able to do it again.
+    {
+    SCOPED_TRACE("Sixth read empty");
     EXPECT_TRUE(lf->next(lease));
     EXPECT_TRUE(lf->next(lease));
     EXPECT_FALSE(lease);
     EXPECT_FALSE(lease);
     checkStats(*lf, 6, 3, 1, 0, 0, 0);
     checkStats(*lf, 6, 3, 1, 0, 0, 0);
+    }
 }
 }
 
 
 // This test checks creation of the lease file and writing leases.
 // This test checks creation of the lease file and writing leases.
@@ -206,31 +238,43 @@ TEST_F(CSVLeaseFile6Test, recreate) {
     ASSERT_TRUE(io_.exists());
     ASSERT_TRUE(io_.exists());
 
 
     // Verify the counters are cleared
     // Verify the counters are cleared
+    {
+    SCOPED_TRACE("Check stats are empty");
     checkStats(*lf, 0, 0, 0, 0, 0, 0);
     checkStats(*lf, 0, 0, 0, 0, 0, 0);
+    }
 
 
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1"),
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1"),
                                makeDUID(DUID0, sizeof(DUID0)),
                                makeDUID(DUID0, sizeof(DUID0)),
                                7, 100, 200, 50, 80, 8, true, true,
                                7, 100, 200, 50, 80, 8, true, true,
                                "host.example.com"));
                                "host.example.com"));
     lease->cltt_ = 0;
     lease->cltt_ = 0;
+    {
+    SCOPED_TRACE("First write");
     ASSERT_NO_THROW(lf->append(*lease));
     ASSERT_NO_THROW(lf->append(*lease));
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
+    }
 
 
     lease.reset(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:2::10"),
     lease.reset(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:2::10"),
                            makeDUID(DUID1, sizeof(DUID1)),
                            makeDUID(DUID1, sizeof(DUID1)),
                            8, 150, 300, 40, 70, 6, false, false,
                            8, 150, 300, 40, 70, 6, false, false,
                            "", HWAddrPtr(), 128));
                            "", HWAddrPtr(), 128));
     lease->cltt_ = 0;
     lease->cltt_ = 0;
+    {
+    SCOPED_TRACE("Second write");
     ASSERT_NO_THROW(lf->append(*lease));
     ASSERT_NO_THROW(lf->append(*lease));
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
+    }
 
 
     lease.reset(new Lease6(Lease::TYPE_PD, IOAddress("3000:1:1::"),
     lease.reset(new Lease6(Lease::TYPE_PD, IOAddress("3000:1:1::"),
                            makeDUID(DUID0, sizeof(DUID0)),
                            makeDUID(DUID0, sizeof(DUID0)),
                            7, 150, 300, 40, 70, 10, false, false,
                            7, 150, 300, 40, 70, 10, false, false,
                            "", HWAddrPtr(), 64));
                            "", HWAddrPtr(), 64));
     lease->cltt_ = 0;
     lease->cltt_ = 0;
+    {
+    SCOPED_TRACE("Third write");
     ASSERT_NO_THROW(lf->append(*lease));
     ASSERT_NO_THROW(lf->append(*lease));
     checkStats(*lf, 0, 0, 0, 3, 3, 0);
     checkStats(*lf, 0, 0, 0, 3, 3, 0);
+    }
 
 
     EXPECT_EQ("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
     EXPECT_EQ("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
               "lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
               "lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"

+ 51 - 8
src/lib/dhcpsrv/tests/lease_file_loader_unittest.cc

@@ -117,8 +117,12 @@ public:
     /// should have for comparison.
     /// should have for comparison.
     ///
     ///
     /// @param lease_file A reference to the file we are using
     /// @param lease_file A reference to the file we are using
-    /// @param the statistics, in order, reads attempted, leases read, errors
-    /// while reading, writes attempted, leases written, and errors while writing
+    /// @param reads the number of attempted reads
+    /// @param read_leases the number of valid leases read
+    /// @param read_errs the number of errors while reading leases
+    /// @param writes the number of attempted writes
+    /// @param write_leases the number of leases successfully written
+    /// @param write_errs the number of errors while writing
     ///
     ///
     /// @tparam LeaseFileType A @c CSVLeaseFile4 or @c CSVLeaseFile6.
     /// @tparam LeaseFileType A @c CSVLeaseFile4 or @c CSVLeaseFile6.
     template<typename LeaseFileType>
     template<typename LeaseFileType>
@@ -126,12 +130,12 @@ public:
                     uint32_t reads, uint32_t read_leases,
                     uint32_t reads, uint32_t read_leases,
                     uint32_t read_errs, uint32_t writes,
                     uint32_t read_errs, uint32_t writes,
                     uint32_t write_leases, uint32_t write_errs) const {
                     uint32_t write_leases, uint32_t write_errs) const {
-        EXPECT_EQ(lease_file.getReads(), reads);
-        EXPECT_EQ(lease_file.getReadLeases(), read_leases);
-        EXPECT_EQ(lease_file.getReadErrs(), read_errs);
-        EXPECT_EQ(lease_file.getWrites(), writes);
-        EXPECT_EQ(lease_file.getWriteLeases(), write_leases);
-        EXPECT_EQ(lease_file.getWriteErrs(), write_errs);
+        EXPECT_EQ(reads, lease_file.getReads());
+        EXPECT_EQ(read_leases, lease_file.getReadLeases());
+        EXPECT_EQ(read_errs, lease_file.getReadErrs());
+        EXPECT_EQ(writes, lease_file.getWrites());
+        EXPECT_EQ(write_leases, lease_file.getWriteLeases());
+        EXPECT_EQ(write_errs, lease_file.getWriteErrs());
     }
     }
 
 
     /// @brief Name of the test lease file.
     /// @brief Name of the test lease file.
@@ -201,7 +205,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite4) {
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 10));
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 10));
 
 
     // We should have made 6 attempts to read, with 4 leases read and 1 error
     // We should have made 6 attempts to read, with 4 leases read and 1 error
+    {
+    SCOPED_TRACE("Read leases");
     checkStats(*lf, 6, 4, 1, 0, 0, 0);
     checkStats(*lf, 6, 4, 1, 0, 0, 0);
+    }
 
 
     // There are two unique leases.
     // There are two unique leases.
     ASSERT_EQ(2, storage.size());
     ASSERT_EQ(2, storage.size());
@@ -228,7 +235,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite4) {
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
 
 
     // We should have made 2 attempts to write, with 2 leases written and 0 errors
     // We should have made 2 attempts to write, with 2 leases written and 0 errors
+    {
+    SCOPED_TRACE("Write leases");
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
+    }
 }
 }
 
 
 // This test verifies that the lease with a valid lifetime of 0
 // This test verifies that the lease with a valid lifetime of 0
@@ -263,7 +273,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite4LeaseRemove) {
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 10));
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 10));
 
 
     // We should have made 5 attempts to read, with 4 leases read and 0 error
     // We should have made 5 attempts to read, with 4 leases read and 0 error
+    {
+    SCOPED_TRACE("Read leases");
     checkStats(*lf, 5, 4, 0, 0, 0, 0);
     checkStats(*lf, 5, 4, 0, 0, 0, 0);
+    }
 
 
     // There should only be one lease. The one with the valid_lifetime
     // There should only be one lease. The one with the valid_lifetime
     // of 0 should be removed.
     // of 0 should be removed.
@@ -277,7 +290,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite4LeaseRemove) {
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
 
 
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
+    {
+    SCOPED_TRACE("Write leases");
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
+    }
 }
 }
 
 
 // This test verifies that the DHCPv6 leases can be loaded from the lease
 // This test verifies that the DHCPv6 leases can be loaded from the lease
@@ -318,7 +334,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite6) {
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease6>(*lf, storage, 10));
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease6>(*lf, storage, 10));
 
 
     // We should have made 7 attempts to read, with 5 leases read and 1 error
     // We should have made 7 attempts to read, with 5 leases read and 1 error
+    {
+    SCOPED_TRACE("Read leases");
     checkStats(*lf, 7, 5, 1, 0, 0, 0);
     checkStats(*lf, 7, 5, 1, 0, 0, 0);
+    }
 
 
     // There should be 3 unique leases.
     // There should be 3 unique leases.
     ASSERT_EQ(3, storage.size());
     ASSERT_EQ(3, storage.size());
@@ -345,7 +364,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite6) {
     writeLeases<Lease6, CSVLeaseFile6, Lease6Storage>(*lf, storage, test_str);
     writeLeases<Lease6, CSVLeaseFile6, Lease6Storage>(*lf, storage, test_str);
 
 
     // We should have made 3 attempts to write, with 3 leases written and 0 errors
     // We should have made 3 attempts to write, with 3 leases written and 0 errors
+    {
+    SCOPED_TRACE("Write leases");
     checkStats(*lf, 0, 0, 0, 3, 3, 0);
     checkStats(*lf, 0, 0, 0, 3, 3, 0);
+    }
 }
 }
 
 
 // This test verifies that the lease with a valid lifetime of 0
 // This test verifies that the lease with a valid lifetime of 0
@@ -380,7 +402,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite6LeaseRemove) {
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease6>(*lf, storage, 10));
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease6>(*lf, storage, 10));
 
 
     // We should have made 5 attempts to read, with 4 leases read and 0 error
     // We should have made 5 attempts to read, with 4 leases read and 0 error
+    {
+    SCOPED_TRACE("Read leases");
     checkStats(*lf, 5, 4, 0, 0, 0, 0);
     checkStats(*lf, 5, 4, 0, 0, 0, 0);
+    }
 
 
     // There should be only one lease for 2001:db8:2::10. The other one
     // There should be only one lease for 2001:db8:2::10. The other one
     // should have been deleted (or rather not loaded).
     // should have been deleted (or rather not loaded).
@@ -394,7 +419,10 @@ TEST_F(LeaseFileLoaderTest, loadWrite6LeaseRemove) {
     writeLeases<Lease6, CSVLeaseFile6, Lease6Storage>(*lf, storage, test_str);
     writeLeases<Lease6, CSVLeaseFile6, Lease6Storage>(*lf, storage, test_str);
 
 
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
+    {
+    SCOPED_TRACE("Write leases");
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
+    }
 }
 }
 
 
 // This test verifies that the exception is thrown when the specific
 // This test verifies that the exception is thrown when the specific
@@ -426,7 +454,10 @@ TEST_F(LeaseFileLoaderTest, loadMaxErrors) {
                  util::CSVFileError);
                  util::CSVFileError);
 
 
     // We should have made 6 attempts to read, with 2 leases read and 4 error
     // We should have made 6 attempts to read, with 2 leases read and 4 error
+    {
+    SCOPED_TRACE("Read leases 1");
     checkStats(*lf, 6, 2, 4, 0, 0, 0);
     checkStats(*lf, 6, 2, 4, 0, 0, 0);
+    }
 
 
     lf->close();
     lf->close();
     ASSERT_NO_THROW(lf->open());
     ASSERT_NO_THROW(lf->open());
@@ -437,7 +468,10 @@ TEST_F(LeaseFileLoaderTest, loadMaxErrors) {
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 4));
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 4));
 
 
     // We should have made 8 attempts to read, with 3 leases read and 4 error
     // We should have made 8 attempts to read, with 3 leases read and 4 error
+    {
+    SCOPED_TRACE("Read leases 2");
     checkStats(*lf, 8, 3, 4, 0, 0, 0);
     checkStats(*lf, 8, 3, 4, 0, 0, 0);
+    }
 
 
     ASSERT_EQ(2, storage.size());
     ASSERT_EQ(2, storage.size());
 
 
@@ -453,7 +487,10 @@ TEST_F(LeaseFileLoaderTest, loadMaxErrors) {
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
 
 
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
+    {
+    SCOPED_TRACE("Write leases");
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
     checkStats(*lf, 0, 0, 0, 2, 2, 0);
+    }
 }
 }
 
 
 // This test verifies that the lease with a valid lifetime set to 0 is
 // This test verifies that the lease with a valid lifetime set to 0 is
@@ -480,7 +517,10 @@ TEST_F(LeaseFileLoaderTest, loadWriteLeaseWithZeroLifetime) {
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 0));
     ASSERT_NO_THROW(LeaseFileLoader::load<Lease4>(*lf, storage, 0));
 
 
     // We should have made 3 attempts to read, with 2 leases read and 0 error
     // We should have made 3 attempts to read, with 2 leases read and 0 error
+    {
+    SCOPED_TRACE("Read leases");
     checkStats(*lf, 3, 2, 0, 0, 0, 0);
     checkStats(*lf, 3, 2, 0, 0, 0, 0);
+    }
 
 
     // The first lease should be present.
     // The first lease should be present.
     Lease4Ptr lease = getLease<Lease4Ptr>("192.0.2.1", storage);
     Lease4Ptr lease = getLease<Lease4Ptr>("192.0.2.1", storage);
@@ -494,6 +534,9 @@ TEST_F(LeaseFileLoaderTest, loadWriteLeaseWithZeroLifetime) {
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
     writeLeases<Lease4, CSVLeaseFile4, Lease4Storage>(*lf, storage, test_str);
 
 
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
     // We should have made 1 attempts to write, with 1 leases written and 0 errors
+    {
+    SCOPED_TRACE("Write leases");
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
     checkStats(*lf, 0, 0, 0, 1, 1, 0);
+    }
 }
 }
 } // end of anonymous namespace
 } // end of anonymous namespace