Skip to content

Commit

Permalink
libpod: use io.Writer vs io.WriteCloser for attach streams
Browse files Browse the repository at this point in the history
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 <pholzing@redhat.com>
  • Loading branch information
Luap99 authored and ashley-cui committed Jul 20, 2023
1 parent 574b782 commit 598ebe8
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 29 deletions.
23 changes: 4 additions & 19 deletions libpod/container_top_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions libpod/define/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/bindings/containers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions pkg/bindings/containers/types_execstartandattach_options.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 598ebe8

Please sign in to comment.