Merge pull request #14385 from hashicorp/f-cg-use-kill

cgroups: refactor v2 kill path to use cgroups.kill interface file

.changelog/14371.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+cgroups: use cgroup.kill interface file when using cgroups v2
+```

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
+}

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)
+}

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
 }