From 42ea0bf9c735aa5da6a53d32a0b8bdb11a09d524 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 10 Jul 2023 10:52:44 +0200 Subject: [PATCH] libpod: use io.Writer vs io.WriteCloser for attach streams We never ever close the stream so we do not need the Close() function in th ebackend, the caller should close when required which may no be the case, i.e. when os.Stdout/err is used. This should not be a breaking change as the io.Writer is a subset of io.WriteCloser, therfore all code should still compile while allowing to pass in Writers without Close(). This is useful for podman top where we exec ps in the container via podman exec. Signed-off-by: Paul Holzinger --- libpod/container_top_linux.go | 23 ++++--------------- libpod/define/config.go | 4 ++-- pkg/bindings/containers/types.go | 4 ++-- .../types_execstartandattach_options.go | 12 +++++----- 4 files changed, 14 insertions(+), 29 deletions(-) diff --git a/libpod/container_top_linux.go b/libpod/container_top_linux.go index 1da4919398..e460902083 100644 --- a/libpod/container_top_linux.go +++ b/libpod/container_top_linux.go @@ -355,16 +355,10 @@ func (c *Container) execPSinContainer(args []string) ([]string, error) { defer wPipe.Close() defer rPipe.Close() - rErrPipe, wErrPipe, err := os.Pipe() - if err != nil { - return nil, err - } - defer wErrPipe.Close() - defer rErrPipe.Close() - + var errBuf bytes.Buffer streams := new(define.AttachStreams) streams.OutputStream = wPipe - streams.ErrorStream = wErrPipe + streams.ErrorStream = &errBuf streams.AttachOutput = true streams.AttachError = true @@ -375,13 +369,6 @@ func (c *Container) execPSinContainer(args []string) ([]string, error) { stdout = append(stdout, scanner.Text()) } }() - stderr := []string{} - go func() { - scanner := bufio.NewScanner(rErrPipe) - for scanner.Scan() { - stderr = append(stderr, scanner.Text()) - } - }() cmd := append([]string{"ps"}, args...) config := new(ExecConfig) @@ -390,15 +377,13 @@ func (c *Container) execPSinContainer(args []string) ([]string, error) { if err != nil { return nil, err } else if ec != 0 { - return nil, fmt.Errorf("runtime failed with exit status: %d and output: %s", ec, strings.Join(stderr, " ")) + return nil, fmt.Errorf("runtime failed with exit status: %d and output: %s", ec, errBuf.String()) } if logrus.GetLevel() >= logrus.DebugLevel { // If we're running in debug mode or higher, we might want to have a // look at stderr which includes debug logs from conmon. - for _, log := range stderr { - logrus.Debugf("%s", log) - } + logrus.Debugf(errBuf.String()) } return stdout, nil diff --git a/libpod/define/config.go b/libpod/define/config.go index 7295f1425e..e5729d47ea 100644 --- a/libpod/define/config.go +++ b/libpod/define/config.go @@ -51,9 +51,9 @@ const ( // AttachStreams contains streams that will be attached to the container type AttachStreams struct { // OutputStream will be attached to container's STDOUT - OutputStream io.WriteCloser + OutputStream io.Writer // ErrorStream will be attached to container's STDERR - ErrorStream io.WriteCloser + ErrorStream io.Writer // InputStream will be attached to container's STDIN InputStream *bufio.Reader // AttachOutput is whether to attach to STDOUT diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go index 6b25aae7e3..eaab8b76ec 100644 --- a/pkg/bindings/containers/types.go +++ b/pkg/bindings/containers/types.go @@ -295,9 +295,9 @@ type ResizeExecTTYOptions struct { //go:generate go run ../generator/generator.go ExecStartAndAttachOptions type ExecStartAndAttachOptions struct { // OutputStream will be attached to container's STDOUT - OutputStream *io.WriteCloser + OutputStream *io.Writer // ErrorStream will be attached to container's STDERR - ErrorStream *io.WriteCloser + ErrorStream *io.Writer // InputStream will be attached to container's STDIN InputStream *bufio.Reader // AttachOutput is whether to attach to STDOUT diff --git a/pkg/bindings/containers/types_execstartandattach_options.go b/pkg/bindings/containers/types_execstartandattach_options.go index df7ac45d1c..759676f2ff 100644 --- a/pkg/bindings/containers/types_execstartandattach_options.go +++ b/pkg/bindings/containers/types_execstartandattach_options.go @@ -20,30 +20,30 @@ func (o *ExecStartAndAttachOptions) ToParams() (url.Values, error) { } // WithOutputStream set field OutputStream to given value -func (o *ExecStartAndAttachOptions) WithOutputStream(value io.WriteCloser) *ExecStartAndAttachOptions { +func (o *ExecStartAndAttachOptions) WithOutputStream(value io.Writer) *ExecStartAndAttachOptions { o.OutputStream = &value return o } // GetOutputStream returns value of field OutputStream -func (o *ExecStartAndAttachOptions) GetOutputStream() io.WriteCloser { +func (o *ExecStartAndAttachOptions) GetOutputStream() io.Writer { if o.OutputStream == nil { - var z io.WriteCloser + var z io.Writer return z } return *o.OutputStream } // WithErrorStream set field ErrorStream to given value -func (o *ExecStartAndAttachOptions) WithErrorStream(value io.WriteCloser) *ExecStartAndAttachOptions { +func (o *ExecStartAndAttachOptions) WithErrorStream(value io.Writer) *ExecStartAndAttachOptions { o.ErrorStream = &value return o } // GetErrorStream returns value of field ErrorStream -func (o *ExecStartAndAttachOptions) GetErrorStream() io.WriteCloser { +func (o *ExecStartAndAttachOptions) GetErrorStream() io.Writer { if o.ErrorStream == nil { - var z io.WriteCloser + var z io.Writer return z } return *o.ErrorStream