From 2ea9dac5e1d00b2820bd7156e3bea4b9fd98c1e6 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 24 Aug 2020 11:35:01 -0400 Subject: Send HTTP Hijack headers after successful attach Our previous flow was to perform a hijack before passing a connection into Libpod, and then Libpod would attach to the container's attach socket and begin forwarding traffic. A problem emerges: we write the attach header as soon as the attach complete. As soon as we write the header, the client assumes that all is ready, and sends a Start request. This Start may be processed *before* we successfully finish attaching, causing us to lose output. The solution is to handle hijacking inside Libpod. Unfortunately, this requires a downright extensive refactor of the Attach and HTTP Exec StartAndAttach code. I think the result is an improvement in some places (a lot more errors will be handled with a proper HTTP error code, before the hijack occurs) but other parts, like the relocation of printing container logs, are just *bad*. Still, we need this fixed now to get CI back into good shape... Fixes #7195 Signed-off-by: Matthew Heon --- libpod/oci.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'libpod/oci.go') diff --git a/libpod/oci.go b/libpod/oci.go index 89850affc..924c32510 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -1,8 +1,7 @@ package libpod import ( - "bufio" - "net" + "net/http" "github.com/containers/podman/v2/libpod/define" "k8s.io/client-go/tools/remotecommand" @@ -63,7 +62,7 @@ type OCIRuntime interface { // used instead. Detach keys of "" will disable detaching via keyboard. // The streams parameter will determine which streams to forward to the // client. - HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool) error + HTTPAttach(ctr *Container, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, streamAttach, streamLogs bool) error // AttachResize resizes the terminal in use by the given container. AttachResize(ctr *Container, newSize remotecommand.TerminalSize) error @@ -80,7 +79,7 @@ type OCIRuntime interface { // Maintains the same invariants as ExecContainer (returns on session // start, with a goroutine running in the background to handle attach). // The HTTP attach itself maintains the same invariants as HTTPAttach. - ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, cancel <-chan bool) (int, chan error, error) + ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) // ExecContainerDetached executes a command in a running container, but // does not attach to it. Returns the PID of the exec session and an // error (if starting the exec session failed) -- cgit v1.2.3-54-g00ecf