Enabled bind mounts in sub-containers

With multi-gofers, bind mounts in sub-containers should
just work. Removed restrictions and added test. There are
also a few cleanups along the way, e.g. retry unmounting
in case cleanup races with gofer teardown.

PiperOrigin-RevId: 211699569
Change-Id: Ic0a69c29d7c31cd7e038909cc686c6ac98703374

4 files changed, 82 insertions, 22 deletions

diff --git a/runsc/boot/fds.go b/runsc/boot/fds.go
index 0449e243d..9de5a78b1 100644
--- a/runsc/boot/fds.go
+++ b/runsc/boot/fds.go
@@ -28,11 +28,6 @@ import (
 
 // createFDMap creates an fd map that contains stdin, stdout, and stderr. If
 // console is true, then ioctl calls will be passed through to the host fd.
-//
-// TODO: We currently arn't passing any FDs in to the sandbox, so
-// there's not much else for this function to do.  It will get more complicated
-// when gofers enter the picture.  Also the LISTEN_FDS environment variable
-// allows passing arbitrary FDs to the sandbox, which we do not yet support.
 func createFDMap(ctx context.Context, k *kernel.Kernel, l *limits.LimitSet, console bool) (*kernel.FDMap, error) {
 	fdm := k.NewFDMap()
 	defer fdm.DecRef()
diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go
index 4a11b30f1..772df40fe 100644
--- a/runsc/boot/fs.go
+++ b/runsc/boot/fs.go
@@ -685,11 +685,6 @@ func setFileSystemForProcess(procArgs *kernel.CreateProcessArgs, spec *specs.Spe
 	// Mount all submounts.
 	mounts := compileMounts(spec)
 	for _, m := range mounts {
-		// TODO: Enable bind mounts in child containers.
-		if m.Type == bind {
-			log.Infof("Bind mounts in child containers are not yet supported: %+v", m)
-			continue
-		}
 		dest := filepath.Join(containerRoot, m.Destination)
 		if err := mountSubmount(rootCtx, conf, k.RootMountNamespace(), fds, m, mounts, dest); err != nil {
 			return fmt.Errorf("error mounting filesystem for container: %v", err)
diff --git a/runsc/container/container.go b/runsc/container/container.go
index a3454eb8f..a4a3ed56d 100644
--- a/runsc/container/container.go
+++ b/runsc/container/container.go
@@ -520,24 +520,35 @@ func (c *Container) Destroy() error {
 	c.Status = Stopped
 	c.Sandbox = nil
 
+	if err := c.destroyGofer(); err != nil {
+		return fmt.Errorf("error destroying gofer: %v", err)
+	}
+
+	if err := os.RemoveAll(c.Root); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("error deleting container root directory %q: %v", c.Root, err)
+	}
+
+	return nil
+}
+
+func (c *Container) destroyGofer() error {
 	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
 		}
 	}
 
-	if err := destroyFS(c.Spec); err != nil {
-		return fmt.Errorf("error destroying container fs: %v", err)
-	}
-
-	if err := os.RemoveAll(c.Root); err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("error deleting container root directory %q: %v", c.Root, err)
+	// Gofer process may take some time to teardown. Retry in case of failure.
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	b := backoff.WithContext(backoff.NewConstantBackOff(100*time.Millisecond), ctx)
+	err := backoff.Retry(func() error { return destroyFS(c.Spec) }, b)
+	if err == nil {
+		// Success!
+		c.GoferPid = 0
 	}
-
-	return nil
+	return err
 }
 
 // IsRunning returns true if the sandbox or gofer process is running.
@@ -549,8 +560,9 @@ func (c *Container) IsRunning() bool {
 		// 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)
+			if err := c.destroyGofer(); err != nil {
+				log.Warningf("Error destroying gofer: %v", err)
+			}
 
 			// Don't wait for gofer to die. Return 'running' and hope gofer is dead
 			// next time around.
diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go
index cf5140b4e..3bdfbaca3 100644
--- a/runsc/container/multi_container_test.go
+++ b/runsc/container/multi_container_test.go
@@ -15,7 +15,9 @@
 package container
 
 import (
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"testing"
@@ -179,3 +181,59 @@ func TestMultiContainerWait(t *testing.T) {
 		t.Errorf("failed to wait for %q to start: %v", strings.Join(containers[0].Spec.Process.Args, " "), err)
 	}
 }
+
+// TestMultiContainerMount tests that bind mounts can be used with multiple
+// containers.
+func TestMultiContainerMount(t *testing.T) {
+	rootDir, err := testutil.SetupRootDir()
+	if err != nil {
+		t.Fatalf("error creating root dir: %v", err)
+	}
+	defer os.RemoveAll(rootDir)
+
+	cmd1 := []string{"sleep", "100"}
+
+	// 'src != dst' ensures that 'dst' doesn't exist in the host and must be
+	// properly mapped inside the container to work.
+	src, err := ioutil.TempDir(testutil.TmpDir(), "container")
+	if err != nil {
+		t.Fatal("ioutil.TempDir failed:", err)
+	}
+	dst := src + ".dst"
+	cmd2 := []string{"touch", filepath.Join(dst, "file")}
+
+	sps, ids := createSpecs(cmd1, cmd2)
+	sps[1].Mounts = append(sps[1].Mounts, specs.Mount{
+		Source:      src,
+		Destination: dst,
+		Type:        "bind",
+	})
+
+	// Setup the containers.
+	var containers []*Container
+	for i, spec := range sps {
+		conf := testutil.TestConfig()
+		bundleDir, err := testutil.SetupContainerInRoot(rootDir, spec, conf)
+		if err != nil {
+			t.Fatalf("error setting up container: %v", err)
+		}
+		defer os.RemoveAll(bundleDir)
+		cont, err := Create(ids[i], spec, conf, bundleDir, "", "")
+		if err != nil {
+			t.Fatalf("error creating container: %v", err)
+		}
+		defer cont.Destroy()
+		if err := cont.Start(conf); err != nil {
+			t.Fatalf("error starting container: %v", err)
+		}
+		containers = append(containers, cont)
+	}
+
+	ws, err := containers[1].Wait()
+	if err != nil {
+		t.Error("error waiting on container:", err)
+	}
+	if !ws.Exited() || ws.ExitStatus() != 0 {
+		t.Error("container failed, waitStatus:", ws)
+	}
+}