diff options
-rwxr-xr-x | contrib/cirrus/setup_environment.sh | 3 | ||||
-rw-r--r-- | contrib/spec/podman.spec.in | 2 | ||||
-rw-r--r-- | libpod/container_exec.go | 8 | ||||
-rw-r--r-- | libpod/oci.go | 3 | ||||
-rw-r--r-- | libpod/oci_conmon_exec_linux.go | 4 | ||||
-rw-r--r-- | pkg/api/handlers/compat/exec.go | 48 | ||||
-rw-r--r-- | pkg/api/server/register_exec.go | 6 | ||||
-rw-r--r-- | pkg/bindings/containers/exec.go | 29 | ||||
-rw-r--r-- | pkg/domain/infra/abi/containers.go | 7 | ||||
-rw-r--r-- | pkg/domain/infra/tunnel/containers.go | 23 | ||||
-rw-r--r-- | pkg/env/env.go | 13 | ||||
-rw-r--r-- | pkg/env/env_supported.go | 15 | ||||
-rw-r--r-- | pkg/env/env_unsupported.go | 8 | ||||
-rw-r--r-- | pkg/specgen/generate/container_create.go | 17 | ||||
-rw-r--r-- | test/e2e/exec_test.go | 5 |
15 files changed, 145 insertions, 46 deletions
diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh index 945b33909..25b7ff941 100755 --- a/contrib/cirrus/setup_environment.sh +++ b/contrib/cirrus/setup_environment.sh @@ -46,6 +46,9 @@ case "${OS_RELEASE_ID}" in workaround_bfq_bug + # HACK: Need Conmon 2.0.17, currently in updates-testing on F31. + dnf update -y --enablerepo=updates-testing conmon + if [[ "$ADD_SECOND_PARTITION" == "true" ]]; then bash "$SCRIPT_BASE/add_second_partition.sh" fi diff --git a/contrib/spec/podman.spec.in b/contrib/spec/podman.spec.in index 701b5f0b1..9ab0db3ae 100644 --- a/contrib/spec/podman.spec.in +++ b/contrib/spec/podman.spec.in @@ -33,7 +33,7 @@ # People want conmon packaged with the copr rpm %global import_path_conmon github.com/containers/conmon %global git_conmon https://%{import_path_conmon} -%global commit_conmon d532caebc788fafdd2a305b68cd1983b4039bea4 +%global commit_conmon 41877362fc4685d55e0473d2e4a1cbe5e1debee0 %global shortcommit_conmon %(c=%{commit_conmon}; echo ${c:0:7}) Name: podman diff --git a/libpod/container_exec.go b/libpod/container_exec.go index f2943b73c..a0e8904dc 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -69,6 +69,10 @@ type ExecConfig struct { // The ID of the exec session, and the ID of the container the exec // session is a part of (in that order). ExitCommand []string `json:"exitCommand,omitempty"` + // ExitCommandDelay is a delay (in seconds) between the container + // exiting, and the exit command being executed. If set to 0, there is + // no delay. If set, ExitCommand must also be set. + ExitCommandDelay uint `json:"exitCommandDelay,omitempty"` } // ExecSession contains information on a single exec session attached to a given @@ -165,6 +169,9 @@ func (c *Container) ExecCreate(config *ExecConfig) (string, error) { if len(config.Command) == 0 { return "", errors.Wrapf(define.ErrInvalidArg, "must provide a non-empty command to start an exec session") } + if config.ExitCommandDelay > 0 && len(config.ExitCommand) == 0 { + return "", errors.Wrapf(define.ErrInvalidArg, "must provide a non-empty exit command if giving an exit command delay") + } // Verify that we are in a good state to continue if !c.ensureState(define.ContainerStateRunning) { @@ -984,6 +991,7 @@ func prepareForExec(c *Container, session *ExecSession) (*ExecOptions, error) { opts.PreserveFDs = session.Config.PreserveFDs opts.DetachKeys = session.Config.DetachKeys opts.ExitCommand = session.Config.ExitCommand + opts.ExitCommandDelay = session.Config.ExitCommandDelay return opts, nil } diff --git a/libpod/oci.go b/libpod/oci.go index 7c5218319..684a7ba42 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -172,6 +172,9 @@ type ExecOptions struct { // ExitCommand is a command that will be run after the exec session // exits. ExitCommand []string + // ExitCommandDelay is a delay (in seconds) between the exec session + // exiting, and the exit command being invoked. + ExitCommandDelay uint } // HTTPAttachStreams informs the HTTPAttach endpoint which of the container's diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 51819f90a..bc39100f8 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -421,12 +421,14 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex for _, arg := range options.ExitCommand[1:] { args = append(args, []string{"--exit-command-arg", arg}...) } + if options.ExitCommandDelay > 0 { + args = append(args, []string{"--exit-delay", fmt.Sprintf("%d", options.ExitCommandDelay)}...) + } } logrus.WithFields(logrus.Fields{ "args": args, }).Debugf("running conmon: %s", r.conmonPath) - // TODO: Need to pass this back so we can wait on it. execCmd := exec.Command(r.conmonPath, args...) // TODO: This is commented because it doesn't make much sense in HTTP 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 diff --git a/pkg/bindings/containers/exec.go b/pkg/bindings/containers/exec.go index 2aeeae1f8..73cfb5079 100644 --- a/pkg/bindings/containers/exec.go +++ b/pkg/bindings/containers/exec.go @@ -1,6 +1,7 @@ package containers import ( + "bytes" "context" "net/http" "strings" @@ -69,3 +70,31 @@ func ExecInspect(ctx context.Context, sessionID string) (*define.InspectExecSess return respStruct, nil } + +// ExecStart starts (but does not attach to) a given exec session. +func ExecStart(ctx context.Context, sessionID string) error { + conn, err := bindings.GetClient(ctx) + if err != nil { + return err + } + + logrus.Debugf("Starting exec session ID %q", sessionID) + + // We force Detach to true + body := struct { + Detach bool `json:"Detach"` + }{ + Detach: true, + } + bodyJSON, err := json.Marshal(body) + if err != nil { + return err + } + + resp, err := conn.DoRequest(bytes.NewReader(bodyJSON), http.MethodPost, "/exec/%s/start", nil, nil, sessionID) + if err != nil { + return err + } + + return resp.Process(nil) +} diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index e982c7c11..706fcec47 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -613,12 +613,11 @@ func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrId s if err != nil { return "", errors.Wrapf(err, "error retrieving Libpod configuration to build exec exit command") } - podmanPath, err := os.Executable() + // TODO: Add some ability to toggle syslog + exitCommandArgs, err := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, false, true, true) if err != nil { - return "", errors.Wrapf(err, "error retrieving executable to build exec exit command") + return "", errors.Wrapf(err, "error constructing exit command for exec session") } - // TODO: Add some ability to toggle syslog - exitCommandArgs := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, podmanPath, false, true, true) execConfig.ExitCommand = exitCommandArgs // Create and start the exec session diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index e1c859e7c..af9d94293 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -376,7 +376,7 @@ func (ic *ContainerEngine) ContainerAttach(ctx context.Context, nameOrId string, return containers.Attach(ic.ClientCxt, nameOrId, &options.DetachKeys, nil, bindings.PTrue, options.Stdin, options.Stdout, options.Stderr, nil) } -func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions, streams define.AttachStreams) (int, error) { +func makeExecConfig(options entities.ExecOptions) *handlers.ExecCreateConfig { env := []string{} for k, v := range options.Envs { env = append(env, fmt.Sprintf("%s=%s", k, v)) @@ -395,6 +395,12 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, o createConfig.WorkingDir = options.WorkDir createConfig.Cmd = options.Cmd + return createConfig +} + +func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions, streams define.AttachStreams) (int, error) { + createConfig := makeExecConfig(options) + sessionID, err := containers.ExecCreate(ic.ClientCxt, nameOrId, createConfig) if err != nil { return 125, err @@ -412,8 +418,19 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, o return inspectOut.ExitCode, nil } -func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrID string, options entities.ExecOptions) (string, error) { - return "", errors.New("not implemented") +func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrId string, options entities.ExecOptions) (string, error) { + createConfig := makeExecConfig(options) + + sessionID, err := containers.ExecCreate(ic.ClientCxt, nameOrId, createConfig) + if err != nil { + return "", err + } + + if err := containers.ExecStart(ic.ClientCxt, sessionID); err != nil { + return "", err + } + + return sessionID, nil } func startAndAttach(ic *ContainerEngine, name string, detachKeys *string, input, output, errput *os.File) error { //nolint diff --git a/pkg/env/env.go b/pkg/env/env.go index c6a1a0d28..a16007a50 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -20,18 +20,6 @@ var DefaultEnvVariables = map[string]string{ const whiteSpaces = " \t" -// ParseSlice parses the specified slice and transforms it into an environment -// map. -func ParseSlice(s []string) (map[string]string, error) { - env := make(map[string]string, len(s)) - for _, e := range s { - if err := parseEnv(env, e); err != nil { - return nil, err - } - } - return env, nil -} - // Slice transforms the specified map of environment variables into a // slice. If a value is non-empty, the key and value are joined with '='. func Slice(m map[string]string) []string { @@ -96,7 +84,6 @@ func parseEnv(env map[string]string, line string) error { if data[0] == "" { return errors.Errorf("invalid environment variable: %q", line) } - // trim the front of a variable, but nothing else name := strings.TrimLeft(data[0], whiteSpaces) if strings.ContainsAny(name, whiteSpaces) { diff --git a/pkg/env/env_supported.go b/pkg/env/env_supported.go new file mode 100644 index 000000000..8be9f9592 --- /dev/null +++ b/pkg/env/env_supported.go @@ -0,0 +1,15 @@ +// +build linux darwin + +package env + +// ParseSlice parses the specified slice and transforms it into an environment +// map. +func ParseSlice(s []string) (map[string]string, error) { + env := make(map[string]string, len(s)) + for _, e := range s { + if err := parseEnv(env, e); err != nil { + return nil, err + } + } + return env, nil +} diff --git a/pkg/env/env_unsupported.go b/pkg/env/env_unsupported.go new file mode 100644 index 000000000..a71c2956d --- /dev/null +++ b/pkg/env/env_unsupported.go @@ -0,0 +1,8 @@ +// +build !linux,!darwin + +package env + +func ParseSlice(s []string) (map[string]string, error) { + m := make(map[string]string) + return m, nil +} diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index ffd7fd4dd..7ddfed339 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -107,12 +107,12 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener } options = append(options, opts...) - podmanPath, err := os.Executable() + // TODO: Enable syslog support - we'll need to put this in SpecGen. + exitCommandArgs, err := CreateExitCommandArgs(rt.StorageConfig(), rtc, false, s.Remove, false) if err != nil { return nil, err } - // TODO: Enable syslog support - we'll need to put this in SpecGen. - options = append(options, libpod.WithExitCommand(CreateExitCommandArgs(rt.StorageConfig(), rtc, podmanPath, false, s.Remove, false))) + options = append(options, libpod.WithExitCommand(exitCommandArgs)) runtimeSpec, err := SpecGenToOCI(ctx, s, rt, rtc, newImage, finalMounts) if err != nil { @@ -229,13 +229,18 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. return options, nil } -func CreateExitCommandArgs(storageConfig storage.StoreOptions, config *config.Config, podmanPath string, syslog, rm bool, exec bool) []string { +func CreateExitCommandArgs(storageConfig storage.StoreOptions, config *config.Config, syslog, rm, exec bool) ([]string, error) { // We need a cleanup process for containers in the current model. // But we can't assume that the caller is Podman - it could be another // user of the API. // As such, provide a way to specify a path to Podman, so we can // still invoke a cleanup process. + podmanPath, err := os.Executable() + if err != nil { + return nil, err + } + command := []string{podmanPath, "--root", storageConfig.GraphRoot, "--runroot", storageConfig.RunRoot, @@ -265,9 +270,11 @@ func CreateExitCommandArgs(storageConfig storage.StoreOptions, config *config.Co command = append(command, "--rm") } + // This has to be absolutely last, to ensure that the exec session ID + // will be added after it by Libpod. if exec { command = append(command, "--exec") } - return command + return command, nil } diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go index 8ec666c2b..b152da9e6 100644 --- a/test/e2e/exec_test.go +++ b/test/e2e/exec_test.go @@ -25,6 +25,10 @@ var _ = Describe("Podman exec", func() { podmanTest = PodmanTestCreate(tempdir) podmanTest.Setup() podmanTest.SeedImages() + // HACK: Remove this once we get Conmon 2.0.17 on Ubuntu + if podmanTest.Host.Distribution == "ubuntu" { + Skip("Unable to perform test on Ubuntu distributions due to too-old Conmon (need 2.0.17)") + } }) AfterEach(func() { @@ -284,7 +288,6 @@ var _ = Describe("Podman exec", func() { }) It("podman exec --detach", func() { - Skip(v2remotefail) ctrName := "testctr" ctr := podmanTest.Podman([]string{"run", "-t", "-i", "-d", "--name", ctrName, ALPINE, "top"}) ctr.WaitWithDefaultTimeout() |