Add/adjust tests/code after rebasing on newer retention API commits.

Author: dsessler7

internal/collector/config.go

@@ -149,16 +149,10 @@ func generateLogrotateConfig(logFilePath string, retentionPeriod *v1beta1.Durati
 	)
 }
 
-// FIXME: If the rotate count is 0, logrotate will not keep any archives, it will
-// essentially just empty out the working file during rotation, but it will still
-// rotate on the interval specified (hourly, daily, weekly, etc). If the user
-// sets the retentionPeriod to 0, we currently default to hourly regardless of
-// what unit the user provides. E.g. If the user sets the retentionPeriod to "0w"
-// they might expect that it will rotate once a week, but not keep any archives,
-// but we will actually rotate every hour... Is there a way to get the unit
-// provided in the Duration??
+// parseDurationForLogrotate takes a retention period and returns the rotate
+// number and interval string that should be used in the logrotate config
 func parseDurationForLogrotate(retentionPeriod *v1beta1.Duration) (int, string) {
-	hours := math.Round(retentionPeriod.Duration.Hours())
+	hours := math.Round(retentionPeriod.AsDuration().Hours())
 	if hours < 24 {
 		return int(hours), "hourly"
 	}

internal/collector/config_test.go

@@ -65,12 +65,74 @@ service:
 }
 
 // TODO: write this test after rebasing on new retention API changes.
-// func TestGenerateLogrotateConfig(t *testing.T) {
-
-// }
+func TestGenerateLogrotateConfig(t *testing.T) {
+	for _, tt := range []struct {
+		logFilePath      string
+		retentionPeriod  string
+		postrotateScript string
+		result           string
+	}{
+		{
+			logFilePath:      "/this/is/a/file.path",
+			retentionPeriod:  "12h",
+			postrotateScript: "echo 'Hello, World'",
+			result: `/this/is/a/file.path {
+      rotate 12
+      missingok
+      sharedscripts
+      notifempty
+      nocompress
+      hourly
+      postrotate
+            echo 'Hello, World'
+      endscript
+}
+`,
+		},
+		{
+			logFilePath:      "/tmp/test.log",
+			retentionPeriod:  "5 days",
+			postrotateScript: "",
+			result: `/tmp/test.log {
+      rotate 5
+      missingok
+      sharedscripts
+      notifempty
+      nocompress
+      daily
+      postrotate
+            
+      endscript
+}
+`,
+		},
+		{
+			logFilePath:      "/tmp/test.log",
+			retentionPeriod:  "5wk",
+			postrotateScript: "pkill -HUP --exact pgbouncer",
+			result: `/tmp/test.log {
+      rotate 35
+      missingok
+      sharedscripts
+      notifempty
+      nocompress
+      daily
+      postrotate
+            pkill -HUP --exact pgbouncer
+      endscript
+}
+`,
+		},
+	} {
+		t.Run(tt.retentionPeriod, func(t *testing.T) {
+			duration, err := v1beta1.NewDuration(tt.retentionPeriod)
+			assert.NilError(t, err)
+			result := generateLogrotateConfig(tt.logFilePath, duration, tt.postrotateScript)
+			assert.Equal(t, tt.result, result)
+		})
+	}
+}
 
-// FIXME: This test is currently broken. Fix after rebasing on new
-// retention API changes.
 func TestParseDurationForLogrotate(t *testing.T) {
 	for _, tt := range []struct {
 		retentionPeriod string
@@ -88,7 +150,12 @@ func TestParseDurationForLogrotate(t *testing.T) {
 			interval:        "daily",
 		},
 		{
-			retentionPeriod: "36hour",
+			retentionPeriod: "35hour",
+			number:          1,
+			interval:        "daily",
+		},
+		{
+			retentionPeriod: "36 hours",
 			number:          2,
 			interval:        "daily",
 		},
@@ -119,8 +186,7 @@ func TestParseDurationForLogrotate(t *testing.T) {
 		},
 	} {
 		t.Run(tt.retentionPeriod, func(t *testing.T) {
-			var duration *v1beta1.Duration
-			err := duration.UnmarshalJSON([]byte(tt.retentionPeriod))
+			duration, err := v1beta1.NewDuration(tt.retentionPeriod)
 			assert.NilError(t, err)
 			number, interval := parseDurationForLogrotate(duration)
 			assert.Equal(t, tt.number, number)

internal/collector/logrotate.conf

@@ -6,6 +6,6 @@
       nocompress
       %s
       postrotate
-           %s
+            %s
       endscript
 }