From 8e482462fb325168403966090331dc2f1c957028 Mon Sep 17 00:00:00 2001
From: Jimmi Dyson <jimmidyson@gmail.com>
Date: Tue, 7 Jul 2015 15:10:48 +0100
Subject: [PATCH] Fixes #8945: Cleanup log symlinks on remove & dead log
 symlinks

---
 pkg/kubelet/container_gc.go        | 37 ++++++++++++++++++++++++++++--
 pkg/kubelet/dockertools/docker.go  |  6 +++++
 pkg/kubelet/dockertools/manager.go |  2 +-
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/pkg/kubelet/container_gc.go b/pkg/kubelet/container_gc.go
index c5e8ffa06a7..c8ae50f30f1 100644
--- a/pkg/kubelet/container_gc.go
+++ b/pkg/kubelet/container_gc.go
@@ -18,6 +18,9 @@ package kubelet
 
 import (
 	"fmt"
+	"os"
+	"path"
+	"path/filepath"
 	"sort"
 	"time"
 
@@ -55,6 +58,9 @@ type realContainerGC struct {
 
 	// Policy for garbage collection.
 	policy ContainerGCPolicy
+
+	// The path to the symlinked docker logs
+	containerLogsDir string
 }
 
 // New containerGC instance with the specified policy.
@@ -64,8 +70,9 @@ func newContainerGC(dockerClient dockertools.DockerInterface, policy ContainerGC
 	}
 
 	return &realContainerGC{
-		dockerClient: dockerClient,
-		policy:       policy,
+		dockerClient:     dockerClient,
+		policy:           policy,
+		containerLogsDir: containerLogsDir,
 	}, nil
 }
 
@@ -79,6 +86,13 @@ type containerGCInfo struct {
 
 	// Creation time for the container.
 	createTime time.Time
+
+	// Full pod name, including namespace in the format `namespace_podName`.
+	// This comes from dockertools.ParseDockerName(...)
+	podNameWithNamespace string
+
+	// Container name in pod
+	containerName string
 }
 
 // Containers are considered for eviction as units of (UID, container name) pair.
@@ -157,6 +171,18 @@ func (cgc *realContainerGC) GarbageCollect() error {
 		}
 	}
 
+	// Remove dead symlinks - should only happen on upgrade
+	// from a k8s version without proper log symlink cleanup
+	logSymlinks, _ := filepath.Glob(path.Join(cgc.containerLogsDir, fmt.Sprintf("*.%s", dockertools.LogSuffix)))
+	for _, logSymlink := range logSymlinks {
+		if _, err = os.Stat(logSymlink); os.IsNotExist(err) {
+			err = os.Remove(logSymlink)
+			if err != nil {
+				glog.Warningf("Failed to remove container log dead symlink %q: %v", logSymlink, err)
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -179,6 +205,11 @@ func (cgc *realContainerGC) removeOldestN(containers []containerGCInfo, toRemove
 		if err != nil {
 			glog.Warningf("Failed to remove dead container %q: %v", containers[i].name, err)
 		}
+		symlinkPath := dockertools.LogSymlink(cgc.containerLogsDir, containers[i].podNameWithNamespace, containers[i].containerName, containers[i].id)
+		err = os.Remove(symlinkPath)
+		if !os.IsNotExist(err) {
+			glog.Warningf("Failed to remove container %q log symlink %q: %v", containers[i].name, symlinkPath, err)
+		}
 	}
 
 	// Assume we removed the containers so that we're not too aggressive.
@@ -223,6 +254,8 @@ func (cgc *realContainerGC) evictableContainers() (containersByEvictUnit, []cont
 				uid:  containerName.PodUID,
 				name: containerName.ContainerName,
 			}
+			containerInfo.podNameWithNamespace = containerName.PodFullName
+			containerInfo.containerName = containerName.ContainerName
 			evictUnits[key] = append(evictUnits[key], containerInfo)
 		}
 	}
diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go
index fe3a145bec3..f29a1f2a62b 100644
--- a/pkg/kubelet/dockertools/docker.go
+++ b/pkg/kubelet/dockertools/docker.go
@@ -21,6 +21,7 @@ import (
 	"math/rand"
 	"net/http"
 	"os"
+	"path"
 	"strconv"
 	"strings"
 
@@ -42,6 +43,7 @@ const (
 	PodInfraContainerName  = leaky.PodInfraContainerName
 	DockerPrefix           = "docker://"
 	PodInfraContainerImage = "gcr.io/google_containers/pause:0.8.0"
+	LogSuffix              = "log"
 )
 
 const (
@@ -276,6 +278,10 @@ func ParseDockerName(name string) (dockerName *KubeletContainerName, hash uint64
 	return &KubeletContainerName{podFullName, podUID, containerName}, hash, nil
 }
 
+func LogSymlink(containerLogsDir, podFullName, containerName, dockerId string) string {
+	return path.Join(containerLogsDir, fmt.Sprintf("%s_%s-%s.%s", podFullName, containerName, dockerId, LogSuffix))
+}
+
 // Get a docker endpoint, either from the string passed in, or $DOCKER_HOST environment variables
 func getDockerEndpoint(dockerEndpoint string) string {
 	var endpoint string
diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go
index 8be453cb1e9..c7a9f22bcd6 100644
--- a/pkg/kubelet/dockertools/manager.go
+++ b/pkg/kubelet/dockertools/manager.go
@@ -1207,7 +1207,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 	// labels for Cloud Logging.
 	podFullName := kubecontainer.GetPodFullName(pod)
 	containerLogFile := path.Join(dm.dockerRoot, "containers", id, fmt.Sprintf("%s-json.log", id))
-	symlinkFile := path.Join(dm.containerLogsDir, fmt.Sprintf("%s_%s-%s.log", podFullName, container.Name, id))
+	symlinkFile := LogSymlink(dm.containerLogsDir, podFullName, container.Name, id)
 	if err = dm.os.Symlink(containerLogFile, symlinkFile); err != nil {
 		glog.Errorf("Failed to create symbolic link to the log file of pod %q container %q: %v", podFullName, container.Name, err)
 	}