| 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 |
| |