fix duplicate retention check

plugins/inputs/logfile/logfile.go

@@ -149,7 +149,7 @@ func (t *LogFile) FindLogSrc() []logs.LogSrc {
 
 	t.cleanUpStoppedTailerSrc()
 
-	// If a log group has retention settings defined in more than one place, stop the agent
+	// If a log group has differing retentionInDays values defined in multiple places, stop the agent
 	t.checkForDuplicateRetentionSettings()
 	// Create a "tailer" for each file
 	for i := range t.FileConfig {
@@ -398,22 +398,26 @@ func (t *LogFile) cleanUpStoppedTailerSrc() {
 	}
 }
 
+// checkForDuplicateRetentionSettings: Checks if a log group has retention set differently in multiple places and stops the agent if found
 func (t *LogFile) checkForDuplicateRetentionSettings() {
 	configMap := make(map[string]int)
-
 	for i := range t.FileConfig {
-		fileconfig := &t.FileConfig[i]
-		if fileconfig.LogGroupName != "" {
-			logGroup := strings.ToLower(fileconfig.LogGroupName)
-			configMap[logGroup] += 1
+		fileConfig := &t.FileConfig[i]
+		logGroup := strings.ToLower(fileConfig.LogGroupName)
+		// if retention is 0, -1 or less it's either invalid or default
+		if fileConfig.RetentionInDays < 1 {
+			continue
 		}
-
-	}
-	for i := range t.FileConfig {
-		fileconfig := &t.FileConfig[i]
-		// log group has Retention settings in multiple places: throw an error
-		if fileconfig.LogGroupName != "" && configMap[strings.ToLower(fileconfig.LogGroupName)] > 1 {
-			panic(fmt.Sprintf("error: retention for the same log group set in multiple places. Log Group Name: %v", fileconfig.LogGroupName))
+		// if the configMap[logGroup] exists, retention has been set for the same logGroup somewhere
+		if configMap[logGroup] != 0 {
+			// different retentions has been set for the same log group, panic and stop the agent
+			if configMap[logGroup] != fileConfig.RetentionInDays {
+				panic(fmt.Sprintf("error: The Log Group has differing retentionInDays values defined in two or more places. Log Group Name: %v", fileConfig.LogGroupName))
+			}
+			// The same retention for a log group has been configured in multiple places. Unset it so that the retention api is only called once
+			fileConfig.RetentionInDays = -1
+		} else {
+			configMap[logGroup] = fileConfig.RetentionInDays
 		}
 	}
 }

plugins/inputs/logfile/logfile_test.go

@@ -774,7 +774,7 @@ func TestLogsFileRecreate(t *testing.T) {
 	defer lsrc.Stop()
 
 	// Waiting 10 seconds for the recreated temp file to be detected is plenty sufficient on any OS.
-	for start := time.Now(); time.Since(start) < 10 * time.Second; {
+	for start := time.Now(); time.Since(start) < 10*time.Second; {
 		lsrcs = tt.FindLogSrc()
 		if len(lsrcs) > 0 {
 			break
@@ -1087,6 +1087,121 @@ func TestCheckForDuplicateRetentionSettingsPanics(t *testing.T) {
 			LogGroupName:    logGroupName,
 			RetentionInDays: 3,
 		},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: 3,
+		},
 	}
 	assert.Panics(t, func() { tt.checkForDuplicateRetentionSettings() }, "Did not panic after finding duplicate log group")
 }
+
+func TestCheckForDuplicateRetentionSettingsWithDefaultRetention(t *testing.T) {
+	tt := NewLogFile()
+	logGroupName := "DuplicateLogGroupName"
+	tt.FileConfig = []FileConfig{{
+		FilePath:        "SampleFilePath",
+		FromBeginning:   true,
+		LogGroupName:    logGroupName,
+		RetentionInDays: -1,
+	},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: -1,
+		},
+	}
+	assert.NotPanics(t, func() { tt.checkForDuplicateRetentionSettings() }, "Panicked when no duplicate retention settings exists")
+}
+
+func TestCheckForDuplicateRetentionWithDefaultAndNonDefaultValue(t *testing.T) {
+	tt := NewLogFile()
+	logGroupName := "DuplicateLogGroupName"
+	tt.FileConfig = []FileConfig{{
+		FilePath:        "SampleFilePath",
+		FromBeginning:   true,
+		LogGroupName:    logGroupName,
+		RetentionInDays: -1,
+	},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: 3,
+		},
+	}
+	assert.NotPanics(t, func() { tt.checkForDuplicateRetentionSettings() }, "Panicked when no duplicate retention settings exists")
+}
+
+func TestCheckForDuplicateRetentionSettingsDifferentLogGroups(t *testing.T) {
+	tt := NewLogFile()
+	tt.FileConfig = []FileConfig{{
+		FilePath:        "SampleFilePath",
+		FromBeginning:   true,
+		LogGroupName:    "logGroupName1",
+		RetentionInDays: 5,
+	},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    "logGroupName2",
+			RetentionInDays: 3,
+		},
+	}
+	assert.NotPanics(t, func() { tt.checkForDuplicateRetentionSettings() }, "Panicked when no duplicate retention settings exists")
+}
+
+func TestCheckDuplicateRetentionWithDefaultAndSetLogGroups(t *testing.T) {
+	tt := NewLogFile()
+	logGroupName := "DuplicateLogGroupName"
+	tt.FileConfig = []FileConfig{{
+		FilePath:        "SampleFilePath",
+		FromBeginning:   true,
+		LogGroupName:    logGroupName,
+		RetentionInDays: 3,
+	},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: 5,
+		},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: -1,
+		},
+	}
+	assert.Panics(t, func() { tt.checkForDuplicateRetentionSettings() }, "Did not panic after finding duplicate log group")
+}
+
+func TestCheckForDuplicateRetentionSettingsWithDefault(t *testing.T) {
+	tt := NewLogFile()
+	logGroupName := "DuplicateLogGroupName"
+	tt.FileConfig = []FileConfig{{
+		FilePath:        "SampleFilePath",
+		FromBeginning:   true,
+		LogGroupName:    logGroupName,
+		RetentionInDays: 5,
+	},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: 5,
+		},
+		{
+			FilePath:        "SampleFilePath",
+			FromBeginning:   true,
+			LogGroupName:    logGroupName,
+			RetentionInDays: 5,
+		},
+	}
+	assert.NotPanics(t, func() { tt.checkForDuplicateRetentionSettings() }, "Panicked when no duplicate retention settings exists")
+	assert.Equal(t, tt.FileConfig[0].RetentionInDays, 5)
+	assert.Equal(t, tt.FileConfig[1].RetentionInDays, -1)
+	assert.Equal(t, tt.FileConfig[2].RetentionInDays, -1)
+}