Add tracking for exec session IDs

Exec sessions now have an ID generated and assigned to their PID
and stored in the database state. This allows us to track what
exec sessions are currently active.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #412
Approved by: baude

5 files changed, 135 insertions, 8 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index b7e523d7a..c1c1689df 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -5,8 +5,11 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
+	"time"
 
 	"github.com/docker/docker/daemon/caps"
+	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/term"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod/driver"
@@ -235,7 +238,86 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
 		capList = caps.GetAllCapabilities()
 	}
 
-	return c.runtime.ociRuntime.execContainer(c, cmd, tty, user, capList, env)
+	// Generate exec session ID
+	// Ensure we don't conflict with an existing session ID
+	sessionID := stringid.GenerateNonCryptoID()
+	found := true
+	// This really ought to be a do-while, but Go doesn't have those...
+	for found {
+		found = false
+		for id, _ := range c.state.ExecSessions {
+			if id == sessionID {
+				found = true
+				break
+			}
+		}
+		if found == true {
+			sessionID = stringid.GenerateNonCryptoID()
+		}
+	}
+
+	execCmd, err := c.runtime.ociRuntime.execContainer(c, cmd, capList, env, tty, user, sessionID)
+	if err != nil {
+		return errors.Wrapf(err, "error creating exec command for container %s", c.ID())
+	}
+
+	if err := execCmd.Start(); err != nil {
+		return errors.Wrapf(err, "error starting exec command for container %s", c.ID())
+	}
+
+	pidFile := c.execPidPath(sessionID)
+	const pidWaitTimeout = 250
+
+	// Wait until runc makes the pidfile
+	// TODO: If runc errors before the PID file is created, we have to wait for timeout here
+	if err := WaitForFile(pidFile, pidWaitTimeout * time.Millisecond); err != nil {
+		logrus.Debugf("Timed out waiting for pidfile from runc for container %s exec", c.ID())
+
+		// Check if an error occurred in the process before we made a pidfile
+		// TODO: Wait() here is a poor choice - is there a way to see if
+		// a process has finished, instead of waiting for it to finish?
+		if err := execCmd.Wait(); err != nil {
+			return err
+		}
+
+		return errors.Wrapf(err, "timed out waiting for runc to create pidfile for exec session in container %s", c.ID())
+	}
+
+	// Pidfile exists, read it
+	contents, err := ioutil.ReadFile(pidFile)
+	if err != nil {
+		// We don't know the PID of the exec session
+		// However, it may still be alive
+		// TODO handle this better
+		return errors.Wrapf(err, "could not read pidfile for exec session %s in container %s", sessionID, c.ID())
+	}
+	pid, err := strconv.ParseInt(string(contents), 10, 32)
+	if err != nil {
+		// As above, we don't have a valid PID, but the exec session is likely still alive
+		// TODO handle this better
+		return errors.Wrapf(err, "error parsing PID of exec session %s in container %s", sessionID, c.ID())
+	}
+
+	// We have the PID, add it to state
+	if c.state.ExecSessions == nil {
+		c.state.ExecSessions = make(map[string]int)
+	}
+	c.state.ExecSessions[sessionID] = int(pid)
+	if err := c.save(); err != nil {
+		// Now we have a PID but we can't save it in the DB
+		// TODO handle this better
+		return errors.Wrapf(err, "error saving exec sessions %s for container %s", sessionID, c.ID())
+	}
+
+	waitErr := execCmd.Wait()
+
+	// Remove the exec session from state
+	delete(c.state.ExecSessions, sessionID)
+	if err := c.save(); err != nil {
+		logrus.Errorf("Error removing exec session %s from container %s state: %v", sessionID, c.ID(), err)
+	}
+
+	return waitErr
 }
 
 // Attach attaches to a container
diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go
index 124126708..de5770f8f 100644
--- a/libpod/container_inspect.go
+++ b/libpod/container_inspect.go
@@ -20,6 +20,11 @@ func (c *Container) getContainerInspectData(size bool, driverData *inspect.Data)
 		args = args[1:]
 	}
 
