Do not share container log driver for exec

When the container uses journald logging, we don't want to
automatically use the same driver for its exec sessions. If we do
we will pollute the journal (particularly in the case of
healthchecks) with large amounts of undesired logs. Instead,
force exec sessions logs to file for now; we can add a log-driver
flag later (we'll probably want to add a `podman logs` command
that reads exec session logs at the same time).

As part of this, add support for the new 'none' logs driver in
Conmon. It will be the default log driver for exec sessions, and
can be optionally selected for containers.

Great thanks to Joe Gooch (mrwizard@dok.org) for adding support
to Conmon for a null log driver, and wiring it in here.

Fixes #6555

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

contrib/cirrus/setup_environment.sh

@@ -50,6 +50,7 @@ case "${OS_RELEASE_ID}" in
 	    if [[ "$OS_RELEASE_VER" == "20" ]]; then
 		    apt-get install -y python-is-python3
 	    fi
+	    apt-get upgrade -y conmon
         ;;
     fedora)
         # All SELinux distros need this for systemd-in-a-container

docs/source/markdown/podman-create.1.md

@@ -419,7 +419,7 @@ Not implemented
 
 **--log-driver**="*k8s-file*"
 
-Logging driver for the container.  Currently available options are *k8s-file* and *journald*, with *json-file* aliased to *k8s-file* for scripting compatibility.
+Logging driver for the container.  Currently available options are *k8s-file*, *journald*, and *none*, with *json-file* aliased to *k8s-file* for scripting compatibility.
 
 **--log-opt**=*path*
 

docs/source/markdown/podman-run.1.md

@@ -432,7 +432,7 @@ Not implemented.
 
 **--log-driver**="*driver*"
 
-Logging driver for the container. Currently available options are **k8s-file** and **journald**, with **json-file** aliased to **k8s-file** for scripting compatibility.
+Logging driver for the container. Currently available options are **k8s-file**, **journald**, and **none**, with **json-file** aliased to **k8s-file** for scripting compatibility.
 
 **--log-opt**=*name*=*value*
 

libpod/container_log.go

