From ce56b688712dad1095ab0c520fab5c0b7cedc68b Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 15 Apr 2020 16:10:33 -0400 Subject: Wire in endpoint for ExecStart This is still very early not not well tested, and missing resize capability, but it does provide the first bits of exec. Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/containers_attach.go | 11 +++-- pkg/api/handlers/compat/exec.go | 74 ++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) (limited to 'pkg/api/handlers/compat') diff --git a/pkg/api/handlers/compat/containers_attach.go b/pkg/api/handlers/compat/containers_attach.go index 52c851b8c..3c9a6fd69 100644 --- a/pkg/api/handlers/compat/containers_attach.go +++ b/pkg/api/handlers/compat/containers_attach.go @@ -13,6 +13,12 @@ import ( "k8s.io/client-go/tools/remotecommand" ) +// AttachHeader is the literal header sent for upgraded/hijacked connections for +// attach, sourced from Docker at: +// https://raw.githubusercontent.com/moby/moby/b95fad8e51bd064be4f4e58a996924f343846c85/api/server/router/container/container_routes.go +// Using literally to ensure compatibility with existing clients. +const AttachHeader = "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n" + func AttachContainer(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) @@ -106,10 +112,7 @@ func AttachContainer(w http.ResponseWriter, r *http.Request) { return } - // This header string sourced from Docker: - // https://raw.githubusercontent.com/moby/moby/b95fad8e51bd064be4f4e58a996924f343846c85/api/server/router/container/container_routes.go - // Using literally to ensure compatibility with existing clients. - fmt.Fprintf(connection, "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n") + fmt.Fprintf(connection, AttachHeader) logrus.Debugf("Hijack for attach of container %s successful", ctr.ID()) diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index ec1a8ac96..897f4e6bd 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -11,6 +11,7 @@ import ( "github.com/containers/libpod/pkg/api/handlers" "github.com/containers/libpod/pkg/api/handlers/utils" "github.com/gorilla/mux" + "github.com/gorilla/schema" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -105,3 +106,76 @@ func ExecInspectHandler(w http.ResponseWriter, r *http.Request) { utils.WriteResponse(w, http.StatusOK, inspectOut) } + +// ExecStartHandler runs a given exec session. +func ExecStartHandler(w http.ResponseWriter, r *http.Request) { + runtime := r.Context().Value("runtime").(*libpod.Runtime) + decoder := r.Context().Value("decoder").(*schema.Decoder) + + sessionID := mux.Vars(r)["id"] + + // TODO: Need to support these + query := struct { + Detach bool `schema:"Detach"` + Tty bool `schema:"Tty"` + }{ + // override any golang type defaults + } + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String())) + return + } + + if _, found := r.URL.Query()["Detach"]; found { + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + errors.Errorf("Detached exec is not yet supported")) + return + } + if _, found := r.URL.Query()["Tty"]; found { + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + errors.Errorf("overriding terminal setting in ExecStart is not yet supported")) + return + } + + sessionCtr, err := runtime.GetExecSessionContainer(sessionID) + if err != nil { + utils.Error(w, fmt.Sprintf("No such exec session: %s", sessionID), http.StatusNotFound, err) + return + } + + logrus.Debugf("Starting exec session %s of container %s", sessionID, sessionCtr.ID()) + + state, err := sessionCtr.State() + if err != nil { + utils.InternalServerError(w, err) + return + } + if state != define.ContainerStateRunning { + utils.Error(w, http.StatusText(http.StatusConflict), http.StatusConflict, errors.Errorf("cannot exec in a container that is not running; container %s is %s", sessionCtr.ID(), state.String())) + return + } + + // Hijack the connection + hijacker, ok := w.(http.Hijacker) + if !ok { + utils.InternalServerError(w, errors.Errorf("unable to hijack connection")) + return + } + + connection, buffer, err := hijacker.Hijack() + if err != nil { + utils.InternalServerError(w, errors.Wrapf(err, "error hijacking connection")) + return + } + + fmt.Fprintf(connection, AttachHeader) + + logrus.Debugf("Hijack for attach of container %s exec session %s successful", sessionCtr.ID(), sessionID) + + 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) + } + + logrus.Debugf("Attach for container %s exec session %s completed successfully", sessionCtr.ID(), sessionID) +} -- cgit v1.2.3-54-g00ecf From cf1f13af986b1e81bc17f58aae428610c14afc4f Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 15 Apr 2020 16:46:58 -0400 Subject: Add APIv2 handler for resizing exec sessions Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/exec.go | 40 ++++++++++++++++++++++++++++++++++++++++ pkg/api/server/register_exec.go | 16 ++++++++-------- 2 files changed, 48 insertions(+), 8 deletions(-) (limited to 'pkg/api/handlers/compat') diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index 897f4e6bd..df4950947 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -14,6 +14,7 @@ import ( "github.com/gorilla/schema" "github.com/pkg/errors" "github.com/sirupsen/logrus" + "k8s.io/client-go/tools/remotecommand" ) // ExecCreateHandler creates an exec session for a given container. @@ -107,6 +108,45 @@ func ExecInspectHandler(w http.ResponseWriter, r *http.Request) { utils.WriteResponse(w, http.StatusOK, inspectOut) } +// ExecResizeHandler resizes a given exec session's TTY. +func ExecResizeHandler(w http.ResponseWriter, r *http.Request) { + runtime := r.Context().Value("runtime").(*libpod.Runtime) + decoder := r.Context().Value("decoder").(*schema.Decoder) + + sessionID := mux.Vars(r)["id"] + + query := struct { + Height uint16 `schema:"h"` + Width uint16 `schema:"w"` + }{ + // override any golang type defaults + } + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, + errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String())) + return + } + + sessionCtr, err := runtime.GetExecSessionContainer(sessionID) + if err != nil { + utils.Error(w, fmt.Sprintf("No such exec session: %s", sessionID), http.StatusNotFound, err) + return + } + + newSize := remotecommand.TerminalSize{ + Width: query.Width, + Height: query.Height, + } + + if err := sessionCtr.ExecResize(sessionID, newSize); err != nil { + utils.InternalServerError(w, err) + return + } + + // This is a 201 some reason, not a 204. + utils.WriteResponse(w, http.StatusCreated, "") +} + // ExecStartHandler runs a given exec session. func ExecStartHandler(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) diff --git a/pkg/api/server/register_exec.go b/pkg/api/server/register_exec.go index d37c3eb11..f1f1cc7e9 100644 --- a/pkg/api/server/register_exec.go +++ b/pkg/api/server/register_exec.go @@ -114,7 +114,7 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // $ref: "#/responses/InternalError" r.Handle(VersionedPath("/exec/{id}/start"), s.APIHandler(compat.ExecStartHandler)).Methods(http.MethodPost) // Added non version path to URI to support docker non versioned paths - r.Handle("/exec/{id}/start", s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) + r.Handle("/exec/{id}/start", s.APIHandler(compat.ExecStartHandler)).Methods(http.MethodPost) // swagger:operation POST /exec/{id}/resize compat resizeExec // --- // tags: @@ -145,9 +145,9 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // $ref: "#/responses/NoSuchExecInstance" // 500: // $ref: "#/responses/InternalError" - r.Handle(VersionedPath("/exec/{id}/resize"), s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) + r.Handle(VersionedPath("/exec/{id}/resize"), s.APIHandler(compat.ExecResizeHandler)).Methods(http.MethodPost) // Added non version path to URI to support docker non versioned paths - r.Handle("/exec/{id}/resize", s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) + r.Handle("/exec/{id}/resize", s.APIHandler(compat.ExecResizeHandler)).Methods(http.MethodPost) // swagger:operation GET /exec/{id}/json compat inspectExec // --- // tags: @@ -264,10 +264,10 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // properties: // Detach: // type: boolean - // description: Detach from the command + // description: Detach from the command. Not presently supported. // Tty: // type: boolean - // description: Allocate a pseudo-TTY + // description: Allocate a pseudo-TTY. Not presently supported. // produces: // - application/json // responses: @@ -276,10 +276,10 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // 404: // $ref: "#/responses/NoSuchExecInstance" // 409: - // description: container is stopped or paused + // description: container is not running. // 500: // $ref: "#/responses/InternalError" - r.Handle(VersionedPath("/libpod/exec/{id}/start"), s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) + r.Handle(VersionedPath("/libpod/exec/{id}/start"), s.APIHandler(compat.ExecStartHandler)).Methods(http.MethodPost) // swagger:operation POST /libpod/exec/{id}/resize libpod libpodResizeExec // --- // tags: @@ -310,7 +310,7 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // $ref: "#/responses/NoSuchExecInstance" // 500: // $ref: "#/responses/InternalError" - r.Handle(VersionedPath("/libpod/exec/{id}/resize"), s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) + r.Handle(VersionedPath("/libpod/exec/{id}/resize"), s.APIHandler(compat.ExecResizeHandler)).Methods(http.MethodPost) // swagger:operation GET /libpod/exec/{id}/json libpod libpodInspectExec // --- // tags: -- cgit v1.2.3-54-g00ecf From 0f0abe290927cd17542953042885b554dbffd83e Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 14 May 2020 16:31:55 -0400 Subject: Prune stale exec sessions on inspect The usual flow for exec is going to be: - Create exec session - Start and attach to exec session - Exec session exits, attach session terminates - Client does an exec inspect to pick up exit code The safest point to remove the exec session, without doing any database changes to track stale sessions, is to remove during the last part of this - the single inspect after the exec session exits. This is definitely different from Docker (which would retain the exec session for up to 10 minutes after it exits, where we will immediately discard) but should be close enough to be not noticeable in regular usage. Signed-off-by: Matthew Heon --- libpod/container_exec.go | 2 +- pkg/api/handlers/compat/exec.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'pkg/api/handlers/compat') diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 979594eb4..6ad767b4b 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -104,7 +104,7 @@ func (e *ExecSession) Inspect() (*define.InspectExecSession, error) { } output := new(define.InspectExecSession) - output.CanRemove = e.State != define.ExecStateRunning + output.CanRemove = e.State == define.ExecStateStopped output.ContainerID = e.ContainerId if e.Config.DetachKeys != nil { output.DetachKeys = *e.Config.DetachKeys diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index df4950947..f97fecca2 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -106,6 +106,15 @@ func ExecInspectHandler(w http.ResponseWriter, r *http.Request) { } utils.WriteResponse(w, http.StatusOK, inspectOut) + + // Only for the Compat API: we want to remove sessions that were + // stopped. This is very hacky, but should suffice for now. + if !utils.IsLibpodRequest(r) && inspectOut.CanRemove { + logrus.Infof("Pruning stale exec session %s from container %s", sessionID, sessionCtr.ID()) + if err := sessionCtr.ExecRemove(sessionID, false); err != nil && errors.Cause(err) != define.ErrNoSuchExecSession { + logrus.Errorf("Error removing stale exec session %s from container %s: %v", sessionID, sessionCtr.ID(), err) + } + } } // ExecResizeHandler resizes a given exec session's TTY. -- cgit v1.2.3-54-g00ecf From 08d04c2e0512cd67e81965f4f6aee26a83fd4046 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 14 May 2020 16:50:44 -0400 Subject: Parameters for ExecStart are body, not query Oops. Misread the docs when I initially implemented this. Nice and easy fix, at least. Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/exec.go | 24 +++++++----------------- pkg/api/handlers/types.go | 5 +++++ 2 files changed, 12 insertions(+), 17 deletions(-) (limited to 'pkg/api/handlers/compat') diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index f97fecca2..6f62dca11 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -159,33 +159,23 @@ func ExecResizeHandler(w http.ResponseWriter, r *http.Request) { // ExecStartHandler runs a given exec session. func ExecStartHandler(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) - decoder := r.Context().Value("decoder").(*schema.Decoder) sessionID := mux.Vars(r)["id"] - // TODO: Need to support these - query := struct { - Detach bool `schema:"Detach"` - Tty bool `schema:"Tty"` - }{ - // override any golang type defaults - } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, - errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String())) - return - } + // TODO: We should read/support Tty and Detach from here. + bodyParams := new(handlers.ExecStartConfig) - if _, found := r.URL.Query()["Detach"]; found { + if err := json.NewDecoder(r.Body).Decode(&bodyParams); err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, - errors.Errorf("Detached exec is not yet supported")) + errors.Wrapf(err, "failed to decode parameters for %s", r.URL.String())) return } - if _, found := r.URL.Query()["Tty"]; found { + if bodyParams.Detach == true { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, - errors.Errorf("overriding terminal setting in ExecStart is not yet supported")) + errors.Errorf("Detached exec is not yet supported")) return } + // TODO: Verify TTY setting against what inspect session was made with sessionCtr, err := runtime.GetExecSessionContainer(sessionID) if err != nil { diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index 2075d29df..d8cdd9caf 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -170,6 +170,11 @@ type ExecCreateResponse struct { docker.IDResponse } +type ExecStartConfig struct { + Detach bool `json:"Detach"` + Tty bool `json:"Tty"` +} + func ImageToImageSummary(l *libpodImage.Image) (*entities.ImageSummary, error) { containers, err := l.Containers() if err != nil { -- cgit v1.2.3-54-g00ecf From 79ad6bb53d2409c04f54c292c62bb8021002cfe1 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 14 May 2020 17:34:42 -0400 Subject: Fix lint Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/exec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg/api/handlers/compat') diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index 6f62dca11..6ff9f607a 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -170,7 +170,7 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) { errors.Wrapf(err, "failed to decode parameters for %s", r.URL.String())) return } - if bodyParams.Detach == true { + if bodyParams.Detach { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, errors.Errorf("Detached exec is not yet supported")) return -- cgit v1.2.3-54-g00ecf From 6d1e5c713a3a62ba671c6966af23dc86ed2270dd Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 15 May 2020 14:43:26 -0400 Subject: Drop APIv2 resize endpoint Jhon is working on an alternative version that will combine container and exec session resize, so we'll wait for that. Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/exec.go | 41 ----------------------------------------- pkg/api/server/register_exec.go | 6 +++--- 2 files changed, 3 insertions(+), 44 deletions(-) (limited to 'pkg/api/handlers/compat') diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index 6ff9f607a..6865a3319 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -11,10 +11,8 @@ import ( "github.com/containers/libpod/pkg/api/handlers" "github.com/containers/libpod/pkg/api/handlers/utils" "github.com/gorilla/mux" - "github.com/gorilla/schema" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "k8s.io/client-go/tools/remotecommand" ) // ExecCreateHandler creates an exec session for a given container. @@ -117,45 +115,6 @@ func ExecInspectHandler(w http.ResponseWriter, r *http.Request) { } } -// ExecResizeHandler resizes a given exec session's TTY. -func ExecResizeHandler(w http.ResponseWriter, r *http.Request) { - runtime := r.Context().Value("runtime").(*libpod.Runtime) - decoder := r.Context().Value("decoder").(*schema.Decoder) - - sessionID := mux.Vars(r)["id"] - - query := struct { - Height uint16 `schema:"h"` - Width uint16 `schema:"w"` - }{ - // override any golang type defaults - } - if err := decoder.Decode(&query, r.URL.Query()); err != nil { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, - errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String())) - return - } - - sessionCtr, err := runtime.GetExecSessionContainer(sessionID) - if err != nil { - utils.Error(w, fmt.Sprintf("No such exec session: %s", sessionID), http.StatusNotFound, err) - return - } - - newSize := remotecommand.TerminalSize{ - Width: query.Width, - Height: query.Height, - } - - if err := sessionCtr.ExecResize(sessionID, newSize); err != nil { - utils.InternalServerError(w, err) - return - } - - // This is a 201 some reason, not a 204. - utils.WriteResponse(w, http.StatusCreated, "") -} - // ExecStartHandler runs a given exec session. func ExecStartHandler(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value("runtime").(*libpod.Runtime) diff --git a/pkg/api/server/register_exec.go b/pkg/api/server/register_exec.go index 1571de90b..19b7e2fcd 100644 --- a/pkg/api/server/register_exec.go +++ b/pkg/api/server/register_exec.go @@ -145,9 +145,9 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // $ref: "#/responses/NoSuchExecInstance" // 500: // $ref: "#/responses/InternalError" - r.Handle(VersionedPath("/exec/{id}/resize"), s.APIHandler(compat.ExecResizeHandler)).Methods(http.MethodPost) + r.Handle(VersionedPath("/exec/{id}/resize"), s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) // Added non version path to URI to support docker non versioned paths - r.Handle("/exec/{id}/resize", s.APIHandler(compat.ExecResizeHandler)).Methods(http.MethodPost) + r.Handle("/exec/{id}/resize", s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) // swagger:operation GET /exec/{id}/json compat inspectExec // --- // tags: @@ -310,7 +310,7 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // $ref: "#/responses/NoSuchExecInstance" // 500: // $ref: "#/responses/InternalError" - r.Handle(VersionedPath("/libpod/exec/{id}/resize"), s.APIHandler(compat.ExecResizeHandler)).Methods(http.MethodPost) + r.Handle(VersionedPath("/libpod/exec/{id}/resize"), s.APIHandler(compat.UnsupportedHandler)).Methods(http.MethodPost) // swagger:operation GET /libpod/exec/{id}/json libpod libpodInspectExec // --- // tags: -- cgit v1.2.3-54-g00ecf