Honor maxAge for log collector if set in the spec

Author: divolgin

pkg/collect/logs.go

@@ -110,27 +110,7 @@ func getPodLogs(ctx context.Context, client *kubernetes.Clientset, pod corev1.Po
 		Container: container,
 	}
 
-	defaultMaxLines := int64(10000)
-	if limits == nil || limits.MaxLines == 0 {
-		podLogOpts.TailLines = &defaultMaxLines
-	} else {
-		podLogOpts.TailLines = &limits.MaxLines
-	}
-	if limits != nil && !limits.SinceTime.IsZero() {
-		podLogOpts.SinceTime = &limits.SinceTime
-
-	} else if limits != nil && limits.MaxAge != "" {
-		parsedDuration, err := time.ParseDuration(limits.MaxAge)
-		if err != nil {
-			logger.Printf("unable to parse time duration %s\n", limits.MaxAge)
-		} else {
-			now := time.Now()
-			then := now.Add(0 - parsedDuration)
-			kthen := metav1.NewTime(then)
-
-			podLogOpts.SinceTime = &kthen
-		}
-	}
+	setLogLimits(&podLogOpts, limits, convertMaxAgeToTime)
 
 	fileKey := fmt.Sprintf("%s/%s", name, pod.Name)
 	if container != "" {
@@ -172,6 +152,48 @@ func getPodLogs(ctx context.Context, client *kubernetes.Clientset, pod corev1.Po
 	return result, nil
 }
 
+func convertMaxAgeToTime(maxAge string) *metav1.Time {
+	parsedDuration, err := time.ParseDuration(maxAge)
+	if err != nil {
+		logger.Printf("unable to parse time duration %s\n", maxAge)
+		return nil
+	}
+
+	now := time.Now()
+	then := now.Add(0 - parsedDuration)
+	kthen := metav1.NewTime(then)
+
+	return &kthen
+}
+
+func setLogLimits(podLogOpts *corev1.PodLogOptions, limits *troubleshootv1beta2.LogLimits, maxAgeParser func(maxAge string) *metav1.Time) {
+	if podLogOpts == nil {
+		return
+	}
+
+	defaultMaxLines := int64(10000)
+	if limits == nil {
+		podLogOpts.TailLines = &defaultMaxLines
+		return
+	}
+
+	if !limits.SinceTime.IsZero() {
+		podLogOpts.SinceTime = &limits.SinceTime
+		return
+	}
+
+	if limits.MaxAge != "" {
+		podLogOpts.SinceTime = maxAgeParser(limits.MaxAge)
+		return
+	}
+
+	if limits.MaxLines == 0 {
+		podLogOpts.TailLines = &defaultMaxLines
+	} else {
+		podLogOpts.TailLines = &limits.MaxLines
+	}
+}
+
 func getLogsErrorsFileName(logsCollector *troubleshootv1beta2.Logs) string {
 	if len(logsCollector.Name) > 0 {
 		return fmt.Sprintf("%s/errors.json", logsCollector.Name)

pkg/collect/logs_test.go

@@ -0,0 +1,79 @@
+package collect
+
+import (
+	"testing"
+	"time"
+
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func Test_setLogLimits(t *testing.T) {
+	defaultMaxLines := int64(10000)
+	customLines := int64(20)
+	maxAge := "10h"
+	sinceWhen := metav1.NewTime(time.Now().Add(-10 * time.Hour))
+
+	convertMaxAgeToTime := func(maxAge string) *metav1.Time {
+		return &sinceWhen
+	}
+
+	tests := []struct {
+		name     string
+		limits   *troubleshootv1beta2.LogLimits
+		expected corev1.PodLogOptions
+		validate func(t *testing.T, podLogOpts *corev1.PodLogOptions)
+	}{
+		{
+			name:   "default limits",
+			limits: nil,
+			expected: corev1.PodLogOptions{
+				TailLines: &defaultMaxLines,
+			},
+		},
+		{
+			name: "custom limit lines",
+			limits: &troubleshootv1beta2.LogLimits{
+				MaxLines: customLines,
+			},
+			expected: corev1.PodLogOptions{
+				TailLines: &customLines,
+			},
+		},
+		{
+			name: "max age",
+			limits: &troubleshootv1beta2.LogLimits{
+				MaxAge: maxAge,
+			},
+			expected: corev1.PodLogOptions{
+				SinceTime: &sinceWhen,
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			req := require.New(t)
+
+			actual := corev1.PodLogOptions{}
+			setLogLimits(&actual, test.limits, convertMaxAgeToTime)
+
+			if test.expected.TailLines != nil {
+				req.NotNil(actual.TailLines)
+				assert.Equal(t, *test.expected.TailLines, *actual.TailLines)
+			} else {
+				req.Nil(actual.TailLines)
+			}
+
+			if test.expected.SinceTime != nil {
+				req.NotNil(actual.SinceTime)
+				assert.Equal(t, *test.expected.SinceTime, *actual.SinceTime)
+			} else {
+				req.Nil(actual.SinceTime)
+			}
+		})
+	}
+}