From f7608c4cef3b91f2a1b23ecc1b4f42abdee3a9ec Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 10 Jun 2019 21:20:45 -0400 Subject: [PATCH 1/2] exec: use an independent name=systemd cgroup path We aim for containers to be part of a new cgroups hierarchy independent from nomad agent. However, we've been setting a relative path as libcontainer `cfg.Cgroups.Path`, which makes libcontainer concatinate the executor process cgroup with passed cgroup, as set in [1]. By setting an absolute path, we ensure that all cgroups subsystem (including `name=systemd` get a dedicated one). This matches behavior in Nomad 0.8, and behavior of how Docker and OCI sets CgroupsPath[2] Fixes #5736 [1] https://github.com/hashicorp/nomad/blob/d7edf9b2e42348865908735996359c7869fb16b5/vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs/apply_raw.go#L326-L340 [2] https://github.com/moby/moby/blob/238f8eaa31aa74be843c81703fabf774863ec30c/vendor/github.com/containerd/containerd/oci/spec.go#L229 --- drivers/exec/driver_test.go | 2 +- drivers/shared/executor/executor_linux.go | 2 +- .../shared/executor/executor_linux_test.go | 53 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 6319ed69a..671c5753b 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -436,7 +436,7 @@ func TestExecDriver_HandlerExec(t *testing.T) { } // Skip systemd and rdma cgroups; rdma was added in most recent kernels and libcontainer/docker // don't isolate them by default. - if strings.HasPrefix(line, "1:name=systemd") || strings.Contains(line, ":rdma:") { + if strings.Contains(line, ":rdma:") { continue } if !strings.Contains(line, ":/nomad/") { diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 14a406aae..3987b1984 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -684,7 +684,7 @@ func configureCgroups(cfg *lconfigs.Config, command *ExecCommand) error { } id := uuid.Generate() - cfg.Cgroups.Path = filepath.Join(defaultCgroupParent, id) + cfg.Cgroups.Path = filepath.Join("/", defaultCgroupParent, id) if command.Resources == nil || command.Resources.NomadResources == nil { return nil diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index ed87a1dc7..407d261aa 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -164,6 +164,59 @@ passwd` }, func(err error) { t.Error(err) }) } +// TestExecutor_CgroupPaths asserts that process starts with independent cgroups +// hierarchy created for this process +func TestExecutor_CgroupPaths(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) + + 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 systemd and rdma cgroups; rdma was added in most recent kernels and libcontainer/docker + // don't isolate them 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) + } + } + return true, nil + }, func(err error) { t.Error(err) }) +} + func TestUniversalExecutor_LookupTaskBin(t *testing.T) { t.Parallel() require := require.New(t) From 5734c8a6488ae6d8163376b7ffab4a368a42c2c9 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 11 Jun 2019 13:00:26 -0400 Subject: [PATCH 2/2] update comment --- drivers/exec/driver_test.go | 4 ++-- drivers/shared/executor/executor_linux_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 671c5753b..e0680eb49 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -434,8 +434,8 @@ func TestExecDriver_HandlerExec(t *testing.T) { if line == "" { continue } - // Skip systemd and rdma cgroups; rdma was added in most recent kernels and libcontainer/docker - // don't isolate them by default. + // 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 } diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 407d261aa..c8e07370b 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -203,8 +203,8 @@ func TestExecutor_CgroupPaths(t *testing.T) { continue } - // Skip systemd and rdma cgroups; rdma was added in most recent kernels and libcontainer/docker - // don't isolate them by default. + // 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 }