From 5d5c8af930b38e9c8687da7a0afb6a905dad60b3 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 29 Aug 2022 14:50:10 -0500 Subject: [PATCH] cgroups: refactor v2 kill path to use cgroups.kill interface file This PR refactors the cgroups v2 group kill code path to use the cgroups.kill interface file for destroying the cgroup. Previously we copied the freeze + sigkill + unfreeze pattern from the v1 code, but v2 provides a more efficient and more race-free way to handle this. Closes #14371 --- .changelog/14371.txt | 3 ++ client/lib/cgutil/editor.go | 27 +++++++++++++++ client/lib/cgutil/editor_test.go | 39 +++++++++++++++++++++ client/lib/cgutil/group_killer.go | 57 +++++++------------------------ 4 files changed, 81 insertions(+), 45 deletions(-) create mode 100644 .changelog/14371.txt create mode 100644 client/lib/cgutil/editor.go create mode 100644 client/lib/cgutil/editor_test.go diff --git a/.changelog/14371.txt b/.changelog/14371.txt new file mode 100644 index 000000000..b4bdc9723 --- /dev/null +++ b/.changelog/14371.txt @@ -0,0 +1,3 @@ +```release-note:improvement +cgroups: use cgroup.kill interface file when using cgroups v2 +``` diff --git a/client/lib/cgutil/editor.go b/client/lib/cgutil/editor.go new file mode 100644 index 000000000..4f354b98e --- /dev/null +++ b/client/lib/cgutil/editor.go @@ -0,0 +1,27 @@ +//go:build linux + +package cgutil + +import ( + "os" + "path/filepath" + "strings" +) + +// editor provides a simple mechanism for reading and writing cgroup files. +type editor struct { + fromRoot string +} + +func (e *editor) path(file string) string { + return filepath.Join(CgroupRoot, e.fromRoot, file) +} + +func (e *editor) write(file, content string) error { + return os.WriteFile(e.path(file), []byte(content), 0o644) +} + +func (e *editor) read(file string) (string, error) { + b, err := os.ReadFile(e.path(file)) + return strings.TrimSpace(string(b)), err +} diff --git a/client/lib/cgutil/editor_test.go b/client/lib/cgutil/editor_test.go new file mode 100644 index 000000000..ad9a5c319 --- /dev/null +++ b/client/lib/cgutil/editor_test.go @@ -0,0 +1,39 @@ +//go:build linux + +package cgutil + +import ( + "os" + "path/filepath" + "testing" + + "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/helper/uuid" + "github.com/shoenig/test/must" +) + +func createCG(t *testing.T) (string, func()) { + name := uuid.Short() + ".scope" + path := filepath.Join(CgroupRoot, name) + err := os.Mkdir(path, 0o755) + must.NoError(t, err) + + return name, func() { + _ = os.Remove(path) + } +} + +func TestCG_editor(t *testing.T) { + testutil.CgroupsCompatibleV2(t) + + cg, rm := createCG(t) + t.Cleanup(rm) + + edits := &editor{cg} + writeErr := edits.write("cpu.weight.nice", "13") + must.NoError(t, writeErr) + + b, readErr := edits.read("cpu.weight.nice") + must.NoError(t, readErr) + must.Eq(t, "13", b) +} diff --git a/client/lib/cgutil/group_killer.go b/client/lib/cgutil/group_killer.go index 9f966c499..f6e82dbf8 100644 --- a/client/lib/cgutil/group_killer.go +++ b/client/lib/cgutil/group_killer.go @@ -6,13 +6,12 @@ import ( "errors" "fmt" "os" - "path/filepath" + "strconv" "time" "github.com/hashicorp/go-hclog" "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fs" - "github.com/opencontainers/runc/libcontainer/cgroups/fs2" "github.com/opencontainers/runc/libcontainer/configs" ) @@ -96,56 +95,24 @@ func (d *killer) v1(cgroup *configs.Cgroup) error { } func (d *killer) v2(cgroup *configs.Cgroup) error { - if cgroup == nil { + if cgroup == nil || cgroup.Path == "" { return errors.New("missing cgroup") } - path := filepath.Join(CgroupRoot, cgroup.Path) - - existingPIDs, err := cgroups.GetPids(path) - if err != nil { - return fmt.Errorf("failed to determine pids in cgroup: %w", err) - } - - d.logger.Trace("killing processes", "cgroup_path", path, "cgroup_version", "v2", "executor_pid", d.pid, "existing_pids", existingPIDs) - - mgr, err := fs2.NewManager(cgroup, "") - if err != nil { - return fmt.Errorf("failed to create v2 cgroup manager: %w", err) - } - - // move executor PID into the root init.scope so we can kill the task pids - // without killing the executor (which is the process running this code, doing - // the killing) - init, err := fs2.NewManager(nil, filepath.Join(CgroupRoot, "init.scope")) - if err != nil { - return fmt.Errorf("failed to create v2 init cgroup manager: %w", err) - } - if err = init.Apply(d.pid); err != nil { - return fmt.Errorf("failed to move executor pid into init.scope cgroup: %w", err) - } - - d.logger.Trace("move of executor pid into init.scope complete", "pid", d.pid) - - // ability to freeze the cgroup - freeze := func() { - _ = mgr.Freeze(configs.Frozen) - } - - // ability to thaw the cgroup - thaw := func() { - _ = mgr.Freeze(configs.Thawed) - } - - // do the common kill logic - - if err = d.kill(path, freeze, thaw); err != nil { + // move executor (d.PID) into init.scope + editSelf := &editor{"init.scope"} + if err := editSelf.write("cgroup.procs", strconv.Itoa(d.pid)); err != nil { return err } - // note: do NOT remove the cgroup from disk; leave that to the alloc-level - // cpuset mananager. + // write "1" to cgroup.kill + editTask := &editor{cgroup.Path} + if err := editTask.write("cgroup.kill", "1"); err != nil { + return err + } + // note: do NOT remove the cgroup from disk; leave that to the Client, at + // least until #14375 is implemented. return nil }