Feat: Add log_path support in containers.conf

Added log_path variable in containers/common, User sets default log path in containers.conf under the `[containers]` section.
The directory has to exist beforehand. Container logs go under this directory, sub-directories named with the container id
and inside the sub-directory a ctr.log file will be created where the container logs for the corresponding container will go.
This path can be overridden by using the `--log-opt` flag.

Signed-off-by: Joshua Arrevillaga <[email protected]>

Author: ajoshua2004

libpod/options.go

@@ -6,6 +6,8 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"os"
+	"path/filepath"
 	"strings"
 	"syscall"
 	"time"
@@ -1041,8 +1043,16 @@ func WithLogPath(path string) CtrCreateOption {
 		if path == "" {
 			return fmt.Errorf("log path must be set: %w", define.ErrInvalidArg)
 		}
+		if isDirectory(path) {
+			containerDir := filepath.Join(path, ctr.ID())
+			if err := os.Mkdir(containerDir, 0755); err != nil {
+				return fmt.Errorf("failed to create container log directory %s: %w", containerDir, err)
+			}
 
-		ctr.config.LogPath = path
+			ctr.config.LogPath = filepath.Join(containerDir, "ctr.log")
+		} else {
+			ctr.config.LogPath = path
+		}
 
 		return nil
 	}

libpod/util.go

@@ -285,3 +285,11 @@ func evalSymlinksIfExists(toCheck string) (string, error) {
 	}
 	return checkedVal, nil
 }
+
+func isDirectory(path string) bool {
+	info, err := os.Stat(path)
+	if err != nil {
+		return false
+	}
+	return info.IsDir()
+}

pkg/specgen/generate/kube/kube.go

@@ -228,6 +228,10 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 
 	s.ImageVolumes = opts.ImageVolumes
 
+	if rtc.Containers.LogPath != "" {
+		s.LogConfiguration.Path = rtc.Containers.LogPath
+	}
+
 	s.LogConfiguration.Options = make(map[string]string)
 	for _, o := range opts.LogOptions {
 		opt, val, hasVal := strings.Cut(o, "=")

pkg/specgenutil/specgen.go

@@ -844,6 +844,10 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		return err
 	}
 
+	if rtc.Containers.LogPath != "" {
+		s.LogConfiguration.Path = rtc.Containers.LogPath
+	}
+
 	logOpts := make(map[string]string)
 	for _, o := range c.LogOptions {
 		key, val, hasVal := strings.Cut(o, "=")
@@ -865,6 +869,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 			logOpts[key] = val
 		}
 	}
+
 	if len(s.LogConfiguration.Options) == 0 || len(c.LogOptions) != 0 {
 		s.LogConfiguration.Options = logOpts
 	}

test/e2e/play_kube_test.go

@@ -6310,4 +6310,85 @@ spec:
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(ExitWithError(125, "invalid signal: noSuchSignal"))
 	})
+
+	It("test with custom log path from containers.conf", func() {
+		customLogPath := filepath.Join(podmanTest.TempDir, "custom-logs")
+		expectedMessage := "Pod started, checking logs from test"
+
+		conffile := filepath.Join(podmanTest.TempDir, "containers.conf")
+		configContent := fmt.Sprintf(`[containers]
+log_driver = "k8s-file"
+log_path = "%s"
+`, customLogPath)
+
+		err := os.WriteFile(conffile, []byte(configContent), 0644)
+		Expect(err).ToNot(HaveOccurred())
+
+		err = os.MkdirAll(customLogPath, 0755)
+		Expect(err).ToNot(HaveOccurred())
+
+		os.Setenv("CONTAINERS_CONF_OVERRIDE", conffile)
+		defer os.Unsetenv("CONTAINERS_CONF_OVERRIDE")
+
+		if IsRemote() {
+			podmanTest.RestartRemoteService()
+		}
+
+		kubeYaml := filepath.Join(podmanTest.TempDir, "test-pod.yaml")
+		podYamlContent := fmt.Sprintf(`apiVersion: v1
+kind: Pod
+metadata:
+  name: log-test-pod
+spec:
+  restartPolicy: Never
+  containers:
+  - name: logger-container
+    image: %s
+    command: ["/bin/sh", "-c", "echo '%s'; sleep 2"]
+`, CITEST_IMAGE, expectedMessage)
+
+		err = os.WriteFile(kubeYaml, []byte(podYamlContent), 0644)
+		Expect(err).ToNot(HaveOccurred())
+
+		podmanTest.PodmanExitCleanly("kube", "play", kubeYaml)
+
+		podmanTest.PodmanExitCleanly("wait", "log-test-pod-logger-container")
+
+		customLogDirs, err := os.ReadDir(customLogPath)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(customLogDirs).To(HaveLen(2), "Should have exactly two container log directories (infra + app container)")
+
+		var appContainerLogDir string
+		var logContent string
+		found := false
+
+		for _, dir := range customLogDirs {
+			if !dir.IsDir() {
+				continue
+			}
+			containerLogDir := dir.Name()
+			logFilePath := filepath.Join(customLogPath, containerLogDir, "ctr.log")
+
+			if _, err := os.Stat(logFilePath); err != nil {
+				continue
+			}
+
+			content, err := os.ReadFile(logFilePath)
+			if err != nil {
+				continue
+			}
+
+			if strings.Contains(string(content), expectedMessage) {
+				appContainerLogDir = containerLogDir
+				logContent = string(content)
+				found = true
+				break
+			}
+		}
+
+		Expect(found).To(BeTrue(), "Should find log file with expected message")
+
+		Expect(appContainerLogDir).ToNot(BeEmpty(), "Should have found application container log directory")
+		Expect(logContent).To(ContainSubstring(expectedMessage), "Log file should contain the expected message")
+	})
 })

