log: handle discard all logfiles properly (#6945)

* 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.

Co-Authored-By: Freddy <freddygv@users.noreply.github.com>
This commit is contained in:
Hans Hasselberg 2019-12-18 22:31:22 +01:00 committed by GitHub
parent 396e636572
commit 1bf94b01e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 6 deletions

View file

@ -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 {
@ -104,8 +105,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
@ -123,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

View file

@ -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
}
}

View file

@ -283,7 +283,7 @@ The options below are all specified on the command-line.
* <a name="_log_rotate_duration"></a><a href="#_log_rotate_duration">`-log-rotate-duration`</a> - 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.
* <a name="_log_rotate_max_files"></a><a href="#_log_rotate_max_files">`-log-rotate-max-files`</a> - 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.
* <a name="_log_rotate_max_files"></a><a href="#_log_rotate_max_files">`-log-rotate-max-files`</a> - 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.
* <a name="_join"></a><a href="#_join">`-join`</a> - Address of another agent
to join upon starting up. This can be