diff options
author | Fabricio Voznika <fvoznika@google.com> | 2020-06-10 17:03:38 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-06-10 17:05:23 -0700 |
commit | ab4c85189313097dbac5d5531f9ff6a08d9ba289 (patch) | |
tree | d99894f3d86406af3f17455a2f4839fbc4e2fab7 | |
parent | 41d9e536d559a2066ed6c92ed30aa2757d3596bb (diff) |
Cgroup fixes
- Set hugetlb related fields
- Add realtime scheduler related fields
- Beef up unit tests
Updates #2713
PiperOrigin-RevId: 315797979
-rw-r--r-- | runsc/cgroup/BUILD | 4 | ||||
-rw-r--r-- | runsc/cgroup/cgroup.go | 41 | ||||
-rw-r--r-- | runsc/cgroup/cgroup_test.go | 582 |
3 files changed, 618 insertions, 9 deletions
diff --git a/runsc/cgroup/BUILD b/runsc/cgroup/BUILD index c087e1a3c..7e34a284a 100644 --- a/runsc/cgroup/BUILD +++ b/runsc/cgroup/BUILD @@ -20,4 +20,8 @@ go_test( srcs = ["cgroup_test.go"], library = ":cgroup", tags = ["local"], + deps = [ + "//pkg/test/testutil", + "@com_github_opencontainers_runtime-spec//specs-go:go_default_library", + ], ) diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go index 5698cab4f..e5cc9d622 100644 --- a/runsc/cgroup/cgroup.go +++ b/runsc/cgroup/cgroup.go @@ -43,6 +43,7 @@ var controllers = map[string]config{ "blkio": config{ctrlr: &blockIO{}}, "cpu": config{ctrlr: &cpu{}}, "cpuset": config{ctrlr: &cpuSet{}}, + "hugetlb": config{ctrlr: &hugeTLB{}, optional: true}, "memory": config{ctrlr: &memory{}}, "net_cls": config{ctrlr: &networkClass{}}, "net_prio": config{ctrlr: &networkPrio{}}, @@ -52,7 +53,6 @@ var controllers = map[string]config{ // irrelevant for a sandbox. "devices": config{ctrlr: &noop{}}, "freezer": config{ctrlr: &noop{}}, - "hugetlb": config{ctrlr: &noop{}, optional: true}, "perf_event": config{ctrlr: &noop{}}, "rdma": config{ctrlr: &noop{}, optional: true}, "systemd": config{ctrlr: &noop{}}, @@ -125,7 +125,7 @@ func fillFromAncestor(path string) (string, error) { return val, nil } - // File is not set, recurse to parent and then set here. + // File is not set, recurse to parent and then set here. name := filepath.Base(path) parent := filepath.Dir(filepath.Dir(path)) val, err = fillFromAncestor(filepath.Join(parent, name)) @@ -446,7 +446,13 @@ func (*cpu) set(spec *specs.LinuxResources, path string) error { if err := setOptionalValueInt(path, "cpu.cfs_quota_us", spec.CPU.Quota); err != nil { return err } - return setOptionalValueUint(path, "cpu.cfs_period_us", spec.CPU.Period) + if err := setOptionalValueUint(path, "cpu.cfs_period_us", spec.CPU.Period); err != nil { + return err + } + if err := setOptionalValueUint(path, "cpu.rt_period_us", spec.CPU.RealtimePeriod); err != nil { + return err + } + return setOptionalValueInt(path, "cpu.rt_runtime_us", spec.CPU.RealtimeRuntime) } type cpuSet struct{} @@ -487,13 +493,17 @@ func (*blockIO) set(spec *specs.LinuxResources, path string) error { } for _, dev := range spec.BlockIO.WeightDevice { - val := fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, dev.Weight) - if err := setValue(path, "blkio.weight_device", val); err != nil { - return err + if dev.Weight != nil { + val := fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, *dev.Weight) + if err := setValue(path, "blkio.weight_device", val); err != nil { + return err + } } - val = fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, dev.LeafWeight) - if err := setValue(path, "blkio.leaf_weight_device", val); err != nil { - return err + if dev.LeafWeight != nil { + val := fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, *dev.LeafWeight) + if err := setValue(path, "blkio.leaf_weight_device", val); err != nil { + return err + } } } if err := setThrottle(path, "blkio.throttle.read_bps_device", spec.BlockIO.ThrottleReadBpsDevice); err != nil { @@ -551,3 +561,16 @@ func (*pids) set(spec *specs.LinuxResources, path string) error { val := strconv.FormatInt(spec.Pids.Limit, 10) return setValue(path, "pids.max", val) } + +type hugeTLB struct{} + +func (*hugeTLB) set(spec *specs.LinuxResources, path string) error { + for _, limit := range spec.HugepageLimits { + name := fmt.Sprintf("hugetlb.%s.limit_in_bytes", limit.Pagesize) + val := strconv.FormatUint(limit.Limit, 10) + if err := setValue(path, name, val); err != nil { + return err + } + } + return nil +} diff --git a/runsc/cgroup/cgroup_test.go b/runsc/cgroup/cgroup_test.go index 548c80e9a..4db5ee5c3 100644 --- a/runsc/cgroup/cgroup_test.go +++ b/runsc/cgroup/cgroup_test.go @@ -15,7 +15,14 @@ package cgroup import ( + "io/ioutil" + "os" + "path/filepath" + "strings" "testing" + + specs "github.com/opencontainers/runtime-spec/specs-go" + "gvisor.dev/gvisor/pkg/test/testutil" ) func TestUninstallEnoent(t *testing.T) { @@ -65,3 +72,578 @@ func TestCountCpuset(t *testing.T) { }) } } + +func uint16Ptr(v uint16) *uint16 { + return &v +} + +func uint32Ptr(v uint32) *uint32 { + return &v +} + +func int64Ptr(v int64) *int64 { + return &v +} + +func uint64Ptr(v uint64) *uint64 { + return &v +} + +func boolPtr(v bool) *bool { + return &v +} + +func checkDir(t *testing.T, dir string, contents map[string]string) { + all, err := ioutil.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir(%q): %v", dir, err) + } + fileCount := 0 + for _, file := range all { + if file.IsDir() { + // Only want to compare files. + continue + } + fileCount++ + + want, ok := contents[file.Name()] + if !ok { + t.Errorf("file not expected: %q", file.Name()) + continue + } + gotBytes, err := ioutil.ReadFile(filepath.Join(dir, file.Name())) + if err != nil { + t.Fatal(err.Error()) + } + got := strings.TrimSuffix(string(gotBytes), "\n") + if got != want { + t.Errorf("wrong file content, file: %q, want: %q, got: %q", file.Name(), want, got) + } + } + if fileCount != len(contents) { + t.Errorf("file is missing, want: %v, got: %v", contents, all) + } +} + +func makeLinuxWeightDevice(major, minor int64, weight, leafWeight *uint16) specs.LinuxWeightDevice { + rv := specs.LinuxWeightDevice{ + Weight: weight, + LeafWeight: leafWeight, + } + rv.Major = major + rv.Minor = minor + return rv +} + +func makeLinuxThrottleDevice(major, minor int64, rate uint64) specs.LinuxThrottleDevice { + rv := specs.LinuxThrottleDevice{ + Rate: rate, + } + rv.Major = major + rv.Minor = minor + return rv +} + +func TestBlockIO(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxBlockIO + wants map[string]string + }{ + { + name: "simple", + spec: &specs.LinuxBlockIO{ + Weight: uint16Ptr(1), + LeafWeight: uint16Ptr(2), + }, + wants: map[string]string{ + "blkio.weight": "1", + "blkio.leaf_weight": "2", + }, + }, + { + name: "weight_device", + spec: &specs.LinuxBlockIO{ + WeightDevice: []specs.LinuxWeightDevice{ + makeLinuxWeightDevice(1, 2, uint16Ptr(3), uint16Ptr(4)), + }, + }, + wants: map[string]string{ + "blkio.weight_device": "1:2 3", + "blkio.leaf_weight_device": "1:2 4", + }, + }, + { + name: "weight_device_nil_values", + spec: &specs.LinuxBlockIO{ + WeightDevice: []specs.LinuxWeightDevice{ + makeLinuxWeightDevice(1, 2, nil, nil), + }, + }, + }, + { + name: "throttle", + spec: &specs.LinuxBlockIO{ + ThrottleReadBpsDevice: []specs.LinuxThrottleDevice{ + makeLinuxThrottleDevice(1, 2, 3), + }, + ThrottleReadIOPSDevice: []specs.LinuxThrottleDevice{ + makeLinuxThrottleDevice(4, 5, 6), + }, + ThrottleWriteBpsDevice: []specs.LinuxThrottleDevice{ + makeLinuxThrottleDevice(7, 8, 9), + }, + ThrottleWriteIOPSDevice: []specs.LinuxThrottleDevice{ + makeLinuxThrottleDevice(10, 11, 12), + }, + }, + wants: map[string]string{ + "blkio.throttle.read_bps_device": "1:2 3", + "blkio.throttle.read_iops_device": "4:5 6", + "blkio.throttle.write_bps_device": "7:8 9", + "blkio.throttle.write_iops_device": "10:11 12", + }, + }, + { + name: "nil_values", + spec: &specs.LinuxBlockIO{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + BlockIO: tc.spec, + } + ctrlr := blockIO{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +func TestCPU(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxCPU + wants map[string]string + }{ + { + name: "all", + spec: &specs.LinuxCPU{ + Shares: uint64Ptr(1), + Quota: int64Ptr(2), + Period: uint64Ptr(3), + RealtimeRuntime: int64Ptr(4), + RealtimePeriod: uint64Ptr(5), + }, + wants: map[string]string{ + "cpu.shares": "1", + "cpu.cfs_quota_us": "2", + "cpu.cfs_period_us": "3", + "cpu.rt_runtime_us": "4", + "cpu.rt_period_us": "5", + }, + }, + { + name: "nil_values", + spec: &specs.LinuxCPU{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + CPU: tc.spec, + } + ctrlr := cpu{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +func TestCPUSet(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxCPU + wants map[string]string + }{ + { + name: "all", + spec: &specs.LinuxCPU{ + Cpus: "foo", + Mems: "bar", + }, + wants: map[string]string{ + "cpuset.cpus": "foo", + "cpuset.mems": "bar", + }, + }, + // Don't test nil values because they are copied from the parent. + // See TestCPUSetAncestor(). + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + CPU: tc.spec, + } + ctrlr := cpuSet{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +// TestCPUSetAncestor checks that, when not available, value is read from +// parent directory. +func TestCPUSetAncestor(t *testing.T) { + // Prepare master directory with cgroup files that will be propagated to + // children. + grandpa, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(grandpa) + + if err := ioutil.WriteFile(filepath.Join(grandpa, "cpuset.cpus"), []byte("parent-cpus"), 0666); err != nil { + t.Fatalf("ioutil.WriteFile(): %v", err) + } + if err := ioutil.WriteFile(filepath.Join(grandpa, "cpuset.mems"), []byte("parent-mems"), 0666); err != nil { + t.Fatalf("ioutil.WriteFile(): %v", err) + } + + for _, tc := range []struct { + name string + spec *specs.LinuxCPU + }{ + { + name: "nil_values", + spec: &specs.LinuxCPU{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Create empty files in intermediate directory. They should be ignored + // when reading, and then populated from parent. + parent, err := ioutil.TempDir(grandpa, "parent") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(parent) + if _, err := os.Create(filepath.Join(parent, "cpuset.cpus")); err != nil { + t.Fatalf("os.Create(): %v", err) + } + if _, err := os.Create(filepath.Join(parent, "cpuset.mems")); err != nil { + t.Fatalf("os.Create(): %v", err) + } + + // cgroup files mmust exist. + dir, err := ioutil.TempDir(parent, "child") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + if _, err := os.Create(filepath.Join(dir, "cpuset.cpus")); err != nil { + t.Fatalf("os.Create(): %v", err) + } + if _, err := os.Create(filepath.Join(dir, "cpuset.mems")); err != nil { + t.Fatalf("os.Create(): %v", err) + } + + spec := &specs.LinuxResources{ + CPU: tc.spec, + } + ctrlr := cpuSet{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + want := map[string]string{ + "cpuset.cpus": "parent-cpus", + "cpuset.mems": "parent-mems", + } + // Both path and dir must have been populated from grandpa. + checkDir(t, parent, want) + checkDir(t, dir, want) + }) + } +} + +func TestHugeTlb(t *testing.T) { + for _, tc := range []struct { + name string + spec []specs.LinuxHugepageLimit + wants map[string]string + }{ + { + name: "single", + spec: []specs.LinuxHugepageLimit{ + { + Pagesize: "1G", + Limit: 123, + }, + }, + wants: map[string]string{ + "hugetlb.1G.limit_in_bytes": "123", + }, + }, + { + name: "multiple", + spec: []specs.LinuxHugepageLimit{ + { + Pagesize: "1G", + Limit: 123, + }, + { + Pagesize: "2G", + Limit: 456, + }, + { + Pagesize: "1P", + Limit: 789, + }, + }, + wants: map[string]string{ + "hugetlb.1G.limit_in_bytes": "123", + "hugetlb.2G.limit_in_bytes": "456", + "hugetlb.1P.limit_in_bytes": "789", + }, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + HugepageLimits: tc.spec, + } + ctrlr := hugeTLB{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +func TestMemory(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxMemory + wants map[string]string + }{ + { + name: "all", + spec: &specs.LinuxMemory{ + Limit: int64Ptr(1), + Reservation: int64Ptr(2), + Swap: int64Ptr(3), + Kernel: int64Ptr(4), + KernelTCP: int64Ptr(5), + Swappiness: uint64Ptr(6), + DisableOOMKiller: boolPtr(true), + }, + wants: map[string]string{ + "memory.limit_in_bytes": "1", + "memory.soft_limit_in_bytes": "2", + "memory.memsw.limit_in_bytes": "3", + "memory.kmem.limit_in_bytes": "4", + "memory.kmem.tcp.limit_in_bytes": "5", + "memory.swappiness": "6", + "memory.oom_control": "1", + }, + }, + { + // Disable OOM killer should only write when set to true. + name: "oomkiller", + spec: &specs.LinuxMemory{ + DisableOOMKiller: boolPtr(false), + }, + }, + { + name: "nil_values", + spec: &specs.LinuxMemory{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + Memory: tc.spec, + } + ctrlr := memory{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +func TestNetworkClass(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxNetwork + wants map[string]string + }{ + { + name: "all", + spec: &specs.LinuxNetwork{ + ClassID: uint32Ptr(1), + }, + wants: map[string]string{ + "net_cls.classid": "1", + }, + }, + { + name: "nil_values", + spec: &specs.LinuxNetwork{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + Network: tc.spec, + } + ctrlr := networkClass{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +func TestNetworkPriority(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxNetwork + wants map[string]string + }{ + { + name: "all", + spec: &specs.LinuxNetwork{ + Priorities: []specs.LinuxInterfacePriority{ + { + Name: "foo", + Priority: 1, + }, + }, + }, + wants: map[string]string{ + "net_prio.ifpriomap": "foo 1", + }, + }, + { + name: "nil_values", + spec: &specs.LinuxNetwork{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + Network: tc.spec, + } + ctrlr := networkPrio{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} + +func TestPids(t *testing.T) { + for _, tc := range []struct { + name string + spec *specs.LinuxPids + wants map[string]string + }{ + { + name: "all", + spec: &specs.LinuxPids{Limit: 1}, + wants: map[string]string{ + "pids.max": "1", + }, + }, + { + name: "nil_values", + spec: &specs.LinuxPids{}, + }, + { + name: "nil", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dir, err := ioutil.TempDir(testutil.TmpDir(), "cgroup") + if err != nil { + t.Fatalf("error creating temporary directory: %v", err) + } + defer os.RemoveAll(dir) + + spec := &specs.LinuxResources{ + Pids: tc.spec, + } + ctrlr := pids{} + if err := ctrlr.set(spec, dir); err != nil { + t.Fatalf("ctrlr.set(): %v", err) + } + checkDir(t, dir, tc.wants) + }) + } +} |