From 3d82845bf04d5aaeb14c45d0812c4f236da361c7 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:52:02 +0100 Subject: [PATCH 1/5] 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. Read the unified control files instead: 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. Version() lets a caller ask which hierarchy it is talking to. Relates to #807. Signed-off-by: Daniel Golle --- cgroups/cgroups.go | 15 +- cgroups/cgroups_v1.go | 5 + cgroups/cgroups_v2.go | 276 +++++++++++++++++++++++++++++++++++-- cgroups/cgroups_v2_test.go | 250 +++++++++++++++++++++++++++++++++ 4 files changed, 528 insertions(+), 18 deletions(-) create mode 100644 cgroups/cgroups_v2_test.go diff --git a/cgroups/cgroups.go b/cgroups/cgroups.go index 48beba06..7bfdb35d 100644 --- a/cgroups/cgroups.go +++ b/cgroups/cgroups.go @@ -20,6 +20,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) @@ -37,7 +38,7 @@ func FindCgroup() (Cgroup, error) { } defer f.Close() - cgroupv2 := false + unifiedPath := "" scanner := bufio.NewScanner(f) for scanner.Scan() { text := scanner.Text() @@ -60,9 +61,10 @@ func FindCgroup() (Cgroup, error) { } return cg, nil } else if postSeparatorFields[0] == "cgroup2" { - cgroupv2 = true + // A unified hierarchy is only used when no legacy + // controller is mounted, so keep looking. + unifiedPath = fields[4] continue - // TODO cgroupv2 unimplemented } } @@ -70,8 +72,11 @@ func FindCgroup() (Cgroup, error) { return nil, err } - if cgroupv2 { - return nil, fmt.Errorf("cgroupv2 is not supported yet") + if unifiedPath != "" { + cg := &CgroupV2{ + MountPath: unifiedPath, + } + return cg, nil } return nil, fmt.Errorf("cgroup is not found") } diff --git a/cgroups/cgroups_v1.go b/cgroups/cgroups_v1.go index 17ae6673..73931548 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..045f7a23 100644 --- a/cgroups/cgroups_v2.go +++ b/cgroups/cgroups_v2.go @@ -2,8 +2,14 @@ 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 @@ -11,37 +17,281 @@ type CgroupV2 struct { MountPath string } +// unlimited is how the unified hierarchy spells "no limit". +const unlimited = "max" + +// unifiedController is the controller field of the unified hierarchy entry +// in /proc//cgroup, which is always empty. +const unifiedController = "" + +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(cg.MountPath, 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 := GetSubsystemPath(pid, unifiedController) + if err != nil { + return "", err + } + if !strings.Contains(subPath, cgPath) { + return "", fmt.Errorf("cgroup %s is not mounted as expected", cgPath) + } + + return filepath.Join(cg.MountPath, 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..e18291bd --- /dev/null +++ b/cgroups/cgroups_v2_test.go @@ -0,0 +1,250 @@ +package cgroups + +import ( + "os" + "path/filepath" + "testing" +) + +var _ Cgroup = (*CgroupV2)(nil) + +const testCgroupPath = "/cgrouptest" + +func newTestCgroupV2(t *testing.T, files map[string]string) *CgroupV2 { + t.Helper() + + mountPath := t.TempDir() + cgDir := filepath.Join(mountPath, testCgroupPath) + if err := os.Mkdir(cgDir, 0o755); err != nil { + t.Fatal(err) + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(cgDir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + + return &CgroupV2{MountPath: mountPath} +} + +func TestCgroupV2Version(t *testing.T) { + cg := &CgroupV2{} + if cg.Version() != 2 { + t.Errorf("Version() == %d, expected 2", cg.Version()) + } +} + +func TestCgroupV2MissingCgroup(t *testing.T) { + cg := &CgroupV2{MountPath: t.TempDir()} + if _, err := cg.GetPidsData(0, testCgroupPath); err == nil { + t.Error("expected an error for a cgroup that was never created") + } +} + +func TestCgroupV2GetPidsData(t *testing.T) { + for _, test := range []struct { + content string + expected int64 + }{ + {content: "1000\n", expected: 1000}, + {content: "max\n", expected: -1}, + } { + cg := newTestCgroupV2(t, map[string]string{"pids.max": test.content}) + lp, err := cg.GetPidsData(0, testCgroupPath) + if err != nil { + t.Fatal(err) + } + if *lp.Limit != test.expected { + t.Errorf("pids.max %q: limit == %d, expected %d", test.content, *lp.Limit, test.expected) + } + } +} + +func TestCgroupV2GetMemoryData(t *testing.T) { + for _, test := range []struct { + files map[string]string + description string + reservation int64 + limit int64 + swap int64 + swapReported bool + }{ + { + description: "limits in bytes", + files: map[string]string{ + "memory.max": "50593792\n", + "memory.low": "33554432\n", + "memory.swap.max": "16515072\n", + }, + limit: 50593792, + reservation: 33554432, + swap: 67108864, + swapReported: true, + }, + { + description: "unlimited memory and swap", + files: map[string]string{ + "memory.max": "max\n", + "memory.low": "0\n", + "memory.swap.max": "max\n", + }, + limit: -1, + reservation: 0, + swap: -1, + swapReported: true, + }, + { + description: "no swap accounting", + files: map[string]string{ + "memory.max": "50593792\n", + "memory.low": "0\n", + }, + limit: 50593792, + reservation: 0, + }, + } { + cg := newTestCgroupV2(t, test.files) + lm, err := cg.GetMemoryData(0, testCgroupPath) + 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 test.swapReported != (lm.Swap != nil) { + t.Fatalf("%s: swap reported == %t, expected %t", test.description, lm.Swap != nil, test.swapReported) + } + if test.swapReported && *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) { + for _, test := range []struct { + files map[string]string + description string + cpus string + mems string + quota int64 + period uint64 + }{ + { + description: "quota and explicit cpuset", + files: map[string]string{ + "cpu.max": "50000 100000\n", + "cpuset.cpus": "0-1\n", + "cpuset.mems": "0\n", + "cpu.weight": "100\n", + }, + quota: 50000, + period: 100000, + cpus: "0-1", + mems: "0", + }, + { + description: "no quota, inherited cpuset", + files: map[string]string{ + "cpu.max": "max 100000\n", + "cpuset.cpus": "\n", + "cpuset.cpus.effective": "0-3\n", + "cpuset.mems": "\n", + "cpuset.mems.effective": "0\n", + }, + quota: -1, + period: 100000, + cpus: "0-3", + mems: "0", + }, + } { + cg := newTestCgroupV2(t, test.files) + lc, err := cg.GetCPUData(0, testCgroupPath) + if err != nil { + t.Fatalf("%s: %v", test.description, err) + } + if *lc.Quota != test.quota { + t.Errorf("%s: quota == %d, expected %d", test.description, *lc.Quota, test.quota) + } + if *lc.Period != test.period { + t.Errorf("%s: period == %d, expected %d", test.description, *lc.Period, test.period) + } + if lc.Cpus != test.cpus { + t.Errorf("%s: cpus == %q, expected %q", test.description, lc.Cpus, test.cpus) + } + if lc.Mems != test.mems { + t.Errorf("%s: mems == %q, expected %q", test.description, lc.Mems, test.mems) + } + if lc.Shares != nil { + t.Errorf("%s: reported shares, which cannot be recovered from cpu.weight", test.description) + } + } +} + +func TestCgroupV2GetBlockIOData(t *testing.T) { + cg := newTestCgroupV2(t, map[string]string{ + "io.max": "8:0 rbps=102400 wbps=204800 riops=max wiops=1000\n259:0 rbps=max wbps=max riops=max wiops=max\n", + }) + lb, err := cg.GetBlockIOData(0, testCgroupPath) + 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 lb.ThrottleReadBpsDevice[0].Major != 8 || lb.ThrottleReadBpsDevice[0].Minor != 0 { + t.Errorf("read bps device == %d:%d, expected 8:0", lb.ThrottleReadBpsDevice[0].Major, lb.ThrottleReadBpsDevice[0].Minor) + } + 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") + } + + var limit uint64 = 2 * (1 << 30) + files := map[string]string{ + "hugetlb." + pageSizes[0] + ".max": "2147483648\n", + } + cg := newTestCgroupV2(t, files) + lh, err := cg.GetHugepageLimitData(0, testCgroupPath) + if err != nil { + t.Fatal(err) + } + + if len(lh) != 1 { + t.Fatalf("hugepage limits == %v, expected a single entry", lh) + } + if lh[0].Pagesize != pageSizes[0] || lh[0].Limit != limit { + t.Errorf("hugepage limit == %s/%d, expected %s/%d", lh[0].Pagesize, lh[0].Limit, pageSizes[0], limit) + } +} + +func TestCgroupV2UnreadableControllers(t *testing.T) { + cg := newTestCgroupV2(t, nil) + if _, err := cg.GetDevicesData(0, testCgroupPath); err == nil { + t.Error("expected an error for the devices controller") + } + if _, err := cg.GetNetworkData(0, testCgroupPath); err == nil { + t.Error("expected an error for the network controllers") + } +} From 97523e6089a33174ffc10437480946fdfaccb40e Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:55:55 +0100 Subject: [PATCH 2/5] 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 944dab97439cb928ffe7f408521eb9c64e77700b Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:56:48 +0100 Subject: [PATCH 3/5] 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 5f035b212766703bdc5c2844a51f80922ca2d99d Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Sun, 20 Sep 2026 19:58:55 +0100 Subject: [PATCH 4/5] 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 7bfdb35d..9f2b4054 100644 --- a/cgroups/cgroups.go +++ b/cgroups/cgroups.go @@ -21,6 +21,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 73931548..5219bcf1 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 045f7a23..6073a171 100644 --- a/cgroups/cgroups_v2.go +++ b/cgroups/cgroups_v2.go @@ -40,6 +40,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(cg.MountPath, cgPath) +} + func (cg *CgroupV2) dir(pid int, cgPath string) (string, error) { if filepath.IsAbs(cgPath) { path := filepath.Join(cg.MountPath, 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 368ead04177f70d289e73ee5e22d9c56d1f73c4e Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Mon, 21 Sep 2026 12:11:44 +0100 Subject: [PATCH 5/5] 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. 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 | 85 +++++++++++++++++++ cmd/runtimetest/main_test.go | 67 +++++++++++++++ .../linux_cgroups_devices.go | 39 +++++++-- 3 files changed, 185 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..15457d07 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -663,6 +663,90 @@ 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 +} + +// 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. +func deviceDeniedByCgroup(path string) bool { + f, err := os.OpenFile(path, os.O_RDONLY, 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 + } + expected := !deviceReadAllowed(spec.Linux.Resources.Devices, device.Type, 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 +1381,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..a6907a28 --- /dev/null +++ b/cmd/runtimetest/main_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "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") + } +} 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()) }