From e7f4e98c450e2381c70109b308591853f9ac909e Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 18 May 2020 14:37:09 -0400 Subject: Add exit commands to exec sessions These are required for detached exec, where they will be used to clean up and remove exec sessions when they exit. As part of this, move all Exec related functionality for the Conmon OCI runtime into a separate file; the existing one was around 2000 lines. Signed-off-by: Matthew Heon --- libpod/container_exec.go | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'libpod/container_exec.go') diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 6ad767b4b..088bd0fd5 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -62,6 +62,10 @@ type ExecConfig struct { // given is the number that will be passed into the exec session, // starting at 3. PreserveFDs uint `json:"preserveFds,omitempty"` + // 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. + ExitCommand []string `json:"exitCommand,omitempty"` } // ExecSession contains information on a single exec session attached to a given -- cgit v1.2.3-54-g00ecf From 43413887c0d8a20095e454d1046df46a42810e75 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 18 May 2020 16:22:56 -0400 Subject: Add backend code for detached exec As part of the massive exec rework, I stubbed out a function for non-detached exec, which is implemented here. It's largely similar to the existing exec functions, but missing a few pieces. This also involves implemented a new OCI runtime call for detached exec. Again, very similar to the other functions, but with a few missing pieces. Signed-off-by: Matthew Heon --- libpod/container_exec.go | 49 +++++++++++++++++++++++++++++++++++++---- libpod/oci.go | 12 ++++++---- libpod/oci_conmon_exec_linux.go | 43 ++++++++++++++++++++++++++++++++++++ libpod/oci_missing.go | 5 +++++ 4 files changed, 101 insertions(+), 8 deletions(-) (limited to 'libpod/container_exec.go') diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 088bd0fd5..a38f22488 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -214,11 +214,52 @@ func (c *Container) ExecCreate(config *ExecConfig) (string, error) { } // ExecStart starts an exec session in the container, but does not attach to it. -// Returns immediately upon starting the exec session. +// Returns immediately upon starting the exec session, unlike other ExecStart +// functions, which will only return when the exec session exits. func (c *Container) ExecStart(sessionID string) error { - // Will be implemented in part 2, migrating Start and implementing - // detached Start. - return define.ErrNotImplemented + if !c.batched { + c.lock.Lock() + defer c.lock.Unlock() + + if err := c.syncContainer(); err != nil { + return err + } + } + + // Verify that we are in a good state to continue + if !c.ensureState(define.ContainerStateRunning) { + return errors.Wrapf(define.ErrCtrStateInvalid, "can only start exec sessions when their container is running") + } + + session, ok := c.state.ExecSessions[sessionID] + if !ok { + return errors.Wrapf(define.ErrNoSuchExecSession, "container %s has no exec session with ID %s", c.ID(), sessionID) + } + + if session.State != define.ExecStateCreated { + return errors.Wrapf(define.ErrExecSessionStateInvalid, "can only start created exec sessions, while container %s session %s state is %q", c.ID(), session.ID(), session.State.String()) + } + + logrus.Infof("Going to start container %s exec session %s and attach to it", c.ID(), session.ID()) + + opts, err := prepareForExec(c, session) + if err != nil { + return err + } + + pid, err := c.ociRuntime.ExecContainerDetached(c, session.ID(), opts, session.Config.AttachStdin) + if err != nil { + return err + } + + c.newContainerEvent(events.Exec) + logrus.Debugf("Successfully started exec session %s in container %s", session.ID(), c.ID()) + + // Update and save session to reflect PID/running + session.PID = pid + session.State = define.ExecStateRunning + + return c.save() } // ExecStartAndAttach starts and attaches to an exec session in a container. diff --git a/libpod/oci.go b/libpod/oci.go index da4030c14..7c5218319 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -68,10 +68,10 @@ type OCIRuntime interface { AttachResize(ctr *Container, newSize remotecommand.TerminalSize) error // ExecContainer executes a command in a running container. - // Returns an int (exit code), error channel (errors from attach), and - // error (errors that occurred attempting to start the exec session). - // This returns once the exec session is running - not once it has - // completed, as one might expect. The attach session will remain + // Returns an int (PID of exec session), error channel (errors from + // attach), and error (errors that occurred attempting to start the exec + // session). This returns once the exec session is running - not once it + // has completed, as one might expect. The attach session will remain // running, in a goroutine that will return via the chan error in the // return signature. ExecContainer(ctr *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams) (int, chan error, error) @@ -81,6 +81,10 @@ type OCIRuntime interface { // start, with a goroutine running in the background to handle attach). // The HTTP attach itself maintains the same invariants as HTTPAttach. ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, httpConn net.Conn, httpBuf *bufio.ReadWriter, streams *HTTPAttachStreams, cancel <-chan bool) (int, chan error, error) + // ExecContainerDetached executes a command in a running container, but + // does not attach to it. Returns the PID of the exec session and an + // error (if starting the exec session failed) + ExecContainerDetached(ctr *Container, sessionID string, options *ExecOptions, stdin bool) (int, error) // ExecAttachResize resizes the terminal of a running exec session. Only // allowed with sessions that were created with a TTY. ExecAttachResize(ctr *Container, sessionID string, newSize remotecommand.TerminalSize) error diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 3fc866796..51819f90a 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -143,6 +143,49 @@ func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, o return pid, attachChan, err } +// ExecContainerDetached executes a command in a running container, but does +// not attach to it. +func (r *ConmonOCIRuntime) ExecContainerDetached(ctr *Container, sessionID string, options *ExecOptions, stdin bool) (int, error) { + if options == nil { + return -1, errors.Wrapf(define.ErrInvalidArg, "must provide exec options to ExecContainerHTTP") + } + + var ociLog string + if logrus.GetLevel() != logrus.DebugLevel && r.supportsJSON { + ociLog = ctr.execOCILog(sessionID) + } + + execCmd, pipes, err := r.startExec(ctr, sessionID, options, stdin, ociLog) + if err != nil { + return -1, err + } + + defer func() { + pipes.cleanup() + }() + + // Wait for Conmon to tell us we're ready to attach. + // We aren't actually *going* to attach, but this means that we're good + // to proceed. + if _, err := readConmonPipeData(pipes.attachPipe, ""); err != nil { + return -1, err + } + + // Start the exec session + if err := writeConmonPipeData(pipes.startPipe); err != nil { + return -1, err + } + + // Wait for conmon to succeed, when return. + if err := execCmd.Wait(); err != nil { + return -1, errors.Wrapf(err, "cannot run conmon") + } + + pid, err := readConmonPipeData(pipes.syncPipe, ociLog) + + return pid, err +} + // ExecAttachResize resizes the TTY of the given exec session. func (r *ConmonOCIRuntime) ExecAttachResize(ctr *Container, sessionID string, newSize remotecommand.TerminalSize) error { controlFile, err := openControlFile(ctr, ctr.execBundlePath(sessionID)) diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index 626740f72..4da16876c 100644 --- a/libpod/oci_missing.go +++ b/libpod/oci_missing.go @@ -130,6 +130,11 @@ func (r *MissingRuntime) ExecContainerHTTP(ctr *Container, sessionID string, opt return -1, nil, r.printError() } +// ExecContainerDetached is not available as the runtime is missing +func (r *MissingRuntime) ExecContainerDetached(ctr *Container, sessionID string, options *ExecOptions, stdin bool) (int, error) { + return -1, r.printError() +} + // ExecAttachResize is not available as the runtime is missing. func (r *MissingRuntime) ExecAttachResize(ctr *Container, sessionID string, newSize remotecommand.TerminalSize) error { return r.printError() -- 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 'libpod/container_exec.go') 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 'libpod/container_exec.go') 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