From ba545c49a216d20be83bdfb0357a4d5f0abe6800 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 10 Dec 2020 17:39:57 +0100 Subject: podman logs honor stderr correctly Make the ContainerLogsOptions support two io.Writers, one for stdout and the other for stderr. The logline already includes the information to which Writer it has to be written. Signed-off-by: Paul Holzinger --- cmd/podman/containers/logs.go | 3 ++- libpod/logs/log.go | 16 ++++++++++++++++ pkg/domain/entities/containers.go | 6 ++++-- pkg/domain/infra/abi/containers.go | 4 ++-- pkg/domain/infra/tunnel/containers.go | 20 ++++++++++++++------ test/e2e/logs_test.go | 17 +++++++++++++++++ test/e2e/play_kube_test.go | 4 ++-- test/e2e/toolbox_test.go | 2 +- 8 files changed, 58 insertions(+), 14 deletions(-) diff --git a/cmd/podman/containers/logs.go b/cmd/podman/containers/logs.go index d4ede370a..9b562afd8 100644 --- a/cmd/podman/containers/logs.go +++ b/cmd/podman/containers/logs.go @@ -122,6 +122,7 @@ func logs(_ *cobra.Command, args []string) error { } logsOptions.Since = since } - logsOptions.Writer = os.Stdout + logsOptions.StdoutWriter = os.Stdout + logsOptions.StderrWriter = os.Stderr return registry.ContainerEngine().ContainerLogs(registry.GetContext(), args, logsOptions.ContainerLogsOptions) } diff --git a/libpod/logs/log.go b/libpod/logs/log.go index a9554088b..d3d83747f 100644 --- a/libpod/logs/log.go +++ b/libpod/logs/log.go @@ -210,3 +210,19 @@ func NewLogLine(line string) (*LogLine, error) { func (l *LogLine) Partial() bool { return l.ParseLogType == PartialLogType } + +func (l *LogLine) Write(stdout io.Writer, stderr io.Writer, logOpts *LogOptions) { + switch l.Device { + case "stdout": + if stdout != nil { + fmt.Fprintln(stdout, l.String(logOpts)) + } + case "stderr": + if stderr != nil { + fmt.Fprintln(stderr, l.String(logOpts)) + } + default: + // Warn the user if the device type does not match. Most likely the file is corrupted. + logrus.Warnf("unknown Device type '%s' in log file from Container %s", l.Device, l.CID) + } +} diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 39d679eaf..01086a2b3 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -227,8 +227,10 @@ type ContainerLogsOptions struct { Tail int64 // Show timestamps in the logs. Timestamps bool - // Write the logs to Writer. - Writer io.Writer + // Write the stdout to this Writer. + StdoutWriter io.Writer + // Write the stderr to this Writer. + StderrWriter io.Writer } // ExecOptions describes the cli values to exec into diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index ff4277a2e..ec65dbe44 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -925,7 +925,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta } func (ic *ContainerEngine) ContainerLogs(ctx context.Context, containers []string, options entities.ContainerLogsOptions) error { - if options.Writer == nil { + if options.StdoutWriter == nil && options.StderrWriter == nil { return errors.New("no io.Writer set for container logs") } @@ -963,7 +963,7 @@ func (ic *ContainerEngine) ContainerLogs(ctx context.Context, containers []strin }() for line := range logChannel { - fmt.Fprintln(options.Writer, line.String(logOpts)) + line.Write(options.StdoutWriter, options.StderrWriter, logOpts) } return nil diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index e65fef0a4..94ec2a7b9 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -360,11 +360,12 @@ func (ic *ContainerEngine) ContainerCreate(ctx context.Context, s *specgen.SpecG func (ic *ContainerEngine) ContainerLogs(_ context.Context, nameOrIDs []string, options entities.ContainerLogsOptions) error { since := options.Since.Format(time.RFC3339) tail := strconv.FormatInt(options.Tail, 10) - stdout := options.Writer != nil + stdout := options.StdoutWriter != nil + stderr := options.StderrWriter != nil opts := containers.LogOptions{ Follow: &options.Follow, Since: &since, - Stderr: &stdout, + Stderr: &stderr, Stdout: &stdout, Tail: &tail, Timestamps: &options.Timestamps, @@ -372,10 +373,11 @@ func (ic *ContainerEngine) ContainerLogs(_ context.Context, nameOrIDs []string, } var err error - outCh := make(chan string) + stdoutCh := make(chan string) + stderrCh := make(chan string) ctx, cancel := context.WithCancel(context.Background()) go func() { - err = containers.Logs(ic.ClientCxt, nameOrIDs[0], opts, outCh, outCh) + err = containers.Logs(ic.ClientCxt, nameOrIDs[0], opts, stdoutCh, stderrCh) cancel() }() @@ -383,8 +385,14 @@ func (ic *ContainerEngine) ContainerLogs(_ context.Context, nameOrIDs []string, select { case <-ctx.Done(): return err - case line := <-outCh: - _, _ = io.WriteString(options.Writer, line+"\n") + case line := <-stdoutCh: + if options.StdoutWriter != nil { + _, _ = io.WriteString(options.StdoutWriter, line+"\n") + } + case line := <-stderrCh: + if options.StderrWriter != nil { + _, _ = io.WriteString(options.StderrWriter, line+"\n") + } } } } diff --git a/test/e2e/logs_test.go b/test/e2e/logs_test.go index a749a86ff..aae6d4f02 100644 --- a/test/e2e/logs_test.go +++ b/test/e2e/logs_test.go @@ -355,4 +355,21 @@ var _ = Describe("Podman logs", func() { Expect(outlines[0]).To(Equal("1\r")) Expect(outlines[1]).To(Equal("2\r")) }) + + It("podman logs test stdout and stderr", func() { + cname := "log-test" + logc := podmanTest.Podman([]string{"run", "--name", cname, ALPINE, "sh", "-c", "echo stdout; echo stderr >&2"}) + logc.WaitWithDefaultTimeout() + Expect(logc).To(Exit(0)) + + wait := podmanTest.Podman([]string{"wait", cname}) + wait.WaitWithDefaultTimeout() + Expect(wait).To(Exit(0)) + + results := podmanTest.Podman([]string{"logs", cname}) + results.WaitWithDefaultTimeout() + Expect(results).To(Exit(0)) + Expect(results.OutputToString()).To(Equal("stdout")) + Expect(results.ErrorToString()).To(Equal("stderr")) + }) }) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 5ecfdd6b5..3a2387559 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -1072,7 +1072,7 @@ var _ = Describe("Podman play kube", func() { logs := podmanTest.Podman([]string{"logs", getCtrNameInPod(pod)}) logs.WaitWithDefaultTimeout() Expect(logs.ExitCode()).To(Equal(0)) - Expect(logs.OutputToString()).To(ContainSubstring("Operation not permitted")) + Expect(logs.ErrorToString()).To(ContainSubstring("Operation not permitted")) }) It("podman play kube seccomp pod level", func() { @@ -1099,7 +1099,7 @@ var _ = Describe("Podman play kube", func() { logs := podmanTest.Podman([]string{"logs", getCtrNameInPod(pod)}) logs.WaitWithDefaultTimeout() Expect(logs.ExitCode()).To(Equal(0)) - Expect(logs.OutputToString()).To(ContainSubstring("Operation not permitted")) + Expect(logs.ErrorToString()).To(ContainSubstring("Operation not permitted")) }) It("podman play kube with pull policy of never should be 125", func() { diff --git a/test/e2e/toolbox_test.go b/test/e2e/toolbox_test.go index 7393b13cb..6f04ce48c 100644 --- a/test/e2e/toolbox_test.go +++ b/test/e2e/toolbox_test.go @@ -239,7 +239,7 @@ var _ = Describe("Toolbox-specific testing", func() { session = podmanTest.Podman([]string{"logs", "test"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - Expect(session.OutputToString()).To(ContainSubstring(expectedOutput)) + Expect(session.ErrorToString()).To(ContainSubstring(expectedOutput)) }) It("podman create --userns=keep-id + podman exec - adding group with groupadd", func() { -- cgit v1.2.3-54-g00ecf