From 867d49957bb1160f2bed740e715a0f6b2cf16a17 Mon Sep 17 00:00:00 2001 From: baude Date: Thu, 26 Sep 2019 15:41:46 -0500 Subject: Add ability to redirect bash for run -i Signed-off-by: baude --- pkg/adapter/containers_remote.go | 9 +++++++-- pkg/varlinkapi/attach.go | 1 - pkg/varlinkapi/virtwriter/virtwriter.go | 24 ++++++++++++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) (limited to 'pkg') diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go index f7cb28b0c..f4e83a975 100644 --- a/pkg/adapter/containers_remote.go +++ b/pkg/adapter/containers_remote.go @@ -1092,6 +1092,7 @@ func configureVarlinkAttachStdio(reader *bufio.Reader, writer *bufio.Writer, std // These are the special writers that encode input from the client. varlinkStdinWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.ToStdin) varlinkResizeWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.TerminalResize) + varlinkHangupWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.HangUpFromClient) go func() { // Read from the wire and direct to stdout or stderr @@ -1117,7 +1118,6 @@ func configureVarlinkAttachStdio(reader *bufio.Reader, writer *bufio.Writer, std } } }() - if stdin != nil { // Takes stdinput and sends it over the wire after being encoded go func() { @@ -1126,7 +1126,12 @@ func configureVarlinkAttachStdio(reader *bufio.Reader, writer *bufio.Writer, std sendGenericError(ecChan) errChan <- err } - + _, err := varlinkHangupWriter.Write([]byte("EOF")) + if err != nil { + logrus.Errorf("unable to notify server to hangup: %q", err) + } + err = varlinkStdinWriter.Close() + errChan <- err }() } return errChan diff --git a/pkg/varlinkapi/attach.go b/pkg/varlinkapi/attach.go index f8557ae0c..37adbbf55 100644 --- a/pkg/varlinkapi/attach.go +++ b/pkg/varlinkapi/attach.go @@ -70,7 +70,6 @@ func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string, detachKeys st } reader, writer, _, pw, streams := setupStreams(call) - go func() { if err := virtwriter.Reader(reader, nil, nil, pw, resize, nil); err != nil { errChan <- err diff --git a/pkg/varlinkapi/virtwriter/virtwriter.go b/pkg/varlinkapi/virtwriter/virtwriter.go index 27ecd1f52..dd171943f 100644 --- a/pkg/varlinkapi/virtwriter/virtwriter.go +++ b/pkg/varlinkapi/virtwriter/virtwriter.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "encoding/json" "io" + "time" "github.com/pkg/errors" "k8s.io/client-go/tools/remotecommand" @@ -26,8 +27,14 @@ const ( TerminalResize SocketDest = iota // Quit and detach Quit SocketDest = iota + // Quit from the client + HangUpFromClient SocketDest = iota ) +// ClientHangup signifies that the client wants to drop its +// connection from the server +var ClientHangup = errors.New("client hangup") + // IntToSocketDest returns a socketdest based on integer input func IntToSocketDest(i int) SocketDest { switch i { @@ -41,6 +48,8 @@ func IntToSocketDest(i int) SocketDest { return TerminalResize case Quit.Int(): return Quit + case HangUpFromClient.Int(): + return HangUpFromClient default: return ToStderr } @@ -65,7 +74,7 @@ func NewVirtWriteCloser(w *bufio.Writer, dest SocketDest) VirtWriteCloser { // Close is a required method for a writecloser func (v VirtWriteCloser) Close() error { - return nil + return v.writer.Flush() } // Write prepends a header to the input message. The header is @@ -96,7 +105,6 @@ func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remote if r == nil { return errors.Errorf("Reader must not be nil") } - for { n, err := io.ReadFull(r, headerBytes) if err != nil { @@ -107,7 +115,6 @@ func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remote } messageSize = int64(binary.BigEndian.Uint32(headerBytes[4:8])) - switch IntToSocketDest(int(headerBytes[0])) { case ToStdout: if output != nil { @@ -161,7 +168,16 @@ func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remote execEcChan <- int(ecInt) } return nil - + case HangUpFromClient: + // This sleep allows the pipes to flush themselves before tearing everything down. + // It makes me sick to do it but after a full day I cannot put my finger on the race + // that occurs when closing things up. It would require a significant rewrite of code + // to make the pipes close down properly. Given that we are currently discussing a + // rewrite of all things remote, this hardly seems worth resolving. + // + // reproducer: echo hello | (podman-remote run -i alpine cat) + time.Sleep(1 * time.Second) + return ClientHangup default: // Something really went wrong return errors.New("unknown multiplex destination") -- cgit v1.2.3-54-g00ecf