[5324] Log file rotation now works when configured to do so

doc/guide/logging.xml
    Updated logging section with more explanation of maxsize and maxver

src/lib/dhcpsrv/logging_info.cc
    LoggingInfo::toSpec()
        Now sets maxsize and maxver in created spec

src/lib/dhcpsrv/tests/logging_info_unittest.cc
    TEST_F(LoggingInfoTest, defaults)
        Added checks for default maxsize and maxver

src/lib/dhcpsrv/tests/logging_unittest.cc
    LoggingTest:
        Added support for log files

    TEST_F(LoggingTest, logRotate) - new test the ensures
    logs rotate when configured to do so

src/lib/testutils/dhcp_test_lib.sh.in
    Added code to remove log lock file.  When rotation is enabled,
    lock files are automatically enabled.

diff --git a/doc/guide/logging.xml b/doc/guide/logging.xml
index 6b871e236645451bd7b4c324c94f9f7579ac8adc..52f9ef8b636421676f8ed3a173795a48f7be46aa 100644 (file)
--- a/doc/guide/logging.xml
+++ b/doc/guide/logging.xml
@@ -628,15 +628,17 @@
         <section>
           <title>maxsize (integer)</title>
           <para>
-            Only relevant when destination is file, this is maximum file size of
-            output files in bytes. When the maximum size is reached, the file is
-            renamed and a new file opened.  (For example, a ".1" is appended to
-            the name &mdash; if a ".1" file exists, it is renamed ".2", etc.)
+            Only relevant when the destination is a file. This is maximum size
+            in bytes that a log file may reach.  When the maximum size is
+            reached, the file is renamed and a new file opened. For example,
+            a ".1" is appended to the name &mdash; if a ".1" file exists, it
+            is renamed ".2", etc. This is referred to as rotation.
           </para>
           <para>
-            If this is set to 0 or omitted, no maximum file size is used.
+            The default value is 204800. This is also the smallest value that
+            may be specified without disabling rotation.  Any value less than
+            this, including 0, disables rotation.
           </para>
-
           <note>
             <simpara>
               Due to a limitation of the underlying logging library (log4cplus),
@@ -655,9 +657,13 @@
         <section>
           <title>maxver (integer)</title>
           <para>
-            Maximum number of old log files to keep around when rolling the
-            output file. Only relevant when <option>output</option> is
-            <quote>file</quote>.
+            Only relevant when the destination is file and rotation is enabled
+            (i.e. maxsize is large enough).  This is maximum number of rotated
+            versions that will be kept. Once that number of files has been
+            reached, the oldest file, "log-name.maxver", will be discarded
+            each time the log rotates. In other words, at most there will be
+            the active log file plus maxver rotated files.  The minimum and
+            default value is 1.
           </para>
         </section>
       </section>

diff --git a/src/lib/dhcpsrv/logging_info.cc b/src/lib/dhcpsrv/logging_info.cc
index 12f5c2173559149ee8fe2e46d198847df5d8b72b..9cedf6c4ca05be7eb4e0334d82ca2446e15a64af 100644 (file)
--- a/src/lib/dhcpsrv/logging_info.cc
+++ b/src/lib/dhcpsrv/logging_info.cc
@@ -142,6 +142,8 @@ LoggingInfo::toSpec() const {
             // Not a recognized destination, assume a file.
             option.destination = OutputOption::DEST_FILE;
             option.filename = dest->output_;
+            option.maxsize = dest->maxsize_;
+            option.maxver = dest->maxver_;
         }
 
         // Copy the immediate flush flag

diff --git a/src/lib/dhcpsrv/tests/logging_info_unittest.cc b/src/lib/dhcpsrv/tests/logging_info_unittest.cc
index 9f5424d4dd1bc5baa3409f580074319644906a2e..3df9bdc477797f7b4dcc329ea87e128ebfd38443 100644 (file)
--- a/src/lib/dhcpsrv/tests/logging_info_unittest.cc
+++ b/src/lib/dhcpsrv/tests/logging_info_unittest.cc
@@ -91,6 +91,9 @@ TEST_F(LoggingInfoTest, defaults) {
     ASSERT_EQ(1, info_verbose.destinations_.size());
     EXPECT_EQ("stdout", info_verbose.destinations_[0].output_);
 
+    EXPECT_EQ(204800, info_verbose.destinations_[0].maxsize_);
+    EXPECT_EQ(1, info_verbose.destinations_[0].maxver_);
+
     expected = header + "DEBUG" + dbglvl + "99" + trailer;
     runToElementTest<LoggingInfo>(expected, info_verbose);
 }

