New pod log directory /var/log/pods/NAMESPACE_NAME_UID.

Signed-off-by: Lantao Liu <lantaol@google.com>

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 {
+func BuildContainerLogsDirectory(podNamespace, podName string, podUID types.UID, containerName string) string {
-	return filepath.Join(buildPodLogsDirectory(podUID), containerName)
+	return filepath.Join(buildPodLogsDirectory(podNamespace, podName, podUID), containerName)
 }
 
 // buildPodLogsDirectory builds absolute log directory path for a pod sandbox.
-func buildPodLogsDirectory(podUID types.UID) string {
+func buildPodLogsDirectory(podNamespace, podName string, podUID types.UID) string {
-	return filepath.Join(podLogsRootDirectory, string(podUID))
+	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.

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(status.GetLogPath())
-					tMessage = m.readLastStringFromContainerLogs(path)
 				}
 			}
 			// 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 := status.GetLogPath()
-	path := buildFullContainerLogsPath(labeledInfo.PodUID, labeledInfo.ContainerName, annotatedInfo.RestartCount)
 	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)
 	}

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

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
 			}

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"}
+	podStateProvider.existingPods["321"] = struct{}{}
-	removed := []string{filepath.Join(podLogsRootDirectory, "789"), filepath.Join(podLogsRootDirectory, "012")}
+	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()

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

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

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
 	}

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
 }

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("sandbox0-ns", "sandbox0-name", types.UID("sandbox0-uid"), cName0): containerLogStats0,
-		kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox0-uid"), cName1): containerLogStats1,
+		kuberuntime.BuildContainerLogsDirectory("sandbox0-ns", "sandbox0-name", types.UID("sandbox0-uid"), cName1): containerLogStats1,
-		kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox1-uid"), cName2): containerLogStats2,
+		kuberuntime.BuildContainerLogsDirectory("sandbox1-ns", "sandbox1-name", types.UID("sandbox1-uid"), cName2): containerLogStats2,
-		kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox2-uid"), cName3): containerLogStats4,
+		kuberuntime.BuildContainerLogsDirectory("sandbox2-ns", "sandbox2-name", types.UID("sandbox2-uid"), cName3): containerLogStats4,
-		kuberuntime.BuildContainerLogsDirectory(types.UID("sandbox3-uid"), cName5): containerLogStats5,
+		kuberuntime.BuildContainerLogsDirectory("sandbox3-ns", "sandbox3-name", types.UID("sandbox3-uid"), cName5): containerLogStats5,
 	}
 	fakeLogStatsProvider := NewFakeLogMetricsService(fakeLogStats)
 

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