@@ -22,12 +22,21 @@ func (r *Runtime) Log(containers []*Container, options *logs.LogOptions, logChan
 
 // ReadLog reads a containers log based on the input options and returns loglines over a channel.
 func (c *Container) ReadLog(options *logs.LogOptions, logChannel chan *logs.LogLine) error {
-	// TODO Skip sending logs until journald logs can be read
+	switch c.LogDriver() {
-	// TODO make this not a magic string
+	case define.NoLogging:
-	if c.LogDriver() == define.JournaldLogging {
+		return errors.Wrapf(define.ErrNoLogs, "this container is using the 'none' log driver, cannot read logs")
+	case define.JournaldLogging:
+		// TODO Skip sending logs until journald logs can be read
 		return c.readFromJournal(options, logChannel)
+	case define.JSONLogging:
+		// TODO provide a separate implementation of this when Conmon
+		// has support.
+		fallthrough
+	case define.KubernetesLogging, "":
+		return c.readFromLogFile(options, logChannel)
+	default:
+		return errors.Wrapf(define.ErrInternal, "unrecognized log driver %q, cannot read logs", c.LogDriver())
 	}
-	return c.readFromLogFile(options, logChannel)
 }
 
 func (c *Container) readFromLogFile(options *logs.LogOptions, logChannel chan *logs.LogLine) error {

libpod/define/config.go

@@ -72,3 +72,6 @@ const KubernetesLogging = "k8s-file"
 
 // JSONLogging is the string conmon expects when specifying to use the json logging format
 const JSONLogging = "json-file"
+
+// NoLogging is the string conmon expects when specifying to use no log driver whatsoever
+const NoLogging = "none"

libpod/define/errors.go

@@ -79,6 +79,9 @@ var (
 	// ErrNoCgroups indicates that the container does not have its own
 	// CGroup.
 	ErrNoCgroups = errors.New("this container does not have a cgroup")
+	// ErrNoLogs indicates that this container is not creating a log so log
+	// operations cannot be performed on it
+	ErrNoLogs = errors.New("this container is not logging output")
 
 	// ErrRootless indicates that the given command cannot but run without
 	// root.

libpod/oci_conmon_exec_linux.go

@@ -392,7 +392,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
 		return nil, nil, err
 	}
 
-	args := r.sharedConmonArgs(c, sessionID, c.execBundlePath(sessionID), c.execPidPath(sessionID), c.execLogPath(sessionID), c.execExitFileDir(sessionID), ociLog, "")
+	args := r.sharedConmonArgs(c, sessionID, c.execBundlePath(sessionID), c.execPidPath(sessionID), c.execLogPath(sessionID), c.execExitFileDir(sessionID), ociLog, define.NoLogging, "")
 
 	if options.PreserveFDs > 0 {
 		args = append(args, formatRuntimeOpts("--preserve-fds", fmt.Sprintf("%d", options.PreserveFDs))...)

libpod/oci_conmon_linux.go

@@ -881,7 +881,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 		return err
 	}
 
-	args := r.sharedConmonArgs(ctr, ctr.ID(), ctr.bundlePath(), filepath.Join(ctr.state.RunDir, "pidfile"), ctr.LogPath(), r.exitsDir, ociLog, logTag)
+	args := r.sharedConmonArgs(ctr, ctr.ID(), ctr.bundlePath(), filepath.Join(ctr.state.RunDir, "pidfile"), ctr.LogPath(), r.exitsDir, ociLog, ctr.LogDriver(), logTag)
 
 	if ctr.config.Spec.Process.Terminal {
 		args = append(args, "-t")
@@ -1137,7 +1137,7 @@ func (r *ConmonOCIRuntime) configureConmonEnv(runtimeDir string) ([]string, []*o
 }
 
 // sharedConmonArgs takes common arguments for exec and create/restore and formats them for the conmon CLI
-func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, pidPath, logPath, exitDir, ociLogPath, logTag string) []string {
+func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, pidPath, logPath, exitDir, ociLogPath, logDriver, logTag string) []string {
 	// set the conmon API version to be able to use the correct sync struct keys
 	args := []string{
 		"--api-version", "1",
@@ -1155,12 +1155,14 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
 		args = append(args, "-s")
 	}
 
-	var logDriver string
+	var logDriverArg string
-	switch ctr.LogDriver() {
+	switch logDriver {
 	case define.JournaldLogging:
-		logDriver = define.JournaldLogging
+		logDriverArg = define.JournaldLogging
 	case define.JSONLogging:
 		fallthrough
+	case define.NoLogging:
+		logDriverArg = define.NoLogging
 	default: //nolint-stylecheck
 		// No case here should happen except JSONLogging, but keep this here in case the options are extended
 		logrus.Errorf("%s logging specified but not supported. Choosing k8s-file logging instead", ctr.LogDriver())
@@ -1170,10 +1172,10 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
 		// since the former case is obscure, and the latter case isn't an error, let's silently fallthrough
 		fallthrough
 	case define.KubernetesLogging:
-		logDriver = fmt.Sprintf("%s:%s", define.KubernetesLogging, logPath)
+		logDriverArg = fmt.Sprintf("%s:%s", define.KubernetesLogging, logPath)
 	}
 
-	args = append(args, "-l", logDriver)
+	args = append(args, "-l", logDriverArg)
 	if r.logSizeMax >= 0 {
 		args = append(args, "--log-size-max", fmt.Sprintf("%v", r.logSizeMax))
 	}

libpod/options.go

@@ -993,7 +993,7 @@ func WithLogDriver(driver string) CtrCreateOption {
 		switch driver {
 		case "":
 			return errors.Wrapf(define.ErrInvalidArg, "log driver must be set")
-		case define.JournaldLogging, define.KubernetesLogging, define.JSONLogging:
+		case define.JournaldLogging, define.KubernetesLogging, define.JSONLogging, define.NoLogging:
 			break
 		default:
 			return errors.Wrapf(define.ErrInvalidArg, "invalid log driver")

libpod/runtime_ctr.go

@@ -321,7 +321,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 		ctrNamedVolumes = append(ctrNamedVolumes, newVol)
 	}
 
-	if ctr.config.LogPath == "" && ctr.config.LogDriver != define.JournaldLogging {
+	if ctr.config.LogPath == "" && ctr.config.LogDriver != define.JournaldLogging && ctr.config.LogDriver != define.NoLogging {
 		ctr.config.LogPath = filepath.Join(ctr.config.StaticDir, "ctr.log")
 	}
 

test/e2e/exec_test.go

@@ -25,10 +25,6 @@ var _ = Describe("Podman exec", func() {
 		podmanTest = PodmanTestCreate(tempdir)
 		podmanTest.Setup()
 		podmanTest.SeedImages()
-		// HACK: Remove this once we get Conmon 2.0.17 on Ubuntu
-		if podmanTest.Host.Distribution == "ubuntu" {
-			Skip("Unable to perform test on Ubuntu distributions due to too-old Conmon (need 2.0.17)")
-		}
 	})
 
 	AfterEach(func() {

test/e2e/logs_test.go

@@ -300,4 +300,15 @@ var _ = Describe("Podman logs", func() {
 		results.WaitWithDefaultTimeout()
 		Expect(results).To(Exit(0))
 	})
+
+	It("podman logs with log-driver=none errors", func() {
+		ctrName := "logsctr"
+		logc := podmanTest.Podman([]string{"run", "--name", ctrName, "-d", "--log-driver", "none", ALPINE, "top"})
+		logc.WaitWithDefaultTimeout()
+		Expect(logc).To(Exit(0))
+
+		logs := podmanTest.Podman([]string{"logs", "-f", ctrName})
+		logs.WaitWithDefaultTimeout()
+		Expect(logs).To(Not(Exit(0)))
+	})
 })