bugfix: container log path map not initialized for existing businesses

Signed-off-by: Starnop <[email protected]>

Author: starnop

cri/v1alpha2/cri.go

@@ -53,6 +53,7 @@ const (
 	containerTypeLabelSandbox   = "sandbox"
 	containerTypeLabelContainer = "container"
 	sandboxIDLabelKey           = "io.kubernetes.sandbox.id"
+	containerLogPathLabelKey    = "io.kubernetes.container.logpath"
 
 	// sandboxContainerName is a string to include in the pouch container so
 	// that users can easily identify the sandboxes.
@@ -251,8 +252,7 @@ func (c *CriManager) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
 		return nil, err
 	}
 	sandboxMeta := &SandboxMeta{
-		ID:              id,
-		ContainerLogMap: make(map[string]string),
+		ID: id,
 	}
 	if err := c.SandboxStore.Put(sandboxMeta); err != nil {
 		return nil, err
@@ -681,6 +681,12 @@ func (c *CriManager) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	labels[containerTypeLabelKey] = containerTypeLabelContainer
 	// Write the sandbox ID in the labels.
 	labels[sandboxIDLabelKey] = podSandboxID
+	// Get container log.
+	var logPath string
+	if config.GetLogPath() != "" {
+		logPath = filepath.Join(sandboxConfig.GetLogDirectory(), config.GetLogPath())
+		labels[containerLogPathLabelKey] = logPath
+	}
 
 	image := ""
 	if iSpec := config.GetImage(); iSpec != nil {
@@ -762,14 +768,7 @@ func (c *CriManager) CreateContainer(ctx context.Context, r *runtime.CreateConta
 		}
 	}()
 
-	// Get container log.
-	if config.GetLogPath() != "" {
-		logPath := filepath.Join(sandboxConfig.GetLogDirectory(), config.GetLogPath())
-		sandboxMeta.ContainerLogMap[containerID] = logPath
-		if err := c.SandboxStore.Put(sandboxMeta); err != nil {
-			return nil, err
-		}
-
+	if logPath != "" {
 		if err := c.ContainerMgr.AttachCRILog(ctx, containerID, logPath); err != nil {
 			return nil, err
 		}
@@ -978,15 +977,7 @@ func (c *CriManager) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
 		imageRef = imageInfo.RepoDigests[0]
 	}
 
-	podSandboxID := container.Config.Labels[sandboxIDLabelKey]
-	res, err := c.SandboxStore.Get(podSandboxID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get metadata of %q from SandboxStore: %v", podSandboxID, err)
-	}
-	sandboxMeta := res.(*SandboxMeta)
-	logDirectory := sandboxMeta.Config.GetLogDirectory()
-	// TODO: let the container manager handle the log stuff for CRI.
-	logPath := makeupLogPath(logDirectory, metadata)
+	logPath := labels[containerLogPathLabelKey]
 
 	resources := container.HostConfig.Resources
 	diskQuota := container.Config.DiskQuota
@@ -1142,23 +1133,9 @@ func (c *CriManager) ReopenContainerLog(ctx context.Context, r *runtime.ReopenCo
 		return nil, errors.Wrap(errtypes.ErrPreCheckFailed, "container is not running")
 	}
 
-	// get the container's podSandbox id.
-	podSandboxID, ok := container.Config.Labels[sandboxIDLabelKey]
-	if !ok {
-		return nil, fmt.Errorf("failed to get the sandboxId of container %q", containerID)
-	}
-
 	// get logPath of container
-	res, err := c.SandboxStore.Get(podSandboxID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get metadata of %q from SandboxStore: %v", podSandboxID, err)
-	}
-	sandboxMeta, ok := res.(*SandboxMeta)
-	if !ok {
-		return nil, fmt.Errorf("failed to type asseration for sandboxMeta: %v", res)
-	}
-	logPath, ok := sandboxMeta.ContainerLogMap[containerID]
-	if !ok {
+	logPath := container.Config.Labels[containerLogPathLabelKey]
+	if logPath == "" {
 		logrus.Warnf("log path of container: %q is empty", containerID)
 		return &runtime.ReopenContainerLogResponse{}, nil
 	}

cri/v1alpha2/cri_types.go

@@ -20,9 +20,6 @@ type SandboxMeta struct {
 
 	// NetNS is the sandbox's network namespace
 	NetNS string
-
-	// ContainerLogMap store the mapping of container id and CRI logPath.
-	ContainerLogMap map[string]string
 }
 
 // Key returns sandbox's id.

cri/v1alpha2/cri_utils.go

@@ -552,11 +552,6 @@ func parseContainerName(name string) (*runtime.ContainerMetadata, error) {
 	}, nil
 }
 
-// makeupLogPath makes up the log path of container from log directory and its metadata.
-func makeupLogPath(logDirectory string, metadata *runtime.ContainerMetadata) string {
-	return filepath.Join(logDirectory, metadata.Name, fmt.Sprintf("%d.log", metadata.Attempt))
-}
-
 // modifyContainerNamespaceOptions apply namespace options for container.
 func modifyContainerNamespaceOptions(nsOpts *runtime.NamespaceOption, podSandboxID string, hostConfig *apitypes.HostConfig) {
 	sandboxNSMode := fmt.Sprintf("container:%v", podSandboxID)

cri/v1alpha2/cri_utils_test.go

@@ -510,32 +510,6 @@ func Test_parseContainerName(t *testing.T) {
 	}
 }
 
-func Test_makeupLogPath(t *testing.T) {
-	testCases := []struct {
-		logDirectory  string
-		containerMeta *runtime.ContainerMetadata
-		expected      string
-	}{
-		{
-			logDirectory:  "/var/log/pods/099f1c2b79126109140a1f77e211df00",
-			containerMeta: &runtime.ContainerMetadata{Name: "kube-scheduler", Attempt: 0},
-			expected:      "/var/log/pods/099f1c2b79126109140a1f77e211df00/kube-scheduler/0.log",
-		},
-		{
-			logDirectory:  "/var/log/pods/d875aada-9920-11e8-bfef-0242ac11001e/",
-			containerMeta: &runtime.ContainerMetadata{Name: "kube-proxy", Attempt: 10},
-			expected:      "/var/log/pods/d875aada-9920-11e8-bfef-0242ac11001e/kube-proxy/10.log",
-		},
-	}
-
-	for _, test := range testCases {
-		logPath := makeupLogPath(test.logDirectory, test.containerMeta)
-		if !reflect.DeepEqual(test.expected, logPath) {
-			t.Fatalf("unexpected logPath returned by makeupLogPath")
-		}
-	}
-}
-
 func Test_toCriContainerState(t *testing.T) {
 	testCases := []struct {
 		input    apitypes.Status