Initial change for multi-gofer support

PiperOrigin-RevId: 209647293
Change-Id: I980fca1257ea3fcce796388a049c353b0303a8a5

runsc/cmd/debug.go

@@ -92,7 +92,7 @@ func (d *Debug) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 	}
 
 	log.Infof("Found sandbox %q, PID: %d", c.Sandbox.ID, c.Sandbox.Pid)
-	if !c.Sandbox.IsRunning() {
+	if !c.IsRunning() {
 		Fatalf("sandbox %q is not running", c.Sandbox.ID)
 	}
 

runsc/container/BUILD

@@ -23,10 +23,10 @@ go_library(
     deps = [
         "//pkg/log",
         "//pkg/sentry/control",
-        "//pkg/syserror",
         "//runsc/boot",
         "//runsc/sandbox",
         "//runsc/specutils",
+        "@com_github_cenkalti_backoff//:go_default_library",
         "@com_github_opencontainers_runtime-spec//specs-go:go_default_library",
     ],
 )

runsc/container/container.go

@@ -16,6 +16,7 @@
 package container
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -27,10 +28,10 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/cenkalti/backoff"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"gvisor.googlesource.com/gvisor/pkg/log"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/control"
-	"gvisor.googlesource.com/gvisor/pkg/syserror"
 	"gvisor.googlesource.com/gvisor/runsc/boot"
 	"gvisor.googlesource.com/gvisor/runsc/sandbox"
 	"gvisor.googlesource.com/gvisor/runsc/specutils"
@@ -89,6 +90,10 @@ type Container struct {
 	// Status is the current container Status.
 	Status Status `json:"status"`
 
+	// GoferPid is the pid of the gofer running along side the sandbox. May
+	// be 0 if the gofer has been killed or it's not being used.
+	GoferPid int `json:"goferPid"`
+
 	// Sandbox is the sandbox this container is running in. It will be nil
 	// if the container is not in state Running or Created.
 	Sandbox *sandbox.Sandbox `json:"sandbox"`
@@ -130,12 +135,11 @@ func Load(rootDir, id string) (*Container, error) {
 	// This is inherently racey.
 	if c.Status == Running || c.Status == Created {
 		// Check if the sandbox process is still running.
-		if c.Sandbox.IsRunning() {
+		if c.IsRunning() {
 			// TODO: Send a message into the sandbox to
 			// see if this particular container is still running.
 		} else {
-			// Sandbox no longer exists, so this container
-			// definitely does not exist.
+			// Sandbox no longer exists, so this container definitely does not exist.
 			c.Status = Stopped
 			c.Sandbox = nil
 		}
@@ -221,12 +225,13 @@ func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSo
 		log.Debugf("Creating new sandbox for container %q", id)
 		// Start a new sandbox for this container. Any errors after this point
 		// must destroy the container.
-		s, err := sandbox.Create(id, spec, conf, bundleDir, consoleSocket)
+		s, goferPid, err := sandbox.Create(id, spec, conf, bundleDir, consoleSocket)
 		if err != nil {
 			c.Destroy()
 			return nil, err
 		}
 		c.Sandbox = s
+		c.GoferPid = goferPid
 	} else {
 		// This is sort of confusing. For a sandbox with a root
 		// container and a child container in it, runsc sees:
@@ -398,7 +403,18 @@ func (c *Container) WaitPID(pid int32) (syscall.WaitStatus, error) {
 	if c.Status == Stopped {
 		return 0, fmt.Errorf("container is stopped")
 	}
-	return c.Sandbox.WaitPID(pid, c.ID)
+	ws, err := c.Sandbox.WaitPID(pid, c.ID)
+	if err != nil {
+		return 0, err
+	}
+	if c.Sandbox.IsRootContainer(c.ID) {
+		// If waiting for the root, give some time for the sandbox process to exit
+		// to prevent races with resources that might still be in use.
+		if err := c.waitForStopped(); err != nil {
+			return 0, err
+		}
+	}
+	return ws, nil
 }
 
 // Signal sends the signal to the container.
@@ -502,6 +518,14 @@ func (c *Container) Destroy() error {
 			log.Warningf("Failed to destroy sandbox %q: %v", c.Sandbox.ID, err)
 		}
 	}
+	if c.GoferPid != 0 {
+		log.Debugf("Killing gofer for container %q, PID: %d", c.ID, c.GoferPid)
+		if err := syscall.Kill(c.GoferPid, syscall.SIGKILL); err != nil {
+			log.Warningf("error sending signal %d to pid %d: %v", syscall.SIGKILL, c.GoferPid, err)
+		} else {
+			c.GoferPid = 0
+		}
+	}
 
 	c.Sandbox = nil
 	c.Status = Stopped
@@ -509,29 +533,38 @@ func (c *Container) Destroy() error {
 	return nil
 }
 
+// IsRunning returns true if the sandbox or gofer process is running.
+func (c *Container) IsRunning() bool {
+	if c.Status == Stopped {
+		return false
+	}
+	if c.Sandbox != nil && c.Sandbox.IsRunning() {
+		return true
+	}
+	if c.GoferPid != 0 {
+		// Send a signal 0 to the gofer process.
+		if err := syscall.Kill(c.GoferPid, 0); err == nil {
+			log.Warningf("Found orphan gofer process, pid: %d", c.GoferPid)
+			// Attempt to kill gofer if it's orphan.
+			syscall.Kill(c.GoferPid, syscall.SIGKILL)
+
+			// Don't wait for gofer to die. Return 'running' and hope gofer is dead
+			// next time around.
+			return true
+		}
+	}
+	return false
+}
+
 // DestroyAndWait frees all resources associated with the container
 // and waits for destroy to finish before returning.
+//
+// TODO: This only works for single container.
 func (c *Container) DestroyAndWait() error {
-	sandboxPid := c.Sandbox.Pid
-	goferPid := c.Sandbox.GoferPid
-
 	if err := c.Destroy(); err != nil {
 		return fmt.Errorf("error destroying container %v: %v", c, err)
 	}
-
-	if sandboxPid != 0 {
-		if err := waitForDeath(sandboxPid, 5*time.Second); err != nil {
-			return fmt.Errorf("error waiting for sandbox death: %v", err)
-		}
-	}
-
-	if goferPid != 0 {
-		if err := waitForDeath(goferPid, 5*time.Second); err != nil {
-			return fmt.Errorf("error waiting for gofer death: %v", err)
-		}
-	}
-
-	return nil
+	return c.waitForStopped()
 }
 
 // save saves the container metadata to a file.
@@ -551,29 +584,15 @@ func (c *Container) save() error {
 	return nil
 }
 
-// waitForDeath ensures that process is dead before proceeding.
-//
-// This is racy because the kernel can potentially reuse the pid in the time
-// between the process' death and the first check after the process has ended.
-func waitForDeath(pid int, timeout time.Duration) error {
-	backoff := 1 * time.Millisecond
-	for start := time.Now(); time.Now().Sub(start) < timeout; {
-
-		if err := syscall.Kill(pid, 0); err != nil {
-			if err == syserror.ESRCH {
-				// pid does not exist so process must have died
-				return nil
-			}
-			return fmt.Errorf("error killing pid (%d): %v", pid, err)
-		}
-		// pid is still alive.
-
-		// Process continues to run, backoff and retry.
-		time.Sleep(backoff)
-		backoff *= 2
-		if backoff > 1*time.Second {
-			backoff = 1 * time.Second
+func (c *Container) waitForStopped() error {
+	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
+	defer cancel()
+	b := backoff.WithContext(backoff.NewConstantBackOff(100*time.Millisecond), ctx)
+	op := func() error {
+		if !c.IsRunning() {
+			return fmt.Errorf("container is still running")
 		}
+		return nil
 	}
-	return fmt.Errorf("timed out waiting for process (%d)", pid)
+	return backoff.Retry(op, b)
 }

runsc/container/container_test.go

@@ -350,7 +350,7 @@ func TestLifecycle(t *testing.T) {
 		// ourselves.
 		p, _ := os.FindProcess(s.Sandbox.Pid)
 		p.Wait()
-		g, _ := os.FindProcess(s.Sandbox.GoferPid)
+		g, _ := os.FindProcess(s.GoferPid)
 		g.Wait()
 
 		// Load the container from disk and check the status.
@@ -1701,3 +1701,48 @@ func TestContainerVolumeContentsShared(t *testing.T) {
 		t.Errorf("stat %q got error %v, wanted ErrNotExist", filename, err)
 	}
 }
+
+func TestGoferExits(t *testing.T) {
+	spec := testutil.NewSpecWithArgs("/bin/sleep", "10000")
+	conf := testutil.TestConfig()
+	rootDir, bundleDir, err := testutil.SetupContainer(spec, conf)
+	if err != nil {
+		t.Fatalf("error setting up container: %v", err)
+	}
+	defer os.RemoveAll(rootDir)
+	defer os.RemoveAll(bundleDir)
+
+	// Create and start the container.
+	c, err := container.Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "")
+	if err != nil {
+		t.Fatalf("error creating container: %v", err)
+	}
+	defer c.Destroy()
+	if err := c.Start(conf); err != nil {
+		t.Fatalf("error starting container: %v", err)
+	}
+
+	sandboxProc, err := os.FindProcess(c.Sandbox.Pid)
+	if err != nil {
+		t.Fatalf("error finding sandbox process: %v", err)
+	}
+	gofer, err := os.FindProcess(c.GoferPid)
+	if err != nil {
+		t.Fatalf("error finding sandbox process: %v", err)
+	}
+
+	// Kill sandbox and expect gofer to exit on its own.
+	if err := sandboxProc.Kill(); err != nil {
+		t.Fatalf("error killing sandbox process: %v", err)
+	}
+	if _, err := sandboxProc.Wait(); err != nil {
+		t.Fatalf("error waiting for sandbox process: %v", err)
+	}
+
+	if _, err := gofer.Wait(); err != nil {
+		t.Fatalf("error waiting for gofer process: %v", err)
+	}
+	if c.IsRunning() {
+		t.Errorf("container shouldn't be running, container: %+v", c)
+	}
+}

runsc/sandbox/BUILD

@@ -1,6 +1,6 @@
 package(licenses = ["notice"])  # Apache 2.0
 
-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "sandbox",
@@ -29,17 +29,3 @@ go_library(
         "@org_golang_x_sys//unix:go_default_library",
     ],
 )
-
-go_test(
-    name = "sandbox_test",
-    size = "small",
-    srcs = ["sandbox_test.go"],
-    data = [
-        "//runsc",
-    ],
-    embed = [":sandbox"],
-    deps = [
-        "//pkg/log",
-        "//runsc/test/testutil",
-    ],
-)

runsc/sandbox/sandbox.go

@@ -40,46 +40,45 @@ import (
 // It is used to start/stop sandbox process (and associated processes like
 // gofers), as well as for running and manipulating containers inside a running
 // sandbox.
+//
+// Note: Sandbox must be immutable because a copy of it is saved for each
+// container and changes would not be synchronized to all of them.
 type Sandbox struct {
-	// ID is the id of the sandbox. By convention, this is the same ID as
-	// the first container run in the sandbox.
+	// ID is the id of the sandbox (immutable). By convention, this is the same
+	// ID as the first container run in the sandbox.
 	ID string `json:"id"`
 
-	// Pid is the pid of the running sandbox. May be 0 is the sandbox is
-	// not running.
+	// Pid is the pid of the running sandbox (immutable). May be 0 is the sandbox
+	// is not running.
 	Pid int `json:"pid"`
-
-	// GoferPid is the pid of the gofer running along side the sandbox. May
-	// be 0 if the gofer has been killed or it's not being used.
-	GoferPid int `json:"goferPid"`
 }
 
 // Create creates the sandbox process.
-func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSocket string) (*Sandbox, error) {
+func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSocket string) (*Sandbox, int, error) {
 	s := &Sandbox{ID: id}
 
 	binPath, err := specutils.BinPath()
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
 	// Create the gofer process.
-	ioFiles, err := s.createGoferProcess(spec, conf, bundleDir, binPath)
+	goferPid, ioFiles, err := s.createGoferProcess(spec, conf, bundleDir, binPath)
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
 	// Create the sandbox process.
 	if err := s.createSandboxProcess(spec, conf, bundleDir, consoleSocket, binPath, ioFiles); err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
 	// Wait for the control server to come up (or timeout).
 	if err := s.waitForCreated(10 * time.Second); err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	return s, nil
+	return s, goferPid, nil
 }
 
 // StartRoot starts running the root container process inside the sandbox.
@@ -288,10 +287,10 @@ func (s *Sandbox) connError(err error) error {
 	return fmt.Errorf("error connecting to control server at pid %d: %v", s.Pid, err)
 }
 
-func (s *Sandbox) createGoferProcess(spec *specs.Spec, conf *boot.Config, bundleDir, binPath string) ([]*os.File, error) {
+func (s *Sandbox) createGoferProcess(spec *specs.Spec, conf *boot.Config, bundleDir, binPath string) (int, []*os.File, error) {
 	if conf.FileAccess == boot.FileAccessDirect {
 		// Don't start a gofer. The sandbox will access host FS directly.
-		return nil, nil
+		return 0, nil, nil
 	}
 
 	// Start with the general config flags.
@@ -315,7 +314,7 @@ func (s *Sandbox) createGoferProcess(spec *specs.Spec, conf *boot.Config, bundle
 	for nextFD = 3; nextFD-3 < mountCount; nextFD++ {
 		sandEnd, goferEnd, err := createSocketPair()
 		if err != nil {
-			return nil, err
+			return 0, nil, err
 		}
 		defer goferEnd.Close()
 		sandEnds = append(sandEnds, sandEnd)
@@ -327,7 +326,7 @@ func (s *Sandbox) createGoferProcess(spec *specs.Spec, conf *boot.Config, bundle
 	addr := fsgofer.ControlSocketAddr(s.ID)
 	serverFD, err := server.CreateSocket(addr)
 	if err != nil {
-		return nil, fmt.Errorf("error creating control server socket for sandbox %q: %v", s.ID, err)
+		return 0, nil, fmt.Errorf("error creating control server socket for sandbox %q: %v", s.ID, err)
 	}
 
 	// Add the control server fd.
@@ -349,11 +348,10 @@ func (s *Sandbox) createGoferProcess(spec *specs.Spec, conf *boot.Config, bundle
 	// Start the gofer in the given namespace.
 	log.Debugf("Starting gofer: %s %v", binPath, args)
 	if err := startInNS(cmd, nss); err != nil {
-		return nil, err
+		return 0, nil, err
 	}
-	s.GoferPid = cmd.Process.Pid
 	log.Infof("Gofer started, pid: %d", cmd.Process.Pid)
-	return sandEnds, nil
+	return cmd.Process.Pid, sandEnds, nil
 }
 
 // createSocketPair creates a pair of files wrapping a socket pair.
@@ -562,24 +560,9 @@ func (s *Sandbox) WaitPID(pid int32, cid string) (syscall.WaitStatus, error) {
 		PID: pid,
 		CID: cid,
 	}
-
 	if err := conn.Call(boot.ContainerWaitPID, args, &ws); err != nil {
 		return ws, fmt.Errorf("error waiting on PID %d in sandbox %q: %v", pid, s.ID, err)
 	}
-
-	if s.IsRootContainer(cid) {
-		// If waiting for the root, give some time for the sandbox process to exit
-		// to prevent races with resources that might still be in use.
-		timeout := time.Now().Add(time.Second)
-		log.Debugf("Waiting for the sandbox process to exit")
-		for s.IsRunning() {
-			if time.Now().After(timeout) {
-				log.Debugf("Timeout waiting for sandbox process to exit")
-				break
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-	}
 	return ws, nil
 }
 
@@ -602,15 +585,8 @@ func (s *Sandbox) Destroy() error {
 	if s.Pid != 0 {
 		// TODO: Too harsh?
 		log.Debugf("Killing sandbox %q", s.ID)
-		killProcess(s.Pid, unix.SIGKILL)
-		s.Pid = 0
+		signalProcess(s.Pid, unix.SIGKILL)
 	}
-	if s.GoferPid != 0 {
-		log.Debugf("Killing gofer for sandbox %q", s.ID)
-		killProcess(s.GoferPid, unix.SIGKILL)
-		s.GoferPid = 0
-	}
-
 	return nil
 }
 
@@ -689,19 +665,8 @@ func (s *Sandbox) Resume(cid string) error {
 func (s *Sandbox) IsRunning() bool {
 	if s.Pid != 0 {
 		// Send a signal 0 to the sandbox process.
-		if err := killProcess(s.Pid, 0); err == nil {
-			return true
-		}
-	}
-	if s.GoferPid != 0 {
-		// Send a signal 0 to the gofer process.
-		if err := killProcess(s.GoferPid, 0); err == nil {
-			log.Warningf("Found orphan gofer process, pid: %d", s.GoferPid)
-			// Attempt to kill gofer if it's orphan.
-			killProcess(s.GoferPid, unix.SIGKILL)
-
-			// Don't wait for gofer to die. Return 'running' and hope gofer is dead
-			// next time around.
+		if err := signalProcess(s.Pid, 0); err == nil {
+			// Succeeded, process is running.
 			return true
 		}
 	}
@@ -724,10 +689,10 @@ func (s *Sandbox) Stacks() (string, error) {
 	return stacks, nil
 }
 
-// killProcess sends a signal to the host process (i.e. a sandbox or gofer
+// signalProcess sends a signal to the host process (i.e. a sandbox or gofer
 // process). Sandbox.Signal should be used to send a signal to a process
 // running inside the sandbox.
-func killProcess(pid int, sig syscall.Signal) error {
+func signalProcess(pid int, sig syscall.Signal) error {
 	if err := syscall.Kill(pid, sig); err != nil {
 		return fmt.Errorf("error sending signal %d to pid %d: %v", sig, pid, err)
 	}

runsc/sandbox/sandbox_test.go

@@ -1,75 +0,0 @@
-// Copyright 2018 Google Inc.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package sandbox
-
-import (
-	"os"
-	"testing"
-
-	"gvisor.googlesource.com/gvisor/pkg/log"
-	"gvisor.googlesource.com/gvisor/runsc/test/testutil"
-)
-
-func init() {
-	log.SetLevel(log.Debug)
-	if err := testutil.ConfigureExePath(); err != nil {
-		panic(err.Error())
-	}
-}
-
-func TestGoferExits(t *testing.T) {
-	spec := testutil.NewSpecWithArgs("/bin/sleep", "10000")
-	conf := testutil.TestConfig()
-	rootDir, bundleDir, err := testutil.SetupContainer(spec, conf)
-	if err != nil {
-		t.Fatalf("error setting up container: %v", err)
-	}
-	defer os.RemoveAll(rootDir)
-	defer os.RemoveAll(bundleDir)
-
-	// Create, start and wait for the container.
-	s, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "")
-	if err != nil {
-		t.Fatalf("error creating container: %v", err)
-	}
-	defer s.Destroy()
-	if err := s.StartRoot(spec, conf); err != nil {
-		t.Fatalf("error starting container: %v", err)
-	}
-
-	sandboxProc, err := os.FindProcess(s.Pid)
-	if err != nil {
-		t.Fatalf("error finding sandbox process: %v", err)
-	}
-	gofer, err := os.FindProcess(s.GoferPid)
-	if err != nil {
-		t.Fatalf("error finding sandbox process: %v", err)
-	}
-
-	// Kill sandbox and expect gofer to exit on its own.
-	if err := sandboxProc.Kill(); err != nil {
-		t.Fatalf("error killing sandbox process: %v", err)
-	}
-	if _, err := sandboxProc.Wait(); err != nil {
-		t.Fatalf("error waiting for sandbox process: %v", err)
-	}
-
-	if _, err := gofer.Wait(); err != nil {
-		t.Fatalf("error waiting for gofer process: %v", err)
-	}
-	if s.IsRunning() {
-		t.Errorf("Sandbox shouldn't be running, sandbox: %+v", s)
-	}
-}