From c804b28373425a238b4c8acf518deaafcb11c7d3 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 | 59 ++++++++----------------------- 4 files changed, 83 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 000000000000..b4bdc97236e4 --- /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 000000000000..4f354b98eee5 --- /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 000000000000..ad9a5c3194c9 --- /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 9f966c499025..1c0188111240 100644 --- a/client/lib/cgutil/group_killer.go +++ b/client/lib/cgutil/group_killer.go @@ -6,14 +6,14 @@ 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" + "github.com/shoenig/netlog" ) // freezer is the name of the cgroup subsystem used for stopping / starting @@ -35,7 +35,8 @@ type GroupKiller interface { // NewGroupKiller creates a GroupKiller with executor PID pid. func NewGroupKiller(logger hclog.Logger, pid int) GroupKiller { return &killer{ - logger: logger.Named("group_killer"), + // logger: logger.Named("group_killer"), + logger: netlog.New("KILL"), pid: pid, } } @@ -96,56 +97,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) + // 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 } - // do the common kill logic - - if err = d.kill(path, freeze, thaw); err != nil { + // 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 alloc-level - // cpuset mananager. - + // note: do NOT remove the cgroup from disk; leave that to the Client, at + // least until #14375 is implemented. return nil }