Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 1 | From b2d875981cec8eda9c041f858004f613ea928895 Mon Sep 17 00:00:00 2001 |
| 2 | From: Lorenz Brun <lorenz@monogon.tech> |
| 3 | Date: Wed, 16 Mar 2022 18:10:09 +0100 |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 4 | Subject: [PATCH] Drop legacy log path |
| 5 | |
| 6 | --- |
| 7 | pkg/kubelet/kubelet.go | 7 ---- |
| 8 | .../kuberuntime/kuberuntime_container.go | 32 --------------- |
| 9 | pkg/kubelet/kuberuntime/kuberuntime_gc.go | 39 ------------------- |
| 10 | pkg/kubelet/runonce.go | 8 ---- |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 11 | test/e2e_node/log_path_test.go | 18 --------- |
| 12 | 5 files changed, 104 deletions(-) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 13 | |
| 14 | diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 15 | index 2013c871a60..c3fd4221544 100644 |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 16 | --- a/pkg/kubelet/kubelet.go |
| 17 | +++ b/pkg/kubelet/kubelet.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 18 | @@ -1379,13 +1379,6 @@ func (kl *Kubelet) initializeModules() error { |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 19 | return err |
| 20 | } |
| 21 | |
| 22 | - // If the container logs directory does not exist, create it. |
| 23 | - if _, err := os.Stat(ContainerLogsDir); err != nil { |
| 24 | - if err := kl.os.MkdirAll(ContainerLogsDir, 0755); err != nil { |
| 25 | - return fmt.Errorf("failed to create directory %q: %v", ContainerLogsDir, err) |
| 26 | - } |
| 27 | - } |
| 28 | - |
| 29 | // Start the image manager. |
| 30 | kl.imageManager.Start() |
| 31 | |
| 32 | diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 33 | index 9c762ac309e..d1148cc7de5 100644 |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 34 | --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go |
| 35 | +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 36 | @@ -254,25 +254,6 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 37 | } |
| 38 | m.recordContainerEvent(pod, container, containerID, v1.EventTypeNormal, events.StartedContainer, fmt.Sprintf("Started container %s", container.Name)) |
| 39 | |
| 40 | - // Symlink container logs to the legacy container log location for cluster logging |
| 41 | - // support. |
| 42 | - // TODO(random-liu): Remove this after cluster logging supports CRI container log path. |
| 43 | - containerMeta := containerConfig.GetMetadata() |
| 44 | - sandboxMeta := podSandboxConfig.GetMetadata() |
| 45 | - legacySymlink := legacyLogSymlink(containerID, containerMeta.Name, sandboxMeta.Name, |
| 46 | - sandboxMeta.Namespace) |
| 47 | - containerLog := filepath.Join(podSandboxConfig.LogDirectory, containerConfig.LogPath) |
| 48 | - // only create legacy symlink if containerLog path exists (or the error is not IsNotExist). |
| 49 | - // Because if containerLog path does not exist, only dangling legacySymlink is created. |
| 50 | - // This dangling legacySymlink is later removed by container gc, so it does not make sense |
| 51 | - // to create it in the first place. it happens when journald logging driver is used with docker. |
| 52 | - if _, err := m.osInterface.Stat(containerLog); !os.IsNotExist(err) { |
| 53 | - if err := m.osInterface.Symlink(containerLog, legacySymlink); err != nil { |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 54 | - klog.ErrorS(err, "Failed to create legacy symbolic link", "path", legacySymlink, |
| 55 | - "containerID", containerID, "containerLogPath", containerLog) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 56 | - } |
| 57 | - } |
| 58 | - |
| 59 | // Step 4: execute the post start hook. |
| 60 | if container.Lifecycle != nil && container.Lifecycle.PostStart != nil { |
| 61 | kubeContainerID := kubecontainer.ContainerID{ |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 62 | @@ -967,19 +948,6 @@ func (m *kubeGenericRuntimeManager) removeContainerLog(containerID string) error |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 63 | return err |
| 64 | } |
| 65 | |
| 66 | - status, err := m.runtimeService.ContainerStatus(containerID) |
| 67 | - if err != nil { |
| 68 | - return fmt.Errorf("failed to get container status %q: %v", containerID, err) |
| 69 | - } |
| 70 | - // Remove the legacy container log symlink. |
| 71 | - // TODO(random-liu): Remove this after cluster logging supports CRI container log path. |
| 72 | - labeledInfo := getContainerInfoFromLabels(status.Labels) |
| 73 | - legacySymlink := legacyLogSymlink(containerID, labeledInfo.ContainerName, labeledInfo.PodName, |
| 74 | - labeledInfo.PodNamespace) |
| 75 | - if err := m.osInterface.Remove(legacySymlink); err != nil && !os.IsNotExist(err) { |
| 76 | - return fmt.Errorf("failed to remove container %q log legacy symbolic link %q: %v", |
| 77 | - containerID, legacySymlink, err) |
| 78 | - } |
| 79 | return nil |
| 80 | } |
| 81 | |
| 82 | diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 83 | index 610026661b7..a09dec91fbb 100644 |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 84 | --- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go |
| 85 | +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go |
| 86 | @@ -18,7 +18,6 @@ package kuberuntime |
| 87 | |
| 88 | import ( |
| 89 | "fmt" |
| 90 | - "os" |
| 91 | "path/filepath" |
| 92 | "sort" |
| 93 | "time" |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 94 | @@ -347,44 +346,6 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error { |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 95 | } |
| 96 | } |
| 97 | } |
| 98 | - |
| 99 | - // Remove dead container log symlinks. |
| 100 | - // TODO(random-liu): Remove this after cluster logging supports CRI container log path. |
| 101 | - logSymlinks, _ := osInterface.Glob(filepath.Join(legacyContainerLogsDir, fmt.Sprintf("*.%s", legacyLogSuffix))) |
| 102 | - for _, logSymlink := range logSymlinks { |
| 103 | - if _, err := osInterface.Stat(logSymlink); os.IsNotExist(err) { |
| 104 | - if containerID, err := getContainerIDFromLegacyLogSymlink(logSymlink); err == nil { |
| 105 | - status, err := cgc.manager.runtimeService.ContainerStatus(containerID) |
| 106 | - if err != nil { |
| 107 | - // TODO: we should handle container not found (i.e. container was deleted) case differently |
| 108 | - // once https://github.com/kubernetes/kubernetes/issues/63336 is resolved |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 109 | - klog.InfoS("Error getting ContainerStatus for containerID", "containerID", containerID, "err", err) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 110 | - } else if status.State != runtimeapi.ContainerState_CONTAINER_EXITED { |
| 111 | - // Here is how container log rotation works (see containerLogManager#rotateLatestLog): |
| 112 | - // |
| 113 | - // 1. rename current log to rotated log file whose filename contains current timestamp (fmt.Sprintf("%s.%s", log, timestamp)) |
| 114 | - // 2. reopen the container log |
| 115 | - // 3. if #2 fails, rename rotated log file back to container log |
| 116 | - // |
| 117 | - // There is small but indeterministic amount of time during which log file doesn't exist (between steps #1 and #2, between #1 and #3). |
| 118 | - // Hence the symlink may be deemed unhealthy during that period. |
| 119 | - // See https://github.com/kubernetes/kubernetes/issues/52172 |
| 120 | - // |
| 121 | - // We only remove unhealthy symlink for dead containers |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 122 | - klog.V(5).InfoS("Container is still running, not removing symlink", "containerID", containerID, "path", logSymlink) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 123 | - continue |
| 124 | - } |
| 125 | - } else { |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 126 | - klog.V(4).InfoS("Unable to obtain container ID", "err", err) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 127 | - } |
| 128 | - err := osInterface.Remove(logSymlink) |
| 129 | - if err != nil { |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 130 | - klog.ErrorS(err, "Failed to remove container log dead symlink", "path", logSymlink) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 131 | - } else { |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 132 | - klog.V(4).InfoS("Removed symlink", "path", logSymlink) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 133 | - } |
| 134 | - } |
| 135 | - } |
| 136 | return nil |
| 137 | } |
| 138 | |
| 139 | diff --git a/pkg/kubelet/runonce.go b/pkg/kubelet/runonce.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 140 | index 19b8a4f6a7b..2f0aad713e3 100644 |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 141 | --- a/pkg/kubelet/runonce.go |
| 142 | +++ b/pkg/kubelet/runonce.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 143 | @@ -19,7 +19,6 @@ package kubelet |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 144 | import ( |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 145 | "context" |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 146 | "fmt" |
| 147 | - "os" |
| 148 | "time" |
| 149 | |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 150 | v1 "k8s.io/api/core/v1" |
| 151 | @@ -49,13 +48,6 @@ func (kl *Kubelet) RunOnce(updates <-chan kubetypes.PodUpdate) ([]RunPodResult, |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 152 | return nil, err |
| 153 | } |
| 154 | |
| 155 | - // If the container logs directory does not exist, create it. |
| 156 | - if _, err := os.Stat(ContainerLogsDir); err != nil { |
| 157 | - if err := kl.os.MkdirAll(ContainerLogsDir, 0755); err != nil { |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 158 | - klog.ErrorS(err, "Failed to create directory", "path", ContainerLogsDir) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 159 | - } |
| 160 | - } |
| 161 | - |
| 162 | select { |
| 163 | case u := <-updates: |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 164 | klog.InfoS("Processing manifest with pods", "numPods", len(u.Pods)) |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 165 | diff --git a/test/e2e_node/log_path_test.go b/test/e2e_node/log_path_test.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 166 | index cfdd9823cb1..35f3b7be465 100644 |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 167 | --- a/test/e2e_node/log_path_test.go |
| 168 | +++ b/test/e2e_node/log_path_test.go |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 169 | @@ -22,8 +22,6 @@ import ( |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 170 | v1 "k8s.io/api/core/v1" |
| 171 | metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" |
| 172 | "k8s.io/apimachinery/pkg/util/uuid" |
| 173 | - "k8s.io/kubernetes/pkg/kubelet" |
| 174 | - kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" |
| 175 | "k8s.io/kubernetes/test/e2e/framework" |
| 176 | e2epod "k8s.io/kubernetes/test/e2e/framework/pod" |
| 177 | e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" |
Lorenz Brun | d13c1c6 | 2022-03-30 19:58:58 +0200 | [diff] [blame] | 178 | @@ -144,22 +142,6 @@ var _ = SIGDescribe("ContainerLogPath [NodeConformance]", func() { |
Lorenz Brun | 7b82227 | 2021-02-03 17:03:41 +0100 | [diff] [blame] | 179 | err := createAndWaitPod(makeLogPod(logPodName, logString)) |
| 180 | framework.ExpectNoError(err, "Failed waiting for pod: %s to enter success state", logPodName) |
| 181 | }) |
| 182 | - ginkgo.It("should print log to correct log path", func() { |
| 183 | - |
| 184 | - logDir := kubelet.ContainerLogsDir |
| 185 | - |
| 186 | - // get containerID from created Pod |
| 187 | - createdLogPod, err := podClient.Get(context.TODO(), logPodName, metav1.GetOptions{}) |
| 188 | - logContainerID := kubecontainer.ParseContainerID(createdLogPod.Status.ContainerStatuses[0].ContainerID) |
| 189 | - framework.ExpectNoError(err, "Failed to get pod: %s", logPodName) |
| 190 | - |
| 191 | - // build log file path |
| 192 | - expectedlogFile := logDir + "/" + logPodName + "_" + f.Namespace.Name + "_" + logContainerName + "-" + logContainerID.ID + ".log" |
| 193 | - |
| 194 | - logCheckPodName := "log-check-" + string(uuid.NewUUID()) |
| 195 | - err = createAndWaitPod(makeLogCheckPod(logCheckPodName, logString, expectedlogFile)) |
| 196 | - framework.ExpectNoError(err, "Failed waiting for pod: %s to enter success state", logCheckPodName) |
| 197 | - }) |
| 198 | |
| 199 | ginkgo.It("should print log to correct cri log path", func() { |
| 200 | |
| 201 | -- |
| 202 | 2.25.1 |
| 203 | |