diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index ccabf8d004a..517562c6d1a 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -163,24 +163,31 @@ func getStableKey(pod *v1.Pod, container *v1.Container) string { return fmt.Sprintf("%s_%s_%s_%s_%s", pod.Name, pod.Namespace, string(pod.UID), container.Name, hash) } +// logPathDelimiter is the delimiter used in the log path. +const logPathDelimiter = "_" + // buildContainerLogsPath builds log path for container relative to pod logs directory. func buildContainerLogsPath(containerName string, restartCount int) string { return filepath.Join(containerName, fmt.Sprintf("%d.log", restartCount)) } -// buildFullContainerLogsPath builds absolute log path for container. -func buildFullContainerLogsPath(podUID types.UID, containerName string, restartCount int) string { - return filepath.Join(buildPodLogsDirectory(podUID), buildContainerLogsPath(containerName, restartCount)) -} - // BuildContainerLogsDirectory builds absolute log directory path for a container in pod. -func BuildContainerLogsDirectory(podUID types.UID, containerName string) string { - return filepath.Join(buildPodLogsDirectory(podUID), containerName) +func BuildContainerLogsDirectory(podNamespace, podName string, podUID types.UID, containerName string) string { + return filepath.Join(buildPodLogsDirectory(podNamespace, podName, podUID), containerName) } // buildPodLogsDirectory builds absolute log directory path for a pod sandbox. -func buildPodLogsDirectory(podUID types.UID) string { - return filepath.Join(podLogsRootDirectory, string(podUID)) +func buildPodLogsDirectory(podNamespace, podName string, podUID types.UID) string { + return filepath.Join(podLogsRootDirectory, strings.Join([]string{podNamespace, podName, + string(podUID)}, logPathDelimiter)) +} + +// parsePodUIDFromLogsDirectory parses pod logs directory name and returns the pod UID. +// It supports both the old pod log directory /var/log/pods/UID, and the new pod log +// directory /var/log/pods/NAMESPACE_NAME_UID. +func parsePodUIDFromLogsDirectory(name string) types.UID { + parts := strings.Split(name, logPathDelimiter) + return types.UID(parts[len(parts)-1]) } // toKubeRuntimeStatus converts the runtimeapi.RuntimeStatus to kubecontainer.RuntimeStatus. diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index b5826ab47dd..5e5733e5597 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -205,7 +205,7 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Contai } command, args := kubecontainer.ExpandContainerCommandAndArgs(container, opts.Envs) - logDir := BuildContainerLogsDirectory(kubetypes.UID(pod.UID), container.Name) + logDir := BuildContainerLogsDirectory(pod.Namespace, pod.Name, pod.UID, container.Name) err = m.osInterface.MkdirAll(logDir, 0755) if err != nil { return nil, cleanupAction, fmt.Errorf("create container log directory for container %s failed: %v", container.Name, err) @@ -402,7 +402,6 @@ func (m *kubeGenericRuntimeManager) getPodContainerStatuses(uid kubetypes.UID, n if status.State == runtimeapi.ContainerState_CONTAINER_EXITED { // Populate the termination message if needed. annotatedInfo := getContainerInfoFromAnnotations(status.Annotations) - labeledInfo := getContainerInfoFromLabels(status.Labels) fallbackToLogs := annotatedInfo.TerminationMessagePolicy == v1.TerminationMessageFallbackToLogsOnError && cStatus.ExitCode != 0 tMessage, checkLogs := getTerminationMessage(status, annotatedInfo.TerminationMessagePath, fallbackToLogs) if checkLogs { @@ -413,8 +412,7 @@ func (m *kubeGenericRuntimeManager) getPodContainerStatuses(uid kubetypes.UID, n tMessage = fmt.Sprintf("Error reading termination message from logs: %v", err) } } else { - path := buildFullContainerLogsPath(uid, labeledInfo.ContainerName, annotatedInfo.RestartCount) - tMessage = m.readLastStringFromContainerLogs(path) + tMessage = m.readLastStringFromContainerLogs(status.GetLogPath()) } } // Use the termination message written by the application is not empty @@ -824,8 +822,7 @@ func (m *kubeGenericRuntimeManager) removeContainerLog(containerID string) error return fmt.Errorf("failed to get container status %q: %v", containerID, err) } labeledInfo := getContainerInfoFromLabels(status.Labels) - annotatedInfo := getContainerInfoFromAnnotations(status.Annotations) - path := buildFullContainerLogsPath(labeledInfo.PodUID, labeledInfo.ContainerName, annotatedInfo.RestartCount) + path := status.GetLogPath() if err := m.osInterface.Remove(path); err != nil && !os.IsNotExist(err) { return fmt.Errorf("failed to remove container %q log %q: %v", containerID, path, err) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go index fd0c92e096e..3f8a95e57bc 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go @@ -61,7 +61,7 @@ func TestRemoveContainer(t *testing.T) { err = m.removeContainer(containerID) assert.NoError(t, err) // Verify container log is removed - expectedContainerLogPath := filepath.Join(podLogsRootDirectory, "12345678", "foo", "0.log") + expectedContainerLogPath := filepath.Join(podLogsRootDirectory, "new_bar_12345678", "foo", "0.log") expectedContainerLogSymlink := legacyLogSymlink(containerID, "foo", "bar", "new") assert.Equal(t, fakeOS.Removes, []string{expectedContainerLogPath, expectedContainerLogSymlink}) // Verify container is removed diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go index 1bc77c1739c..97bd3fff24d 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go @@ -339,7 +339,7 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error { } for _, dir := range dirs { name := dir.Name() - podUID := types.UID(name) + podUID := parsePodUIDFromLogsDirectory(name) if !cgc.podStateProvider.IsPodDeleted(podUID) { continue } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go index 8c565d56753..d5fb572a7fa 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go @@ -412,10 +412,16 @@ func TestPodLogDirectoryGC(t *testing.T) { // pod log directories without corresponding pods should be removed. podStateProvider.existingPods["123"] = struct{}{} podStateProvider.existingPods["456"] = struct{}{} + podStateProvider.existingPods["321"] = struct{}{} podStateProvider.runningPods["123"] = struct{}{} podStateProvider.runningPods["456"] = struct{}{} - files := []string{"123", "456", "789", "012"} - removed := []string{filepath.Join(podLogsRootDirectory, "789"), filepath.Join(podLogsRootDirectory, "012")} + podStateProvider.existingPods["321"] = struct{}{} + files := []string{"123", "456", "789", "012", "name_namespace_321", "name_namespace_654"} + removed := []string{ + filepath.Join(podLogsRootDirectory, "789"), + filepath.Join(podLogsRootDirectory, "012"), + filepath.Join(podLogsRootDirectory, "name_namespace_654"), + } ctrl := gomock.NewController(t) defer ctrl.Finish() diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 4266eae13cc..8de68a7e4bc 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -17,6 +17,7 @@ limitations under the License. package kuberuntime import ( + "path/filepath" "reflect" "sort" "testing" @@ -158,6 +159,7 @@ func makeFakeContainer(t *testing.T, m *kubeGenericRuntimeManager, template cont State: template.state, Labels: containerConfig.Labels, Annotations: containerConfig.Annotations, + LogPath: filepath.Join(sandboxConfig.GetLogDirectory(), containerConfig.GetLogPath()), }, SandboxID: podSandboxID, } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go index cf0db44db06..320d897c86d 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go @@ -103,7 +103,7 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxConfig(pod *v1.Pod, attemp podSandboxConfig.Hostname = hostname } - logDir := buildPodLogsDirectory(pod.UID) + logDir := buildPodLogsDirectory(pod.Namespace, pod.Name, pod.UID) podSandboxConfig.LogDirectory = logDir portMappings := []*runtimeapi.PortMapping{} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go index 9d0b7c89b16..5ef8eba1ca3 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go @@ -44,7 +44,7 @@ func TestCreatePodSandbox(t *testing.T) { fakeOS := m.osInterface.(*containertest.FakeOS) fakeOS.MkdirAllFn = func(path string, perm os.FileMode) error { // Check pod logs root directory is created. - assert.Equal(t, filepath.Join(podLogsRootDirectory, "12345678"), path) + assert.Equal(t, filepath.Join(podLogsRootDirectory, pod.Namespace+"_"+pod.Name+"_12345678"), path) assert.Equal(t, os.FileMode(0755), perm) return nil } diff --git a/pkg/kubelet/stats/cri_stats_provider.go b/pkg/kubelet/stats/cri_stats_provider.go index f1dbc34ff06..ad26cce3243 100644 --- a/pkg/kubelet/stats/cri_stats_provider.go +++ b/pkg/kubelet/stats/cri_stats_provider.go @@ -189,7 +189,7 @@ func (p *criStatsProvider) listPodStats(updateCPUNanoCoreUsage bool) ([]statsapi } // Fill available stats for full set of required pod stats - cs := p.makeContainerStats(stats, container, &rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata().GetUid(), updateCPUNanoCoreUsage) + cs := p.makeContainerStats(stats, container, &rootFsInfo, fsIDtoInfo, podSandbox.GetMetadata(), updateCPUNanoCoreUsage) p.addPodNetworkStats(ps, podSandboxID, caInfos, cs, containerNetworkStats[podSandboxID]) p.addPodCPUMemoryStats(ps, types.UID(podSandbox.Metadata.Uid), allInfos, cs) @@ -476,7 +476,7 @@ func (p *criStatsProvider) makeContainerStats( container *runtimeapi.Container, rootFsInfo *cadvisorapiv2.FsInfo, fsIDtoInfo map[runtimeapi.FilesystemIdentifier]*cadvisorapiv2.FsInfo, - uid string, + meta *runtimeapi.PodSandboxMetadata, updateCPUNanoCoreUsage bool, ) *statsapi.ContainerStats { result := &statsapi.ContainerStats{ @@ -543,7 +543,12 @@ func (p *criStatsProvider) makeContainerStats( result.Rootfs.Inodes = imageFsInfo.Inodes } } - containerLogPath := kuberuntime.BuildContainerLogsDirectory(types.UID(uid), container.GetMetadata().GetName()) + // NOTE: This doesn't support the old pod log path, `/var/log/pods/UID`. For containers + // using old log path, empty log stats are returned. This is fine, because we don't + // officially support in-place upgrade anyway. + containerLogPath := kuberuntime.BuildContainerLogsDirectory(meta.GetNamespace(), + meta.GetName(), types.UID(meta.GetUid()), container.GetMetadata().GetName()) + // TODO(random-liu): Collect log stats for logs under the pod log directory. result.Logs = p.getContainerLogStats(containerLogPath, rootFsInfo) return result } diff --git a/pkg/kubelet/stats/cri_stats_provider_test.go b/pkg/kubelet/stats/cri_stats_provider_test.go index 1ba3878f8f4..cc76c6849e7 100644 --- a/pkg/kubelet/stats/cri_stats_provider_test.go +++ b/pkg/kubelet/stats/cri_stats_provider_test.go @@ -166,11 +166,11 @@ func TestCRIListPodStats(t *testing.T) { } fakeLogStats := map[string]*volume.Metrics{ - kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox0-uid"), cName0): containerLogStats0, - kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox0-uid"), cName1): containerLogStats1, - kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox1-uid"), cName2): containerLogStats2, - kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox2-uid"), cName3): containerLogStats4, - kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox3-uid"), cName5): containerLogStats5, + kuberuntime.BuildContainerLogsDirectory("sandbox0-ns", "sandbox0-name", types.UID("sandbox0-uid"), cName0): containerLogStats0, + kuberuntime.BuildContainerLogsDirectory("sandbox0-ns", "sandbox0-name", types.UID("sandbox0-uid"), cName1): containerLogStats1, + kuberuntime.BuildContainerLogsDirectory("sandbox1-ns", "sandbox1-name", types.UID("sandbox1-uid"), cName2): containerLogStats2, + kuberuntime.BuildContainerLogsDirectory("sandbox2-ns", "sandbox2-name", types.UID("sandbox2-uid"), cName3): containerLogStats4, + kuberuntime.BuildContainerLogsDirectory("sandbox3-ns", "sandbox3-name", types.UID("sandbox3-uid"), cName5): containerLogStats5, } fakeLogStatsProvider := NewFakeLogMetricsService(fakeLogStats) diff --git a/test/e2e_node/log_path_test.go b/test/e2e_node/log_path_test.go index c175c19fce9..fff10b49ee1 100644 --- a/test/e2e_node/log_path_test.go +++ b/test/e2e_node/log_path_test.go @@ -157,10 +157,12 @@ var _ = framework.KubeDescribe("ContainerLogPath [NodeConformance]", func() { // get podID from created Pod createdLogPod, err := podClient.Get(logPodName, metav1.GetOptions{}) + podNs := createdLogPod.Namespace + podName := createdLogPod.Name podID := string(createdLogPod.UID) // build log cri file path - expectedCRILogFile := logCRIDir + "/" + podID + "/" + logContainerName + "/0.log" + expectedCRILogFile := logCRIDir + "/" + podNs + "_" + podName + "_" + podID + "/" + logContainerName + "/0.log" logCRICheckPodName := "log-cri-check-" + string(uuid.NewUUID()) err = createAndWaitPod(makeLogCheckPod(logCRICheckPodName, logString, expectedCRILogFile))