From 05a034118fcb5057593cb1492f1c9be59f7a88d8 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 18 May 2020 17:27:27 -0400 Subject: Add CLI frontend for detached exec Add a new ContainerEngine method for creating a detached exec session, and wire in the frontend code to do this. As part of this, move Streams out of ExecOptions to the function signature in an effort to share the struct between both methods. Fixes #5884 Signed-off-by: Matthew Heon --- pkg/domain/entities/containers.go | 1 - pkg/domain/entities/engine_container.go | 3 +- pkg/domain/infra/abi/containers.go | 56 ++++++++++++++++++++++++- pkg/domain/infra/abi/terminal/terminal_linux.go | 14 +------ pkg/domain/infra/tunnel/containers.go | 6 ++- 5 files changed, 63 insertions(+), 17 deletions(-) (limited to 'pkg/domain') diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 3cc4b6db1..f362f88a6 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -242,7 +242,6 @@ type ExecOptions struct { Latest bool PreserveFDs uint Privileged bool - Streams define.AttachStreams Tty bool User string WorkDir string diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index e77f0758b..3d5161745 100644 --- a/pkg/domain/entities/engine_container.go +++ b/pkg/domain/entities/engine_container.go @@ -19,7 +19,8 @@ type ContainerEngine interface { ContainerCp(ctx context.Context, source, dest string, options ContainerCpOptions) (*ContainerCpReport, error) ContainerCreate(ctx context.Context, s *specgen.SpecGenerator) (*ContainerCreateReport, error) ContainerDiff(ctx context.Context, nameOrId string, options DiffOptions) (*DiffReport, error) - ContainerExec(ctx context.Context, nameOrId string, options ExecOptions) (int, error) + ContainerExec(ctx context.Context, nameOrId string, options ExecOptions, streams define.AttachStreams) (int, error) + ContainerExecDetached(ctx context.Context, nameOrID string, options ExecOptions) (string, error) ContainerExists(ctx context.Context, nameOrId string) (*BoolReport, error) ContainerExport(ctx context.Context, nameOrId string, options ContainerExportOptions) error ContainerInit(ctx context.Context, namesOrIds []string, options ContainerInitOptions) ([]*ContainerInitReport, error) diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 035efe575..18282bb79 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -536,7 +536,22 @@ func (ic *ContainerEngine) ContainerAttach(ctx context.Context, nameOrId string, return nil } -func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions) (int, error) { +func makeExecConfig(options entities.ExecOptions) *libpod.ExecConfig { + execConfig := new(libpod.ExecConfig) + execConfig.Command = options.Cmd + execConfig.Terminal = options.Tty + execConfig.Privileged = options.Privileged + execConfig.Environment = options.Envs + execConfig.User = options.User + execConfig.WorkDir = options.WorkDir + execConfig.DetachKeys = &options.DetachKeys + execConfig.PreserveFDs = options.PreserveFDs + execConfig.AttachStdin = options.Interactive + + return execConfig +} + +func checkExecPreserveFDs(options entities.ExecOptions) (int, error) { ec := define.ExecErrorCodeGeneric if options.PreserveFDs > 0 { entries, err := ioutil.ReadDir("/proc/self/fd") @@ -559,15 +574,52 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, o } } } + return ec, nil +} + +func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions, streams define.AttachStreams) (int, error) { + ec, err := checkExecPreserveFDs(options) + if err != nil { + return ec, err + } ctrs, err := getContainersByContext(false, options.Latest, []string{nameOrId}, ic.Libpod) if err != nil { return ec, err } ctr := ctrs[0] - ec, err = terminal.ExecAttachCtr(ctx, ctr, options.Tty, options.Privileged, options.Envs, options.Cmd, options.User, options.WorkDir, &options.Streams, options.PreserveFDs, options.DetachKeys) + + execConfig := makeExecConfig(options) + + ec, err = terminal.ExecAttachCtr(ctx, ctr, execConfig, &streams) return define.TranslateExecErrorToExitCode(ec, err), err } +func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrId string, options entities.ExecOptions) (string, error) { + _, err := checkExecPreserveFDs(options) + if err != nil { + return "", err + } + ctrs, err := getContainersByContext(false, options.Latest, []string{nameOrId}, ic.Libpod) + if err != nil { + return "", err + } + ctr := ctrs[0] + + execConfig := makeExecConfig(options) + + // Create and start the exec session + id, err := ctr.ExecCreate(execConfig) + if err != nil { + return "", err + } + + // TODO: we should try and retrieve exit code if this fails. + if err := ctr.ExecStart(id); err != nil { + return "", err + } + return id, nil +} + func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []string, options entities.ContainerStartOptions) ([]*entities.ContainerStartReport, error) { var reports []*entities.ContainerStartReport var exitCode = define.ExecErrorCodeGeneric diff --git a/pkg/domain/infra/abi/terminal/terminal_linux.go b/pkg/domain/infra/abi/terminal/terminal_linux.go index 15701342f..8d9cdde03 100644 --- a/pkg/domain/infra/abi/terminal/terminal_linux.go +++ b/pkg/domain/infra/abi/terminal/terminal_linux.go @@ -15,13 +15,13 @@ import ( ) // ExecAttachCtr execs and attaches to a container -func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, tty, privileged bool, env map[string]string, cmd []string, user, workDir string, streams *define.AttachStreams, preserveFDs uint, detachKeys string) (int, error) { +func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpod.ExecConfig, streams *define.AttachStreams) (int, error) { resize := make(chan remotecommand.TerminalSize) haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd())) // Check if we are attached to a terminal. If we are, generate resize // events, and set the terminal to raw mode - if haveTerminal && tty { + if haveTerminal && execConfig.Terminal { cancel, oldTermState, err := handleTerminalAttach(ctx, resize) if err != nil { return -1, err @@ -34,16 +34,6 @@ func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, tty, privileged b }() } - execConfig := new(libpod.ExecConfig) - execConfig.Command = cmd - execConfig.Terminal = tty - execConfig.Privileged = privileged - execConfig.Environment = env - execConfig.User = user - execConfig.WorkDir = workDir - execConfig.DetachKeys = &detachKeys - execConfig.PreserveFDs = preserveFDs - return ctr.Exec(execConfig, streams, resize) } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index cebd332e3..b460e3e91 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -329,10 +329,14 @@ 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) } -func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions) (int, error) { +func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrId string, options entities.ExecOptions, streams define.AttachStreams) (int, error) { return 125, errors.New("not implemented") } +func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrID string, options entities.ExecOptions) (string, error) { + return "", errors.New("not implemented") +} + func startAndAttach(ic *ContainerEngine, name string, detachKeys *string, input, output, errput *os.File) error { //nolint attachErr := make(chan error) go func() { -- cgit v1.2.3-54-g00ecf From 5ec56dc79093e52abd8e72913aa62932bf12ba6a Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 19 May 2020 15:54:53 -0400 Subject: Add ability to clean up exec sessions with cleanup We need to be able to use cleanup processes to remove exec sessions as part of detached exec. This PR adds that ability. A new flag is added to `podman container cleanup`, `--exec`, to specify an exec session to be cleaned up. As part of this, ensure that `ExecCleanup` can clean up exec sessions that were running, but have since exited. This ensures that we can come back to an exec session that was running but has since stopped, and clean it up. Signed-off-by: Matthew Heon --- cmd/podman/containers/cleanup.go | 14 ++++++++++++++ docs/source/markdown/podman-container-cleanup.1.md | 7 +++++++ libpod/container_exec.go | 22 +++++++++++++++++++++- pkg/domain/entities/containers.go | 1 + pkg/domain/infra/abi/containers.go | 14 ++++++++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) (limited to 'pkg/domain') diff --git a/cmd/podman/containers/cleanup.go b/cmd/podman/containers/cleanup.go index 2bcd1c1e9..257efb971 100644 --- a/cmd/podman/containers/cleanup.go +++ b/cmd/podman/containers/cleanup.go @@ -7,6 +7,7 @@ import ( "github.com/containers/libpod/cmd/podman/registry" "github.com/containers/libpod/cmd/podman/utils" "github.com/containers/libpod/pkg/domain/entities" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -43,6 +44,7 @@ func init() { flags := cleanupCommand.Flags() flags.BoolVarP(&cleanupOptions.All, "all", "a", false, "Cleans up all containers") flags.BoolVarP(&cleanupOptions.Latest, "latest", "l", false, "Act on the latest container podman is aware of") + flags.StringVar(&cleanupOptions.Exec, "exec", "", "Clean up the given exec session instead of the container") flags.BoolVar(&cleanupOptions.Remove, "rm", false, "After cleanup, remove the container entirely") flags.BoolVar(&cleanupOptions.RemoveImage, "rmi", false, "After cleanup, remove the image entirely") @@ -52,6 +54,18 @@ func cleanup(cmd *cobra.Command, args []string) error { var ( errs utils.OutputErrors ) + + if cleanupOptions.Exec != "" { + switch { + case cleanupOptions.All: + return errors.Errorf("exec and all options conflict") + case len(args) > 1: + return errors.Errorf("cannot use exec option when more than one container is given") + case cleanupOptions.RemoveImage: + return errors.Errorf("exec and rmi options conflict") + } + } + responses, err := registry.ContainerEngine().ContainerCleanup(registry.GetContext(), args, cleanupOptions) if err != nil { return err diff --git a/docs/source/markdown/podman-container-cleanup.1.md b/docs/source/markdown/podman-container-cleanup.1.md index 66a6cff62..a200c2c36 100644 --- a/docs/source/markdown/podman-container-cleanup.1.md +++ b/docs/source/markdown/podman-container-cleanup.1.md @@ -16,6 +16,13 @@ Sometimes container's mount points and network stacks can remain if the podman c Cleanup all containers. +**--exec**=_session_ + +Clean up an exec session for a single container. +Can only be specified if a single container is being cleaned up (conflicts with **--all** as such). +If **--rm** is not specified, temporary files for the exec session will be cleaned up; if it is, the exec session will be removed from the container. +Conflicts with **--rmi** as the container is not being cleaned up so the image cannot be removed. + **--latest**, **-l** Instead of providing the container name or ID, use the last created container. If you use methods other than Podman to run containers such as CRI-O, the last started container could be from either of those methods. diff --git a/libpod/container_exec.go b/libpod/container_exec.go index a38f22488..efbd49b15 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -556,7 +556,27 @@ func (c *Container) ExecCleanup(sessionID string) error { } if session.State == define.ExecStateRunning { - return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot clean up container %s exec session %s as it is running", c.ID(), session.ID()) + // Check if the exec session is still running. + alive, err := c.ociRuntime.ExecUpdateStatus(c, session.ID()) + if err != nil { + return err + } + + if alive { + return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot clean up container %s exec session %s as it is running", c.ID(), session.ID()) + } + + exitCode, err := c.readExecExitCode(session.ID()) + if err != nil { + return err + } + session.ExitCode = exitCode + session.PID = 0 + session.State = define.ExecStateStopped + + if err := c.save(); err != nil { + return err + } } logrus.Infof("Cleaning up container %s exec session %s", c.ID(), session.ID()) diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index f362f88a6..8d85a9b23 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -310,6 +310,7 @@ type ContainerRunReport struct { // cleanup command type ContainerCleanupOptions struct { All bool + Exec string Latest bool Remove bool RemoveImage bool diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 18282bb79..e8eb91644 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -888,6 +888,20 @@ func (ic *ContainerEngine) ContainerCleanup(ctx context.Context, namesOrIds []st for _, ctr := range ctrs { var err error report := entities.ContainerCleanupReport{Id: ctr.ID()} + + if options.Exec != "" { + if options.Remove { + if err := ctr.ExecRemove(options.Exec, false); err != nil { + return nil, err + } + } else { + if err := ctr.ExecCleanup(options.Exec); err != nil { + return nil, err + } + } + return []*entities.ContainerCleanupReport{}, nil + } + if options.Remove { err = ic.Libpod.RemoveContainer(ctx, ctr, false, true) if err != nil { -- cgit v1.2.3-54-g00ecf From 6b9e9610d8ef380d32a682d058157db0f08dd873 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 20 May 2020 15:29:24 -0400 Subject: Enable cleanup processes for detached exec The cleanup command creation logic is made public as part of this and wired such that we can call it both within SpecGen (to make container exit commands) and from the ABI detached exec handler. Exit commands are presently only used for detached exec, but theoretically could be turned on for all exec sessions if we wanted (I'm declining to do this because of potential overhead). I also forgot to copy the exit command from the exec config into the ExecOptions struct used by the OCI runtime, so it was not being added. There are also two significant bugfixes for exec in here. One is for updating the status of running exec sessions - this was always failing as I had coded it to remove the exit file *before* reading it, instead of after (oops). The second was that removing a running exec session would always fail because I inverted the check to see if it was running. Signed-off-by: Matthew Heon --- cmd/podman/containers/cleanup.go | 10 ++++++++++ libpod/container_exec.go | 27 ++++++++++++++++++--------- libpod/runtime_ctr.go | 2 ++ pkg/domain/infra/abi/containers.go | 16 +++++++++++++++- pkg/specgen/generate/container_create.go | 21 +++++++++++++-------- 5 files changed, 58 insertions(+), 18 deletions(-) (limited to 'pkg/domain') diff --git a/cmd/podman/containers/cleanup.go b/cmd/podman/containers/cleanup.go index 257efb971..619031208 100644 --- a/cmd/podman/containers/cleanup.go +++ b/cmd/podman/containers/cleanup.go @@ -8,6 +8,7 @@ import ( "github.com/containers/libpod/cmd/podman/utils" "github.com/containers/libpod/pkg/domain/entities" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -68,6 +69,12 @@ func cleanup(cmd *cobra.Command, args []string) error { responses, err := registry.ContainerEngine().ContainerCleanup(registry.GetContext(), args, cleanupOptions) if err != nil { + // `podman container cleanup` is almost always run in the + // background. Our only way of relaying information to the user + // is via syslog. + // As such, we need to logrus.Errorf our errors to ensure they + // are properly printed if --syslog is set. + logrus.Errorf("Error running container cleanup: %v", err) return err } for _, r := range responses { @@ -76,12 +83,15 @@ func cleanup(cmd *cobra.Command, args []string) error { continue } if r.RmErr != nil { + logrus.Errorf("Error removing container: %v", r.RmErr) errs = append(errs, r.RmErr) } if r.RmiErr != nil { + logrus.Errorf("Error removing image: %v", r.RmiErr) errs = append(errs, r.RmiErr) } if r.CleanErr != nil { + logrus.Errorf("Error cleaning up container: %v", r.CleanErr) errs = append(errs, r.CleanErr) } } diff --git a/libpod/container_exec.go b/libpod/container_exec.go index efbd49b15..f2943b73c 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -65,6 +65,9 @@ type ExecConfig struct { // ExitCommand is the exec session's exit command. // This command will be executed when the exec session exits. // If unset, no command will be executed. + // Two arguments will be appended to the exit command by Libpod: + // 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"` } @@ -195,6 +198,10 @@ func (c *Container) ExecCreate(config *ExecConfig) (string, error) { return "", errors.Wrapf(err, "error copying exec configuration into exec session") } + if len(session.Config.ExitCommand) > 0 { + session.Config.ExitCommand = append(session.Config.ExitCommand, []string{session.ID(), c.ID()}...) + } + if c.state.ExecSessions == nil { c.state.ExecSessions = make(map[string]*ExecSession) } @@ -606,11 +613,11 @@ func (c *Container) ExecRemove(sessionID string, force bool) error { // Update status of exec session if running, so we cna check if it // stopped in the meantime. if session.State == define.ExecStateRunning { - stopped, err := c.ociRuntime.ExecUpdateStatus(c, session.ID()) + running, err := c.ociRuntime.ExecUpdateStatus(c, session.ID()) if err != nil { return err } - if stopped { + if !running { session.State = define.ExecStateStopped // TODO: should we retrieve exit code here? // TODO: Might be worth saving state here. @@ -865,13 +872,6 @@ func (c *Container) getActiveExecSessions() ([]string, error) { continue } if !alive { - if err := c.cleanupExecBundle(id); err != nil { - if lastErr != nil { - logrus.Errorf("Error checking container %s exec sessions: %v", c.ID(), lastErr) - } - lastErr = err - } - _, isLegacy := c.state.LegacyExecSessions[id] if isLegacy { delete(c.state.LegacyExecSessions, id) @@ -891,6 +891,12 @@ func (c *Container) getActiveExecSessions() ([]string, error) { needSave = true } + if err := c.cleanupExecBundle(id); err != nil { + if lastErr != nil { + logrus.Errorf("Error checking container %s exec sessions: %v", c.ID(), lastErr) + } + lastErr = err + } } else { activeSessions = append(activeSessions, id) } @@ -911,6 +917,8 @@ func (c *Container) getActiveExecSessions() ([]string, error) { func (c *Container) removeAllExecSessions() error { knownSessions := c.getKnownExecSessions() + logrus.Debugf("Removing all exec sessions for container %s", c.ID()) + var lastErr error for _, id := range knownSessions { if err := c.ociRuntime.ExecStopContainer(c, id, c.StopTimeout()); err != nil { @@ -975,6 +983,7 @@ func prepareForExec(c *Container, session *ExecSession) (*ExecOptions, error) { opts.User = user opts.PreserveFDs = session.Config.PreserveFDs opts.DetachKeys = session.Config.DetachKeys + opts.ExitCommand = session.Config.ExitCommand return opts, nil } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index c670822a0..655b42e51 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -390,6 +390,8 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, } } + logrus.Debugf("Removing container %s", c.ID()) + // We need to lock the pod before we lock the container. // To avoid races around removing a container and the pod it is in. // Don't need to do this in pod removal case - we're evicting the entire diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index e8eb91644..b4e38ca23 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -607,6 +607,20 @@ func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrId s execConfig := makeExecConfig(options) + // Make an exit command + storageConfig := ic.Libpod.StorageConfig() + runtimeConfig, err := ic.Libpod.GetConfig() + if err != nil { + return "", errors.Wrapf(err, "error retrieving Libpod configuration to build exec exit command") + } + podmanPath, err := os.Executable() + if err != nil { + return "", errors.Wrapf(err, "error retrieving executable to build exec exit command") + } + // 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 id, err := ctr.ExecCreate(execConfig) if err != nil { @@ -615,7 +629,7 @@ func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrId s // TODO: we should try and retrieve exit code if this fails. if err := ctr.ExecStart(id); err != nil { - return "", err + return "", err } return id, nil } diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index f3aaf96bf..ffd7fd4dd 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -111,7 +111,8 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener if err != nil { return nil, err } - options = append(options, createExitCommandOption(s, rt.StorageConfig(), rtc, podmanPath)) + // 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))) runtimeSpec, err := SpecGenToOCI(ctx, s, rt, rtc, newImage, finalMounts) if err != nil { @@ -228,7 +229,7 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. return options, nil } -func createExitCommandOption(s *specgen.SpecGenerator, storageConfig storage.StoreOptions, config *config.Config, podmanPath string) libpod.CtrCreateOption { +func CreateExitCommandArgs(storageConfig storage.StoreOptions, config *config.Config, podmanPath string, syslog, rm bool, exec bool) []string { // 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. @@ -255,14 +256,18 @@ func createExitCommandOption(s *specgen.SpecGenerator, storageConfig storage.Sto command = append(command, []string{"--events-backend", config.Engine.EventsLogger}...) } - // TODO Mheon wants to leave this for now - //if s.sys { - // command = append(command, "--syslog", "true") - //} + if syslog { + command = append(command, "--syslog", "true") + } command = append(command, []string{"container", "cleanup"}...) - if s.Remove { + if rm { command = append(command, "--rm") } - return libpod.WithExitCommand(command) + + if exec { + command = append(command, "--exec") + } + + return command } -- cgit v1.2.3-54-g00ecf