From e0d940463475b907d56aa548401f8ba916cec21b Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 2 Jun 2020 14:02:54 -0400 Subject: Enable detached exec for remote The biggest obstacle here was cleanup - we needed a way to remove detached exec sessions after they exited, but there's no way to tell if an exec session will be attached or detached when it's created, and that's when we must add the exit command that would do the removal. The solution was adding a delay to the exit command (5 minutes), which gives sufficient time for attached exec sessions to retrieve the exit code of the session after it exits, but still guarantees that they will be removed, even for detached sessions. This requires Conmon 2.0.17, which has the new `--exit-delay` flag. As part of the exit command rework, we can drop the hack we were using to clean up exec sessions (remove them as part of inspect). This is a lot cleaner, and I'm a lot happier about it. Otherwise, this is just plumbing - we need a bindings call for detached exec, and that needed to be added to the tunnel mode backend for entities. Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/exec.go | 48 ++++++++++++++++++++++++++++------------- pkg/api/server/register_exec.go | 6 +++--- 2 files changed, 36 insertions(+), 18 deletions(-) (limited to 'pkg/api') diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index 6865a3319..8f7016903 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -10,6 +10,7 @@ import ( "github.com/containers/libpod/libpod/define" "github.com/containers/libpod/pkg/api/handlers" "github.com/containers/libpod/pkg/api/handlers/utils" + "github.com/containers/libpod/pkg/specgen/generate" "github.com/gorilla/mux" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -54,6 +55,24 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) { libpodConfig.Privileged = input.Privileged libpodConfig.User = input.User + // Make our exit command + storageConfig := runtime.StorageConfig() + runtimeConfig, err := runtime.GetConfig() + if err != nil { + utils.InternalServerError(w, err) + return + } + exitCommandArgs, err := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, false, true, true) + if err != nil { + utils.InternalServerError(w, err) + return + } + libpodConfig.ExitCommand = exitCommandArgs + + // Run the exit command after 5 minutes, to mimic Docker's exec cleanup + // behavior. + libpodConfig.ExitCommandDelay = 5 * 60 + sessID, err := ctr.ExecCreate(libpodConfig) if err != nil { if errors.Cause(err) == define.ErrCtrStateInvalid { @@ -104,15 +123,6 @@ 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) - } - } } // ExecStartHandler runs a given exec session. @@ -121,7 +131,7 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) { sessionID := mux.Vars(r)["id"] - // TODO: We should read/support Tty and Detach from here. + // TODO: We should read/support Tty from here. bodyParams := new(handlers.ExecStartConfig) if err := json.NewDecoder(r.Body).Decode(&bodyParams); err != nil { @@ -129,11 +139,6 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) { errors.Wrapf(err, "failed to decode parameters for %s", r.URL.String())) return } - if bodyParams.Detach { - utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, - 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) @@ -154,6 +159,19 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) { return } + if bodyParams.Detach { + // If we are detaching, we do NOT want to hijack. + // Instead, we perform a detached start, and return 200 if + // successful. + if err := sessionCtr.ExecStart(sessionID); err != nil { + utils.InternalServerError(w, err) + return + } + // This is a 200 despite having no content + utils.WriteResponse(w, http.StatusOK, "") + return + } + // Hijack the connection hijacker, ok := w.(http.Hijacker) if !ok { diff --git a/pkg/api/server/register_exec.go b/pkg/api/server/register_exec.go index 17181d286..af9a83496 100644 --- a/pkg/api/server/register_exec.go +++ b/pkg/api/server/register_exec.go @@ -13,7 +13,7 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // tags: // - exec (compat) // summary: Create an exec instance - // description: Run a command inside a running container. + // description: Create an exec session to run a command inside a running container. Exec sessions will be automatically removed 5 minutes after they exit. // parameters: // - in: path // name: name @@ -153,7 +153,7 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // tags: // - exec (compat) // summary: Inspect an exec instance - // description: Return low-level information about an exec instance. Stale (stopped) exec sessions will be auto-removed after inspect runs. + // description: Return low-level information about an exec instance. // parameters: // - in: path // name: id @@ -182,7 +182,7 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // tags: // - exec // summary: Create an exec instance - // description: Run a command inside a running container. + // description: Create an exec session to run a command inside a running container. Exec sessions will be automatically removed 5 minutes after they exit. // parameters: // - in: path // name: name -- cgit v1.2.3-54-g00ecf