From 4d98a0637eee66923b854ef75653f74aeb37240b Mon Sep 17 00:00:00 2001
From: Lorenz Brun <lorenz@monogon.tech>
Date: Wed, 16 Mar 2022 18:10:09 +0100
Subject: [PATCH] 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 5d848de88fb..0495eaed7d1 100644
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -1506,13 +1506,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)
-		}
-	}
-
 	if sysruntime.GOOS == "windows" {
 		// On Windows we should not allow other users to read the logs directory
 		// to avoid allowing non-root containers from reading the logs of other containers.
diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go
index b049540d0d3..7b1baa29c14 100644
--- a/pkg/kubelet/kuberuntime/kuberuntime_container.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go
@@ -285,25 +285,6 @@ func (m *kubeGenericRuntimeManager) startContainer(ctx context.Context, 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{
@@ -1317,23 +1298,6 @@ func (m *kubeGenericRuntimeManager) removeContainerLog(ctx context.Context, cont
 		return err
 	}
 
-	resp, err := m.runtimeService.ContainerStatus(ctx, 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 6189b1f07ca..3cd3daa5a6c 100644
--- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go
@@ -19,7 +19,6 @@ package kuberuntime
 import (
 	"context"
 	"fmt"
-	"os"
 	"path/filepath"
 	"sort"
 	"time"
@@ -347,51 +346,6 @@ func (cgc *containerGC) evictPodLogsDirectories(ctx context.Context, allSourcesR
 			}
 		}
 	}
-
-	// 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(ctx, 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 448df444869..d5ecd47337f 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"
@@ -50,13 +49,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 a1589e6d91e..179206eb1dd 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", framework.WithNodeConformance(), func()
 				err := createAndWaitPod(ctx, 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(ctx context.Context) {
-
-				logDir := kubelet.ContainerLogsDir
-
-				// get containerID from created Pod
-				createdLogPod, err := podClient.Get(ctx, 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(ctx, 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(ctx context.Context) {
 
-- 
2.44.1