sandbox: fix sandbox logPath when crio restarts

We weren't setting the logPath of the sandbox when restoring sandboxes
and containers upon a crio restarts. That means that if you restart
CRI-O you get sandboxes with empty logPath. That means that when you're
starting a container in a restored sandbox you get a relative logPath
for the container:

sandboxLogPath: "/var/something"
- restore
sandboxLogPath: ""
- create container foo
containerLogPath: "foo_attempt.log"

With this patch we actually get an absolute path (which is correct):

sandboxLogPath: "/var/something"
- restore
sandboxLogPath: "/var/something"
- create container foo
containerLogPath: "/var/something/foo_attempt.log"

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

lib/container_server.go

@@ -335,7 +335,7 @@ func (c *ContainerServer) LoadSandbox(id string) error {
 	privileged := isTrue(m.Annotations[annotations.PrivilegedRuntime])
 	trusted := isTrue(m.Annotations[annotations.TrustedSandbox])
 
-	sb, err := sandbox.New(id, name, m.Annotations[annotations.KubeName], filepath.Dir(m.Annotations[annotations.LogPath]), "", labels, kubeAnnotations, processLabel, mountLabel, &metadata, m.Annotations[annotations.ShmPath], "", privileged, trusted, m.Annotations[annotations.ResolvPath], "", nil)
+	sb, err := sandbox.New(id, m.Annotations[annotations.Namespace], name, m.Annotations[annotations.KubeName], filepath.Dir(m.Annotations[annotations.LogPath]), labels, kubeAnnotations, processLabel, mountLabel, &metadata, m.Annotations[annotations.ShmPath], m.Annotations[annotations.CgroupParent], privileged, trusted, m.Annotations[annotations.ResolvPath], m.Annotations[annotations.HostName], nil)
 	if err != nil {
 		return err
 	}

pkg/annotations/annotations.go

@@ -19,6 +19,9 @@ const (
 	// HostName is the container host name annotation
 	HostName = "io.kubernetes.cri-o.HostName"
 
+	// CgroupParent is the sandbox cgroup parent
+	CgroupParent = "io.kubernetes.cri-o.CgroupParent"
+
 	// IP is the container ipv4 or ipv6 address
 	IP = "io.kubernetes.cri-o.IP"
 
@@ -49,6 +52,9 @@ const (
 	// Name is the pod name annotation
 	Name = "io.kubernetes.cri-o.Name"
 
+	// Namespace is the pod namespace annotation
+	Namespace = "io.kubernetes.cri-o.Namespace"
+
 	// PrivilegedRuntime is the annotation for the privileged runtime path
 	PrivilegedRuntime = "io.kubernetes.cri-o.PrivilegedRuntime"
 

server/container_create.go

@@ -773,7 +773,7 @@ func (s *Server) setupOCIHooks(specgen *generate.Generator, sb *sandbox.Sandbox,
 	}
 	return nil
 }
-func (s *Server) createSandboxContainer(ctx context.Context, containerID string, containerName string, sb *sandbox.Sandbox, SandboxConfig *pb.PodSandboxConfig, containerConfig *pb.ContainerConfig) (*oci.Container, error) {
+func (s *Server) createSandboxContainer(ctx context.Context, containerID string, containerName string, sb *sandbox.Sandbox, sandboxConfig *pb.PodSandboxConfig, containerConfig *pb.ContainerConfig) (*oci.Container, error) {
 	if sb == nil {
 		return nil, errors.New("createSandboxContainer needs a sandbox")
 	}
@@ -871,15 +871,19 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 
 	}
 
-	logPath := containerConfig.LogPath
+	logPath := containerConfig.GetLogPath()
+	sboxLogDir := sandboxConfig.GetLogDirectory()
+	if sboxLogDir == "" {
+		sboxLogDir = sb.LogDir()
+	}
 	if logPath == "" {
-		// TODO: Should we use sandboxConfig.GetLogDirectory() here?
-		logPath = filepath.Join(sb.LogDir(), containerID+".log")
+		logPath = filepath.Join(sboxLogDir, containerID+".log")
 	}
 	if !filepath.IsAbs(logPath) {
 		// XXX: It's not really clear what this should be versus the sbox logDirectory.
 		logrus.Warnf("requested logPath for ctr id %s is a relative path: %s", containerID, logPath)
-		logPath = filepath.Join(sb.LogDir(), logPath)
+		logPath = filepath.Join(sboxLogDir, logPath)
+		logrus.Warnf("logPath from relative path is now absolute: %s", logPath)
 	}
 
 	// Handle https://issues.k8s.io/44043
@@ -888,8 +892,8 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 	}
 
 	logrus.WithFields(logrus.Fields{
-		"sbox.logdir": sb.LogDir(),
-		"ctr.logfile": containerConfig.LogPath,
+		"sbox.logdir": sboxLogDir,
+		"ctr.logfile": containerConfig.GetLogPath(),
 		"log_path":    logPath,
 	}).Debugf("setting container's log_path")
 

server/sandbox_run.go

@@ -255,7 +255,7 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	}
 
 	// set log directory
-	logDir := req.GetConfig().LogDirectory
+	logDir := req.GetConfig().GetLogDirectory()
 	if logDir == "" {
 		logDir = filepath.Join(s.config.LogDir, id)
 	}
@@ -343,6 +343,7 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	g.AddAnnotation(annotations.Annotations, string(kubeAnnotationsJSON))
 	g.AddAnnotation(annotations.LogPath, logPath)
 	g.AddAnnotation(annotations.Name, name)
+	g.AddAnnotation(annotations.Namespace, namespace)
 	g.AddAnnotation(annotations.ContainerType, annotations.ContainerTypeSandbox)
 	g.AddAnnotation(annotations.SandboxID, id)
 	g.AddAnnotation(annotations.ContainerName, containerName)
@@ -384,6 +385,7 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 			g.SetLinuxCgroupsPath(cgPath)
 		}
 	}
+	g.AddAnnotation(annotations.CgroupParent, cgroupParent)
 
 	sb, err := sandbox.New(id, namespace, name, kubeName, logDir, labels, kubeAnnotations, processLabel, mountLabel, metadata, shmPath, cgroupParent, privileged, trusted, resolvPath, hostname, portMappings)
 	if err != nil {