test/e2e/run_test.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"syscall"
@@ -2456,4 +2457,84 @@ WORKDIR /madethis`, BB)
 		Expect(inspectData[0].Config.Annotations).To(Not(HaveKey(annoName)))
 		Expect(inspectData[0].Config.Annotations).To(Not(HaveKey("testlabel")))
 	})
+
+	It("podman run log-opt overrides containers.conf path", func() {
+		expectedMessage := "CLI override test message"
+		confLogPath := filepath.Join(podmanTest.TempDir, "conf-logs")
+
+		conffile := filepath.Join(podmanTest.TempDir, "containers.conf")
+		configContent := fmt.Sprintf(`[containers]
+log_driver = "k8s-file"
+log_path = "%s"
+`, confLogPath)
+
+		err := os.WriteFile(conffile, []byte(configContent), 0644)
+		Expect(err).ToNot(HaveOccurred())
+
+		err = os.MkdirAll(confLogPath, 0755)
+		Expect(err).ToNot(HaveOccurred())
+
+		os.Setenv("CONTAINERS_CONF_OVERRIDE", conffile)
+		defer os.Unsetenv("CONTAINERS_CONF_OVERRIDE")
+
+		if IsRemote() {
+			podmanTest.RestartRemoteService()
+		}
+
+		cliLogPath := filepath.Join(podmanTest.TempDir, "cli-override.log")
+		podmanTest.PodmanExitCleanly("run", "--rm", "--log-driver", "k8s-file", "--log-opt", fmt.Sprintf("path=%s", cliLogPath), ALPINE, "echo", expectedMessage)
+
+		confLogDirs, err := os.ReadDir(confLogPath)
+		Expect(err).ToNot(HaveOccurred(), "Should be able to read config log directory that we created")
+		Expect(confLogDirs).To(BeEmpty(), "Config file log path should not be used when CLI overrides")
+
+		content, err := os.ReadFile(cliLogPath)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(string(content)).To(ContainSubstring(expectedMessage))
+		Expect(string(content)).To(MatchRegexp(`\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+.*stdout F ` + regexp.QuoteMeta(expectedMessage)))
+
+		_ = os.Remove(cliLogPath)
+	})
+
+	It("podman run uses containers.conf log_path", func() {
+		expectedMessage := "Config file path test message"
+		confLogPath := filepath.Join(podmanTest.TempDir, "conf-logs")
+
+		conffile := filepath.Join(podmanTest.TempDir, "containers.conf")
+		configContent := fmt.Sprintf(`[containers]
+log_driver = "k8s-file"
+log_path = "%s"
+`, confLogPath)
+
+		err := os.WriteFile(conffile, []byte(configContent), 0644)
+		Expect(err).ToNot(HaveOccurred())
+
+		err = os.MkdirAll(confLogPath, 0755)
+		Expect(err).ToNot(HaveOccurred())
+
+		os.Setenv("CONTAINERS_CONF_OVERRIDE", conffile)
+		defer os.Unsetenv("CONTAINERS_CONF_OVERRIDE")
+
+		if IsRemote() {
+			podmanTest.RestartRemoteService()
+		}
+
+		containerName := "test-conf-log-container"
+
+		podmanTest.PodmanExitCleanly("run", "--name", containerName, ALPINE, "echo", expectedMessage)
+		session := podmanTest.PodmanExitCleanly("inspect", "--format", "{{.Id}}", containerName)
+
+		containerID := strings.TrimSpace(session.OutputToString())
+		logFilePath := filepath.Join(confLogPath, containerID, "ctr.log")
+
+		inspectSession := podmanTest.PodmanExitCleanly("inspect", "--format", "{{.HostConfig.LogConfig.Path}}", containerName)
+		inspectedPath := strings.TrimSpace(inspectSession.OutputToString())
+		Expect(inspectedPath).To(Equal(logFilePath), "Log path in inspect data should match the path from containers.conf")
+
+		content, err := os.ReadFile(logFilePath)
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(string(content)).To(ContainSubstring(expectedMessage), "Log should contain expected message")
+		Expect(string(content)).To(MatchRegexp(`\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+.*stdout F `+regexp.QuoteMeta(expectedMessage)), "Log should follow k8s-file format")
+	})
 })