diff --git a/src/lib/dhcpsrv/tests/logging_unittest.cc b/src/lib/dhcpsrv/tests/logging_unittest.cc
index da447736afb5e2a5b7d5a88ec60bee370e8504a5..19c947aca0268d3a437538326ffab982119a07f4 100644 (file)
--- a/src/lib/dhcpsrv/tests/logging_unittest.cc
+++ b/src/lib/dhcpsrv/tests/logging_unittest.cc
@@ -36,7 +36,7 @@ class LoggingTest : public ::testing::Test {
         ///
         /// Reset root logger back to defaults.
         ~LoggingTest() {
-            isc::log::setDefaultLoggingOutput();
+            isc::log::initLogger();
             wipeFiles();
         }
 
@@ -44,20 +44,37 @@ class LoggingTest : public ::testing::Test {
     /// @param rotation number to the append to the end of the file
     std::string logName(int rotation) {
         std::ostringstream os;
-        os << LOG_FILE_NAME << "." << rotation;
+        os << TEST_LOG_NAME << "." << rotation;
         return (os.str());
     }
 
     /// @brief Removes the base log file name and 1 rotation
     void wipeFiles()  {
-        static_cast<void>(remove(LOG_FILE_NAME));
-        static_cast<void>(remove(logName(1).c_str()));
+        static_cast<void>(remove(TEST_LOG_NAME));
+        for (int i = 1; i < TEST_MAX_VERS + 1; ++i) {
+            static_cast<void>(remove(logName(i).c_str()));
+        }
+
+        // Remove the lock file
+        std::ostringstream os;
+        os << TEST_LOG_NAME << ".lock";
+        static_cast<void>(remove(os.str().c_str()));
     }
 
-    static const char* LOG_FILE_NAME;
+    /// @brief Name of the log file
+    static const char* TEST_LOG_NAME;
+
+    /// @brief Maximum log size
+    static const int TEST_MAX_SIZE;
+
+    /// @brief Maximum rotated log versions
+    static const int TEST_MAX_VERS;
+
 };
 
-const char* LoggingTest::LOG_FILE_NAME = "kea.test.log";
+const char* LoggingTest::TEST_LOG_NAME = "kea.test.log";
+const int LoggingTest::TEST_MAX_SIZE = 207800;  // Slightly larger than default
+const int LoggingTest::TEST_MAX_VERS = 2;       // More than the default of 1
 
 // Tests that the spec file is valid.
 TEST_F(LoggingTest, basicSpec) {
@@ -271,7 +288,8 @@ TEST_F(LoggingTest, multipleLoggingDestinations) {
 // we can correcty configure logging such that rotation occurs as
 // expected.
 TEST_F(LoggingTest, logRotate) {
-    int rotate_size = 2048;
+    wipeFiles();
+
     std::ostringstream os;
     os <<
         "{ \"loggers\": ["
@@ -280,11 +298,12 @@ TEST_F(LoggingTest, logRotate) {
         "        \"output_options\": ["
         "            {"
         "                \"output\": \""
-        << LOG_FILE_NAME << "\","  <<
+        << TEST_LOG_NAME << "\","  <<
         "                \"flush\": true,"
         "                \"maxsize\":"
-        << rotate_size << "," <<
-        "                \"maxver\": 1"
+        << TEST_MAX_SIZE << "," <<
+        "                \"maxver\":"
+        << TEST_MAX_VERS <<
         "            }"
         "        ],"
         "        \"debuglevel\": 99,"
@@ -292,10 +311,6 @@ TEST_F(LoggingTest, logRotate) {
         "    }"
         "]}";
 
-
-    // Make sure there aren't any left over.
-    wipeFiles();
-
     // Create our server config container.
     SrvConfigPtr server_cfg(new SrvConfig());
 
@@ -309,23 +324,27 @@ TEST_F(LoggingTest, logRotate) {
     ASSERT_NO_THROW(parser.parseConfiguration(config));
     ASSERT_NO_THROW(server_cfg->applyLoggingCfg());
 
+    EXPECT_EQ(TEST_MAX_SIZE, server_cfg->getLoggingInfo()[0].destinations_[0].maxsize_);
+    EXPECT_EQ(TEST_MAX_VERS, server_cfg->getLoggingInfo()[0].destinations_[0].maxver_);
+
     // Make sure we have the initial log file.
-    ASSERT_TRUE(isc::test::fileExists(LOG_FILE_NAME));
+    ASSERT_TRUE(isc::test::fileExists(TEST_LOG_NAME));
 
     // Now generate a log we know will be large enough to force a rotation.
     // We borrow a one argument log message for the test.
-    std::string big_arg(rotate_size, 'x');
+    std::string big_arg(TEST_MAX_SIZE, 'x');
     isc::log::Logger logger("kea");
-    LOG_INFO(logger, DHCPSRV_CFGMGR_ADD_IFACE).arg(big_arg);
 
-    // Make sure we now have a rotation file.
-    EXPECT_TRUE(isc::test::fileExists(logName(1).c_str()));
+    for (int i = 1; i < TEST_MAX_VERS + 1; i++) {
+        // Output the big log and make sure we get the expected rotation file.
+        LOG_INFO(logger, DHCPSRV_CFGMGR_ADD_IFACE).arg(big_arg);
+        EXPECT_TRUE(isc::test::fileExists(logName(i).c_str()));
+    }
 
     // Clean up.
     wipeFiles();
 }
 
-
 /// @todo Add tests for malformed logging configuration
 
 /// @todo There is no easy way to test applyConfiguration() and defaultLogging().

diff --git a/src/lib/testutils/dhcp_test_lib.sh.in b/src/lib/testutils/dhcp_test_lib.sh.in
index 69a61d4be3d39eb617f309c15e98fd995c8331b2..f490053b93e164f5913b0e67d6ba756033c2abf9 100644 (file)
--- a/src/lib/testutils/dhcp_test_lib.sh.in
+++ b/src/lib/testutils/dhcp_test_lib.sh.in
@@ -1,4 +1,4 @@
-# Copyright (C) 2014-2015,2017 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2014-2017 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
@@ -307,6 +307,7 @@ cleanup() {
 
     # Remove temporary files.
     rm -rf ${LOG_FILE}
+    rm -rf ${LOG_FILE}.lock
     # Use asterisk to remove all files starting with the given name,
     # in case the LFC has been run. LFC creates files with postfixes
     # appended to the lease file name.