From 2ec937dbfb62d8fa673a16e73db0fa21eb042d3b Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Wed, 25 Sep 2024 14:47:54 +0800 Subject: [PATCH 1/4] add CONFIG_CGROUP_BPF to required kernel config - cgroup v2: check `/sys/fs/cgroup/init.scope/cgroup.freeze` file for freezer support --- validators/cgroup_validator_linux.go | 63 ++++++++++++++++++++++++---- validators/types_unix.go | 7 ++-- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/validators/cgroup_validator_linux.go b/validators/cgroup_validator_linux.go index 23f49b3..62e7693 100644 --- a/validators/cgroup_validator_linux.go +++ b/validators/cgroup_validator_linux.go @@ -31,7 +31,10 @@ import ( "golang.org/x/sys/unix" ) -var _ Validator = &CgroupsValidator{} +var ( + _ Validator = &CgroupsValidator{} + unifiedMountpoint = getUnifiedMountpoint() +) // CgroupsValidator validates cgroup configuration. type CgroupsValidator struct { @@ -44,10 +47,30 @@ func (c *CgroupsValidator) Name() string { } const ( - cgroupsConfigPrefix = "CGROUPS_" - unifiedMountpoint = "/sys/fs/cgroup" + cgroupsConfigPrefix = "CGROUPS_" + defaultUnifiedMountpoint = "/sys/fs/cgroup" + mountsFilePath = "/proc/mounts" ) +// getUnifiedMountpoint will check /proc/mounts and return the cgroup dir +func getUnifiedMountpoint() string { + f, err := os.Open(mountsFilePath) + if err != nil { + fmt.Printf("error checking %q: %v\n", mountsFilePath, err) + } else { + defer f.Close() + scanner := bufio.NewScanner(f) + for scanner.Scan() { + // example fields: `cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0` + fields := strings.Split(scanner.Text(), " ") + if len(fields) >= 3 && (fields[2] == "cgroup2" || fields[2] == "cgroup") { + return fields[1] + } + } + } + return defaultUnifiedMountpoint +} + // Validate is part of the system.Validator interface. func (c *CgroupsValidator) Validate(spec SysSpec) (warns, errs []error) { // Get the subsystems from /sys/fs/cgroup/cgroup.controllers when cgroup v2 is used. @@ -142,15 +165,39 @@ func (c *CgroupsValidator) getCgroupV2Subsystems() ([]string, error) { // Some controllers are implicitly enabled by the kernel. // Those controllers do not appear in /sys/fs/cgroup/cgroup.controllers. // https://github.com/torvalds/linux/blob/v5.3/kernel/cgroup/cgroup.c#L433-L434 - // We assume these are always available, as it is hard to detect availability. - // So, we hardcode the following as "pseudo" controllers. + // For freezer, we use checkCgroupV2Freeze() to check. + // For others, we assume these are always available, as it is hard to detect availability. + // We hardcode the following as initial controllers. // - devices: implemented in kernel 4.15 - // - freezer: implemented in kernel 5.2 - pseudo := []string{"devices", "freezer"} + subsystems := []string{"devices"} + if checkCgroupV2Freeze() { + subsystems = append(subsystems, "freezer") + } data, err := ioutil.ReadFile(filepath.Join(unifiedMountpoint, "cgroup.controllers")) if err != nil { return nil, err } - subsystems := append(pseudo, strings.Fields(string(data))...) + subsystems = append(subsystems, strings.Fields(string(data))...) return subsystems, nil } + +// For freezer which is implemented in kernel 5.2, we can check the existence of `cgroup.freeze`. +func checkCgroupV2Freeze() bool { + tmpDir, err := os.MkdirTemp(unifiedMountpoint, "freezer-test") + if err != nil { + fmt.Printf("error mkdir under %q: %v\n", unifiedMountpoint, err) + return false + } + defer func() { + err := os.RemoveAll(tmpDir) + if err != nil { + fmt.Printf("error remove dir %q: %v\n", tmpDir, err) + } + }() + _, err = os.Stat(filepath.Join(tmpDir, "/cgroup.freeze")) + if err == nil { + return true + } + fmt.Printf("no cgroup.freeze under %q: %v\n", tmpDir, err) + return false +} diff --git a/validators/types_unix.go b/validators/types_unix.go index dcd9d4d..ed16c75 100644 --- a/validators/types_unix.go +++ b/validators/types_unix.go @@ -40,11 +40,12 @@ var DefaultSysSpec = SysSpec{ {Name: "IPC_NS"}, {Name: "UTS_NS"}, {Name: "CGROUPS"}, - {Name: "CGROUP_CPUACCT"}, + {Name: "CGROUP_BPF"}, // cgroups v2 + {Name: "CGROUP_CPUACCT"}, // cgroups v1 cpuacct {Name: "CGROUP_DEVICE"}, - {Name: "CGROUP_FREEZER"}, + {Name: "CGROUP_FREEZER"}, // cgroups v1 freezer {Name: "CGROUP_PIDS"}, - {Name: "CGROUP_SCHED"}, + {Name: "CGROUP_SCHED"}, // cgroups v1 & v2 cpu {Name: "CPUSETS"}, {Name: "MEMCG"}, {Name: "INET"}, From e3026021bb212c642019aab3b121be718730271a Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Thu, 10 Oct 2024 14:04:06 +0800 Subject: [PATCH 2/4] refactor getUnifiedMountpoint --- validators/cgroup_validator_linux.go | 41 +++++++++++++++------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/validators/cgroup_validator_linux.go b/validators/cgroup_validator_linux.go index 62e7693..157a96b 100644 --- a/validators/cgroup_validator_linux.go +++ b/validators/cgroup_validator_linux.go @@ -57,15 +57,16 @@ func getUnifiedMountpoint() string { f, err := os.Open(mountsFilePath) if err != nil { fmt.Printf("error checking %q: %v\n", mountsFilePath, err) - } else { - defer f.Close() - scanner := bufio.NewScanner(f) - for scanner.Scan() { - // example fields: `cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0` - fields := strings.Split(scanner.Text(), " ") - if len(fields) >= 3 && (fields[2] == "cgroup2" || fields[2] == "cgroup") { - return fields[1] - } + return defaultUnifiedMountpoint + } + + defer f.Close() + scanner := bufio.NewScanner(f) + for scanner.Scan() { + // example fields: `cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0` + fields := strings.Split(scanner.Text(), " ") + if len(fields) >= 3 && (fields[2] == "cgroup2" || fields[2] == "cgroup") { + return fields[1] } } return defaultUnifiedMountpoint @@ -170,7 +171,11 @@ func (c *CgroupsValidator) getCgroupV2Subsystems() ([]string, error) { // We hardcode the following as initial controllers. // - devices: implemented in kernel 4.15 subsystems := []string{"devices"} - if checkCgroupV2Freeze() { + freezeSupported, err := checkCgroupV2Freeze() + if err != nil { + fmt.Printf("error checking cgroups v2 freeze: %v\n", err) + } + if freezeSupported { subsystems = append(subsystems, "freezer") } data, err := ioutil.ReadFile(filepath.Join(unifiedMountpoint, "cgroup.controllers")) @@ -181,23 +186,21 @@ func (c *CgroupsValidator) getCgroupV2Subsystems() ([]string, error) { return subsystems, nil } -// For freezer which is implemented in kernel 5.2, we can check the existence of `cgroup.freeze`. -func checkCgroupV2Freeze() bool { +// To check if the freezer controller is enabled in Linux kernels 5.2, we can check the existence of cgroup.freeze. +func checkCgroupV2Freeze() (bool, error) { tmpDir, err := os.MkdirTemp(unifiedMountpoint, "freezer-test") if err != nil { - fmt.Printf("error mkdir under %q: %v\n", unifiedMountpoint, err) - return false + return false, err } defer func() { err := os.RemoveAll(tmpDir) if err != nil { - fmt.Printf("error remove dir %q: %v\n", tmpDir, err) + fmt.Printf("error removing directory %q: %v\n", tmpDir, err) } }() - _, err = os.Stat(filepath.Join(tmpDir, "/cgroup.freeze")) + _, err = os.Stat(filepath.Join(tmpDir, "cgroup.freeze")) if err == nil { - return true + return true, nil } - fmt.Printf("no cgroup.freeze under %q: %v\n", tmpDir, err) - return false + return false, err } From 0b104056af1c46fa15391b4b58525226071706c4 Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Tue, 15 Oct 2024 11:04:04 +0800 Subject: [PATCH 3/4] address Kenta's comments Signed-off-by: Paco Xu --- validators/cgroup_validator_linux.go | 31 ++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/validators/cgroup_validator_linux.go b/validators/cgroup_validator_linux.go index 157a96b..2d35a52 100644 --- a/validators/cgroup_validator_linux.go +++ b/validators/cgroup_validator_linux.go @@ -62,13 +62,30 @@ func getUnifiedMountpoint() string { defer f.Close() scanner := bufio.NewScanner(f) + var cgroupv1MountPoint string for scanner.Scan() { // example fields: `cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0` - fields := strings.Split(scanner.Text(), " ") - if len(fields) >= 3 && (fields[2] == "cgroup2" || fields[2] == "cgroup") { - return fields[1] + fields := strings.Fields(scanner.Text()) + if len(fields) >= 3 { + switch fields[2] { + case "cgroup2": + // return the first cgroup v2 mount point directly + return fields[1] + case "cgroup": + // set the first cgroup v1 mount point only, + // and continue the loop to find if there is a cgroup v2 mount point + if len(cgroupv1MountPoint) == 0 { + cgroupv1MountPoint = fields[1] + } + default: + continue + } } } + // return cgroup v1 mount point if no cgroup v2 mount point is found + if len(cgroupv1MountPoint) != 0 { + return cgroupv1MountPoint + } return defaultUnifiedMountpoint } @@ -199,8 +216,10 @@ func checkCgroupV2Freeze() (bool, error) { } }() _, err = os.Stat(filepath.Join(tmpDir, "cgroup.freeze")) - if err == nil { - return true, nil + if os.IsNotExist(err) { + return false, nil + } else if err != nil { + fmt.Printf("error stat cgroup.freeze file under %q: %v\n", tmpDir, err) } - return false, err + return true, nil } From 4a1c1a32a315386db5973e8b62c2b8b4365c2903 Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Tue, 15 Oct 2024 12:07:12 +0800 Subject: [PATCH 4/4] parse /proc/mounts for the first cgroup path and add a UT --- validators/cgroup_validator_linux.go | 112 ++++++++++++++------------- validators/cgroup_validator_test.go | 73 ++++++++++++++++- validators/types_unix.go | 2 +- 3 files changed, 132 insertions(+), 55 deletions(-) diff --git a/validators/cgroup_validator_linux.go b/validators/cgroup_validator_linux.go index 2d35a52..8337b75 100644 --- a/validators/cgroup_validator_linux.go +++ b/validators/cgroup_validator_linux.go @@ -31,10 +31,7 @@ import ( "golang.org/x/sys/unix" ) -var ( - _ Validator = &CgroupsValidator{} - unifiedMountpoint = getUnifiedMountpoint() -) +var _ Validator = &CgroupsValidator{} // CgroupsValidator validates cgroup configuration. type CgroupsValidator struct { @@ -47,66 +44,73 @@ func (c *CgroupsValidator) Name() string { } const ( - cgroupsConfigPrefix = "CGROUPS_" - defaultUnifiedMountpoint = "/sys/fs/cgroup" - mountsFilePath = "/proc/mounts" + cgroupsConfigPrefix = "CGROUPS_" + mountsFilePath = "/proc/mounts" ) -// getUnifiedMountpoint will check /proc/mounts and return the cgroup dir -func getUnifiedMountpoint() string { - f, err := os.Open(mountsFilePath) +// getUnifiedMountpoint checks if the default mount point is available. +// If not, it parses the mounts file to find a valid cgroup mount point. +func getUnifiedMountpoint(path string) (string, error) { + f, err := os.Open(path) if err != nil { - fmt.Printf("error checking %q: %v\n", mountsFilePath, err) - return defaultUnifiedMountpoint + return "", err } - defer f.Close() scanner := bufio.NewScanner(f) - var cgroupv1MountPoint string + var cgroupV1MountPoint string for scanner.Scan() { - // example fields: `cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0` - fields := strings.Fields(scanner.Text()) + line := scanner.Text() + if !strings.Contains(line, "cgroup") { + continue + } + // Example fields: `cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0`. + fields := strings.Fields(line) if len(fields) >= 3 { switch fields[2] { case "cgroup2": - // return the first cgroup v2 mount point directly - return fields[1] + // Return the first cgroups v2 mount point directly. + return fields[1], nil case "cgroup": - // set the first cgroup v1 mount point only, - // and continue the loop to find if there is a cgroup v2 mount point - if len(cgroupv1MountPoint) == 0 { - cgroupv1MountPoint = fields[1] + // Set the first cgroups v1 mount point only, + // and continue the loop to find if there is a cgroups v2 mount point. + if len(cgroupV1MountPoint) == 0 { + cgroupV1MountPoint = fields[1] } - default: - continue } } } - // return cgroup v1 mount point if no cgroup v2 mount point is found - if len(cgroupv1MountPoint) != 0 { - return cgroupv1MountPoint + // Return cgroups v1 mount point if no cgroups v2 mount point is found. + if len(cgroupV1MountPoint) != 0 { + return cgroupV1MountPoint, nil } - return defaultUnifiedMountpoint + return "", fmt.Errorf("cannot get a cgroupfs mount point from %q", path) } // Validate is part of the system.Validator interface. func (c *CgroupsValidator) Validate(spec SysSpec) (warns, errs []error) { - // Get the subsystems from /sys/fs/cgroup/cgroup.controllers when cgroup v2 is used. + // Get the subsystems from /sys/fs/cgroup/cgroup.controllers when cgroups v2 is used. // /proc/cgroups is meaningless for v2 // https://github.com/torvalds/linux/blob/v5.3/Documentation/admin-guide/cgroup-v2.rst#deprecated-v1-core-features var st unix.Statfs_t - var err error + unifiedMountpoint, err := getUnifiedMountpoint(mountsFilePath) + if err != nil { + return nil, []error{fmt.Errorf("cannot get a cgroup mount point: %w", err)} + } if err := unix.Statfs(unifiedMountpoint, &st); err != nil { return nil, []error{fmt.Errorf("cannot statfs the cgroupv2 root: %w", err)} } var requiredCgroupSpec []string var optionalCgroupSpec []string var subsystems []string + var warn error if st.Type == unix.CGROUP2_SUPER_MAGIC { - subsystems, err = c.getCgroupV2Subsystems() + subsystems, err, warn = c.getCgroupV2Subsystems(unifiedMountpoint) if err != nil { return nil, []error{fmt.Errorf("failed to get cgroups v2 subsystems: %w", err)} } + if warn != nil { + warns = append(warns, warn) + } requiredCgroupSpec = spec.CgroupsV2 optionalCgroupSpec = spec.CgroupsV2Optional } else { @@ -151,11 +155,10 @@ func (c *CgroupsValidator) validateCgroupSubsystems(cgroups, subsystems []string missing = append(missing, cgroup) } return missing - } func (c *CgroupsValidator) getCgroupV1Subsystems() ([]string, error) { - // Get the subsystems from /proc/cgroups when cgroup v1 is used. + // Get the subsystems from /proc/cgroups when cgroups v1 is used. f, err := os.Open("/proc/cgroups") if err != nil { return nil, err @@ -179,47 +182,50 @@ func (c *CgroupsValidator) getCgroupV1Subsystems() ([]string, error) { return subsystems, nil } -func (c *CgroupsValidator) getCgroupV2Subsystems() ([]string, error) { +func (c *CgroupsValidator) getCgroupV2Subsystems(unifiedMountpoint string) ([]string, error, error) { // Some controllers are implicitly enabled by the kernel. // Those controllers do not appear in /sys/fs/cgroup/cgroup.controllers. // https://github.com/torvalds/linux/blob/v5.3/kernel/cgroup/cgroup.c#L433-L434 // For freezer, we use checkCgroupV2Freeze() to check. // For others, we assume these are always available, as it is hard to detect availability. // We hardcode the following as initial controllers. - // - devices: implemented in kernel 4.15 + // - devices: implemented in kernel 4.15. subsystems := []string{"devices"} - freezeSupported, err := checkCgroupV2Freeze() - if err != nil { - fmt.Printf("error checking cgroups v2 freeze: %v\n", err) - } + freezeSupported, warn := checkCgroupV2Freeze(unifiedMountpoint) if freezeSupported { subsystems = append(subsystems, "freezer") } data, err := ioutil.ReadFile(filepath.Join(unifiedMountpoint, "cgroup.controllers")) if err != nil { - return nil, err + return nil, err, warn } subsystems = append(subsystems, strings.Fields(string(data))...) - return subsystems, nil + return subsystems, err, warn } -// To check if the freezer controller is enabled in Linux kernels 5.2, we can check the existence of cgroup.freeze. -func checkCgroupV2Freeze() (bool, error) { - tmpDir, err := os.MkdirTemp(unifiedMountpoint, "freezer-test") - if err != nil { - return false, err +// checkCgroupV2Freeze checks if the freezer controller is enabled in Linux kernels 5.2. +// It determines that by creating a cgroup.freeze file under the unified mountpoint location. +func checkCgroupV2Freeze(unifiedMountpoint string) (isCgroupfs bool, warn error) { + const freezeFile = "cgroup.freeze" + tmpDir, warn := os.MkdirTemp(unifiedMountpoint, "freezer-test") + if warn != nil { + return } defer func() { err := os.RemoveAll(tmpDir) if err != nil { - fmt.Printf("error removing directory %q: %v\n", tmpDir, err) + warn = fmt.Errorf("error removing directory %q: %v", tmpDir, err) } }() - _, err = os.Stat(filepath.Join(tmpDir, "cgroup.freeze")) - if os.IsNotExist(err) { - return false, nil - } else if err != nil { - fmt.Printf("error stat cgroup.freeze file under %q: %v\n", tmpDir, err) + _, warn = os.Stat(filepath.Join(tmpDir, freezeFile)) + if os.IsNotExist(warn) { + return + } else if warn != nil { + // If the err is not NotExist error, it means that `cgroup.freeze` exists. + isCgroupfs = true + warn = fmt.Errorf("could not stat %q file in %q: %v", freezeFile, tmpDir, warn) + return } - return true, nil + isCgroupfs = true + return } diff --git a/validators/cgroup_validator_test.go b/validators/cgroup_validator_test.go index dda6b22..bfc7733 100644 --- a/validators/cgroup_validator_test.go +++ b/validators/cgroup_validator_test.go @@ -20,13 +20,14 @@ limitations under the License. package system import ( + "os" "testing" "github.com/stretchr/testify/assert" ) func TestValidateCgroupSubsystem(t *testing.T) { - // hardcoded cgroup v2 subsystems + // hardcoded cgroups v2 subsystems pseudoSubsystems := []string{"devices", "freezer"} v := &CgroupsValidator{ @@ -93,3 +94,73 @@ func TestValidateCgroupSubsystem(t *testing.T) { }) } } + +func TestGetUnifiedMountpoint(t *testing.T) { + tests := map[string]struct { + mountsFileContent string + expectedErr bool + expectedPath string + }{ + "cgroups v2": { + mountsFileContent: "cgroup2 /sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime 0 0", + expectedErr: false, + expectedPath: "/sys/fs/cgroup", + }, + "cgroups v1": { + mountsFileContent: "cgroup /sys/fs/cgroup cgroup rw,seclabel,nosuid,nodev,noexec,relatime 0 0", + expectedErr: false, + expectedPath: "/sys/fs/cgroup", + }, + "empty file": { + mountsFileContent: "", + expectedErr: true, + expectedPath: "", + }, + "no cgroup mounts": { + mountsFileContent: `proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0 +sysfs /sys sysfs rw,seclabel,nosuid,nodev,noexec,relatime 0 0`, + expectedErr: true, + expectedPath: "", + }, + "multiple cgroups v1 and v2": { + mountsFileContent: `cgroup /sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset +cgroup /sys/fs/cgroup/memory cgroup rw,nosuid,nodev,noexec,relatime,memory +cgroup2 /sys/fs/cgroup/unified cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime`, + expectedErr: false, + expectedPath: "/sys/fs/cgroup/unified", + }, + "cgroups v1 only with multiple subsystems": { + mountsFileContent: `cgroup /sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset +cgroup /sys/fs/cgroup/memory cgroup rw,nosuid,nodev,noexec,relatime,memory`, + expectedErr: false, + expectedPath: "/sys/fs/cgroup/cpuset", // First valid cgroups v1 path + }, + "no valid cgroup": { + mountsFileContent: "proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0\nsysfs /sys sysfs rw,seclabel,nosuid,nodev,noexec,relatime 0 0", + expectedErr: true, + expectedPath: "", + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + tmpFile, err := os.CreateTemp("", "mounts") + assert.NoError(t, err, "Unexpected error creating temp file") + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.Write([]byte(test.mountsFileContent)) + assert.NoError(t, err, "Unexpected error writing to temp file") + tmpFile.Close() + + path, err := getUnifiedMountpoint(tmpFile.Name()) + + if test.expectedErr { + assert.Error(t, err, "Expected error but got none") + } else { + assert.NoError(t, err, "Did not expect error but got one: %s", err) + } + + assert.Equal(t, test.expectedPath, path, "Expected cgroup path mismatch") + }) + } +} diff --git a/validators/types_unix.go b/validators/types_unix.go index ed16c75..802c122 100644 --- a/validators/types_unix.go +++ b/validators/types_unix.go @@ -72,7 +72,7 @@ var DefaultSysSpec = SysSpec{ // and therefore lacks corresponding hugetlb cgroup "hugetlb", // The blkio cgroup is optional since some kernels are compiled without support for block I/O throttling. - // Containerd and cri-o will use blkio to track disk I/O and throttling in both cgroup v1 and v2. + // Containerd and cri-o will use blkio to track disk I/O and throttling in both cgroups v1 and v2. "blkio", }, CgroupsV2: []string{"cpu", "cpuset", "devices", "freezer", "memory", "pids"},