summaryrefslogtreecommitdiff
path: root/libpod
diff options
context:
space:
mode:
authorMatthew Heon <matthew.heon@pm.me>2020-05-14 17:32:44 -0400
committerMatthew Heon <matthew.heon@pm.me>2020-05-14 17:32:44 -0400
commit50ed292aee513667684e695b3d4b061fbac3d696 (patch)
treed45c7241adbfe44a888ab4036fb0a6cb7a228ac8 /libpod
parenta6d9cf9a5e46ff3a8f1f8d2515c12ff6d31056b4 (diff)
downloadpodman-50ed292aee513667684e695b3d4b061fbac3d696.tar.gz
podman-50ed292aee513667684e695b3d4b061fbac3d696.tar.bz2
podman-50ed292aee513667684e695b3d4b061fbac3d696.zip
Remove duplicated exec handling code
During the initial workup of HTTP exec, I duplicated most of the existing exec handling code so I could work on it without breaking normal exec (and compare what I was doing to the nroaml version). Now that it's done and working, we can switch over to the refactored version and ditch the original, removing a lot of duplicated code. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Diffstat (limited to 'libpod')
-rw-r--r--libpod/oci_conmon_linux.go177
1 files changed, 15 insertions, 162 deletions
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 56cfa8ccd..1ffacfb0c 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -636,7 +636,6 @@ func (r *ConmonOCIRuntime) AttachResize(ctr *Container, newSize remotecommand.Te
}
// ExecContainer executes a command in a running container
-// TODO: Split into Create/Start/Attach/Wait
func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams) (int, chan error, error) {
if options == nil {
return -1, nil, errors.Wrapf(define.ErrInvalidArg, "must provide an ExecOptions struct to ExecContainer")
@@ -649,178 +648,46 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
return -1, nil, errors.Wrapf(define.ErrEmptyID, "must provide a session ID for exec")
}
- // create sync pipe to receive the pid
- parentSyncPipe, childSyncPipe, err := newPipe()
- if err != nil {
- return -1, nil, errors.Wrapf(err, "error creating socket pair")
- }
-
- defer errorhandling.CloseQuiet(parentSyncPipe)
-
- // create start pipe to set the cgroup before running
- // attachToExec is responsible for closing parentStartPipe
- childStartPipe, parentStartPipe, err := newPipe()
- if err != nil {
- return -1, nil, errors.Wrapf(err, "error creating socket pair")
- }
-
- // We want to make sure we close the parent{Start,Attach}Pipes if we fail
- // but also don't want to close them after attach to exec is called
- attachToExecCalled := false
-
- defer func() {
- if !attachToExecCalled {
- errorhandling.CloseQuiet(parentStartPipe)
- }
- }()
-
- // create the attach pipe to allow attach socket to be created before
- // $RUNTIME exec starts running. This is to make sure we can capture all output
- // from the process through that socket, rather than half reading the log, half attaching to the socket
- // attachToExec is responsible for closing parentAttachPipe
- parentAttachPipe, childAttachPipe, err := newPipe()
- if err != nil {
- return -1, nil, errors.Wrapf(err, "error creating socket pair")
- }
-
- defer func() {
- if !attachToExecCalled {
- errorhandling.CloseQuiet(parentAttachPipe)
- }
- }()
-
- childrenClosed := false
- defer func() {
- if !childrenClosed {
- errorhandling.CloseQuiet(childSyncPipe)
- errorhandling.CloseQuiet(childAttachPipe)
- errorhandling.CloseQuiet(childStartPipe)
- }
- }()
-
- runtimeDir, err := util.GetRuntimeDir()
- if err != nil {
- return -1, nil, err
- }
-
- finalEnv := make([]string, 0, len(options.Env))
- for k, v := range options.Env {
- finalEnv = append(finalEnv, fmt.Sprintf("%s=%s", k, v))
- }
-
- processFile, err := prepareProcessExec(c, options.Cmd, finalEnv, options.Terminal, options.Cwd, options.User, sessionID)
- if err != nil {
- return -1, nil, err
+ // TODO: Should we default this to false?
+ // Or maybe make streams mandatory?
+ attachStdin := true
+ if streams != nil {
+ attachStdin = streams.AttachInput
}
var ociLog string
if logrus.GetLevel() != logrus.DebugLevel && r.supportsJSON {
ociLog = c.execOCILog(sessionID)
}
- args := r.sharedConmonArgs(c, sessionID, c.execBundlePath(sessionID), c.execPidPath(sessionID), c.execLogPath(sessionID), c.execExitFileDir(sessionID), ociLog, "")
-
- if options.PreserveFDs > 0 {
- args = append(args, formatRuntimeOpts("--preserve-fds", fmt.Sprintf("%d", options.PreserveFDs))...)
- }
-
- for _, capability := range options.CapAdd {
- args = append(args, formatRuntimeOpts("--cap", capability)...)
- }
-
- if options.Terminal {
- args = append(args, "-t")
- }
-
- if streams != nil && streams.AttachInput {
- args = append(args, "-i")
- }
- // Append container ID and command
- args = append(args, "-e")
- // TODO make this optional when we can detach
- args = append(args, "--exec-attach")
- args = append(args, "--exec-process-spec", processFile.Name())
-
- logrus.WithFields(logrus.Fields{
- "args": args,
- }).Debugf("running conmon: %s", r.conmonPath)
- execCmd := exec.Command(r.conmonPath, args...)
-
- if streams != nil {
- // Don't add the InputStream to the execCmd. Instead, the data should be passed
- // through CopyDetachable
- if streams.AttachOutput {
- execCmd.Stdout = options.Streams.OutputStream
- }
- if streams.AttachError {
- execCmd.Stderr = options.Streams.ErrorStream
- }
- }
-
- conmonEnv, extraFiles, err := r.configureConmonEnv(runtimeDir)
+ execCmd, pipes, err := r.startExec(c, sessionID, options, attachStdin, ociLog)
if err != nil {
return -1, nil, err
}
- if options.PreserveFDs > 0 {
- for fd := 3; fd < int(3+options.PreserveFDs); fd++ {
- execCmd.ExtraFiles = append(execCmd.ExtraFiles, os.NewFile(uintptr(fd), fmt.Sprintf("fd-%d", fd)))
- }
- }
-
- // we don't want to step on users fds they asked to preserve
- // Since 0-2 are used for stdio, start the fds we pass in at preserveFDs+3
- execCmd.Env = r.conmonEnv
- execCmd.Env = append(execCmd.Env, fmt.Sprintf("_OCI_SYNCPIPE=%d", options.PreserveFDs+3), fmt.Sprintf("_OCI_STARTPIPE=%d", options.PreserveFDs+4), fmt.Sprintf("_OCI_ATTACHPIPE=%d", options.PreserveFDs+5))
- execCmd.Env = append(execCmd.Env, conmonEnv...)
-
- execCmd.ExtraFiles = append(execCmd.ExtraFiles, childSyncPipe, childStartPipe, childAttachPipe)
- execCmd.ExtraFiles = append(execCmd.ExtraFiles, extraFiles...)
- execCmd.Dir = c.execBundlePath(sessionID)
- execCmd.SysProcAttr = &syscall.SysProcAttr{
- Setpgid: true,
- }
-
- err = startCommandGivenSelinux(execCmd)
-
- // We don't need children pipes on the parent side
- errorhandling.CloseQuiet(childSyncPipe)
- errorhandling.CloseQuiet(childAttachPipe)
- errorhandling.CloseQuiet(childStartPipe)
- childrenClosed = true
-
- if err != nil {
- return -1, nil, errors.Wrapf(err, "cannot start container %s", c.ID())
- }
- if err := r.moveConmonToCgroupAndSignal(c, execCmd, parentStartPipe); err != nil {
- return -1, nil, err
- }
-
- if options.PreserveFDs > 0 {
- for fd := 3; fd < int(3+options.PreserveFDs); fd++ {
- // These fds were passed down to the runtime. Close them
- // and not interfere
- if err := os.NewFile(uintptr(fd), fmt.Sprintf("fd-%d", fd)).Close(); err != nil {
- logrus.Debugf("unable to close file fd-%d", fd)
- }
+ // Only close sync pipe. Start and attach are consumed in the attach
+ // goroutine.
+ defer func() {
+ if pipes.syncPipe != nil && !pipes.syncClosed {
+ errorhandling.CloseQuiet(pipes.syncPipe)
+ pipes.syncClosed = true
}
- }
+ }()
// TODO Only create if !detach
// Attach to the container before starting it
attachChan := make(chan error)
go func() {
// attachToExec is responsible for closing pipes
- attachChan <- c.attachToExec(streams, options.DetachKeys, sessionID, parentStartPipe, parentAttachPipe)
+ attachChan <- c.attachToExec(streams, options.DetachKeys, sessionID, pipes.startPipe, pipes.attachPipe)
close(attachChan)
}()
- attachToExecCalled = true
if err := execCmd.Wait(); err != nil {
return -1, nil, errors.Wrapf(err, "cannot run conmon")
}
- pid, err := readConmonPipeData(parentSyncPipe, ociLog)
+ pid, err := readConmonPipeData(pipes.syncPipe, ociLog)
return pid, attachChan, err
}
@@ -1949,9 +1816,6 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
}
}()
- // TODOTODOTODO
- //defer errorhandling.CloseQuiet(parentSyncPipe)
-
// create start pipe to set the cgroup before running
// attachToExec is responsible for closing parentStartPipe
childStartPipe, parentStartPipe, err := newPipe()
@@ -1960,17 +1824,6 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
}
pipes.startPipe = parentStartPipe
- // We want to make sure we close the parent{Start,Attach}Pipes if we fail
- // but also don't want to close them after attach to exec is called
- //attachToExecCalled := false
-
- // TODOTODOTODO
- // defer func() {
- // if !attachToExecCalled {
- // errorhandling.CloseQuiet(parentStartPipe)
- // }
- // }()
-
// create the attach pipe to allow attach socket to be created before
// $RUNTIME exec starts running. This is to make sure we can capture all output
// from the process through that socket, rather than half reading the log, half attaching to the socket