Initial change for multi-gofer support

PiperOrigin-RevId: 209647293
Change-Id: I980fca1257ea3fcce796388a049c353b0303a8a5

3 files changed, 26 insertions, 150 deletions

diff --git a/runsc/sandbox/BUILD b/runsc/sandbox/BUILD
index cdacc5e22..d26a4dac6 100644
--- a/runsc/sandbox/BUILD
+++ b/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",
-    ],
-)
diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go
index a10b79856..e5d1f791d 100644
--- a/runsc/sandbox/sandbox.go
+++ b/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)
 	}
diff --git a/runsc/sandbox/sandbox_test.go b/runsc/sandbox/sandbox_test.go
deleted file mode 100644
index 40337bc53..000000000
--- a/runsc/sandbox/sandbox_test.go
+++ /dev/null
@@ -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)
-	}
-}