From 640717daeb23174a289e6080c2982c3941e54445 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Fri, 13 Dec 2019 10:33:13 +0100 Subject: [PATCH 1/3] Handle discard all logfiles properly Fixes https://github.com/hashicorp/consul/issues/6892. The [docs](https://www.consul.io/docs/agent/options.html#_log_rotate_max_files) are stating: > -log-rotate-max-files - to specify the maximum number of older log > file archives to keep. Defaults to 0 (no files are ever deleted). Set to > -1 to disable rotation and discard all log files. But the `-1` case was not implemented and led to a panic when being used. --- logger/logfile.go | 10 ++++++++-- logger/logfile_test.go | 27 ++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/logger/logfile.go b/logger/logfile.go index 8b6ad4aa2adb..c7a68d120550 100644 --- a/logger/logfile.go +++ b/logger/logfile.go @@ -104,8 +104,14 @@ func (l *LogFile) pruneFiles() error { if err != nil { return err } - // Prune if there are more files stored than the configured max - stale := len(matches) - l.MaxFiles + var stale int + if l.MaxFiles == -1 { + // Prune everything + stale = len(matches) + } else { + // Prune if there are more files stored than the configured max + stale = len(matches) - l.MaxFiles + } for i := 0; i < stale; i++ { if err := os.Remove(matches[i]); err != nil { return err diff --git a/logger/logfile_test.go b/logger/logfile_test.go index 44d55a245f31..95c6c9d0dc4f 100644 --- a/logger/logfile_test.go +++ b/logger/logfile_test.go @@ -136,7 +136,7 @@ func TestLogFile_deleteArchives(t *testing.T) { func TestLogFile_deleteArchivesDisabled(t *testing.T) { t.Parallel() - tempDir := testutil.TempDir(t, "LogWriteDeleteArchivesDisabled") + tempDir := testutil.TempDir(t, t.Name()) defer os.Remove(tempDir) filt := LevelFilter() filt.MinLevel = logutils.LogLevel("INFO") @@ -158,3 +158,28 @@ func TestLogFile_deleteArchivesDisabled(t *testing.T) { return } } + +func TestLogFile_rotationDisabled(t *testing.T) { + t.Parallel() + tempDir := testutil.TempDir(t, t.Name()) + defer os.Remove(tempDir) + filt := LevelFilter() + filt.MinLevel = logutils.LogLevel("INFO") + logFile := LogFile{ + logFilter: filt, + fileName: testFileName, + logPath: tempDir, + MaxBytes: testBytes, + duration: 24 * time.Hour, + MaxFiles: -1, + } + logFile.Write([]byte("[INFO] Hello World")) + logFile.Write([]byte("[INFO] Second File")) + logFile.Write([]byte("[INFO] Third File")) + want := 1 + tempFiles, _ := ioutil.ReadDir(tempDir) + if got := tempFiles; len(got) != want { + t.Errorf("Expected %d files, got %v file(s)", want, len(got)) + return + } +} From 457b05eef109b34353564924c5bde28c7179cf87 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Wed, 18 Dec 2019 11:51:10 +0100 Subject: [PATCH 2/3] Better documentation of log-rotate-max-files=-1. --- logger/logfile.go | 7 ++++--- website/source/docs/agent/options.html.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/logger/logfile.go b/logger/logfile.go index c7a68d120550..7effe7a95754 100644 --- a/logger/logfile.go +++ b/logger/logfile.go @@ -62,10 +62,11 @@ func (l *LogFile) fileNamePattern() string { func (l *LogFile) openNew() error { fileNamePattern := l.fileNamePattern() - // New file name has the format : filename-timestamp.extension + createTime := now() newfileName := fmt.Sprintf(fileNamePattern, strconv.FormatInt(createTime.UnixNano(), 10)) newfilePath := filepath.Join(l.logPath, newfileName) + // Try creating a file. We truncate the file because we are the only authority to write the logs filePointer, err := os.OpenFile(newfilePath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0640) if err != nil { @@ -105,7 +106,7 @@ func (l *LogFile) pruneFiles() error { return err } var stale int - if l.MaxFiles == -1 { + if l.MaxFiles <= -1 { // Prune everything stale = len(matches) } else { @@ -129,7 +130,7 @@ func (l *LogFile) Write(b []byte) (n int, err error) { l.acquire.Lock() defer l.acquire.Unlock() - //Create a new file if we have no file to write to + // Create a new file if we have no file to write to if l.FileInfo == nil { if err := l.openNew(); err != nil { return 0, err diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index ca13ad4f3f8e..d64ff5d395d5 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -283,7 +283,7 @@ The options below are all specified on the command-line. * `-log-rotate-duration` - to specify the maximum duration a log should be written to before it needs to be rotated. Must be a duration value such as 30s. Defaults to 24h. -* `-log-rotate-max-files` - to specify the maximum number of older log file archives to keep. Defaults to 0 (no files are ever deleted). Set to -1 to disable rotation and discard all log files. +* `-log-rotate-max-files` - to specify the maximum number of older log file archives to keep. Defaults to 0 (no files are ever deleted). Set to -1 to truncate the log file when reaching `-log-rotate-bytes` or `-log-rotate-duration`, without rotating the content into archives. * `-join` - Address of another agent to join upon starting up. This can be From 90283dc2da763e1e210bc36ce8a3ee88f33ddaf6 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Wed, 18 Dec 2019 21:24:43 +0100 Subject: [PATCH 3/3] Update website/source/docs/agent/options.html.md Co-Authored-By: Freddy --- website/source/docs/agent/options.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index d64ff5d395d5..748762ec6238 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -283,7 +283,7 @@ The options below are all specified on the command-line. * `-log-rotate-duration` - to specify the maximum duration a log should be written to before it needs to be rotated. Must be a duration value such as 30s. Defaults to 24h. -* `-log-rotate-max-files` - to specify the maximum number of older log file archives to keep. Defaults to 0 (no files are ever deleted). Set to -1 to truncate the log file when reaching `-log-rotate-bytes` or `-log-rotate-duration`, without rotating the content into archives. +* `-log-rotate-max-files` - to specify the maximum number of older log file archives to keep. Defaults to 0 (no files are ever deleted). Set to -1 to discard old log files when a new one is created. * `-join` - Address of another agent to join upon starting up. This can be