executor: move out of all of container cgroups

drivers/shared/executor/executor_linux.go

@@ -63,6 +63,8 @@ type LibcontainerExecutor struct {
 	userProc       *libcontainer.Process
 	userProcExited chan interface{}
 	exitState      *ProcessState
+
+	containerSubsystems []string
 }
 
 func NewExecutorWithIsolation(logger hclog.Logger) Executor {
@@ -202,6 +204,7 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro
 	if err := cgroups.EnterPid(containerState.CgroupPaths, os.Getpid()); err != nil {
 		l.logger.Error("error entering user process cgroups", "executor_pid", os.Getpid(), "error", err)
 	}
+	l.containerSubsystems = keys(containerState.CgroupPaths)
 
 	// start a goroutine to wait on the process to complete, so Wait calls can
 	// be multiplexed
@@ -288,11 +291,7 @@ func (l *LibcontainerExecutor) Shutdown(signal string, grace time.Duration) erro
 	}
 
 	// move executor to root cgroup
-	subsystems, err := cgroups.GetAllSubsystems()
-	if err != nil {
-		return err
-	}
-	if err := JoinRootCgroup(subsystems); err != nil {
+	if err := JoinRootCgroup(l.containerSubsystems); err != nil {
 		return err
 	}
 
@@ -901,3 +900,11 @@ func lookPathIn(path string, root string, bin string) (string, error) {
 	}
 	return "", fmt.Errorf("file %s not found under path %s", bin, root)
 }
+
+func keys(m map[string]string) []string {
+	r := make([]string, 0, len(m))
+	for k, _ := range m {
+		r = append(r, k)
+	}
+	return r
+}

drivers/shared/executor/executor_linux_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/plugins/drivers"
 	tu "github.com/hashicorp/nomad/testutil"
+	"github.com/opencontainers/runc/libcontainer/cgroups"
 	lconfigs "github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/unix"
@@ -217,6 +218,88 @@ func TestExecutor_CgroupPaths(t *testing.T) {
 	}, func(err error) { t.Error(err) })
 }
 
+// TestExecutor_CgroupPaths asserts that all cgroups created for a task
+// are destroyed on shutdown
+func TestExecutor_CgroupPathsAreDestroyed(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+	testutil.ExecCompatible(t)
+
+	testExecCmd := testExecutorCommandWithChroot(t)
+	execCmd, allocDir := testExecCmd.command, testExecCmd.allocDir
+	execCmd.Cmd = "/bin/bash"
+	execCmd.Args = []string{"-c", "sleep 0.2; cat /proc/self/cgroup"}
+	defer allocDir.Destroy()
+
+	execCmd.ResourceLimits = true
+
+	executor := NewExecutorWithIsolation(testlog.HCLogger(t))
+	defer executor.Shutdown("SIGKILL", 0)
+
+	ps, err := executor.Launch(execCmd)
+	require.NoError(err)
+	require.NotZero(ps.Pid)
+
+	state, err := executor.Wait(context.Background())
+	require.NoError(err)
+	require.Zero(state.ExitCode)
+
+	var cgroupsPaths string
+	tu.WaitForResult(func() (bool, error) {
+		output := strings.TrimSpace(testExecCmd.stdout.String())
+		// sanity check that we got some cgroups
+		if !strings.Contains(output, ":devices:") {
+			return false, fmt.Errorf("was expected cgroup files but found:\n%v", output)
+		}
+		lines := strings.Split(output, "\n")
+		for _, line := range lines {
+			// Every cgroup entry should be /nomad/$ALLOC_ID
+			if line == "" {
+				continue
+			}
+
+			// Skip rdma subsystem; rdma was added in most recent kernels and libcontainer/docker
+			// don't isolate it by default.
+			if strings.Contains(line, ":rdma:") {
+				continue
+			}
+
+			if !strings.Contains(line, ":/nomad/") {
+				return false, fmt.Errorf("Not a member of the alloc's cgroup: expected=...:/nomad/... -- found=%q", line)
+			}
+		}
+
+		cgroupsPaths = output
+		return true, nil
+	}, func(err error) { t.Error(err) })
+
+	// shutdown executor and test that cgroups are destroyed
+	executor.Shutdown("SIGKILL", 0)
+
+	// test that the cgroup paths are not visible
+	tmpFile, err := ioutil.TempFile("", "")
+	require.NoError(err)
+	defer os.Remove(tmpFile.Name())
+
+	_, err = tmpFile.WriteString(cgroupsPaths)
+	require.NoError(err)
+	tmpFile.Close()
+
+	subsystems, err := cgroups.ParseCgroupFile(tmpFile.Name())
+	require.NoError(err)
+
+	for subsystem, cgroup := range subsystems {
+		if !strings.Contains(cgroup, "nomad/") {
+			// this should only be rdma at this point
+			continue
+		}
+
+		p, err := getCgroupPathHelper(subsystem, cgroup)
+		require.NoError(err)
+		require.Falsef(cgroups.PathExists(p), "cgroup for %s %s still exists", subsystem, cgroup)
+	}
+}
+
 func TestUniversalExecutor_LookupTaskBin(t *testing.T) {
 	t.Parallel()
 	require := require.New(t)
@@ -484,3 +567,19 @@ func TestExecutor_cmdMounts(t *testing.T) {
 
 	require.EqualValues(t, expected, cmdMounts(input))
 }
+
+func getCgroupPathHelper(subsystem, cgroup string) (string, error) {
+	mnt, root, err := cgroups.FindCgroupMountpointAndRoot("", subsystem)
+	if err != nil {
+		return "", err
+	}
+
+	// This is needed for nested containers, because in /proc/self/cgroup we
+	// see paths from host, which don't exist in container.
+	relCgroup, err := filepath.Rel(root, cgroup)
+	if err != nil {
+		return "", err
+	}
+
+	return filepath.Join(mnt, relCgroup), nil
+}