fix(collectors): store unhealthy pod logs correctly (#909)

The symlinking logs feature led to a regression where symlinks of
unhealthy pods were overwritting logs in the support bundle. This
fix allows the cluster resources collector to instruct the logs
collector not to symlink logs, which in turn ensures logs are not
overwritten.

Fixes: #908
This commit is contained in:
Evans Mungai
2022-12-14 14:47:20 -04:00
committed by GitHub
parent 9c77a0e3da
commit cd1511a8fb
4 changed files with 30 additions and 15 deletions
+3 -8
View File
@@ -167,21 +167,16 @@ func (c *CollectClusterResources) Collect(progressChan chan<- interface{}) (Coll
for _, pod := range unhealthyPods {
allContainers := append(pod.Spec.InitContainers, pod.Spec.Containers...)
for _, container := range allContainers {
logsRoot := ""
if c.BundlePath != "" {
logsRoot = path.Join(c.BundlePath, "cluster-resources", "pods", "logs", pod.Namespace)
}
limits := &troubleshootv1beta2.LogLimits{
MaxLines: 500,
}
podLogs, err := savePodLogs(ctx, logsRoot, client, &pod, "", container.Name, limits, false)
podLogs, err := savePodLogs(ctx, c.BundlePath, client, &pod, "", container.Name, limits, false, false)
if err != nil {
errPath := filepath.Join("cluster-resources", "pods", "logs", pod.Namespace, pod.Name, fmt.Sprintf("%s-logs-errors.log", container.Name))
output.SaveResult(c.BundlePath, errPath, bytes.NewBuffer([]byte(err.Error())))
}
for k, v := range podLogs {
output[filepath.Join("cluster-resources", "pods", "logs", pod.Namespace, k)] = v
}
// Add logs collector results to the rest of the output
output.AddResult(podLogs)
}
}
+11 -5
View File
@@ -71,7 +71,7 @@ func (c *CollectLogs) Collect(progressChan chan<- interface{}) (CollectorResult,
}
for _, containerName := range containerNames {
podLogs, err := savePodLogs(ctx, c.BundlePath, client, &pod, c.Collector.Name, containerName, c.Collector.Limits, false)
podLogs, err := savePodLogs(ctx, c.BundlePath, client, &pod, c.Collector.Name, containerName, c.Collector.Limits, false, true)
if err != nil {
key := fmt.Sprintf("%s/%s-errors.json", c.Collector.Name, pod.Name)
if containerName != "" {
@@ -89,7 +89,7 @@ func (c *CollectLogs) Collect(progressChan chan<- interface{}) (CollectorResult,
}
} else {
for _, container := range c.Collector.ContainerNames {
containerLogs, err := savePodLogs(ctx, c.BundlePath, client, &pod, c.Collector.Name, container, c.Collector.Limits, false)
containerLogs, err := savePodLogs(ctx, c.BundlePath, client, &pod, c.Collector.Name, container, c.Collector.Limits, false, true)
if err != nil {
key := fmt.Sprintf("%s/%s/%s-errors.json", c.Collector.Name, pod.Name, container)
err := output.SaveResult(c.BundlePath, key, marshalErrors([]string{err.Error()}))
@@ -132,8 +132,9 @@ func savePodLogs(
collectorName, container string,
limits *troubleshootv1beta2.LogLimits,
follow bool,
createSymLinks bool,
) (CollectorResult, error) {
return savePodLogsWithInterface(ctx, bundlePath, client, pod, collectorName, container, limits, follow)
return savePodLogsWithInterface(ctx, bundlePath, client, pod, collectorName, container, limits, follow, createSymLinks)
}
func savePodLogsWithInterface(
@@ -144,6 +145,7 @@ func savePodLogsWithInterface(
collectorName, container string,
limits *troubleshootv1beta2.LogLimits,
follow bool,
createSymLinks bool,
) (CollectorResult, error) {
podLogOpts := corev1.PodLogOptions{
Follow: follow,
@@ -185,7 +187,9 @@ func savePodLogsWithInterface(
return nil, errors.Wrap(err, "failed to get log writer")
}
// NOTE: deferred calls are executed in LIFO order i.e called in reverse order
defer result.SymLinkResult(bundlePath, linkRelPathPrefix+".log", filePathPrefix+".log")
if createSymLinks {
defer result.SymLinkResult(bundlePath, linkRelPathPrefix+".log", filePathPrefix+".log")
}
defer result.CloseWriter(bundlePath, filePathPrefix+".log", logWriter)
_, err = io.Copy(logWriter, podLogs)
@@ -207,7 +211,9 @@ func savePodLogsWithInterface(
return nil, errors.Wrap(err, "failed to get previous log writer")
}
// NOTE: deferred calls are executed in LIFO order i.e called in reverse order
defer result.SymLinkResult(bundlePath, linkRelPathPrefix+"-previous.log", filePathPrefix+"-previous.log")
if createSymLinks {
defer result.SymLinkResult(bundlePath, linkRelPathPrefix+"-previous.log", filePathPrefix+"-previous.log")
}
defer result.CloseWriter(bundlePath, filePathPrefix+"-previous.log", logWriter)
_, err = io.Copy(prevLogWriter, podLogs)
+15 -1
View File
@@ -85,12 +85,14 @@ func Test_savePodLogs(t *testing.T) {
name string
withContainerName bool
collectorName string
createSymLinks bool
want CollectorResult
}{
{
name: "with container name",
withContainerName: true,
collectorName: "all-logs",
createSymLinks: true,
want: CollectorResult{
"all-logs/test-pod/nginx.log": []byte("fake logs"),
"all-logs/test-pod/nginx-previous.log": []byte("fake logs"),
@@ -102,6 +104,7 @@ func Test_savePodLogs(t *testing.T) {
name: "without container name",
withContainerName: false,
collectorName: "all-logs",
createSymLinks: true,
want: CollectorResult{
"all-logs/test-pod.log": []byte("fake logs"),
"all-logs/test-pod-previous.log": []byte("fake logs"),
@@ -112,6 +115,7 @@ func Test_savePodLogs(t *testing.T) {
{
name: "without container or collector names",
withContainerName: false,
createSymLinks: true,
want: CollectorResult{
"/test-pod.log": []byte("fake logs"),
"/test-pod-previous.log": []byte("fake logs"),
@@ -119,6 +123,16 @@ func Test_savePodLogs(t *testing.T) {
"cluster-resources/pods/logs/my-namespace/test-pod/nginx-previous.log": []byte("fake logs"),
},
},
{
name: "without sym links",
withContainerName: true,
collectorName: "all-logs",
createSymLinks: false,
want: CollectorResult{
"cluster-resources/pods/logs/my-namespace/test-pod/nginx.log": []byte("fake logs"),
"cluster-resources/pods/logs/my-namespace/test-pod/nginx-previous.log": []byte("fake logs"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -144,7 +158,7 @@ func Test_savePodLogs(t *testing.T) {
if !tt.withContainerName {
containerName = ""
}
got, err := savePodLogsWithInterface(ctx, "", client, pod, tt.collectorName, containerName, limits, false)
got, err := savePodLogsWithInterface(ctx, "", client, pod, tt.collectorName, containerName, limits, false, tt.createSymLinks)
assert.NoError(t, err)
assert.Equal(t, tt.want, got)
})
+1 -1
View File
@@ -179,7 +179,7 @@ func runWithoutTimeout(ctx context.Context, bundlePath string, clientConfig *res
limits := troubleshootv1beta2.LogLimits{
MaxLines: 10000,
}
podLogs, err := savePodLogs(ctx, bundlePath, client, pod, collectorName, "", &limits, true)
podLogs, err := savePodLogs(ctx, bundlePath, client, pod, collectorName, "", &limits, true, true)
if err != nil {
return nil, errors.Wrap(err, "failed to get pod logs")
}