Merge pull request #2491 from 2004joshua/loglocation

add log_path option to container configuration

Author: openshift-merge-bot[bot]

docs/containers.conf.5.md

@@ -294,6 +294,14 @@ the user system_u, and the role system_r.
 
 Logging driver for the container. Currently available options are k8s-file, journald, none and passthrough, with json-file aliased to k8s-file for scripting compatibility.  The journald driver is used by default if the systemd journal is readable and writable.  Otherwise, the k8s-file driver is used.
 
+**log_path**=""
+
+Default path for container logs to be stored in. When empty, logs will be stored 
+in the container's default storage and removed when the container is removed.
+A subdirectory named with the container ID will be created under the specified 
+path, and the log file will have the default name `ctr.log` within that directory.
+This option can be overridden by the `--log-opt` flag.
+
 **log_size_max**=-1
 
 Maximum size allowed for the container's log file. Negative numbers indicate

pkg/config/config.go

@@ -172,6 +172,9 @@ type ContainersConfig struct {
 	// LogDriver  for the container.  For example: k8s-file and journald
 	LogDriver string `toml:"log_driver,omitempty"`
 
+	// LogPath is the path to the container log file.
+	LogPath string `toml:"log_path,omitempty"`
+
 	// LogSizeMax is the maximum number of bytes after which the log file
 	// will be truncated. It can be expressed as a human-friendly string
 	// that is parsed to bytes.
@@ -882,6 +885,10 @@ func (c *ContainersConfig) Validate() error {
 		return err
 	}
 
+	if err := c.validateLogPath(); err != nil {
+		return err
+	}
+
 	if c.LogSizeMax >= 0 && c.LogSizeMax < OCIBufSize {
 		return fmt.Errorf("log size max should be negative or >= %d", OCIBufSize)
 	}

pkg/config/config_local.go

@@ -105,6 +105,20 @@ func (c *ContainersConfig) validateUmask() error {
 	return nil
 }
 
+func (c *ContainersConfig) validateLogPath() error {
+	if c.LogPath == "" {
+		return nil
+	}
+	if !filepath.IsAbs(c.LogPath) {
+		return fmt.Errorf("log_path must be an absolute path - instead got %q", c.LogPath)
+	}
+	if strings.ContainsAny(c.LogPath, "\x00") {
+		return fmt.Errorf("log_path contains null bytes - got %q", c.LogPath)
+	}
+
+	return nil
+}
+
 func isRemote() bool {
 	return false
 }

pkg/config/config_local_test.go

@@ -588,4 +588,40 @@ var _ = Describe("Config Local", func() {
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		gomega.Expect(config2.Machine.Rosetta).To(gomega.BeFalse())
 	})
+	It("should validate log_path correctly", func() {
+		defConf, err := defaultConfig()
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Expect(defConf).NotTo(gomega.BeNil())
+
+		// Test empty path
+		defConf.Containers.LogPath = ""
+		err = defConf.Containers.Validate()
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+		// Test absolute path
+		defConf.Containers.LogPath = "/var/log/containers"
+		err = defConf.Containers.Validate()
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+		// Should fail on relative path
+		defConf.Containers.LogPath = "./logs"
+		err = defConf.Containers.Validate()
+		gomega.Expect(err).To(gomega.HaveOccurred())
+		gomega.Expect(err.Error()).To(gomega.ContainSubstring("absolute path"))
+
+		// Test path with null byte
+		defConf.Containers.LogPath = "/var/log\x00/containers"
+		err = defConf.Containers.Validate()
+		gomega.Expect(err).To(gomega.HaveOccurred())
+		gomega.Expect(err.Error()).To(gomega.ContainSubstring("null bytes"))
+	})
+	It("should parse log_path from config file", func() {
+		config, err := New(nil)
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Expect(config.Containers.LogPath).To(gomega.Equal(""))
+
+		config2, err := NewConfig("testdata/containers_default.conf")
+		gomega.Expect(err).ToNot(gomega.HaveOccurred())
+		gomega.Expect(config2.Containers.LogPath).To(gomega.Equal("/var/log/containers"))
+	})
 })

pkg/config/config_remote.go

@@ -35,3 +35,7 @@ func (c *ContainersConfig) validateTZ() error {
 func (c *ContainersConfig) validateUmask() error {
 	return nil
 }
+
+func (c *ContainersConfig) validateLogPath() error {
+	return nil
+}

pkg/config/containers.conf

@@ -216,6 +216,14 @@ default_sysctls = [
 #
 #log_driver = "k8s-file"
 
+# Default path for container logs to be stored in. When empty, logs will be stored 
+# in the container's default storage and removed when the container is removed.
+# A subdirectory named with the container ID will be created under the specified 
+# path, and the log file will have the default name `ctr.log` within that directory.
+# This option can be overridden by the `--log-opt` flag.
+# 
+#log_path = ""
+
 # Maximum size allowed for the container log file. Negative numbers indicate
 # that no size limit is imposed. If positive, it must be >= 8192 to match or
 # exceed conmon's read buffer. The file is truncated and re-opened so the

pkg/config/containers.conf-freebsd

@@ -169,6 +169,14 @@ default_sysctls = [
 #
 #log_driver = "k8s-file"
 
+# Default path for container logs to be stored in. When empty, logs will be stored 
+# in the container's default storage and removed when the container is removed.
+# A subdirectory named with the container ID will be created under the specified 
+# path, and the log file will have the default name `ctr.log` within that directory.
+# This option can be overridden by the `--log-opt` flag.
+# 
+#log_path = ""
+
 # Maximum size allowed for the container log file. Negative numbers indicate
 # that no size limit is imposed. If positive, it must be >= 8192 to match or
 # exceed conmon's read buffer. The file is truncated and re-opened so the

pkg/config/testdata/containers_default.conf

@@ -85,6 +85,8 @@ label = true
 # limit is never exceeded.
 log_size_max = -1
 
+log_path = "/var/log/containers"
+
 mounts= [
 	"type=glob,source=/tmp/test2*,ro=true",
 	"type=bind,source=/etc/services,destination=/etc/services,ro",