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 <matthew.heon@pm.me>

1 files changed, 17 insertions, 7 deletions

diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go
index 7a62a2b58..1db950f85 100644
--- a/pkg/api/handlers/compat/exec.go
+++ b/pkg/api/handlers/compat/exec.go
@@ -10,6 +10,7 @@ import (
 	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/pkg/api/handlers"
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
+	"github.com/containers/podman/v2/pkg/api/server/idletracker"
 	"github.com/containers/podman/v2/pkg/specgen/generate"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
@@ -173,15 +174,24 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	connection, buffer, err := AttachConnection(w, r)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
-	}
-	logrus.Debugf("Hijack for attach of container %s exec session %s successful", sessionCtr.ID(), sessionID)
+	idleTracker := r.Context().Value("idletracker").(*idletracker.IdleTracker)
+	hijackChan := make(chan bool, 1)
+
+	if err := sessionCtr.ExecHTTPStartAndAttach(sessionID, r, w, nil, nil, nil, hijackChan); err != nil {
+		hijackComplete := <-hijackChan
 
-	if err := sessionCtr.ExecHTTPStartAndAttach(sessionID, connection, buffer, nil, nil, nil); err != nil {
 		logrus.Errorf("Error attaching to container %s exec session %s: %v", sessionCtr.ID(), sessionID, err)
+
+		if hijackComplete {
+			// We do need to tell the idle tracker that the
+			// connection has been closed, though. We can guarantee
+			// that is true after HTTPAttach exits.
+			idleTracker.TrackHijackedClosed()
+		} else {
+			// A hijack was not successfully completed. We need to
+			// report the error normally.
+			utils.InternalServerError(w, err)
+		}
 	}
 
 	logrus.Debugf("Attach for container %s exec session %s completed successfully", sessionCtr.ID(), sessionID)