summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthew Heon <matthew.heon@pm.me>2020-06-10 14:35:00 -0400
committerMatthew Heon <mheon@redhat.com>2020-06-17 11:11:46 -0400
commit0e171b7b3327948f2e9e32d9e496736bd7a48009 (patch)
treec52bdf719ac4fb248cf19662b1fa41c2472e4aec
parent38391ed25fdb1cc53b70a75ee4fbe7ea0fa782c3 (diff)
downloadpodman-0e171b7b3327948f2e9e32d9e496736bd7a48009.tar.gz
podman-0e171b7b3327948f2e9e32d9e496736bd7a48009.tar.bz2
podman-0e171b7b3327948f2e9e32d9e496736bd7a48009.zip
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>
-rwxr-xr-xcontrib/cirrus/setup_environment.sh1
-rw-r--r--docs/source/markdown/podman-create.1.md2
-rw-r--r--docs/source/markdown/podman-run.1.md2
-rw-r--r--libpod/container_log.go17
-rw-r--r--libpod/define/config.go3
-rw-r--r--libpod/define/errors.go3
-rw-r--r--libpod/oci_conmon_exec_linux.go2
-rw-r--r--libpod/oci_conmon_linux.go16
-rw-r--r--libpod/options.go2
-rw-r--r--libpod/runtime_ctr.go2
-rw-r--r--test/e2e/exec_test.go4
-rw-r--r--test/e2e/logs_test.go11
12 files changed, 45 insertions, 20 deletions
diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh
index 323e7c35b..ea2c7d8e0 100755
--- a/contrib/cirrus/setup_environment.sh
+++ b/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
diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md
index 1da9d72e6..65ef2dba0 100644
--- a/docs/source/markdown/podman-create.1.md
+++ b/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*
diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md
index 3e1ade047..53263d2be 100644
--- a/docs/source/markdown/podman-run.1.md
+++ b/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*
diff --git a/libpod/container_log.go b/libpod/container_log.go
index 39c395fe6..ac4720cfb 100644
--- a/libpod/container_log.go
+++ b/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
- // TODO make this not a magic string
- if c.LogDriver() == define.JournaldLogging {
+ switch c.LogDriver() {
+ case define.NoLogging:
+ 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 {
diff --git a/libpod/define/config.go b/libpod/define/config.go
index 5ca4da4af..900a363d8 100644
--- a/libpod/define/config.go
+++ b/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"
diff --git a/libpod/define/errors.go b/libpod/define/errors.go
index 083553b7e..e0c9811fe 100644
--- a/libpod/define/errors.go
+++ b/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.
diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go
index bc39100f8..6be8534d9 100644
--- a/libpod/oci_conmon_exec_linux.go
+++ b/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))...)
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 0921a532b..625a5bf70 100644
--- a/libpod/oci_conmon_linux.go
+++ b/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
- switch ctr.LogDriver() {
+ var logDriverArg string
+ 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))
}
diff --git a/libpod/options.go b/libpod/options.go
index 5a0f60093..3a5dddbed 100644
--- a/libpod/options.go
+++ b/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")
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index f0beb0941..0431861b5 100644
--- a/libpod/runtime_ctr.go
+++ b/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")
}
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index b152da9e6..f44d428d6 100644
--- a/test/e2e/exec_test.go
+++ b/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() {
diff --git a/test/e2e/logs_test.go b/test/e2e/logs_test.go
index f9446e0c6..f19147662 100644
--- a/test/e2e/logs_test.go
+++ b/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)))
+ })
})