summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>2020-06-17 17:17:27 -0400
committerGitHub <noreply@github.com>2020-06-17 17:17:27 -0400
commit7b00e49f657305f233e4e29613821305bf4d2962 (patch)
tree8470c3a4a002f93514762f7fe2eb37cc0396d44b
parentdaabbc1a1e82b840a51b6d769cdf19cfda474144 (diff)
parent0e171b7b3327948f2e9e32d9e496736bd7a48009 (diff)
downloadpodman-7b00e49f657305f233e4e29613821305bf4d2962.tar.gz
podman-7b00e49f657305f233e4e29613821305bf4d2962.tar.bz2
podman-7b00e49f657305f233e4e29613821305bf4d2962.zip
Merge pull request #6560 from mheon/fix_exec_logdriver
Do not share container log driver for exec
-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 03ac8642f..3ec91a3ad 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 539e62819..7e91a06a3 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 c3a84d048..071882bc2 100644
--- a/libpod/container_log.go
+++ b/libpod/container_log.go
@@ -21,12 +21,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 e1c4ddf06..ffc9c1018 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 c12ae9c86..a4a59acb2 100644
--- a/test/e2e/logs_test.go
+++ b/test/e2e/logs_test.go
@@ -300,4 +300,15 @@ var _ = Describe("Podman logs", func() {
Expect(inspect.ErrorToString()).To(ContainSubstring("no such container"))
}
})
+
+ 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)))
+ })
})