runsc: Add exec flag that specifies where to save the sandbox-internal pid.

This is different from the existing -pid-file flag, which saves a host pid.

PiperOrigin-RevId: 212713968
Change-Id: I2c486de8dd5cfd9b923fb0970165ef7c5fc597f0

5 files changed, 107 insertions, 69 deletions

diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go
index 257f275f9..aaac852e0 100644
--- a/runsc/boot/controller.go
+++ b/runsc/boot/controller.go
@@ -41,9 +41,9 @@ const (
 	// container used by "runsc events".
 	ContainerEvent = "containerManager.Event"
 
-	// ContainerExecute is the URPC endpoint for executing a command in a
+	// ContainerExecuteAsync is the URPC endpoint for executing a command in a
 	// container..
-	ContainerExecute = "containerManager.Execute"
+	ContainerExecuteAsync = "containerManager.ExecuteAsync"
 
 	// ContainerPause pauses the container.
 	ContainerPause = "containerManager.Pause"
@@ -233,33 +233,40 @@ type ExecArgs struct {
 	CID string
 }
 
-// Execute runs a command on a created or running sandbox.
-func (cm *containerManager) Execute(e *ExecArgs, waitStatus *uint32) error {
-	log.Debugf("containerManager.Execute: %+v", *e)
+// ExecuteAsync starts running a command on a created or running sandbox. It
+// returns the pid of the new process.
+func (cm *containerManager) ExecuteAsync(args *ExecArgs, pid *int32) error {
+	log.Debugf("containerManager.ExecuteAsync: %+v", args)
 
 	// Get the container Root Dirent from the Task, since we must run this
 	// process with the same Root.
 	cm.l.mu.Lock()
-	tgid, ok := cm.l.containerRootTGIDs[e.CID]
+	tgid, ok := cm.l.containerRootTGIDs[args.CID]
 	cm.l.mu.Unlock()
 	if !ok {
-		return fmt.Errorf("cannot exec in container %q: no such container", e.CID)
+		return fmt.Errorf("cannot exec in container %q: no such container", args.CID)
 	}
 	t := cm.l.k.TaskSet().Root.TaskWithID(kernel.ThreadID(tgid))
 	if t == nil {
-		return fmt.Errorf("cannot exec in container %q: no thread group with ID %d", e.CID, tgid)
+		return fmt.Errorf("cannot exec in container %q: no thread group with ID %d", args.CID, tgid)
 	}
 	t.WithMuLocked(func(t *kernel.Task) {
-		e.Root = t.FSContext().RootDirectory()
+		args.Root = t.FSContext().RootDirectory()
 	})
-	if e.Root != nil {
-		defer e.Root.DecRef()
+	if args.Root != nil {
+		defer args.Root.DecRef()
 	}
 
+	// Start the process.
 	proc := control.Proc{Kernel: cm.l.k}
-	if err := proc.Exec(&e.ExecArgs, waitStatus); err != nil {
-		return fmt.Errorf("error executing: %+v: %v", e, err)
+	newTG, err := control.ExecAsync(&proc, &args.ExecArgs)
+	if err != nil {
+		return fmt.Errorf("error executing: %+v: %v", args, err)
 	}
+
+	// Return the pid of the newly-created process.
+	ts := cm.l.k.TaskSet()
+	*pid = int32(ts.Root.IDOfThreadGroup(newTG))
 	return nil
 }
 
diff --git a/runsc/cmd/exec.go b/runsc/cmd/exec.go
index da1642c08..0d1fa6e20 100644
--- a/runsc/cmd/exec.go
+++ b/runsc/cmd/exec.go
@@ -45,12 +45,13 @@ type Exec struct {
 	cwd string
 	env stringSlice
 	// user contains the UID and GID with which to run the new process.
-	user        user
-	extraKGIDs  stringSlice
-	caps        stringSlice
-	detach      bool
-	processPath string
-	pidFile     string
+	user            user
+	extraKGIDs      stringSlice
+	caps            stringSlice
+	detach          bool
+	processPath     string
+	pidFile         string
+	internalPidFile string
 
 	// consoleSocket is the path to an AF_UNIX socket which will receive a
 	// file descriptor referencing the master end of the console's
@@ -97,6 +98,7 @@ func (ex *Exec) SetFlags(f *flag.FlagSet) {
 	f.BoolVar(&ex.detach, "detach", false, "detach from the container's process")
 	f.StringVar(&ex.processPath, "process", "", "path to the process.json")
 	f.StringVar(&ex.pidFile, "pid-file", "", "filename that the container pid will be written to")
+	f.StringVar(&ex.internalPidFile, "internal-pid-file", "", "filename that the container-internal pid will be written to")
 	f.StringVar(&ex.consoleSocket, "console-socket", "", "path to an AF_UNIX socket which will receive a file descriptor referencing the master end of the console's pseudoterminal")
 }
 
@@ -146,10 +148,25 @@ func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 		}
 	}
 
-	ws, err := c.Execute(e)
+	// Start the new process and get it pid.
+	pid, err := c.Execute(e)
 	if err != nil {
 		Fatalf("error getting processes for container: %v", err)
 	}
+
+	// Write the sandbox-internal pid if required.
+	if ex.internalPidFile != "" {
+		pidStr := []byte(strconv.Itoa(int(pid)))
+		if err := ioutil.WriteFile(ex.internalPidFile, pidStr, 0644); err != nil {
+			Fatalf("error writing internal pid file %q: %v", ex.internalPidFile, err)
+		}
+	}
+
+	// Wait for the process to exit.
+	ws, err := c.WaitPID(pid)
+	if err != nil {
+		Fatalf("error waiting on pid %d: %v", pid, err)
+	}
 	*waitStatus = ws
 	return subcommands.ExitSuccess
 }
diff --git a/runsc/container/container.go b/runsc/container/container.go
index 9a05a1dc5..38848d02f 100644
--- a/runsc/container/container.go
+++ b/runsc/container/container.go
@@ -353,13 +353,14 @@ func Run(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSocke
 	return c.Wait()
 }
 
-// Execute runs the specified command in the container.
-func (c *Container) Execute(e *control.ExecArgs) (syscall.WaitStatus, error) {
-	log.Debugf("Execute in container %q, args: %+v", c.ID, e)
+// Execute runs the specified command in the container. It returns the pid of
+// the newly created process.
+func (c *Container) Execute(args *control.ExecArgs) (int32, error) {
+	log.Debugf("Execute in container %q, args: %+v", c.ID, args)
 	if c.Status != Created && c.Status != Running {
 		return 0, fmt.Errorf("cannot exec in container in state %s", c.Status)
 	}
-	return c.Sandbox.Execute(c.ID, e)
+	return c.Sandbox.Execute(c.ID, args)
 }
 
 // Event returns events for the container.
diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go
index c45eb79a3..790334249 100644
--- a/runsc/container/container_test.go
+++ b/runsc/container/container_test.go
@@ -49,11 +49,11 @@ func init() {
 }
 
 // waitForProcessList waits for the given process list to show up in the container.
-func waitForProcessList(s *Container, expected []*control.Process) error {
+func waitForProcessList(cont *Container, expected []*control.Process) error {
 	var got []*control.Process
 	for start := time.Now(); time.Now().Sub(start) < 10*time.Second; {
 		var err error
-		got, err = s.Processes()
+		got, err = cont.Processes()
 		if err != nil {
 			return fmt.Errorf("error getting process data from container: %v", err)
 		}
@@ -485,12 +485,12 @@ func TestExec(t *testing.T) {
 		defer os.RemoveAll(bundleDir)
 
 		// Create and start the container.
-		s, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "")
+		cont, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "")
 		if err != nil {
 			t.Fatalf("error creating container: %v", err)
 		}
-		defer s.Destroy()
-		if err := s.Start(conf); err != nil {
+		defer cont.Destroy()
+		if err := cont.Start(conf); err != nil {
 			t.Fatalf("error starting container: %v", err)
 		}
 
@@ -513,11 +513,11 @@ func TestExec(t *testing.T) {
 		}
 
 		// Verify that "sleep 100" is running.
-		if err := waitForProcessList(s, expectedPL[:1]); err != nil {
+		if err := waitForProcessList(cont, expectedPL[:1]); err != nil {
 			t.Error(err)
 		}
 
-		execArgs := control.ExecArgs{
+		args := &control.ExecArgs{
 			Filename:         "/bin/sleep",
 			Argv:             []string{"sleep", "5"},
 			WorkingDirectory: "/",
@@ -528,17 +528,19 @@ func TestExec(t *testing.T) {
 		// First, start running exec (whick blocks).
 		status := make(chan error, 1)
 		go func() {
-			exitStatus, err := s.Execute(&execArgs)
+			exitStatus, err := cont.executeSync(args)
 			if err != nil {
+				log.Debugf("error executing: %v", err)
 				status <- err
 			} else if exitStatus != 0 {
+				log.Debugf("bad status: %d", exitStatus)
 				status <- fmt.Errorf("failed with exit status: %v", exitStatus)
 			} else {
 				status <- nil
 			}
 		}()
 
-		if err := waitForProcessList(s, expectedPL); err != nil {
+		if err := waitForProcessList(cont, expectedPL); err != nil {
 			t.Fatal(err)
 		}
 
@@ -548,7 +550,7 @@ func TestExec(t *testing.T) {
 			t.Fatalf("container timed out waiting for exec to finish.")
 		case st := <-status:
 			if st != nil {
-				t.Errorf("container failed to exec %v: %v", execArgs, err)
+				t.Errorf("container failed to exec %v: %v", args, err)
 			}
 		}
 	}
@@ -884,15 +886,18 @@ func TestPauseResume(t *testing.T) {
 		}
 
 		script := fmt.Sprintf("while [[ -f %q ]]; do sleep 0.1; done", lock.Name())
-		execArgs := control.ExecArgs{
+		args := &control.ExecArgs{
 			Filename:         "/bin/bash",
 			Argv:             []string{"bash", "-c", script},
 			WorkingDirectory: "/",
 			KUID:             uid,
 		}
 
-		// First, start running exec (which blocks).
-		go cont.Execute(&execArgs)
+		// First, start running exec.
+		_, err = cont.Execute(args)
+		if err != nil {
+			t.Fatalf("error executing: %v", err)
+		}
 
 		// Verify that "sleep 5" is running.
 		if err := waitForProcessList(cont, expectedPL); err != nil {
@@ -1022,12 +1027,12 @@ func TestCapabilities(t *testing.T) {
 		defer os.RemoveAll(bundleDir)
 
 		// Create and start the container.
-		s, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "")
+		cont, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "")
 		if err != nil {
 			t.Fatalf("error creating container: %v", err)
 		}
-		defer s.Destroy()
-		if err := s.Start(conf); err != nil {
+		defer cont.Destroy()
+		if err := cont.Start(conf); err != nil {
 			t.Fatalf("error starting container: %v", err)
 		}
 
@@ -1048,7 +1053,7 @@ func TestCapabilities(t *testing.T) {
 				Cmd:  "exe",
 			},
 		}
-		if err := waitForProcessList(s, expectedPL[:1]); err != nil {
+		if err := waitForProcessList(cont, expectedPL[:1]); err != nil {
 			t.Fatalf("Failed to wait for sleep to start, err: %v", err)
 		}
 
@@ -1064,7 +1069,7 @@ func TestCapabilities(t *testing.T) {
 		// Need to traverse the intermediate directory.
 		os.Chmod(rootDir, 0755)
 
-		execArgs := control.ExecArgs{
+		args := &control.ExecArgs{
 			Filename:         exePath,
 			Argv:             []string{exePath},
 			WorkingDirectory: "/",
@@ -1074,17 +1079,17 @@ func TestCapabilities(t *testing.T) {
 		}
 
 		// "exe" should fail because we don't have the necessary permissions.
-		if _, err := s.Execute(&execArgs); err == nil {
+		if _, err := cont.executeSync(args); err == nil {
 			t.Fatalf("container executed without error, but an error was expected")
 		}
 
 		// Now we run with the capability enabled and should succeed.
-		execArgs.Capabilities = &auth.TaskCapabilities{
+		args.Capabilities = &auth.TaskCapabilities{
 			EffectiveCaps: auth.CapabilitySetOf(linux.CAP_DAC_OVERRIDE),
 		}
 		// "exe" should not fail this time.
-		if _, err := s.Execute(&execArgs); err != nil {
-			t.Fatalf("container failed to exec %v: %v", execArgs, err)
+		if _, err := cont.executeSync(args); err != nil {
+			t.Fatalf("container failed to exec %v: %v", args, err)
 		}
 	}
 }
@@ -1404,11 +1409,11 @@ func TestContainerVolumeContentsShared(t *testing.T) {
 	filename := filepath.Join(dir, "file")
 
 	// File does not exist yet. Reading from the sandbox should fail.
-	execArgsTestFile := control.ExecArgs{
+	argsTestFile := &control.ExecArgs{
 		Filename: "/usr/bin/test",
 		Argv:     []string{"test", "-f", filename},
 	}
-	if ws, err := c.Execute(&execArgsTestFile); err != nil {
+	if ws, err := c.executeSync(argsTestFile); err != nil {
 		t.Fatalf("unexpected error testing file %q: %v", filename, err)
 	} else if ws.ExitStatus() == 0 {
 		t.Errorf("test %q exited with code %v, wanted not zero", ws.ExitStatus(), err)
@@ -1420,7 +1425,7 @@ func TestContainerVolumeContentsShared(t *testing.T) {
 	}
 
 	// Now we should be able to test the file from within the sandbox.
-	if ws, err := c.Execute(&execArgsTestFile); err != nil {
+	if ws, err := c.executeSync(argsTestFile); err != nil {
 		t.Fatalf("unexpected error testing file %q: %v", filename, err)
 	} else if ws.ExitStatus() != 0 {
 		t.Errorf("test %q exited with code %v, wanted zero", filename, ws.ExitStatus())
@@ -1433,18 +1438,18 @@ func TestContainerVolumeContentsShared(t *testing.T) {
 	}
 
 	// File should no longer exist at the old path within the sandbox.
-	if ws, err := c.Execute(&execArgsTestFile); err != nil {
+	if ws, err := c.executeSync(argsTestFile); err != nil {
 		t.Fatalf("unexpected error testing file %q: %v", filename, err)
 	} else if ws.ExitStatus() == 0 {
 		t.Errorf("test %q exited with code %v, wanted not zero", filename, ws.ExitStatus())
 	}
 
 	// We should be able to test the new filename from within the sandbox.
-	execArgsTestNewFile := control.ExecArgs{
+	argsTestNewFile := &control.ExecArgs{
 		Filename: "/usr/bin/test",
 		Argv:     []string{"test", "-f", newFilename},
 	}
-	if ws, err := c.Execute(&execArgsTestNewFile); err != nil {
+	if ws, err := c.executeSync(argsTestNewFile); err != nil {
 		t.Fatalf("unexpected error testing file %q: %v", newFilename, err)
 	} else if ws.ExitStatus() != 0 {
 		t.Errorf("test %q exited with code %v, wanted zero", newFilename, ws.ExitStatus())
@@ -1456,20 +1461,20 @@ func TestContainerVolumeContentsShared(t *testing.T) {
 	}
 
 	// Renamed file should no longer exist at the old path within the sandbox.
-	if ws, err := c.Execute(&execArgsTestNewFile); err != nil {
+	if ws, err := c.executeSync(argsTestNewFile); err != nil {
 		t.Fatalf("unexpected error testing file %q: %v", newFilename, err)
 	} else if ws.ExitStatus() == 0 {
 		t.Errorf("test %q exited with code %v, wanted not zero", newFilename, ws.ExitStatus())
 	}
 
 	// Now create the file from WITHIN the sandbox.
-	execArgsTouch := control.ExecArgs{
+	argsTouch := &control.ExecArgs{
 		Filename: "/usr/bin/touch",
 		Argv:     []string{"touch", filename},
 		KUID:     auth.KUID(os.Getuid()),
 		KGID:     auth.KGID(os.Getgid()),
 	}
-	if ws, err := c.Execute(&execArgsTouch); err != nil {
+	if ws, err := c.executeSync(argsTouch); err != nil {
 		t.Fatalf("unexpected error touching file %q: %v", filename, err)
 	} else if ws.ExitStatus() != 0 {
 		t.Errorf("touch %q exited with code %v, wanted zero", filename, ws.ExitStatus())
@@ -1486,11 +1491,11 @@ func TestContainerVolumeContentsShared(t *testing.T) {
 	}
 
 	// Delete the file from within the sandbox.
-	execArgsRemove := control.ExecArgs{
+	argsRemove := &control.ExecArgs{
 		Filename: "/bin/rm",
 		Argv:     []string{"rm", filename},
 	}
-	if ws, err := c.Execute(&execArgsRemove); err != nil {
+	if ws, err := c.executeSync(argsRemove); err != nil {
 		t.Fatalf("unexpected error removing file %q: %v", filename, err)
 	} else if ws.ExitStatus() != 0 {
 		t.Errorf("remove %q exited with code %v, wanted zero", filename, ws.ExitStatus())
@@ -1547,6 +1552,19 @@ func TestGoferExits(t *testing.T) {
 	}
 }
 
+// executeSync synchronously executes a new process.
+func (cont *Container) executeSync(args *control.ExecArgs) (syscall.WaitStatus, error) {
+	pid, err := cont.Execute(args)
+	if err != nil {
+		return 0, fmt.Errorf("error executing: %v", err)
+	}
+	ws, err := cont.WaitPID(pid)
+	if err != nil {
+		return 0, fmt.Errorf("error waiting: %v", err)
+	}
+	return ws, nil
+}
+
 func TestMain(m *testing.M) {
 	testutil.RunAsRoot(m)
 }
diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go
index 195deda1e..8e90dcc70 100644
--- a/runsc/sandbox/sandbox.go
+++ b/runsc/sandbox/sandbox.go
@@ -187,8 +187,9 @@ func (s *Sandbox) Processes(cid string) ([]*control.Process, error) {
 	return pl, nil
 }
 
-// Execute runs the specified command in the container.
-func (s *Sandbox) Execute(cid string, e *control.ExecArgs) (syscall.WaitStatus, error) {
+// Execute runs the specified command in the container. It returns the pid of
+// the newly created process.
+func (s *Sandbox) Execute(cid string, args *control.ExecArgs) (int32, error) {
 	log.Debugf("Executing new process in container %q in sandbox %q", cid, s.ID)
 	conn, err := s.sandboxConnect()
 	if err != nil {
@@ -196,20 +197,14 @@ func (s *Sandbox) Execute(cid string, e *control.ExecArgs) (syscall.WaitStatus,
 	}
 	defer conn.Close()
 
-	ea := &boot.ExecArgs{
-		ExecArgs: *e,
-		CID:      cid,
-	}
+	rpcArgs := &boot.ExecArgs{ExecArgs: *args, CID: cid}
 
 	// Send a message to the sandbox control server to start the container.
-	var waitStatus uint32
-	// TODO: Pass in the container id (cid) here. The sandbox
-	// should execute in the context of that container.
-	if err := conn.Call(boot.ContainerExecute, ea, &waitStatus); err != nil {
+	var pid int32
+	if err := conn.Call(boot.ContainerExecuteAsync, rpcArgs, &pid); err != nil {
 		return 0, fmt.Errorf("error executing in sandbox: %v", err)
 	}
-
-	return syscall.WaitStatus(waitStatus), nil
+	return pid, nil
 }
 
 // Event retrieves stats about the sandbox such as memory and CPU utilization.