From 6b4af308e9b420c146983ddb031fd80be0a15ec9 Mon Sep 17 00:00:00 2001
From: Lorenz Brun <lorenz@monogon.tech>
Date: Wed, 16 Mar 2022 18:10:09 +0100
Subject: [PATCH 3/5] Drop legacy log path

---
 pkg/kubelet/kubelet.go                        |  7 ---
 .../kuberuntime/kuberuntime_container.go      | 36 ---------------
 pkg/kubelet/kuberuntime/kuberuntime_gc.go     | 46 -------------------
 pkg/kubelet/runonce.go                        |  8 ----
 test/e2e_node/log_path_test.go                | 18 --------
 5 files changed, 115 deletions(-)

diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go
index 01e9c49c6f1..895fa86fb7e 100644
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -1326,13 +1326,6 @@ func (kl *Kubelet) initializeModules() error {
 		return err
 	}
 
-	// If the container logs directory does not exist, create it.
-	if _, err := os.Stat(ContainerLogsDir); err != nil {
-		if err := kl.os.MkdirAll(ContainerLogsDir, 0755); err != nil {
-			return fmt.Errorf("failed to create directory %q: %v", ContainerLogsDir, err)
-		}
-	}
-
 	// Start the image manager.
 	kl.imageManager.Start()
 
diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go
index 680eab7a56e..52210adb35d 100644
--- a/pkg/kubelet/kuberuntime/kuberuntime_container.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go
@@ -255,25 +255,6 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb
 	}
 	m.recordContainerEvent(pod, container, containerID, v1.EventTypeNormal, events.StartedContainer, fmt.Sprintf("Started container %s", container.Name))
 
-	// Symlink container logs to the legacy container log location for cluster logging
-	// support.
-	// TODO(random-liu): Remove this after cluster logging supports CRI container log path.
-	containerMeta := containerConfig.GetMetadata()
-	sandboxMeta := podSandboxConfig.GetMetadata()
-	legacySymlink := legacyLogSymlink(containerID, containerMeta.Name, sandboxMeta.Name,
-		sandboxMeta.Namespace)
-	containerLog := filepath.Join(podSandboxConfig.LogDirectory, containerConfig.LogPath)
-	// only create legacy symlink if containerLog path exists (or the error is not IsNotExist).
-	// Because if containerLog path does not exist, only dangling legacySymlink is created.
-	// This dangling legacySymlink is later removed by container gc, so it does not make sense
-	// to create it in the first place. it happens when journald logging driver is used with docker.
-	if _, err := m.osInterface.Stat(containerLog); !os.IsNotExist(err) {
-		if err := m.osInterface.Symlink(containerLog, legacySymlink); err != nil {
-			klog.ErrorS(err, "Failed to create legacy symbolic link", "path", legacySymlink,
-				"containerID", containerID, "containerLogPath", containerLog)
-		}
-	}
-
 	// Step 4: execute the post start hook.
 	if container.Lifecycle != nil && container.Lifecycle.PostStart != nil {
 		kubeContainerID := kubecontainer.ContainerID{
@@ -970,23 +951,6 @@ func (m *kubeGenericRuntimeManager) removeContainerLog(containerID string) error
 		return err
 	}
 
-	resp, err := m.runtimeService.ContainerStatus(containerID, false)
-	if err != nil {
-		return fmt.Errorf("failed to get container status %q: %v", containerID, err)
-	}
-	status := resp.GetStatus()
-	if status == nil {
-		return remote.ErrContainerStatusNil
-	}
-	// Remove the legacy container log symlink.
-	// TODO(random-liu): Remove this after cluster logging supports CRI container log path.
-	labeledInfo := getContainerInfoFromLabels(status.Labels)
-	legacySymlink := legacyLogSymlink(containerID, labeledInfo.ContainerName, labeledInfo.PodName,
-		labeledInfo.PodNamespace)
-	if err := m.osInterface.Remove(legacySymlink); err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to remove container %q log legacy symbolic link %q: %v",
-			containerID, legacySymlink, err)
-	}
 	return nil
 }
 
diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go
index a72691bfcfc..a09dec91fbb 100644
--- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go
@@ -18,7 +18,6 @@ package kuberuntime
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 	"sort"
 	"time"
@@ -347,51 +346,6 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error {
 			}
 		}
 	}
