Merge pull request #2605 from kolyshkin/fscommon-II

libct/cgroups: switch to fscommon.{Read,Write}File

libcontainer/cgroups/fs/cpuacct.go

@@ -5,7 +5,6 @@ package fs
 import (
 	"bufio"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -92,11 +91,11 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
 	// Expected format:
 	// user <usage in ticks>
 	// system <usage in ticks>
-	data, err := ioutil.ReadFile(filepath.Join(path, cgroupCpuacctStat))
+	data, err := fscommon.ReadFile(path, cgroupCpuacctStat)
 	if err != nil {
 		return 0, 0, err
 	}
-	fields := strings.Fields(string(data))
+	fields := strings.Fields(data)
 	if len(fields) < 4 {
 		return 0, 0, fmt.Errorf("failure - %s is expected to have at least 4 fields", filepath.Join(path, cgroupCpuacctStat))
 	}
@@ -118,11 +117,11 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
 
 func getPercpuUsage(path string) ([]uint64, error) {
 	percpuUsage := []uint64{}
-	data, err := ioutil.ReadFile(filepath.Join(path, "cpuacct.usage_percpu"))
+	data, err := fscommon.ReadFile(path, "cpuacct.usage_percpu")
 	if err != nil {
 		return percpuUsage, err
 	}
-	for _, value := range strings.Fields(string(data)) {
+	for _, value := range strings.Fields(data) {
 		value, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			return percpuUsage, fmt.Errorf("Unable to convert param value to uint64: %s", err)

libcontainer/cgroups/fs/cpuset.go

@@ -3,8 +3,6 @@
 package fs
 
 import (
-	"bytes"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -81,7 +79,7 @@ func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) erro
 	// 'ensureParent' start with parent because we don't want to
 	// explicitly inherit from parent, it could conflict with
 	// 'cpuset.cpu_exclusive'.
-	if err := s.ensureParent(filepath.Dir(dir), root); err != nil {
+	if err := cpusetEnsureParent(filepath.Dir(dir), root); err != nil {
 		return err
 	}
 	if err := os.MkdirAll(dir, 0755); err != nil {
@@ -103,20 +101,20 @@ func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) erro
 	return cgroups.WriteCgroupProc(dir, pid)
 }
 
-func (s *CpusetGroup) getSubsystemSettings(parent string) (cpus []byte, mems []byte, err error) {
-	if cpus, err = ioutil.ReadFile(filepath.Join(parent, "cpuset.cpus")); err != nil {
+func getCpusetSubsystemSettings(parent string) (cpus, mems string, err error) {
+	if cpus, err = fscommon.ReadFile(parent, "cpuset.cpus"); err != nil {
 		return
 	}
-	if mems, err = ioutil.ReadFile(filepath.Join(parent, "cpuset.mems")); err != nil {
+	if mems, err = fscommon.ReadFile(parent, "cpuset.mems"); err != nil {
 		return
 	}
 	return cpus, mems, nil
 }
 
-// ensureParent makes sure that the parent directory of current is created
+// cpusetEnsureParent makes sure that the parent directory of current is created
 // and populated with the proper cpus and mems files copied from
-// it's parent.
-func (s *CpusetGroup) ensureParent(current, root string) error {
+// its parent.
+func cpusetEnsureParent(current, root string) error {
 	parent := filepath.Dir(current)
 	if libcontainerUtils.CleanPath(parent) == root {
 		return nil
@@ -125,51 +123,47 @@ func (s *CpusetGroup) ensureParent(current, root string) error {
 	if parent == current {
 		return errors.New("cpuset: cgroup parent path outside cgroup root")
 	}
-	if err := s.ensureParent(parent, root); err != nil {
+	if err := cpusetEnsureParent(parent, root); err != nil {
 		return err
 	}
 	if err := os.MkdirAll(current, 0755); err != nil {
 		return err
 	}
-	return s.copyIfNeeded(current, parent)
+	return cpusetCopyIfNeeded(current, parent)
 }
 
-// copyIfNeeded copies the cpuset.cpus and cpuset.mems from the parent
+// cpusetCopyIfNeeded copies the cpuset.cpus and cpuset.mems from the parent
 // directory to the current directory if the file's contents are 0
-func (s *CpusetGroup) copyIfNeeded(current, parent string) error {
-	var (
-		err                      error
-		currentCpus, currentMems []byte
-		parentCpus, parentMems   []byte
-	)
-
-	if currentCpus, currentMems, err = s.getSubsystemSettings(current); err != nil {
+func cpusetCopyIfNeeded(current, parent string) error {
+	currentCpus, currentMems, err := getCpusetSubsystemSettings(current)
+	if err != nil {
 		return err
 	}
-	if parentCpus, parentMems, err = s.getSubsystemSettings(parent); err != nil {
+	parentCpus, parentMems, err := getCpusetSubsystemSettings(parent)
+	if err != nil {
 		return err
 	}
 
-	if s.isEmpty(currentCpus) {
+	if isEmptyCpuset(currentCpus) {
 		if err := fscommon.WriteFile(current, "cpuset.cpus", string(parentCpus)); err != nil {
 			return err
 		}
 	}
-	if s.isEmpty(currentMems) {
+	if isEmptyCpuset(currentMems) {
 		if err := fscommon.WriteFile(current, "cpuset.mems", string(parentMems)); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (s *CpusetGroup) isEmpty(b []byte) bool {
-	return len(bytes.Trim(b, "\n")) == 0
+func isEmptyCpuset(str string) bool {
+	return str == "" || str == "\n"
 }
 
 func (s *CpusetGroup) ensureCpusAndMems(path string, cgroup *configs.Cgroup) error {
 	if err := s.Set(path, cgroup); err != nil {
 		return err
 	}
-	return s.copyIfNeeded(path, filepath.Dir(path))
+	return cpusetCopyIfNeeded(path, filepath.Dir(path))
 }

libcontainer/cgroups/fs/kmem.go

@@ -5,11 +5,11 @@ package fs
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"path/filepath"
 	"strconv"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
+	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"golang.org/x/sys/unix"
 )
 
@@ -42,15 +42,15 @@ func setKernelMemory(path string, kernelMemoryLimit int64) error {
 		// doesn't support it we *must* error out.
 		return errors.New("kernel memory accounting not supported by this kernel")
 	}
-	if err := ioutil.WriteFile(filepath.Join(path, cgroupKernelMemoryLimit), []byte(strconv.FormatInt(kernelMemoryLimit, 10)), 0700); err != nil {
+	if err := fscommon.WriteFile(path, cgroupKernelMemoryLimit, strconv.FormatInt(kernelMemoryLimit, 10)); err != nil {
 		// Check if the error number returned by the syscall is "EBUSY"
 		// The EBUSY signal is returned on attempts to write to the
 		// memory.kmem.limit_in_bytes file if the cgroup has children or
 		// once tasks have been attached to the cgroup
 		if errors.Is(err, unix.EBUSY) {
 			return fmt.Errorf("failed to set %s, because either tasks have already joined this cgroup or it has children", cgroupKernelMemoryLimit)
 		}
-		return fmt.Errorf("failed to write %v to %v: %v", kernelMemoryLimit, cgroupKernelMemoryLimit, err)
+		return err
 	}
 	return nil
 }

libcontainer/cgroups/fs2/create.go

@@ -1,19 +1,17 @@
 package fs2
 
 import (
-	"bytes"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
 
+	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-func supportedControllers(cgroup *configs.Cgroup) ([]byte, error) {
-	const file = UnifiedMountpoint + "/cgroup.controllers"
-	return ioutil.ReadFile(file)
+func supportedControllers(cgroup *configs.Cgroup) (string, error) {
+	return fscommon.ReadFile(UnifiedMountpoint, "/cgroup.controllers")
 }
 
 // needAnyControllers returns whether we enable some supported controllers or not,
@@ -31,7 +29,7 @@ func needAnyControllers(cgroup *configs.Cgroup) (bool, error) {
 		return false, err
 	}
 	avail := make(map[string]struct{})
-	for _, ctr := range strings.Fields(string(content)) {
+	for _, ctr := range strings.Fields(content) {
 		avail[ctr] = struct{}{}
 	}
 
@@ -81,8 +79,12 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
 		return err
 	}
 
-	ctrs := bytes.Fields(content)
-	res := append([]byte("+"), bytes.Join(ctrs, []byte(" +"))...)
+	const (
+		cgTypeFile  = "cgroup.type"
+		cgStCtlFile = "cgroup.subtree_control"
+	)
+	ctrs := strings.Fields(content)
+	res := "+" + strings.Join(ctrs, " +")
 
 	elements := strings.Split(path, "/")
 	elements = elements[3:]
@@ -103,9 +105,9 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
 					}
 				}()
 			}
-			cgTypeFile := filepath.Join(current, "cgroup.type")
-			cgType, _ := ioutil.ReadFile(cgTypeFile)
-			switch string(bytes.TrimSpace(cgType)) {
+			cgType, _ := fscommon.ReadFile(current, cgTypeFile)
+			cgType = strings.TrimSpace(cgType)
+			switch cgType {
 			// If the cgroup is in an invalid mode (usually this means there's an internal
 			// process in the cgroup tree, because we created a cgroup under an
 			// already-populated-by-other-processes cgroup), then we have to error out if
@@ -120,26 +122,25 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
 					// since that means we're a properly delegated cgroup subtree) but in
 					// this case there's not much we can do and it's better than giving an
 					// error.
-					_ = ioutil.WriteFile(cgTypeFile, []byte("threaded"), 0644)
+					_ = fscommon.WriteFile(current, cgTypeFile, "threaded")
 				}
 			// If the cgroup is in (threaded) or (domain threaded) mode, we can only use thread-aware controllers
 			// (and you cannot usually take a cgroup out of threaded mode).
 			case "domain threaded":
 				fallthrough
 			case "threaded":
 				if containsDomainController(c) {
-					return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in %s mode", current, string(bytes.TrimSpace(cgType)))
+					return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in %s mode", current, cgType)
 				}
 			}
 		}
 		// enable all supported controllers
 		if i < len(elements)-1 {
-			file := filepath.Join(current, "cgroup.subtree_control")
-			if err := ioutil.WriteFile(file, res, 0644); err != nil {
+			if err := fscommon.WriteFile(current, cgStCtlFile, res); err != nil {
 				// try write one by one
-				allCtrs := bytes.Split(res, []byte(" "))
+				allCtrs := strings.Split(res, " ")
 				for _, ctr := range allCtrs {
-					_ = ioutil.WriteFile(file, ctr, 0644)
+					_ = fscommon.WriteFile(current, cgStCtlFile, ctr)
 				}
 			}
 			// Some controllers might not be enabled when rootless or containerized,

libcontainer/cgroups/fs2/fs2.go

@@ -4,9 +4,7 @@ package fs2
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
@@ -53,15 +51,14 @@ func (m *manager) getControllers() error {
 		return nil
 	}
 
-	file := filepath.Join(m.dirPath, "cgroup.controllers")
-	data, err := ioutil.ReadFile(file)
+	data, err := fscommon.ReadFile(m.dirPath, "cgroup.controllers")
 	if err != nil {
 		if m.rootless && m.config.Path == "" {
 			return nil
 		}
 		return err
 	}
-	fields := strings.Fields(string(data))
+	fields := strings.Fields(data)
 	m.controllers = make(map[string]struct{}, len(fields))
 	for _, c := range fields {
 		m.controllers[c] = struct{}{}

libcontainer/cgroups/fs2/hugetlb.go

@@ -3,8 +3,6 @@
 package fs2
 
 import (
-	"io/ioutil"
-	"path/filepath"
 	"strconv"
 
 	"github.com/pkg/errors"
@@ -46,12 +44,11 @@ func statHugeTlb(dirPath string, stats *cgroups.Stats) error {
 		hugetlbStats.Usage = value
 
 		fileName := "hugetlb." + pagesize + ".events"
-		filePath := filepath.Join(dirPath, fileName)
-		contents, err := ioutil.ReadFile(filePath)
+		contents, err := fscommon.ReadFile(dirPath, fileName)
 		if err != nil {
 			return errors.Wrap(err, "failed to read stats")
 		}
-		_, value, err = fscommon.GetCgroupParamKeyValue(string(contents))
+		_, value, err = fscommon.GetCgroupParamKeyValue(contents)
 		if err != nil {
 			return errors.Wrap(err, "failed to parse "+fileName)
 		}

libcontainer/cgroups/fs2/pids.go

@@ -3,7 +3,6 @@
 package fs2
 
 import (
-	"io/ioutil"
 	"path/filepath"
 	"strings"
 
@@ -34,15 +33,15 @@ func setPids(dirPath string, cgroup *configs.Cgroup) error {
 func statPidsWithoutController(dirPath string, stats *cgroups.Stats) error {
 	// if the controller is not enabled, let's read PIDS from cgroups.procs
 	// (or threads if cgroup.threads is enabled)
-	contents, err := ioutil.ReadFile(filepath.Join(dirPath, "cgroup.procs"))
+	contents, err := fscommon.ReadFile(dirPath, "cgroup.procs")
 	if errors.Is(err, unix.ENOTSUP) {
-		contents, err = ioutil.ReadFile(filepath.Join(dirPath, "cgroup.threads"))
+		contents, err = fscommon.ReadFile(dirPath, "cgroup.threads")
 	}
 	if err != nil {
 		return err
 	}
 	pids := make(map[string]string)
-	for _, i := range strings.Split(string(contents), "\n") {
+	for _, i := range strings.Split(contents, "\n") {
 		if i != "" {
 			pids[i] = i
 		}

libcontainer/cgroups/fscommon/fscommon_test.go

@@ -9,18 +9,16 @@ import (
 	"strconv"
 	"testing"
 	"time"
-
-	"github.com/opencontainers/runc/libcontainer/cgroups"
 )
 
 func TestWriteCgroupFileHandlesInterrupt(t *testing.T) {
-	if cgroups.IsCgroup2UnifiedMode() {
-		t.Skip("cgroup v2 is not supported")
-	}
-
-	memoryCgroupMount, err := cgroups.FindCgroupMountpoint("", "memory")
-	if err != nil {
-		t.Fatal(err)
+	const (
+		memoryCgroupMount = "/sys/fs/cgroup/memory"
+		memoryLimit       = "memory.limit_in_bytes"
+	)
+	if _, err := os.Stat(memoryCgroupMount); err != nil {
+		// most probably cgroupv2
+		t.Skip(err)
 	}
 
 	cgroupName := fmt.Sprintf("test-eint-%d", time.Now().Nanosecond())
@@ -30,9 +28,14 @@ func TestWriteCgroupFileHandlesInterrupt(t *testing.T) {
 	}
 	defer os.RemoveAll(cgroupPath)
 
+	if _, err := os.Stat(filepath.Join(cgroupPath, memoryLimit)); err != nil {
+		// either cgroupv2, or memory controller is not avalable
+		t.Skip(err)
+	}
+
 	for i := 0; i < 100000; i++ {
 		limit := 1024*1024 + i
-		if err := WriteFile(cgroupPath, "memory.limit_in_bytes", strconv.Itoa(limit)); err != nil {
+		if err := WriteFile(cgroupPath, memoryLimit, strconv.Itoa(limit)); err != nil {
 			t.Fatalf("Failed to write %d on attempt %d: %+v", limit, i, err)
 		}
 	}