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