-
-	// Remove dead container log symlinks.
-	// TODO(random-liu): Remove this after cluster logging supports CRI container log path.
-	logSymlinks, _ := osInterface.Glob(filepath.Join(legacyContainerLogsDir, fmt.Sprintf("*.%s", legacyLogSuffix)))
-	for _, logSymlink := range logSymlinks {
-		if _, err := osInterface.Stat(logSymlink); os.IsNotExist(err) {
-			if containerID, err := getContainerIDFromLegacyLogSymlink(logSymlink); err == nil {
-				resp, err := cgc.manager.runtimeService.ContainerStatus(containerID, false)
-				if err != nil {
-					// TODO: we should handle container not found (i.e. container was deleted) case differently
-					// once https://github.com/kubernetes/kubernetes/issues/63336 is resolved
-					klog.InfoS("Error getting ContainerStatus for containerID", "containerID", containerID, "err", err)
-				} else {
-					status := resp.GetStatus()
-					if status == nil {
-						klog.V(4).InfoS("Container status is nil")
-						continue
-					}
-					if status.State != runtimeapi.ContainerState_CONTAINER_EXITED {
-						// Here is how container log rotation works (see containerLogManager#rotateLatestLog):
-						//
-						// 1. rename current log to rotated log file whose filename contains current timestamp (fmt.Sprintf("%s.%s", log, timestamp))
-						// 2. reopen the container log
-						// 3. if #2 fails, rename rotated log file back to container log
-						//
-						// There is small but indeterministic amount of time during which log file doesn't exist (between steps #1 and #2, between #1 and #3).
-						// Hence the symlink may be deemed unhealthy during that period.
-						// See https://github.com/kubernetes/kubernetes/issues/52172
-						//
-						// We only remove unhealthy symlink for dead containers
-						klog.V(5).InfoS("Container is still running, not removing symlink", "containerID", containerID, "path", logSymlink)
-						continue
-					}
-				}
-			} else {
-				klog.V(4).InfoS("Unable to obtain container ID", "err", err)
-			}
-			err := osInterface.Remove(logSymlink)
-			if err != nil {
-				klog.ErrorS(err, "Failed to remove container log dead symlink", "path", logSymlink)
-			} else {
-				klog.V(4).InfoS("Removed symlink", "path", logSymlink)
-			}
-		}
-	}
 	return nil
 }
 
diff --git a/pkg/kubelet/runonce.go b/pkg/kubelet/runonce.go
index 00f3022af5a..99d1c8b6a8d 100644
--- a/pkg/kubelet/runonce.go
+++ b/pkg/kubelet/runonce.go
@@ -19,7 +19,6 @@ package kubelet
 import (
 	"context"
 	"fmt"
-	"os"
 	"time"
 
 	v1 "k8s.io/api/core/v1"
@@ -49,13 +48,6 @@ func (kl *Kubelet) RunOnce(updates <-chan kubetypes.PodUpdate) ([]RunPodResult,
 		return nil, err
 	}
 
-	// If the container logs directory does not exist, create it.
-	if _, err := os.Stat(ContainerLogsDir); err != nil {
-		if err := kl.os.MkdirAll(ContainerLogsDir, 0755); err != nil {
-			klog.ErrorS(err, "Failed to create directory", "path", ContainerLogsDir)
-		}
-	}
-
 	select {
 	case u := <-updates:
 		klog.InfoS("Processing manifest with pods", "numPods", len(u.Pods))
diff --git a/test/e2e_node/log_path_test.go b/test/e2e_node/log_path_test.go
index cdbc2b6d8ae..3535a81822f 100644
--- a/test/e2e_node/log_path_test.go
+++ b/test/e2e_node/log_path_test.go
@@ -22,8 +22,6 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
-	"k8s.io/kubernetes/pkg/kubelet"
-	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
 	admissionapi "k8s.io/pod-security-admission/api"
@@ -122,22 +120,6 @@ var _ = SIGDescribe("ContainerLogPath [NodeConformance]", func() {
 				err := createAndWaitPod(makeLogPod(logPodName, logString))
 				framework.ExpectNoError(err, "Failed waiting for pod: %s to enter success state", logPodName)
 			})
-			ginkgo.It("should print log to correct log path", func() {
-
-				logDir := kubelet.ContainerLogsDir
-
-				// get containerID from created Pod
-				createdLogPod, err := podClient.Get(context.TODO(), logPodName, metav1.GetOptions{})
-				logContainerID := kubecontainer.ParseContainerID(createdLogPod.Status.ContainerStatuses[0].ContainerID)
-				framework.ExpectNoError(err, "Failed to get pod: %s", logPodName)
-
-				// build log file path
-				expectedlogFile := logDir + "/" + logPodName + "_" + f.Namespace.Name + "_" + logContainerName + "-" + logContainerID.ID + ".log"
-
-				logCheckPodName := "log-check-" + string(uuid.NewUUID())
-				err = createAndWaitPod(makeLogCheckPod(logCheckPodName, logString, expectedlogFile))
-				framework.ExpectNoError(err, "Failed waiting for pod: %s to enter success state", logCheckPodName)
-			})
 
 			ginkgo.It("should print log to correct cri log path", func() {
 
-- 
2.25.1