+	execIDs := []string{}
+	for id, _ := range c.state.ExecSessions {
+		execIDs = append(execIDs, id)
+	}
+
 	data := &inspect.ContainerInspectData{
 		ID:      config.ID,
 		Created: config.CreatedTime,
@@ -50,7 +55,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *inspect.Data)
 		MountLabel:      config.MountLabel,
 		ProcessLabel:    spec.Process.SelinuxLabel,
 		AppArmorProfile: spec.Process.ApparmorProfile,
-		ExecIDs:         []string{}, //TODO
+		ExecIDs:         execIDs,
 		GraphDriver:     driverData,
 		Mounts:          spec.Mounts,
 		NetworkSettings: &inspect.NetworkSettings{
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 37a70a152..61f3410c9 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -100,6 +100,11 @@ func (c *Container) attachSocketPath() string {
 	return filepath.Join(c.runtime.ociRuntime.socketsDir, c.ID(), "attach")
 }
 
+// Get PID file path for a container's exec session
+func (c *Container) execPidPath(sessionID string) string {
+	return filepath.Join(c.state.RunDir, "exec_pid_" + sessionID)
+}
+
 // Sync this container with on-disk state and runc status
 // Should only be called with container lock held
 // This function should suffice to ensure a container's state is accurate and
diff --git a/libpod/oci.go b/libpod/oci.go
index 225643f39..f25c6bdb9 100644
--- a/libpod/oci.go
+++ b/libpod/oci.go
@@ -471,7 +471,15 @@ func (r *OCIRuntime) unpauseContainer(ctr *Container) error {
 // TODO: Add --detach support
 // TODO: Convert to use conmon
 // TODO: add --pid-file and use that to generate exec session tracking
-func (r *OCIRuntime) execContainer(c *Container, cmd []string, tty bool, user string, capAdd, env []string) error {
+func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty bool, user, sessionID string) (*exec.Cmd, error) {
+	if len(cmd) == 0 {
+		return nil, errors.Wrapf(ErrInvalidArg, "must provide a command to execute")
+	}
+
+	if sessionID == "" {
+		return nil, errors.Wrapf(ErrEmptyID, "must provide a session ID for exec")
+	}
+
 	args := []string{}
 
 	// TODO - should we maintain separate logpaths for exec sessions?
@@ -481,6 +489,8 @@ func (r *OCIRuntime) execContainer(c *Container, cmd []string, tty bool, user st
 
 	args = append(args, "--cwd", c.config.Spec.Process.Cwd)
 
+	args = append(args, "--pid-file", c.execPidPath(sessionID))
+
 	if tty {
 		args = append(args, "--tty")
 	}
@@ -512,9 +522,5 @@ func (r *OCIRuntime) execContainer(c *Container, cmd []string, tty bool, user st
 	execCmd.Stderr = os.Stderr
 	execCmd.Stdin = os.Stdin
 
-	if err := execCmd.Start(); err != nil {
-		return errors.Wrapf(err, "error starting exec command for container %s", c.ID())
-	}
-
-	return execCmd.Wait()
+	return execCmd, nil
 }
diff --git a/libpod/util.go b/libpod/util.go
index 0c6700fbf..ca93fc097 100644
--- a/libpod/util.go
+++ b/libpod/util.go
@@ -107,3 +107,32 @@ func MountExists(specMounts []spec.Mount, dest string) bool {
 	}
 	return false
 }
+
+// WaitForFile waits until a file has been created or the given timeout has occurred
+func WaitForFile(path string, timeout time.Duration) error {
+	done := make(chan struct{})
+	chControl := make(chan struct{})
+	go func() {
+		for {
+			select {
+			case <-chControl:
+				return
+			default:
+				_, err := os.Stat(path)
+				if err == nil {
+					close(done)
+					return
+				}
+				time.Sleep(25 * time.Millisecond)
+			}
+		}
+	}()
+
+	select {
+	case <-done:
+		return nil
+	case <-time.After(timeout):
+		close(chControl)
+		return errors.Wrapf(ErrInternal, "timed out waiting for file %s", path)
+	}
+}