From: Thomas Markwalder Date: Tue, 8 Aug 2017 15:17:09 +0000 (-0400) Subject: [5324] Log file rotation now works when configured to do so X-Git-Tag: trac5124a_base~7^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2e2cb37a355409dc6d1dd2afce8557c52240a091;p=thirdparty%2Fkea.git [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 6b871e2366..52f9ef8b63 100644 --- a/doc/guide/logging.xml +++ b/doc/guide/logging.xml @@ -628,15 +628,17 @@
maxsize (integer) - 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 — 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 — if a ".1" file exists, it + is renamed ".2", etc. This is referred to as rotation. - 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. - Due to a limitation of the underlying logging library (log4cplus), @@ -655,9 +657,13 @@
maxver (integer) - Maximum number of old log files to keep around when rolling the - output file. Only relevant when is - file. + 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.
diff --git a/src/lib/dhcpsrv/logging_info.cc b/src/lib/dhcpsrv/logging_info.cc index 12f5c21735..9cedf6c4ca 100644 --- 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 9f5424d4dd..3df9bdc477 100644 --- 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(expected, info_verbose); } diff --git a/src/lib/dhcpsrv/tests/logging_unittest.cc b/src/lib/dhcpsrv/tests/logging_unittest.cc index da447736af..19c947aca0 100644 --- 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(remove(LOG_FILE_NAME)); - static_cast(remove(logName(1).c_str())); + static_cast(remove(TEST_LOG_NAME)); + for (int i = 1; i < TEST_MAX_VERS + 1; ++i) { + static_cast(remove(logName(i).c_str())); + } + + // Remove the lock file + std::ostringstream os; + os << TEST_LOG_NAME << ".lock"; + static_cast(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 69a61d4be3..f490053b93 100644 --- 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.