Add container.Destroy urpc method.

This method will:
1. Stop the container process if it is still running.
2. Unmount all sanadbox-internal mounts for the container.
3. Delete the contaner root directory inside the sandbox.

Destroy is idempotent, and safe to call concurrantly.

This fixes a bug where after stopping a container, we cannot unmount the
container root directory on the host. This bug occured because the sandbox
dirent cache was holding a dirent with a host fd corresponding to a file inside
the container root on the host. The dirent cache did not know that the
container had exited, and kept the FD open, preventing us from unmounting on
the host.

Now that we unmount (and flush) all container mounts inside the sandbox, any
host FDs donated by the gofer will be closed, and we can unmount the container
root on the host.

PiperOrigin-RevId: 213737693
Change-Id: I28c0ff4cd19a08014cdd72fec5154497e92aacc9

2 files changed, 73 insertions, 10 deletions

diff --git a/runsc/container/container.go b/runsc/container/container.go
index 3be88066c..a2582611a 100644
--- a/runsc/container/container.go
+++ b/runsc/container/container.go
@@ -544,16 +544,9 @@ func (c *Container) save() error {
 // to stop. If any of them doesn't stop before timeout, an error is returned.
 func (c *Container) stop() error {
 	if c.Sandbox != nil && c.Sandbox.IsRunning() {
-		log.Debugf("Killing container %q", c.ID)
-		if c.Sandbox.IsRootContainer(c.ID) {
-			if err := c.Sandbox.Destroy(); err != nil {
-				return fmt.Errorf("error destroying sandbox %q: %v", c.Sandbox.ID, err)
-			}
-		} else {
-			if err := c.Signal(syscall.SIGKILL); err != nil {
-				// The container may already be stopped, log the error.
-				log.Warningf("Error sending signal %d to container %q: %v", syscall.SIGKILL, c.ID, err)
-			}
+		log.Debugf("Destroying container %q", c.ID)
+		if err := c.Sandbox.DestroyContainer(c.ID); err != nil {
+			return fmt.Errorf("error destroying container %q: %v", c.ID, err)
 		}
 	}
 
diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go
index d6418efb6..0df587e30 100644
--- a/runsc/container/multi_container_test.go
+++ b/runsc/container/multi_container_test.go
@@ -17,6 +17,7 @@ package container
 import (
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -25,6 +26,7 @@ import (
 
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/control"
+	"gvisor.googlesource.com/gvisor/runsc/boot"
 	"gvisor.googlesource.com/gvisor/runsc/specutils"
 	"gvisor.googlesource.com/gvisor/runsc/test/testutil"
 )
@@ -431,3 +433,71 @@ func TestMultiContainerSignal(t *testing.T) {
 		}
 	}
 }
+
+// TestMultiContainerDestroy checks that container are properly cleaned-up when
+// they are destroyed.
+func TestMultiContainerDestroy(t *testing.T) {
+	for _, conf := range configs(all...) {
+		t.Logf("Running test with conf: %+v", conf)
+
+		rootDir, err := testutil.SetupRootDir()
+		if err != nil {
+			t.Fatalf("error creating root dir: %v", err)
+		}
+		defer os.RemoveAll(rootDir)
+
+		// Two containers that will run for a long time. We will
+		// destroy the second one.
+		specs, ids := createSpecs([]string{"sleep", "100"}, []string{"sleep", "100"})
+
+		// Setup the containers.
+		var containers []*Container
+		for i, spec := range specs {
+			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)
+		}
+
+		// Exec in the root container to check for the existence of the
+		// second containers root filesystem directory.
+		contDir := path.Join(boot.ChildContainersDir, containers[1].ID)
+		args := &control.ExecArgs{
+			Filename: "/usr/bin/test",
+			Argv:     []string{"test", "-d", contDir},
+		}
+		if ws, err := containers[0].executeSync(args); err != nil {
+			t.Fatalf("error executing %+v: %v", args, err)
+		} else if ws.ExitStatus() != 0 {
+			t.Errorf("exec 'test -f %q' got exit status %d, wanted 0", contDir, ws.ExitStatus())
+		}
+
+		// Destory the second container.
+		if err := containers[1].Destroy(); err != nil {
+			t.Fatalf("error destroying container: %v", err)
+		}
+
+		// Now the container dir should be gone.
+		if ws, err := containers[0].executeSync(args); err != nil {
+			t.Fatalf("error executing %+v: %v", args, err)
+		} else if ws.ExitStatus() == 0 {
+			t.Errorf("exec 'test -f %q' got exit status 0, wanted non-zero", contDir)
+		}
+
+		// Check that cont.Destroy is safe to call multiple times.
+		if err := containers[1].Destroy(); err != nil {
+			t.Errorf("error destroying container: %v", err)
+		}
+	}
+}