From 64d06f925c6892c34fe4b06386c9e071083e0a64 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 20 Oct 2026 16:48:51 +0100 Subject: [PATCH 1/6] cgroups: rename GetSubsystemPath to GetV1SubsystemPath The function looks a controller up by name in /proc//cgroup, which only means something for the cgroup v1 hierarchies; the unified hierarchy has a single entry with an empty controller list. Name it for what it does so that a cgroup v2 lookup can sit next to it. Signed-off-by: Daniel Golle --- cgroups/cgroups.go | 4 ++-- cgroups/cgroups_v1.go | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cgroups/cgroups.go b/cgroups/cgroups.go index 48beba06..3cff7c5b 100644 --- a/cgroups/cgroups.go +++ b/cgroups/cgroups.go @@ -76,8 +76,8 @@ func FindCgroup() (Cgroup, error) { return nil, fmt.Errorf("cgroup is not found") } -// GetSubsystemPath gets path of subsystem -func GetSubsystemPath(pid int, subsystem string) (string, error) { +// GetV1SubsystemPath gets the path of a cgroup v1 subsystem +func GetV1SubsystemPath(pid int, subsystem string) (string, error) { contents, err := os.ReadFile(fmt.Sprintf("/proc/%d/cgroup", pid)) if err != nil { return "", err diff --git a/cgroups/cgroups_v1.go b/cgroups/cgroups_v1.go index 17ae6673..ce31ed41 100644 --- a/cgroups/cgroups_v1.go +++ b/cgroups/cgroups_v1.go @@ -52,7 +52,7 @@ func (cg *CgroupV1) GetBlockIOData(pid int, cgPath string) (*rspec.LinuxBlockIO, fileName := strings.Join([]string{"blkio", name}, ".") filePath := filepath.Join(cg.MountPath, "blkio", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "blkio") + subPath, err := GetV1SubsystemPath(pid, "blkio") if err != nil { return nil, err } @@ -227,7 +227,7 @@ func (cg *CgroupV1) GetCPUData(pid int, cgPath string) (*rspec.LinuxCPU, error) fileName := strings.Join([]string{"cpu", name}, ".") filePath := filepath.Join(cg.MountPath, "cpu", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "cpu") + subPath, err := GetV1SubsystemPath(pid, "cpu") if err != nil { return nil, err } @@ -294,7 +294,7 @@ func (cg *CgroupV1) GetCPUData(pid int, cgPath string) (*rspec.LinuxCPU, error) fileName := strings.Join([]string{"cpuset", name}, ".") filePath := filepath.Join(cg.MountPath, "cpuset", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "cpuset") + subPath, err := GetV1SubsystemPath(pid, "cpuset") if err != nil { return nil, err } @@ -324,7 +324,7 @@ func (cg *CgroupV1) GetDevicesData(pid int, cgPath string) ([]rspec.LinuxDeviceC fileName := strings.Join([]string{"devices", "list"}, ".") filePath := filepath.Join(cg.MountPath, "devices", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "devices") + subPath, err := GetV1SubsystemPath(pid, "devices") if err != nil { return nil, err } @@ -447,7 +447,7 @@ func (cg *CgroupV1) GetHugepageLimitData(pid int, cgPath string) ([]rspec.LinuxH maxUsage := strings.Join([]string{"hugetlb", pageSize, "limit_in_bytes"}, ".") filePath := filepath.Join(cg.MountPath, "hugetlb", cgPath, maxUsage) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "hugetlb") + subPath, err := GetV1SubsystemPath(pid, "hugetlb") if err != nil { return lh, err } @@ -494,7 +494,7 @@ func (cg *CgroupV1) GetMemoryData(pid int, cgPath string) (*rspec.LinuxMemory, e fileName := strings.Join([]string{"memory", name}, ".") filePath := filepath.Join(cg.MountPath, "memory", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "memory") + subPath, err := GetV1SubsystemPath(pid, "memory") if err != nil { return nil, err } @@ -587,7 +587,7 @@ func (cg *CgroupV1) GetNetworkData(pid int, cgPath string) (*rspec.LinuxNetwork, fileName := strings.Join([]string{"net_cls", "classid"}, ".") filePath := filepath.Join(cg.MountPath, "net_cls", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "net_cls") + subPath, err := GetV1SubsystemPath(pid, "net_cls") if err != nil { return nil, err } @@ -614,7 +614,7 @@ func (cg *CgroupV1) GetNetworkData(pid int, cgPath string) (*rspec.LinuxNetwork, fileName = strings.Join([]string{"net_prio", "ifpriomap"}, ".") filePath = filepath.Join(cg.MountPath, "net_prio", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "net_prio") + subPath, err := GetV1SubsystemPath(pid, "net_prio") if err != nil { return nil, err } @@ -658,7 +658,7 @@ func (cg *CgroupV1) GetPidsData(pid int, cgPath string) (*rspec.LinuxPids, error fileName := strings.Join([]string{"pids", "max"}, ".") filePath := filepath.Join(cg.MountPath, "pids", cgPath, fileName) if !filepath.IsAbs(cgPath) { - subPath, err := GetSubsystemPath(pid, "pids") + subPath, err := GetV1SubsystemPath(pid, "pids") if err != nil { return nil, err } From 5fa19284ec617033398c3fa6ded61427b6d284fe Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 20 Oct 2026 16:57:29 +0100 Subject: [PATCH 2/6] cgroups: implement the cgroup v2 reader FindCgroup() refused to work on a host that only mounts the unified hierarchy, and CgroupV2 was a stub whose methods carried no receivers, so the type never satisfied the Cgroup interface. Every validation test that reads resource limits back out of the cgroup filesystem failed with "cgroupv2 is not supported yet" on any current distribution. The unified hierarchy is always mounted at /sys/fs/cgroup, so FindCgroup() recognises it by the filesystem type statfs(2) reports there and only parses mountinfo for the cgroup v1 controllers. GetCgroupV2Path() reads a process's place in the hierarchy from the "0::" entry of /proc//cgroup. Read the unified control files: memory.{max,low,swap.max}, cpu.max, cpuset.{cpus,mems}, pids.max, hugetlb..max and io.max. memory.swap.max limits swap alone while the configuration field covers memory plus swap, so the memory limit is added back. cpuset files fall back to the effective set, which is where the unified hierarchy reports an inherited value. Values the unified hierarchy does not carry are left unset rather than guessed, so that a caller can tell them apart from a real zero. That covers swappiness, disableOOMKiller, useHierarchy, kernel, kernelTCP and the block IO weights, and also cpu.shares, which runtimes map onto cpu.weight through a lossy conversion the runtime-spec does not define. The devices and network controllers have no unified counterpart at all: device access is enforced by an eBPF program that cannot be read back, and net_cls and net_prio were never ported, so both report that rather than a missing file. The tests read and write a real cgroup below /sys/fs/cgroup and are skipped on a host without cgroup v2 or without the permission to create one. Version() lets a caller ask which hierarchy it is talking to. Relates to #807. Signed-off-by: Daniel Golle --- cgroups/cgroups.go | 39 +++- cgroups/cgroups_v1.go | 5 + cgroups/cgroups_v2.go | 277 ++++++++++++++++++++++++++-- cgroups/cgroups_v2_test.go | 368 +++++++++++++++++++++++++++++++++++++ 4 files changed, 666 insertions(+), 23 deletions(-) create mode 100644 cgroups/cgroups_v2_test.go diff --git a/cgroups/cgroups.go b/cgroups/cgroups.go index 3cff7c5b..743b69f9 100644 --- a/cgroups/cgroups.go +++ b/cgroups/cgroups.go @@ -2,6 +2,7 @@ package cgroups import ( "bufio" + "errors" "fmt" "os" "path/filepath" @@ -9,6 +10,7 @@ import ( "strings" rspec "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" ) var ( @@ -20,6 +22,7 @@ var ( // Cgroup represents interfaces for cgroup validation type Cgroup interface { + Version() int GetBlockIOData(pid int, cgPath string) (*rspec.LinuxBlockIO, error) GetCPUData(pid int, cgPath string) (*rspec.LinuxCPU, error) GetDevicesData(pid int, cgPath string) ([]rspec.LinuxDeviceCgroup, error) @@ -31,13 +34,18 @@ type Cgroup interface { // FindCgroup gets cgroup root mountpoint func FindCgroup() (Cgroup, error) { + var st unix.Statfs_t + + if err := unix.Statfs(unifiedPath, &st); err == nil && st.Type == unix.CGROUP2_SUPER_MAGIC { + return &CgroupV2{}, nil + } + f, err := os.Open("/proc/self/mountinfo") if err != nil { return nil, err } defer f.Close() - cgroupv2 := false scanner := bufio.NewScanner(f) for scanner.Scan() { text := scanner.Text() @@ -59,10 +67,6 @@ func FindCgroup() (Cgroup, error) { MountPath: filepath.Dir(fields[4]), } return cg, nil - } else if postSeparatorFields[0] == "cgroup2" { - cgroupv2 = true - continue - // TODO cgroupv2 unimplemented } } @@ -70,9 +74,6 @@ func FindCgroup() (Cgroup, error) { return nil, err } - if cgroupv2 { - return nil, fmt.Errorf("cgroupv2 is not supported yet") - } return nil, fmt.Errorf("cgroup is not found") } @@ -97,3 +98,25 @@ func GetV1SubsystemPath(pid int, subsystem string) (string, error) { return "", fmt.Errorf("subsystem %s not found", subsystem) } + +// GetCgroupV2Path gets the path of a process in the unified hierarchy +func GetCgroupV2Path(pid int) (string, error) { + f, err := os.Open(fmt.Sprintf("/proc/%d/cgroup", pid)) + if err != nil { + return "", err + } + defer f.Close() + + s := bufio.NewScanner(f) + for s.Scan() { + // "0::/user.slice/user-1001.slice/session-1.scope" + if path, ok := strings.CutPrefix(s.Text(), "0::"); ok { + return path, nil + } + } + if err := s.Err(); err != nil { + return "", err + } + + return "", errors.New("cgroup v2 path not found") +} diff --git a/cgroups/cgroups_v1.go b/cgroups/cgroups_v1.go index ce31ed41..dce24b0a 100644 --- a/cgroups/cgroups_v1.go +++ b/cgroups/cgroups_v1.go @@ -17,6 +17,11 @@ type CgroupV1 struct { MountPath string } +// Version returns the hierarchy version this implementation reads +func (cg *CgroupV1) Version() int { + return 1 +} + // HugePageSizeUnitList is a list of the units used by the linux kernel when // naming the HugePage control files. // https://www.kernel.org/doc/Documentation/cgroup-v1/hugetlb.txt diff --git a/cgroups/cgroups_v2.go b/cgroups/cgroups_v2.go index d15e647d..adeaac12 100644 --- a/cgroups/cgroups_v2.go +++ b/cgroups/cgroups_v2.go @@ -2,46 +2,293 @@ package cgroups import ( "fmt" + "math" + "os" + "path/filepath" + "strconv" + "strings" rspec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/opencontainers/runtime-tools/specerror" ) // CgroupV2 used for cgroupv2 validation -type CgroupV2 struct { - MountPath string +type CgroupV2 struct{} + +// unifiedPath is where the unified hierarchy is mounted. +const unifiedPath = "/sys/fs/cgroup" + +// unlimited is how the unified hierarchy spells "no limit". +const unlimited = "max" + +func attachError() error { + return specerror.NewError(specerror.CgroupsPathAttach, fmt.Errorf("The runtime MUST consistently attach to the same place in the cgroups hierarchy given the same value of `cgroupsPath`"), rspec.Version) +} + +func parseLimit(value string) (int64, error) { + if value == unlimited { + return -1, nil + } + return strconv.ParseInt(value, 10, 64) +} + +// Version returns the hierarchy version this implementation reads +func (cg *CgroupV2) Version() int { + return 2 +} + +func (cg *CgroupV2) dir(pid int, cgPath string) (string, error) { + if filepath.IsAbs(cgPath) { + path := filepath.Join(unifiedPath, cgPath) + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + return "", specerror.NewError(specerror.CgroupsAbsPathRelToMount, fmt.Errorf("In the case of an absolute path, the runtime MUST take the path to be relative to the cgroups mount point"), rspec.Version) + } + return "", err + } + return path, nil + } + + subPath, err := GetCgroupV2Path(pid) + if err != nil { + return "", err + } + if !strings.Contains(subPath, cgPath) { + return "", fmt.Errorf("cgroup %s is not mounted as expected", cgPath) + } + + return filepath.Join(unifiedPath, subPath), nil +} + +// readValue returns the trimmed contents of a control file, or an empty +// string if the controller does not provide it. +func (cg *CgroupV2) readValue(pid int, cgPath string, name string) (string, error) { + dir, err := cg.dir(pid, cgPath) + if err != nil { + return "", err + } + + contents, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + if os.IsNotExist(err) { + return "", nil + } + return "", err + } + + return strings.TrimSpace(string(contents)), nil +} + +func (cg *CgroupV2) readLimit(pid int, cgPath string, name string) (int64, error) { + value, err := cg.readValue(pid, cgPath, name) + if err != nil { + return 0, err + } + if value == "" { + return 0, attachError() + } + + return parseLimit(value) +} + +// readCPUSet falls back to the effective set because the unified hierarchy +// leaves the configured file empty while the cgroup inherits its parent. +func (cg *CgroupV2) readCPUSet(pid int, cgPath string, name string) (string, error) { + value, err := cg.readValue(pid, cgPath, name) + if err != nil || value != "" { + return value, err + } + + return cg.readValue(pid, cgPath, name+".effective") } // GetBlockIOData gets cgroup blockio data -func GetBlockIOData(pid int, cgPath string) (*rspec.LinuxBlockIO, error) { - return nil, fmt.Errorf("unimplemented yet") +func (cg *CgroupV2) GetBlockIOData(pid int, cgPath string) (*rspec.LinuxBlockIO, error) { + value, err := cg.readValue(pid, cgPath, "io.max") + if err != nil { + return nil, err + } + + lb := &rspec.LinuxBlockIO{} + if value == "" { + return lb, nil + } + + for _, line := range strings.Split(value, "\n") { + if err := addThrottleDevices(lb, line); err != nil { + return nil, err + } + } + + return lb, nil +} + +func addThrottleDevices(lb *rspec.LinuxBlockIO, line string) error { + fields := strings.Fields(line) + if len(fields) < 2 { + return nil + } + + major, minor, err := getDeviceID(fields[0]) + if err != nil { + return err + } + + for _, field := range fields[1:] { + key, value, found := strings.Cut(field, "=") + if !found || value == unlimited { + continue + } + rate, err := strconv.ParseUint(value, 10, 64) + if err != nil { + return err + } + ltd := rspec.LinuxThrottleDevice{} + ltd.Major = major + ltd.Minor = minor + ltd.Rate = rate + switch key { + case "rbps": + lb.ThrottleReadBpsDevice = append(lb.ThrottleReadBpsDevice, ltd) + case "wbps": + lb.ThrottleWriteBpsDevice = append(lb.ThrottleWriteBpsDevice, ltd) + case "riops": + lb.ThrottleReadIOPSDevice = append(lb.ThrottleReadIOPSDevice, ltd) + case "wiops": + lb.ThrottleWriteIOPSDevice = append(lb.ThrottleWriteIOPSDevice, ltd) + } + } + + return nil } // GetCPUData gets cgroup cpus data -func GetCPUData(pid int, cgPath string) (*rspec.LinuxCPU, error) { - return nil, fmt.Errorf("unimplemented yet") +func (cg *CgroupV2) GetCPUData(pid int, cgPath string) (*rspec.LinuxCPU, error) { + value, err := cg.readValue(pid, cgPath, "cpu.max") + if err != nil { + return nil, err + } + if value == "" { + return nil, attachError() + } + fields := strings.Fields(value) + if len(fields) != 2 { + return nil, fmt.Errorf("unexpected cpu.max content %q", value) + } + quota, err := parseLimit(fields[0]) + if err != nil { + return nil, err + } + period, err := strconv.ParseUint(fields[1], 10, 64) + if err != nil { + return nil, err + } + + cpus, err := cg.readCPUSet(pid, cgPath, "cpuset.cpus") + if err != nil { + return nil, err + } + mems, err := cg.readCPUSet(pid, cgPath, "cpuset.mems") + if err != nil { + return nil, err + } + + // cpu.shares has no unified equivalent: runtimes map it onto cpu.weight + // with a lossy conversion that the runtime-spec does not define, so the + // requested share cannot be recovered. + return &rspec.LinuxCPU{ + Quota: "a, + Period: &period, + Cpus: cpus, + Mems: mems, + }, nil } // GetDevicesData gets cgroup devices data -func GetDevicesData(pid int, cgPath string) ([]rspec.LinuxDeviceCgroup, error) { - return nil, fmt.Errorf("unimplemented yet") +func (cg *CgroupV2) GetDevicesData(pid int, cgPath string) ([]rspec.LinuxDeviceCgroup, error) { + return nil, fmt.Errorf("the unified hierarchy has no devices controller, access is governed by an eBPF program that cannot be read back") } // GetHugepageLimitData gets cgroup hugetlb data -func GetHugepageLimitData(pid int, cgPath string) ([]rspec.LinuxHugepageLimit, error) { - return nil, fmt.Errorf("unimplemented yet") +func (cg *CgroupV2) GetHugepageLimitData(pid int, cgPath string) ([]rspec.LinuxHugepageLimit, error) { + pageSizes, err := GetHugePageSize() + if err != nil { + return nil, err + } + + lh := []rspec.LinuxHugepageLimit{} + for _, pageSize := range pageSizes { + value, err := cg.readValue(pid, cgPath, strings.Join([]string{"hugetlb", pageSize, "max"}, ".")) + if err != nil { + return nil, err + } + if value == "" { + continue + } + limit := uint64(math.MaxUint64) + if value != unlimited { + limit, err = strconv.ParseUint(value, 10, 64) + if err != nil { + return nil, err + } + } + pageLimit := rspec.LinuxHugepageLimit{} + pageLimit.Pagesize = pageSize + pageLimit.Limit = limit + lh = append(lh, pageLimit) + } + + return lh, nil } // GetMemoryData gets cgroup memory data func (cg *CgroupV2) GetMemoryData(pid int, cgPath string) (*rspec.LinuxMemory, error) { - return nil, fmt.Errorf("unimplemented yet") + limit, err := cg.readLimit(pid, cgPath, "memory.max") + if err != nil { + return nil, err + } + reservation, err := cg.readLimit(pid, cgPath, "memory.low") + if err != nil { + return nil, err + } + + lm := &rspec.LinuxMemory{ + Limit: &limit, + Reservation: &reservation, + } + + value, err := cg.readValue(pid, cgPath, "memory.swap.max") + if err != nil { + return nil, err + } + if value == "" { + return lm, nil + } + swap, err := parseLimit(value) + if err != nil { + return nil, err + } + // memory.swap.max limits swap alone while memory.swap is memory plus + // swap, so the memory limit has to be added back. + if swap >= 0 && limit >= 0 { + swap += limit + } + lm.Swap = &swap + + return lm, nil } // GetNetworkData gets cgroup network data -func GetNetworkData(pid int, cgPath string) (*rspec.LinuxNetwork, error) { - return nil, fmt.Errorf("unimplemented yet") +func (cg *CgroupV2) GetNetworkData(pid int, cgPath string) (*rspec.LinuxNetwork, error) { + return nil, fmt.Errorf("the unified hierarchy has no net_cls or net_prio controller") } // GetPidsData gets cgroup pid ints data -func GetPidsData(pid int, cgPath string) (*rspec.LinuxPids, error) { - return nil, fmt.Errorf("unimplemented yet") +func (cg *CgroupV2) GetPidsData(pid int, cgPath string) (*rspec.LinuxPids, error) { + limit, err := cg.readLimit(pid, cgPath, "pids.max") + if err != nil { + return nil, err + } + + return &rspec.LinuxPids{Limit: &limit}, nil } diff --git a/cgroups/cgroups_v2_test.go b/cgroups/cgroups_v2_test.go new file mode 100644 index 00000000..66fdd56a --- /dev/null +++ b/cgroups/cgroups_v2_test.go @@ -0,0 +1,368 @@ +package cgroups + +import ( + "errors" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "golang.org/x/sys/unix" +) + +var _ Cgroup = (*CgroupV2)(nil) + +func requireCgroupV2(t *testing.T) { + t.Helper() + + var st unix.Statfs_t + if err := unix.Statfs(unifiedPath, &st); err != nil || st.Type != unix.CGROUP2_SUPER_MAGIC { + t.Skip("cgroup v2 is not mounted at " + unifiedPath) + } +} + +// newTestCgroup creates a cgroup right below the root of the unified +// hierarchy, where the controllers are enabled, and returns it as an +// absolute cgroupsPath. +func newTestCgroup(t *testing.T, controllers ...string) string { + t.Helper() + requireCgroupV2(t) + + subtreeControl := filepath.Join(unifiedPath, "cgroup.subtree_control") + enabled, err := os.ReadFile(subtreeControl) + if err != nil { + t.Skip(err) + } + for _, controller := range controllers { + controller := controller + if slices.Contains(strings.Fields(string(enabled)), controller) { + continue + } + if err := os.WriteFile(subtreeControl, []byte("+"+controller), 0o644); err != nil { + t.Skipf("cannot enable the %s controller at %s: %v", controller, unifiedPath, err) + } + t.Cleanup(func() { + if err := os.WriteFile(subtreeControl, []byte("-"+controller), 0o644); err != nil { + t.Logf("cannot disable the %s controller again: %v", controller, err) + } + }) + } + + dir, err := os.MkdirTemp(unifiedPath, "runtime-tools-test-") + if errors.Is(err, os.ErrPermission) || errors.Is(err, unix.EROFS) { + t.Skipf("cannot create a cgroup: %v", err) + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.Remove(dir); err != nil { + t.Error(err) + } + }) + + return "/" + filepath.Base(dir) +} + +func writeControl(t *testing.T, cgPath string, name string, value string) { + t.Helper() + + if err := os.WriteFile(filepath.Join(unifiedPath, cgPath, name), []byte(value), 0o644); err != nil { + t.Fatal(err) + } +} + +func readControl(t *testing.T, cgPath string, name string) string { + t.Helper() + + contents, err := os.ReadFile(filepath.Join(unifiedPath, cgPath, name)) + if err != nil { + t.Fatal(err) + } + + return strings.TrimSpace(string(contents)) +} + +func TestCgroupV2Version(t *testing.T) { + cg := &CgroupV2{} + if cg.Version() != 2 { + t.Errorf("Version() == %d, expected 2", cg.Version()) + } +} + +func TestFindCgroupV2(t *testing.T) { + requireCgroupV2(t) + + cg, err := FindCgroup() + if err != nil { + t.Fatal(err) + } + if cg.Version() != 2 { + t.Errorf("FindCgroup() found a version %d hierarchy, expected 2", cg.Version()) + } +} + +func TestGetCgroupV2Path(t *testing.T) { + requireCgroupV2(t) + + path, err := GetCgroupV2Path(os.Getpid()) + if err != nil { + t.Fatal(err) + } + if !filepath.IsAbs(path) { + t.Fatalf("cgroup path %q is not absolute", path) + } + if _, err := os.Stat(filepath.Join(unifiedPath, path)); err != nil { + t.Error(err) + } +} + +func TestCgroupV2RelativePath(t *testing.T) { + requireCgroupV2(t) + + path, err := GetCgroupV2Path(os.Getpid()) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(unifiedPath, path, "pids.max")); err != nil { + t.Skipf("the pids controller is not enabled for %s", path) + } + + cg := &CgroupV2{} + lp, err := cg.GetPidsData(os.Getpid(), filepath.Base(path)) + if err != nil { + t.Fatal(err) + } + expected, err := parseLimit(readControl(t, path, "pids.max")) + if err != nil { + t.Fatal(err) + } + if *lp.Limit != expected { + t.Errorf("limit == %d, expected %d", *lp.Limit, expected) + } +} + +func TestCgroupV2MissingCgroup(t *testing.T) { + requireCgroupV2(t) + + cg := &CgroupV2{} + if _, err := cg.GetPidsData(0, "/runtime-tools-test-missing"); err == nil { + t.Error("expected an error for a cgroup that was never created") + } +} + +func TestCgroupV2GetPidsData(t *testing.T) { + cgPath := newTestCgroup(t, "pids") + cg := &CgroupV2{} + + for _, test := range []struct { + value string + expected int64 + }{ + {value: "1000", expected: 1000}, + {value: "max", expected: -1}, + } { + writeControl(t, cgPath, "pids.max", test.value) + lp, err := cg.GetPidsData(0, cgPath) + if err != nil { + t.Fatal(err) + } + if *lp.Limit != test.expected { + t.Errorf("pids.max %q: limit == %d, expected %d", test.value, *lp.Limit, test.expected) + } + } +} + +func TestCgroupV2GetMemoryData(t *testing.T) { + cgPath := newTestCgroup(t, "memory") + cg := &CgroupV2{} + _, err := os.Stat(filepath.Join(unifiedPath, cgPath, "memory.swap.max")) + swapAccounting := err == nil + + for _, test := range []struct { + description string + max string + low string + swapMax string + limit int64 + reservation int64 + swap int64 + }{ + { + description: "limits in bytes", + max: "50593792", + low: "33554432", + swapMax: "16515072", + limit: 50593792, + reservation: 33554432, + swap: 67108864, + }, + { + description: "unlimited memory and swap", + max: "max", + low: "0", + swapMax: "max", + limit: -1, + reservation: 0, + swap: -1, + }, + } { + writeControl(t, cgPath, "memory.max", test.max) + writeControl(t, cgPath, "memory.low", test.low) + if swapAccounting { + writeControl(t, cgPath, "memory.swap.max", test.swapMax) + } + + lm, err := cg.GetMemoryData(0, cgPath) + if err != nil { + t.Fatalf("%s: %v", test.description, err) + } + if *lm.Limit != test.limit { + t.Errorf("%s: limit == %d, expected %d", test.description, *lm.Limit, test.limit) + } + if *lm.Reservation != test.reservation { + t.Errorf("%s: reservation == %d, expected %d", test.description, *lm.Reservation, test.reservation) + } + if swapAccounting != (lm.Swap != nil) { + t.Fatalf("%s: swap reported == %t, expected %t", test.description, lm.Swap != nil, swapAccounting) + } + if swapAccounting && *lm.Swap != test.swap { + t.Errorf("%s: swap == %d, expected %d", test.description, *lm.Swap, test.swap) + } + if lm.Swappiness != nil || lm.DisableOOMKiller != nil || lm.KernelTCP != nil { + t.Errorf("%s: reported a value the unified hierarchy does not have", test.description) + } + } +} + +func TestCgroupV2GetCPUData(t *testing.T) { + cgPath := newTestCgroup(t, "cpu", "cpuset") + cg := &CgroupV2{} + + writeControl(t, cgPath, "cpu.max", "50000 100000") + writeControl(t, cgPath, "cpuset.cpus", "0") + writeControl(t, cgPath, "cpuset.mems", "0") + lc, err := cg.GetCPUData(0, cgPath) + if err != nil { + t.Fatal(err) + } + if *lc.Quota != 50000 || *lc.Period != 100000 { + t.Errorf("quota/period == %d/%d, expected 50000/100000", *lc.Quota, *lc.Period) + } + if lc.Cpus != "0" || lc.Mems != "0" { + t.Errorf("cpus/mems == %q/%q, expected \"0\"/\"0\"", lc.Cpus, lc.Mems) + } + if lc.Shares != nil { + t.Error("reported shares, which cannot be recovered from cpu.weight") + } + + writeControl(t, cgPath, "cpu.max", "max 100000") + writeControl(t, cgPath, "cpuset.cpus", "") + writeControl(t, cgPath, "cpuset.mems", "") + lc, err = cg.GetCPUData(0, cgPath) + if err != nil { + t.Fatal(err) + } + if *lc.Quota != -1 { + t.Errorf("quota == %d, expected -1", *lc.Quota) + } + if cpus := readControl(t, cgPath, "cpuset.cpus.effective"); lc.Cpus != cpus { + t.Errorf("inherited cpus == %q, expected %q", lc.Cpus, cpus) + } + if mems := readControl(t, cgPath, "cpuset.mems.effective"); lc.Mems != mems { + t.Errorf("inherited mems == %q, expected %q", lc.Mems, mems) + } +} + +func TestCgroupV2GetBlockIOData(t *testing.T) { + cgPath := newTestCgroup(t, "io") + cg := &CgroupV2{} + + devices, err := filepath.Glob("/sys/block/*/dev") + if err != nil { + t.Fatal(err) + } + device := "" + for _, dev := range devices { + contents, err := os.ReadFile(dev) + if err != nil { + continue + } + candidate := strings.TrimSpace(string(contents)) + limit := candidate + " rbps=102400 wbps=204800 wiops=1000" + if os.WriteFile(filepath.Join(unifiedPath, cgPath, "io.max"), []byte(limit), 0o644) == nil { + device = candidate + break + } + } + if device == "" { + t.Skip("no block device accepts an io.max limit") + } + + lb, err := cg.GetBlockIOData(0, cgPath) + if err != nil { + t.Fatal(err) + } + if len(lb.ThrottleReadBpsDevice) != 1 || lb.ThrottleReadBpsDevice[0].Rate != 102400 { + t.Errorf("read bps devices == %v, expected a single 102400 entry", lb.ThrottleReadBpsDevice) + } + if len(lb.ThrottleWriteBpsDevice) != 1 || lb.ThrottleWriteBpsDevice[0].Rate != 204800 { + t.Errorf("write bps devices == %v, expected a single 204800 entry", lb.ThrottleWriteBpsDevice) + } + if len(lb.ThrottleReadIOPSDevice) != 0 { + t.Errorf("read iops devices == %v, expected none", lb.ThrottleReadIOPSDevice) + } + if len(lb.ThrottleWriteIOPSDevice) != 1 || lb.ThrottleWriteIOPSDevice[0].Rate != 1000 { + t.Errorf("write iops devices == %v, expected a single 1000 entry", lb.ThrottleWriteIOPSDevice) + } + if len(lb.ThrottleReadBpsDevice) == 1 { + major, minor, err := getDeviceID(device) + if err != nil { + t.Fatal(err) + } + if lb.ThrottleReadBpsDevice[0].Major != major || lb.ThrottleReadBpsDevice[0].Minor != minor { + t.Errorf("read bps device == %d:%d, expected %s", lb.ThrottleReadBpsDevice[0].Major, lb.ThrottleReadBpsDevice[0].Minor, device) + } + } + if lb.Weight != nil || lb.LeafWeight != nil { + t.Error("reported a weight, which is written to a scheduler dependent file") + } +} + +func TestCgroupV2GetHugepageLimitData(t *testing.T) { + pageSizes, err := GetHugePageSize() + if err != nil || len(pageSizes) == 0 { + t.Skip("no hugepage sizes on this host") + } + cgPath := newTestCgroup(t, "hugetlb") + cg := &CgroupV2{} + + var limit uint64 = 2 * (1 << 30) + writeControl(t, cgPath, "hugetlb."+pageSizes[0]+".max", "2147483648") + lh, err := cg.GetHugepageLimitData(0, cgPath) + if err != nil { + t.Fatal(err) + } + + for _, pageLimit := range lh { + if pageLimit.Pagesize != pageSizes[0] { + continue + } + if pageLimit.Limit != limit { + t.Errorf("hugepage limit == %d, expected %d", pageLimit.Limit, limit) + } + return + } + t.Errorf("hugepage limits == %v, expected an entry for %s", lh, pageSizes[0]) +} + +func TestCgroupV2UnreadableControllers(t *testing.T) { + cg := &CgroupV2{} + if _, err := cg.GetDevicesData(0, AbsCgroupPath); err == nil { + t.Error("expected an error for the devices controller") + } + if _, err := cg.GetNetworkData(0, AbsCgroupPath); err == nil { + t.Error("expected an error for the network controllers") + } +} From 7358f9250874b152c213418306f3e8ba1325a20f Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:55:55 +0100 Subject: [PATCH 3/6] validation: skip cgroup v1 only resource fields The cgroup test bundles ask for memory.swappiness, memory.disableOOMKiller, memory.kernel, memory.kernelTCP, blockIO.leafWeight and the block IO weights on every host. None of those exist in the unified hierarchy, and the create operation requires a runtime that cannot apply a property to generate an error and not create the container. A correct cgroup v2 only runtime therefore fails these tests by doing exactly what the spec asks of it. Set those fields only where the hierarchy can carry them. cpu.shares goes the same way: runtimes map it onto cpu.weight with a lossy conversion the runtime-spec does not define, so the configured value cannot be recovered and asserting on it would test a convention rather than the spec. Compare the remaining fields with a helper that reports a value neither side carries as a diagnostic instead of a failure, so that one unsupported field no longer hides the checks that follow it: the block IO and cpu validators used to return early and silently drop every later assertion. linux_cgroups_relative_cpus grows the same behaviour by using the shared CPU validator its siblings already use rather than its own copy of it. Signed-off-by: Daniel Golle --- .../linux_cgroups_blkio.go | 16 +++++-- .../linux_cgroups_cpus/linux_cgroups_cpus.go | 5 +- .../linux_cgroups_memory.go | 10 ++-- .../linux_cgroups_relative_blkio.go | 13 ++++-- .../linux_cgroups_relative_cpus.go | 41 +++-------------- .../linux_cgroups_relative_memory.go | 10 ++-- validation/util/linux_cgroups.go | 31 +++++++++++++ validation/util/linux_resources_blkio.go | 24 ++-------- validation/util/linux_resources_cpus.go | 46 +++---------------- validation/util/linux_resources_memory.go | 28 ++++------- 10 files changed, 92 insertions(+), 132 deletions(-) create mode 100644 validation/util/linux_cgroups.go diff --git a/validation/linux_cgroups_blkio/linux_cgroups_blkio.go b/validation/linux_cgroups_blkio/linux_cgroups_blkio.go index 0004613a..7f6e7cd3 100644 --- a/validation/linux_cgroups_blkio/linux_cgroups_blkio.go +++ b/validation/linux_cgroups_blkio/linux_cgroups_blkio.go @@ -30,13 +30,19 @@ func testBlkioCgroups(rate uint64, isEmpty bool) error { g.SetLinuxCgroupsPath(cgroups.AbsCgroupPath) - if !isEmpty { - g.SetLinuxResourcesBlockIOWeight(weight) - g.SetLinuxResourcesBlockIOLeafWeight(leafWeight) + // The unified hierarchy has no leaf weights, and writes the weight to a + // file that depends on the IO scheduler in use, so it cannot be read + // back to compare it with the configured value. + if !util.UnifiedCgroups() { + if !isEmpty { + g.SetLinuxResourcesBlockIOWeight(weight) + g.SetLinuxResourcesBlockIOLeafWeight(leafWeight) + } + + g.AddLinuxResourcesBlockIOWeightDevice(major, minor, weight) + g.AddLinuxResourcesBlockIOLeafWeightDevice(major, minor, leafWeight) } - g.AddLinuxResourcesBlockIOWeightDevice(major, minor, weight) - g.AddLinuxResourcesBlockIOLeafWeightDevice(major, minor, leafWeight) g.AddLinuxResourcesBlockIOThrottleReadBpsDevice(major, minor, rate) g.AddLinuxResourcesBlockIOThrottleWriteBpsDevice(major, minor, rate) g.AddLinuxResourcesBlockIOThrottleReadIOPSDevice(major, minor, rate) diff --git a/validation/linux_cgroups_cpus/linux_cgroups_cpus.go b/validation/linux_cgroups_cpus/linux_cgroups_cpus.go index ca92d450..74ad9747 100644 --- a/validation/linux_cgroups_cpus/linux_cgroups_cpus.go +++ b/validation/linux_cgroups_cpus/linux_cgroups_cpus.go @@ -59,7 +59,10 @@ func testCPUCgroups() error { g.SetLinuxCgroupsPath(cgroups.AbsCgroupPath) - if c.shares > 0 { + // cpu.shares is mapped onto cpu.weight by a conversion the + // runtime-spec does not define, and the mapping is lossy, so + // the configured value cannot be recovered. + if c.shares > 0 && !util.UnifiedCgroups() { g.SetLinuxResourcesCPUShares(c.shares) } diff --git a/validation/linux_cgroups_memory/linux_cgroups_memory.go b/validation/linux_cgroups_memory/linux_cgroups_memory.go index e83be675..1e2280c8 100644 --- a/validation/linux_cgroups_memory/linux_cgroups_memory.go +++ b/validation/linux_cgroups_memory/linux_cgroups_memory.go @@ -38,10 +38,12 @@ func main() { g.SetLinuxResourcesMemoryLimit(c.limit) g.SetLinuxResourcesMemoryReservation(c.limit) g.SetLinuxResourcesMemorySwap(c.limit) - g.SetLinuxResourcesMemoryKernel(c.limit) - g.SetLinuxResourcesMemoryKernelTCP(c.limit) - g.SetLinuxResourcesMemorySwappiness(c.swappiness) - g.SetLinuxResourcesMemoryDisableOOMKiller(true) + if !util.UnifiedCgroups() { + g.SetLinuxResourcesMemoryKernel(c.limit) + g.SetLinuxResourcesMemoryKernelTCP(c.limit) + g.SetLinuxResourcesMemorySwappiness(c.swappiness) + g.SetLinuxResourcesMemoryDisableOOMKiller(true) + } err = util.RuntimeOutsideValidate(g, t, util.ValidateLinuxResourcesMemory) if err != nil { t.Fail(err.Error()) diff --git a/validation/linux_cgroups_relative_blkio/linux_cgroups_relative_blkio.go b/validation/linux_cgroups_relative_blkio/linux_cgroups_relative_blkio.go index 67b0433e..b9b7ecd4 100644 --- a/validation/linux_cgroups_relative_blkio/linux_cgroups_relative_blkio.go +++ b/validation/linux_cgroups_relative_blkio/linux_cgroups_relative_blkio.go @@ -21,10 +21,15 @@ func main() { util.Fatal(err) } g.SetLinuxCgroupsPath(cgroups.RelCgroupPath) - g.SetLinuxResourcesBlockIOWeight(weight) - g.SetLinuxResourcesBlockIOLeafWeight(leafWeight) - g.AddLinuxResourcesBlockIOWeightDevice(major, minor, weight) - g.AddLinuxResourcesBlockIOLeafWeightDevice(major, minor, leafWeight) + // The unified hierarchy has no leaf weights, and writes the weight to a + // file that depends on the IO scheduler in use, so it cannot be read + // back to compare it with the configured value. + if !util.UnifiedCgroups() { + g.SetLinuxResourcesBlockIOWeight(weight) + g.SetLinuxResourcesBlockIOLeafWeight(leafWeight) + g.AddLinuxResourcesBlockIOWeightDevice(major, minor, weight) + g.AddLinuxResourcesBlockIOLeafWeightDevice(major, minor, leafWeight) + } g.AddLinuxResourcesBlockIOThrottleReadBpsDevice(major, minor, rate) g.AddLinuxResourcesBlockIOThrottleWriteBpsDevice(major, minor, rate) g.AddLinuxResourcesBlockIOThrottleReadIOPSDevice(major, minor, rate) diff --git a/validation/linux_cgroups_relative_cpus/linux_cgroups_relative_cpus.go b/validation/linux_cgroups_relative_cpus/linux_cgroups_relative_cpus.go index b0dff504..10defe10 100644 --- a/validation/linux_cgroups_relative_cpus/linux_cgroups_relative_cpus.go +++ b/validation/linux_cgroups_relative_cpus/linux_cgroups_relative_cpus.go @@ -2,7 +2,6 @@ package main import ( "github.com/mndrix/tap-go" - rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/cgroups" "github.com/opencontainers/runtime-tools/validation/util" ) @@ -24,43 +23,17 @@ func main() { util.Fatal(err) } g.SetLinuxCgroupsPath(cgroups.RelCgroupPath) - g.SetLinuxResourcesCPUShares(shares) + // cpu.shares is mapped onto cpu.weight by a conversion the runtime-spec + // does not define, and the mapping is lossy, so the configured value + // cannot be recovered. + if !util.UnifiedCgroups() { + g.SetLinuxResourcesCPUShares(shares) + } g.SetLinuxResourcesCPUQuota(quota) g.SetLinuxResourcesCPUPeriod(period) g.SetLinuxResourcesCPUCpus(cpus) g.SetLinuxResourcesCPUMems(mems) - err = util.RuntimeOutsideValidate(g, t, func(config *rspec.Spec, t *tap.T, state *rspec.State) error { - cg, err := cgroups.FindCgroup() - t.Ok((err == nil), "find cpus cgroup") - if err != nil { - t.Diagnostic(err.Error()) - return nil - } - - lcd, err := cg.GetCPUData(state.Pid, config.Linux.CgroupsPath) - t.Ok((err == nil), "get cpus cgroup data") - if err != nil { - t.Diagnostic(err.Error()) - return nil - } - - t.Ok(*lcd.Shares == shares, "cpus shares limit is set correctly") - t.Diagnosticf("expect: %d, actual: %d", shares, lcd.Shares) - - t.Ok(*lcd.Quota == quota, "cpus quota is set correctly") - t.Diagnosticf("expect: %d, actual: %d", quota, lcd.Quota) - - t.Ok(*lcd.Period == period, "cpus period is set correctly") - t.Diagnosticf("expect: %d, actual: %d", period, lcd.Period) - - t.Ok(lcd.Cpus == cpus, "cpus cpus is set correctly") - t.Diagnosticf("expect: %s, actual: %s", cpus, lcd.Cpus) - - t.Ok(lcd.Mems == mems, "cpus mems is set correctly") - t.Diagnosticf("expect: %s, actual: %s", mems, lcd.Mems) - - return nil - }) + err = util.RuntimeOutsideValidate(g, t, util.ValidateLinuxResourcesCPU) if err != nil { t.Fail(err.Error()) diff --git a/validation/linux_cgroups_relative_memory/linux_cgroups_relative_memory.go b/validation/linux_cgroups_relative_memory/linux_cgroups_relative_memory.go index 7dcdeb27..075c99b3 100644 --- a/validation/linux_cgroups_relative_memory/linux_cgroups_relative_memory.go +++ b/validation/linux_cgroups_relative_memory/linux_cgroups_relative_memory.go @@ -22,10 +22,12 @@ func main() { g.SetLinuxResourcesMemoryLimit(limit) g.SetLinuxResourcesMemoryReservation(limit) g.SetLinuxResourcesMemorySwap(limit) - g.SetLinuxResourcesMemoryKernel(limit) - g.SetLinuxResourcesMemoryKernelTCP(limit) - g.SetLinuxResourcesMemorySwappiness(swappiness) - g.SetLinuxResourcesMemoryDisableOOMKiller(true) + if !util.UnifiedCgroups() { + g.SetLinuxResourcesMemoryKernel(limit) + g.SetLinuxResourcesMemoryKernelTCP(limit) + g.SetLinuxResourcesMemorySwappiness(swappiness) + g.SetLinuxResourcesMemoryDisableOOMKiller(true) + } err = util.RuntimeOutsideValidate(g, t, util.ValidateLinuxResourcesMemory) if err != nil { t.Fail(err.Error()) diff --git a/validation/util/linux_cgroups.go b/validation/util/linux_cgroups.go new file mode 100644 index 00000000..7662217e --- /dev/null +++ b/validation/util/linux_cgroups.go @@ -0,0 +1,31 @@ +package util + +import ( + "github.com/mndrix/tap-go" + "github.com/opencontainers/runtime-tools/cgroups" +) + +// UnifiedCgroups reports whether the host mounts the unified hierarchy, +// on which several of the cgroup v1 resource fields have no counterpart. +func UnifiedCgroups() bool { + cg, err := cgroups.FindCgroup() + if err != nil { + return false + } + + return cg.Version() == 2 +} + +// checkValue compares a configured resource with the value read back from +// the cgroup filesystem. A field that either side does not carry is +// reported as a diagnostic rather than counted as a failed check, because +// the hierarchy in use may have no way to express it. +func checkValue[T comparable](t *tap.T, name string, expected *T, actual *T) { + if expected == nil || actual == nil { + t.Diagnosticf("skipping %s: not expressed by this cgroup hierarchy", name) + return + } + + t.Ok(*expected == *actual, name+" is set correctly") + t.Diagnosticf("expect: %v, actual: %v", *expected, *actual) +} diff --git a/validation/util/linux_resources_blkio.go b/validation/util/linux_resources_blkio.go index 1d4ac12f..4d5c51aa 100644 --- a/validation/util/linux_resources_blkio.go +++ b/validation/util/linux_resources_blkio.go @@ -24,32 +24,16 @@ func ValidateLinuxResourcesBlockIO(config *rspec.Spec, t *tap.T, state *rspec.St return nil } - if lbd.Weight == nil || config.Linux.Resources.BlockIO.Weight == nil { - t.Diagnostic(fmt.Sprintf("unable to get weight: lbd.Weight == %v, config.Linux.Resources.BlockIO.Weight == %v", lbd.Weight, config.Linux.Resources.BlockIO.Weight)) - return nil - } - - t.Ok(*lbd.Weight == *config.Linux.Resources.BlockIO.Weight, "blkio weight is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.BlockIO.Weight, *lbd.Weight) - - if lbd.LeafWeight == nil || config.Linux.Resources.BlockIO.LeafWeight == nil { - t.Diagnostic(fmt.Sprintf("unable to get leafWeight: lbd.LeafWeight == %v, config.Linux.Resources.BlockIO.LeafWeight == %v", lbd.LeafWeight, config.Linux.Resources.BlockIO.LeafWeight)) - return nil - } - - t.Ok(*lbd.LeafWeight == *config.Linux.Resources.BlockIO.LeafWeight, "blkio leafWeight is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.BlockIO.LeafWeight, *lbd.LeafWeight) + checkValue(t, "blkio weight", config.Linux.Resources.BlockIO.Weight, lbd.Weight) + checkValue(t, "blkio leafWeight", config.Linux.Resources.BlockIO.LeafWeight, lbd.LeafWeight) for _, device := range config.Linux.Resources.BlockIO.WeightDevice { found := false for _, wd := range lbd.WeightDevice { if wd.Major == device.Major && wd.Minor == device.Minor { found = true - t.Ok(*wd.Weight == *device.Weight, fmt.Sprintf("blkio weight for %d:%d is set correctly", device.Major, device.Minor)) - t.Diagnosticf("expect: %d, actual: %d", *device.Weight, *wd.Weight) - - t.Ok(*wd.LeafWeight == *device.LeafWeight, fmt.Sprintf("blkio leafWeight for %d:%d is set correctly", device.Major, device.Minor)) - t.Diagnosticf("expect: %d, actual: %d", *device.LeafWeight, *wd.LeafWeight) + checkValue(t, fmt.Sprintf("blkio weight for %d:%d", device.Major, device.Minor), device.Weight, wd.Weight) + checkValue(t, fmt.Sprintf("blkio leafWeight for %d:%d", device.Major, device.Minor), device.LeafWeight, wd.LeafWeight) } } t.Ok(found, fmt.Sprintf("blkio weightDevice for %d:%d found", device.Major, device.Minor)) diff --git a/validation/util/linux_resources_cpus.go b/validation/util/linux_resources_cpus.go index f37efe9a..23b0f0dc 100644 --- a/validation/util/linux_resources_cpus.go +++ b/validation/util/linux_resources_cpus.go @@ -35,26 +35,9 @@ func ValidateLinuxResourcesCPU(config *rspec.Spec, t *tap.T, state *rspec.State) return nil } - if lcd.Shares == nil || config.Linux.Resources.CPU.Shares == nil { - t.Diagnostic(fmt.Sprintf("unable to get cpu shares, lcd.Shares == %v, config.Linux.Resources.CPU.Shares == %v", lcd.Shares, config.Linux.Resources.CPU.Shares)) - return nil - } - t.Ok(*lcd.Shares == *config.Linux.Resources.CPU.Shares, "cpu shares is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.CPU.Shares, *lcd.Shares) - - if lcd.Period == nil || config.Linux.Resources.CPU.Period == nil { - t.Diagnostic(fmt.Sprintf("unable to get cpu period, lcd.Period == %v, config.Linux.Resources.CPU.Period == %v", lcd.Period, config.Linux.Resources.CPU.Period)) - return nil - } - t.Ok(*lcd.Period == *config.Linux.Resources.CPU.Period, "cpu period is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.CPU.Period, *lcd.Period) - - if lcd.Quota == nil || config.Linux.Resources.CPU.Quota == nil { - t.Diagnostic(fmt.Sprintf("unable to get cpu quota, lcd.Quota == %v, config.Linux.Resources.CPU.Quota == %v", lcd.Quota, config.Linux.Resources.CPU.Quota)) - return nil - } - t.Ok(*lcd.Quota == *config.Linux.Resources.CPU.Quota, "cpu quota is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.CPU.Quota, *lcd.Quota) + checkValue(t, "cpu shares", config.Linux.Resources.CPU.Shares, lcd.Shares) + checkValue(t, "cpu period", config.Linux.Resources.CPU.Period, lcd.Period) + checkValue(t, "cpu quota", config.Linux.Resources.CPU.Quota, lcd.Quota) t.Ok(lcd.Cpus == config.Linux.Resources.CPU.Cpus, "cpu cpus is set correctly") t.Diagnosticf("expect: %s, actual: %s", config.Linux.Resources.CPU.Cpus, lcd.Cpus) @@ -106,26 +89,9 @@ func ValidateLinuxResourcesCPUEmpty(config *rspec.Spec, t *tap.T, state *rspec.S return nil } - if lcd.Shares == nil { - t.Diagnostic(fmt.Sprintf("unable to get cpu shares, lcd.Shares == %v", lcd.Shares)) - return nil - } - t.Ok(*lcd.Shares == defaultShares, "cpu shares is set correctly") - t.Diagnosticf("expect: %d, actual: %d", defaultShares, *lcd.Shares) - - if lcd.Period == nil { - t.Diagnostic(fmt.Sprintf("unable to get cpu period, lcd.Period == %v", lcd.Period)) - return nil - } - t.Ok(*lcd.Period == defaultPeriod, "cpu period is set correctly") - t.Diagnosticf("expect: %d, actual: %d", defaultPeriod, *lcd.Period) - - if lcd.Quota == nil { - t.Diagnostic(fmt.Sprintf("unable to get cpu quota, lcd.Quota == %v", lcd.Quota)) - return nil - } - t.Ok(*lcd.Quota == defaultQuota, "cpu quota is set correctly") - t.Diagnosticf("expect: %d, actual: %d", defaultQuota, *lcd.Quota) + checkValue(t, "cpu shares", &defaultShares, lcd.Shares) + checkValue(t, "cpu period", &defaultPeriod, lcd.Period) + checkValue(t, "cpu quota", &defaultQuota, lcd.Quota) t.Ok(lcd.Cpus == defaultCpus, "cpu cpus is set correctly") t.Diagnosticf("expect: %s, actual: %s", defaultCpus, lcd.Cpus) diff --git a/validation/util/linux_resources_memory.go b/validation/util/linux_resources_memory.go index 305e89d1..3eb4bbf7 100644 --- a/validation/util/linux_resources_memory.go +++ b/validation/util/linux_resources_memory.go @@ -22,26 +22,14 @@ func ValidateLinuxResourcesMemory(config *rspec.Spec, t *tap.T, state *rspec.Sta return nil } - t.Ok(*lm.Limit == *config.Linux.Resources.Memory.Limit, "memory limit is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.Memory.Limit, *lm.Limit) - - t.Ok(*lm.Reservation == *config.Linux.Resources.Memory.Reservation, "memory reservation is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.Memory.Reservation, *lm.Reservation) - - t.Ok(*lm.Swap == *config.Linux.Resources.Memory.Swap, "memory swap is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.Memory.Swap, *lm.Reservation) - - t.Ok(*lm.Kernel == *config.Linux.Resources.Memory.Kernel, "memory kernel is set correctly") //nolint:staticcheck // Ignore SA1019: lm.Kernel is deprecated - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.Memory.Kernel, *lm.Kernel) //nolint:staticcheck // Ignore SA1019: config.Linux.Resources.Memory.Kernel is deprecated - - t.Ok(*lm.KernelTCP == *config.Linux.Resources.Memory.KernelTCP, "memory kernelTCP is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.Memory.KernelTCP, *lm.Kernel) //nolint:staticcheck // Ignore SA1019: lm.Kernel is deprecated - - t.Ok(*lm.Swappiness == *config.Linux.Resources.Memory.Swappiness, "memory swappiness is set correctly") - t.Diagnosticf("expect: %d, actual: %d", *config.Linux.Resources.Memory.Swappiness, *lm.Swappiness) - - t.Ok(*lm.DisableOOMKiller == *config.Linux.Resources.Memory.DisableOOMKiller, "memory oom is set correctly") - t.Diagnosticf("expect: %t, actual: %t", *config.Linux.Resources.Memory.DisableOOMKiller, *lm.DisableOOMKiller) + lrm := config.Linux.Resources.Memory + checkValue(t, "memory limit", lrm.Limit, lm.Limit) + checkValue(t, "memory reservation", lrm.Reservation, lm.Reservation) + checkValue(t, "memory swap", lrm.Swap, lm.Swap) + checkValue(t, "memory kernel", lrm.Kernel, lm.Kernel) //nolint:staticcheck // Ignore SA1019: Kernel is deprecated + checkValue(t, "memory kernelTCP", lrm.KernelTCP, lm.KernelTCP) + checkValue(t, "memory swappiness", lrm.Swappiness, lm.Swappiness) + checkValue(t, "memory oom", lrm.DisableOOMKiller, lm.DisableOOMKiller) return nil } From 22809e49bac5ddf9b86ef863fa3acdbcaca70898 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:56:48 +0100 Subject: [PATCH 4/6] validation: skip the cgroup v1 only resource tests linux.resources.network and linux.resources.devices have no counterpart in the unified hierarchy at all. net_cls and net_prio were never ported, and device access is enforced by an eBPF program that cannot be read back from the filesystem, so there is nothing for these tests to compare the configuration against. Skip the four tests where the host runs a unified hierarchy, the way the suite already skips tests that do not apply to the platform it runs on. Signed-off-by: Daniel Golle --- validation/linux_cgroups_devices/linux_cgroups_devices.go | 7 +++++++ validation/linux_cgroups_network/linux_cgroups_network.go | 6 ++++++ .../linux_cgroups_relative_devices.go | 7 +++++++ .../linux_cgroups_relative_network.go | 7 +++++++ 4 files changed, 27 insertions(+) diff --git a/validation/linux_cgroups_devices/linux_cgroups_devices.go b/validation/linux_cgroups_devices/linux_cgroups_devices.go index ae947ade..b1e532fe 100644 --- a/validation/linux_cgroups_devices/linux_cgroups_devices.go +++ b/validation/linux_cgroups_devices/linux_cgroups_devices.go @@ -1,6 +1,8 @@ package main import ( + "os" + "github.com/mndrix/tap-go" "github.com/opencontainers/runtime-tools/cgroups" "github.com/opencontainers/runtime-tools/validation/util" @@ -9,6 +11,11 @@ import ( func main() { var major1, minor1, major2, minor2, major3, minor3 int64 = 10, 229, 8, 20, 10, 200 + if util.UnifiedCgroups() { + util.Skip("cgroup v1 specific linux.resources.devices test", nil) + os.Exit(0) + } + t := tap.New() t.Header(0) defer t.AutoPlan() diff --git a/validation/linux_cgroups_network/linux_cgroups_network.go b/validation/linux_cgroups_network/linux_cgroups_network.go index 6208cd7c..cfd3b31a 100644 --- a/validation/linux_cgroups_network/linux_cgroups_network.go +++ b/validation/linux_cgroups_network/linux_cgroups_network.go @@ -3,6 +3,7 @@ package main import ( "fmt" "net" + "os" "runtime" "github.com/mndrix/tap-go" @@ -102,6 +103,11 @@ func main() { util.Fatal(fmt.Errorf("linux-specific cgroup test")) } + if util.UnifiedCgroups() { + util.Skip("cgroup v1 specific linux.resources.network test", nil) + os.Exit(0) + } + if err := testNetworkCgroups(); err != nil { util.Fatal(err) } diff --git a/validation/linux_cgroups_relative_devices/linux_cgroups_relative_devices.go b/validation/linux_cgroups_relative_devices/linux_cgroups_relative_devices.go index 679a88f2..8a718667 100644 --- a/validation/linux_cgroups_relative_devices/linux_cgroups_relative_devices.go +++ b/validation/linux_cgroups_relative_devices/linux_cgroups_relative_devices.go @@ -1,6 +1,8 @@ package main import ( + "os" + "github.com/mndrix/tap-go" "github.com/opencontainers/runtime-tools/cgroups" "github.com/opencontainers/runtime-tools/validation/util" @@ -9,6 +11,11 @@ import ( func main() { var major1, minor1, major2, minor2, major3, minor3 int64 = 10, 229, 8, 20, 10, 200 + if util.UnifiedCgroups() { + util.Skip("cgroup v1 specific linux.resources.devices test", nil) + os.Exit(0) + } + t := tap.New() t.Header(0) defer t.AutoPlan() diff --git a/validation/linux_cgroups_relative_network/linux_cgroups_relative_network.go b/validation/linux_cgroups_relative_network/linux_cgroups_relative_network.go index da5e9ee2..32f0536a 100644 --- a/validation/linux_cgroups_relative_network/linux_cgroups_relative_network.go +++ b/validation/linux_cgroups_relative_network/linux_cgroups_relative_network.go @@ -1,6 +1,8 @@ package main import ( + "os" + "github.com/mndrix/tap-go" "github.com/opencontainers/runtime-tools/cgroups" "github.com/opencontainers/runtime-tools/validation/util" @@ -10,6 +12,11 @@ func main() { var id, prio uint32 = 255, 10 ifName := "lo" + if util.UnifiedCgroups() { + util.Skip("cgroup v1 specific linux.resources.network test", nil) + os.Exit(0) + } + t := tap.New() t.Header(0) defer t.AutoPlan() From a82844094ef1f18aa38f0edae148fad2e7a3fcf0 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:58:55 +0100 Subject: [PATCH 5/6] validation: drop hardcoded cgroup paths from delete Both delete tests build the cgroup directory out of a literal /sys/fs/cgroup/pids. On a unified hierarchy that directory never exists, so delete_only_create_resources fails when it tries to populate it and delete_resources passes without testing anything: the path it expects the runtime to have removed was never there. Ask the Cgroup implementation where the cgroup lives instead, which also picks up the real mount point rather than assuming one. Move a process with cgroup.procs, which both hierarchies provide, rather than with the cgroup v1 only tasks file. Fixes #807 Signed-off-by: Daniel Golle --- cgroups/cgroups.go | 1 + cgroups/cgroups_v1.go | 5 +++++ cgroups/cgroups_v2.go | 7 +++++++ .../delete_only_create_resources.go | 11 ++++++++--- validation/delete_resources/delete_resources.go | 8 ++++++-- 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/cgroups/cgroups.go b/cgroups/cgroups.go index 743b69f9..84d8ea26 100644 --- a/cgroups/cgroups.go +++ b/cgroups/cgroups.go @@ -23,6 +23,7 @@ var ( // Cgroup represents interfaces for cgroup validation type Cgroup interface { Version() int + ControllerPath(controller string, cgPath string) string GetBlockIOData(pid int, cgPath string) (*rspec.LinuxBlockIO, error) GetCPUData(pid int, cgPath string) (*rspec.LinuxCPU, error) GetDevicesData(pid int, cgPath string) ([]rspec.LinuxDeviceCgroup, error) diff --git a/cgroups/cgroups_v1.go b/cgroups/cgroups_v1.go index dce24b0a..8137df37 100644 --- a/cgroups/cgroups_v1.go +++ b/cgroups/cgroups_v1.go @@ -22,6 +22,11 @@ func (cg *CgroupV1) Version() int { return 1 } +// ControllerPath returns the directory backing an absolute cgroupsPath +func (cg *CgroupV1) ControllerPath(controller string, cgPath string) string { + return filepath.Join(cg.MountPath, controller, cgPath) +} + // HugePageSizeUnitList is a list of the units used by the linux kernel when // naming the HugePage control files. // https://www.kernel.org/doc/Documentation/cgroup-v1/hugetlb.txt diff --git a/cgroups/cgroups_v2.go b/cgroups/cgroups_v2.go index adeaac12..759417f2 100644 --- a/cgroups/cgroups_v2.go +++ b/cgroups/cgroups_v2.go @@ -37,6 +37,13 @@ func (cg *CgroupV2) Version() int { return 2 } +// ControllerPath returns the directory backing an absolute cgroupsPath. +// The unified hierarchy keeps every controller in one directory, so the +// controller is ignored. +func (cg *CgroupV2) ControllerPath(controller string, cgPath string) string { + return filepath.Join(unifiedPath, cgPath) +} + func (cg *CgroupV2) dir(pid int, cgPath string) (string, error) { if filepath.IsAbs(cgPath) { path := filepath.Join(unifiedPath, cgPath) diff --git a/validation/delete_only_create_resources/delete_only_create_resources.go b/validation/delete_only_create_resources/delete_only_create_resources.go index 04eb60f8..0ef420d4 100644 --- a/validation/delete_only_create_resources/delete_only_create_resources.go +++ b/validation/delete_only_create_resources/delete_only_create_resources.go @@ -11,6 +11,7 @@ import ( tap "github.com/mndrix/tap-go" "github.com/mrunalp/fileutils" rspec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/opencontainers/runtime-tools/cgroups" "github.com/opencontainers/runtime-tools/specerror" "github.com/opencontainers/runtime-tools/validation/util" ) @@ -19,9 +20,13 @@ func main() { t := tap.New() t.Header(0) + cg, err := cgroups.FindCgroup() + if err != nil { + util.Fatal(err) + } + // Create a cgroup - cgPath := "/sys/fs/cgroup" - testPath := filepath.Join(cgPath, "pids", "cgrouptest") + testPath := cg.ControllerPath("pids", cgroups.AbsCgroupPath) os.Mkdir(testPath, 0o755) defer os.RemoveAll(testPath) @@ -61,7 +66,7 @@ func main() { util.Fatal(err) } // Add the container to the cgroup - err = os.WriteFile(filepath.Join(testPath, "tasks"), []byte(strconv.Itoa(state.Pid)), 0o644) + err = os.WriteFile(filepath.Join(testPath, "cgroup.procs"), []byte(strconv.Itoa(state.Pid)), 0o644) if err != nil { util.Fatal(err) } diff --git a/validation/delete_resources/delete_resources.go b/validation/delete_resources/delete_resources.go index 7dbe2721..510fffca 100644 --- a/validation/delete_resources/delete_resources.go +++ b/validation/delete_resources/delete_resources.go @@ -50,6 +50,11 @@ func main() { util.Fatal(err) } + cg, err := cgroups.FindCgroup() + if err != nil { + util.Fatal(err) + } + err = r.Create() if err != nil { util.Fatal(err) @@ -76,8 +81,7 @@ func main() { t.Fail(err.Error()) } - path := filepath.Join("/sys/fs/cgroup/pids", cgroups.AbsCgroupPath) - _, err = os.Stat(path) + _, err = os.Stat(cg.ControllerPath("pids", cgroups.AbsCgroupPath)) util.SpecErrorOK(t, os.IsNotExist(err), specerror.NewError(specerror.DeleteResImplement, fmt.Errorf("Deleting a container MUST delete the resources that were created during the `create` step"), rspec.Version), nil) t.AutoPlan() From 119b0a1cdc4da97abd325fa7207da0f39b14c494 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Mon, 21 Sep 2026 12:11:44 +0100 Subject: [PATCH 6/6] validation: check the device rules by trying them The unified hierarchy applies the same device rules cgroup v1 did, but it publishes no list to read them back from, so linux_cgroups_devices has nothing to compare against and is skipped there. The rules are still in force; only this test's way of looking at them is gone. Look at them from inside the container instead. The three rules the test already configures name no device that the bundle puts in the container, so add a node the first of them allows and one that only the generator's leading deny-all covers. The rules themselves are untouched, which leaves the cgroup v1 path reading exactly what it read before. runtimetest works out what the list permits by applying the entries in the listed order, as a runtime is required to, and compares that with what opening the node does. Only EPERM means the device controller refused, and it is reported before the driver behind the node is consulted, so nothing has to be loaded for the answer to be meaningful. The open does not block, and only character and block devices are compared, since the device controller governs nothing else; a FIFO in linux.devices would otherwise hold the test up waiting for a writer. Devices a runtime must supply are left alone, since runtimes add rules of their own for those. Signed-off-by: Daniel Golle --- cmd/runtimetest/main.go | 103 ++++++++++++++++++ cmd/runtimetest/main_test.go | 100 +++++++++++++++++ .../linux_cgroups_devices.go | 39 ++++++- 3 files changed, 236 insertions(+), 6 deletions(-) create mode 100644 cmd/runtimetest/main_test.go diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index b46536ea..4f0155f1 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -663,6 +663,108 @@ func (c *complianceTester) validateLinuxDevices(spec *rspec.Spec) error { return nil } +func isDefaultDevicePath(path string) bool { + for _, device := range defaultDevices { + if device.Path == path { + return true + } + } + + return false +} + +func deviceRuleMatches(rule rspec.LinuxDeviceCgroup, devType string, major int64, minor int64) bool { + if rule.Type != "" && rule.Type != "a" && rule.Type != devType { + return false + } + if rule.Major != nil && *rule.Major != major { + return false + } + if rule.Minor != nil && *rule.Minor != minor { + return false + } + + return rule.Access == "" || strings.Contains(rule.Access, "r") +} + +// deviceReadAllowed applies the entries in the listed order, as a runtime +// is required to, and reports what the last one to match has to say. A +// device that no entry matches carries no restriction. +func deviceReadAllowed(rules []rspec.LinuxDeviceCgroup, devType string, major int64, minor int64) bool { + allowed := true + for _, rule := range rules { + if deviceRuleMatches(rule, devType, major, minor) { + allowed = rule.Allow + } + } + + return allowed +} + +// deviceCgroupType maps a linux.devices type onto the type the device +// controller knows, which covers character and block devices only. +func deviceCgroupType(devType string) (string, bool) { + switch devType { + case "c", "u": + return "c", true + case "b": + return "b", true + } + + return "", false +} + +// deviceDeniedByCgroup opens a device node to find out whether the device +// controller refuses it. EPERM comes back before the driver behind the +// node is consulted, so a node with nothing behind it answers the question +// just as well as one that could be read. The open does not block, so a +// driver that waits for a peer cannot hold the test up. +func deviceDeniedByCgroup(path string) bool { + f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) + if err == nil { + f.Close() + return false + } + + return errors.Is(err, syscall.EPERM) +} + +func (c *complianceTester) validateLinuxResourcesDevices(spec *rspec.Spec) error { + if spec.Linux == nil || spec.Linux.Resources == nil || len(spec.Linux.Resources.Devices) == 0 { + c.harness.Skip(1, "linux.resources.devices is not set") + return nil + } + + for _, device := range spec.Linux.Devices { + // Runtimes add rules of their own for the devices they are + // required to supply, so those say nothing about the list that + // was configured. + if isDefaultDevicePath(device.Path) { + continue + } + devType, ok := deviceCgroupType(device.Type) + if !ok { + continue + } + expected := !deviceReadAllowed(spec.Linux.Resources.Devices, devType, device.Major, device.Minor) + denied := deviceDeniedByCgroup(device.Path) + rfcError, err := c.Ok(denied == expected, specerror.DevicesApplyInOrder, spec.Version, + fmt.Sprintf("access to %s follows linux.resources.devices", device.Path)) + if err != nil { + return err + } + _ = c.harness.YAML(map[string]string{ + "level": rfcError.Level.String(), + "reference": rfcError.Reference, + "device": fmt.Sprintf("%s %d:%d", device.Type, device.Major, device.Minor), + "expected": fmt.Sprintf("denied: %t", expected), + "actual": fmt.Sprintf("denied: %t", denied), + }) + } + + return nil +} + func (c *complianceTester) validateDevice(device *rspec.LinuxDevice, condition specerror.Code, version string, description string) (err error) { var exists bool fi, err := os.Stat(device.Path) @@ -1297,6 +1399,7 @@ func run(context *cli.Context) error { c.validateDefaultFS, c.validateDefaultDevices, c.validateLinuxDevices, + c.validateLinuxResourcesDevices, c.validateLinuxProcess, c.validateMaskedPaths, c.validateOOMScoreAdj, diff --git a/cmd/runtimetest/main_test.go b/cmd/runtimetest/main_test.go new file mode 100644 index 00000000..324d16e6 --- /dev/null +++ b/cmd/runtimetest/main_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "path/filepath" + "syscall" + "testing" + + rspec "github.com/opencontainers/runtime-spec/specs-go" +) + +func devicePtr(v int64) *int64 { + return &v +} + +func TestDeviceReadAllowed(t *testing.T) { + // The list linux_cgroups_devices builds: the generator's leading + // deny-all followed by the three rules the test adds. + rules := []rspec.LinuxDeviceCgroup{ + {Allow: false, Access: "rwm"}, + {Allow: true, Type: "c", Major: devicePtr(10), Minor: devicePtr(229), Access: "rwm"}, + {Allow: true, Type: "b", Major: devicePtr(8), Minor: devicePtr(20), Access: "rw"}, + {Allow: true, Type: "b", Major: devicePtr(10), Minor: devicePtr(200), Access: "r"}, + } + + for _, test := range []struct { + description string + devType string + rules []rspec.LinuxDeviceCgroup + major int64 + minor int64 + expected bool + }{ + {description: "explicitly allowed", rules: rules, devType: "c", major: 10, minor: 229, expected: true}, + {description: "a minor the allow does not cover", rules: rules, devType: "c", major: 10, minor: 230, expected: false}, + {description: "right numbers, wrong type", rules: rules, devType: "b", major: 10, minor: 229, expected: false}, + {description: "allowed block device", rules: rules, devType: "b", major: 8, minor: 20, expected: true}, + {description: "read-only allow still permits reading", rules: rules, devType: "b", major: 10, minor: 200, expected: true}, + {description: "covered only by the deny-all", rules: rules, devType: "c", major: 1, minor: 3, expected: false}, + {description: "no rules at all", rules: nil, devType: "c", major: 1, minor: 3, expected: true}, + { + description: "a deny that does not mention read", + rules: []rspec.LinuxDeviceCgroup{{Allow: false, Access: "w"}}, + devType: "c", major: 1, minor: 3, expected: true, + }, + { + description: "a later rule overrides an earlier one", + rules: []rspec.LinuxDeviceCgroup{ + {Allow: true, Access: "rwm"}, + {Allow: false, Type: "c", Major: devicePtr(10), Minor: devicePtr(229), Access: "rwm"}, + }, + devType: "c", major: 10, minor: 229, expected: false, + }, + } { + got := deviceReadAllowed(test.rules, test.devType, test.major, test.minor) + if got != test.expected { + t.Errorf("%s: %s %d:%d allowed == %t, expected %t", + test.description, test.devType, test.major, test.minor, got, test.expected) + } + } +} + +func TestIsDefaultDevicePath(t *testing.T) { + if !isDefaultDevicePath("/dev/null") { + t.Error("/dev/null is a default device") + } + if isDefaultDevicePath("/dev/denied") { + t.Error("/dev/denied is not a default device") + } +} + +func TestDeviceCgroupType(t *testing.T) { + for _, test := range []struct { + devType string + expected string + governed bool + }{ + {devType: "c", expected: "c", governed: true}, + {devType: "u", expected: "c", governed: true}, + {devType: "b", expected: "b", governed: true}, + {devType: "p", governed: false}, + } { + got, governed := deviceCgroupType(test.devType) + if got != test.expected || governed != test.governed { + t.Errorf("%s: type == %q governed == %t, expected %q %t", + test.devType, got, governed, test.expected, test.governed) + } + } +} + +func TestDeviceDeniedByCgroupFifo(t *testing.T) { + fifo := filepath.Join(t.TempDir(), "fifo") + if err := syscall.Mkfifo(fifo, 0o600); err != nil { + t.Fatal(err) + } + // Opening a FIFO without a writer blocks unless the open is + // non-blocking. + if deviceDeniedByCgroup(fifo) { + t.Error("a FIFO outside any device rule was reported as denied") + } +} diff --git a/validation/linux_cgroups_devices/linux_cgroups_devices.go b/validation/linux_cgroups_devices/linux_cgroups_devices.go index b1e532fe..67205fb7 100644 --- a/validation/linux_cgroups_devices/linux_cgroups_devices.go +++ b/validation/linux_cgroups_devices/linux_cgroups_devices.go @@ -4,17 +4,28 @@ import ( "os" "github.com/mndrix/tap-go" + rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/cgroups" "github.com/opencontainers/runtime-tools/validation/util" ) -func main() { - var major1, minor1, major2, minor2, major3, minor3 int64 = 10, 229, 8, 20, 10, 200 +func deviceNode(path string, major int64, minor int64) rspec.LinuxDevice { + mode := os.FileMode(0o666) + owner := uint32(0) - if util.UnifiedCgroups() { - util.Skip("cgroup v1 specific linux.resources.devices test", nil) - os.Exit(0) + return rspec.LinuxDevice{ + Path: path, + Type: "c", + Major: major, + Minor: minor, + FileMode: &mode, + UID: &owner, + GID: &owner, } +} + +func main() { + var major1, minor1, major2, minor2, major3, minor3 int64 = 10, 229, 8, 20, 10, 200 t := tap.New() t.Header(0) @@ -28,7 +39,23 @@ func main() { g.AddLinuxResourcesDevice(true, "c", &major1, &minor1, "rwm") g.AddLinuxResourcesDevice(true, "b", &major2, &minor2, "rw") g.AddLinuxResourcesDevice(true, "b", &major3, &minor3, "r") - err = util.RuntimeOutsideValidate(g, t, util.ValidateLinuxResourcesDevices) + + // The rules above name no device that the bundle puts in the + // container, so nothing about them can be observed from the inside. + // Add a node the first rule allows and one that only the leading + // deny-all covers, which leaves the rules themselves untouched. + g.AddDevice(deviceNode("/dev/allowed", major1, minor1)) + g.AddDevice(deviceNode("/dev/denied", major1, minor1+1)) + + // Reading the rules back is a cgroup v1 affair. The unified hierarchy + // applies the same rules but publishes no list to compare against, so + // there they are checked by trying the two nodes above. + if util.UnifiedCgroups() { + g.AddAnnotation("TestName", "check linux.resources.devices") + err = util.RuntimeInsideValidate(g, t, nil) + } else { + err = util.RuntimeOutsideValidate(g, t, util.ValidateLinuxResourcesDevices) + } if err != nil { t.Fail(err.Error()) }