diff --git a/.buildkite/hooks/pre-command b/.buildkite/hooks/pre-command index 591c9dfd4..3170d4931 100644 --- a/.buildkite/hooks/pre-command +++ b/.buildkite/hooks/pre-command @@ -25,9 +25,8 @@ export TOTAL_PARTITIONS=${BUILDKITE_PARALLEL_JOB_COUNT:-1} # Ensure Docker has experimental enabled. EXPERIMENTAL=$(sudo docker version --format='{{.Server.Experimental}}') -CGDRIVER=$(sudo docker info --format='{{.CgroupDriver}}') -if test "${EXPERIMENTAL}" != "true" || test "${CGDRIVER}" != "cgroupfs"; then - make sudo TARGETS=//runsc:runsc ARGS="install --experimental=true --cgroupdriver=cgroupfs" +if test "${EXPERIMENTAL}" != "true"; then + make sudo TARGETS=//runsc:runsc ARGS="install --experimental=true" sudo systemctl restart docker fi diff --git a/Makefile b/Makefile index 27f6d043c..50e5e7da0 100644 --- a/Makefile +++ b/Makefile @@ -161,6 +161,7 @@ dev: $(RUNTIME_BIN) ## Installs a set of local runtimes. Requires sudo. @$(call configure_noreload,$(RUNTIME)-fuse-d,--net-raw --debug --strace --log-packets --fuse) @$(call configure_noreload,$(RUNTIME)-cgroup-d,--net-raw --debug --strace --log-packets --cgroupfs) @$(call configure_noreload,$(RUNTIME)-lisafs-d,--net-raw --debug --strace --log-packets --lisafs) + @$(call configure_noreload,$(RUNTIME)-systemd-d,--net-raw --debug --strace --log-packets --systemd-cgroup) @$(call reload_docker) .PHONY: dev diff --git a/WORKSPACE b/WORKSPACE index 8c4380d6c..9fd92eda8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1491,8 +1491,8 @@ go_repository( go_repository( name = "com_github_coreos_go_systemd_v22", importpath = "github.com/coreos/go-systemd/v22", - sum = "h1:kq/SbG2BCKLkDKkjQf5OWwKWUKj1lgs3lFI4PxnR5lg=", - version = "v22.1.0", + sum = "h1:D9/bQk5vlXQFZ6Kwuu6zaiXJ9oTPe68++AzAJc1DzSI=", + version = "v22.3.2", ) go_repository( @@ -1896,3 +1896,10 @@ go_repository( sum = "h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=", version = "v1.2.0", ) + +go_repository( + name = "com_github_bits_and_blooms_bitset", + importpath = "github.com/bits-and-blooms/bitset", + sum = "h1:M+/hrU9xlMp7t4TyTDQW97d3tRPVuKFC6zBEK16QnXY=", + version = "v1.2.1", +) diff --git a/go.mod b/go.mod index bc0f6eab6..196a58e98 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.17 require ( github.com/BurntSushi/toml v0.3.1 github.com/bazelbuild/rules_go v0.30.0 + github.com/bits-and-blooms/bitset v1.2.0 github.com/cenkalti/backoff v1.1.1-0.20190506075156-2146c9339422 github.com/containerd/cgroups v1.0.1 github.com/containerd/console v1.0.1 @@ -12,6 +13,7 @@ require ( github.com/containerd/fifo v1.0.0 github.com/containerd/go-runc v1.0.0 github.com/containerd/typeurl v1.0.2 + github.com/coreos/go-systemd/v22 v22.3.2 github.com/gofrs/flock v0.8.0 github.com/gogo/protobuf v1.3.2 github.com/google/btree v1.0.1 @@ -39,10 +41,9 @@ require ( github.com/cilium/ebpf v0.4.0 // indirect github.com/containerd/continuity v0.2.1 // indirect github.com/containerd/ttrpc v1.0.2 // indirect - github.com/coreos/go-systemd/v22 v22.1.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/docker/go-units v0.4.0 // indirect - github.com/godbus/dbus/v5 v5.0.3 // indirect + github.com/godbus/dbus/v5 v5.0.4 // indirect github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/google/go-cmp v0.5.6 // indirect diff --git a/go.sum b/go.sum index 85cf2c214..9cfc88980 100644 --- a/go.sum +++ b/go.sum @@ -61,6 +61,8 @@ github.com/bazelbuild/rules_go v0.30.0 h1:kX4jVcstqrsRqKPJSn2mq2o+TI21edRzEJSrEO github.com/bazelbuild/rules_go v0.30.0/go.mod h1:MC23Dc/wkXEyk3Wpq6lCqz0ZAYOZDw2DR5y3N1q2i7M= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= +github.com/bits-and-blooms/bitset v1.2.0 h1:Kn4yilvwNtMACtf1eYDlG8H77R07mZSPbMjLyS07ChA= +github.com/bits-and-blooms/bitset v1.2.0/go.mod h1:gIdJ4wp64HaoK2YrL1Q5/N7Y16edYb8uY+O0FJTyyDA= github.com/cenkalti/backoff v1.1.1-0.20190506075156-2146c9339422 h1:8eZxmY1yvxGHzdzTEhI09npjMVGzNAdrqzruTX6jcK4= github.com/cenkalti/backoff v1.1.1-0.20190506075156-2146c9339422/go.mod h1:b6Nc7NRH5C4aCISLry0tLnTjcuTEvoiqcWDdsU0sOGM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -112,6 +114,8 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/go-systemd/v22 v22.0.0/go.mod h1:xO0FLkIi5MaZafQlIrOotqXZ90ih+1atmu1JpKERPPk= github.com/coreos/go-systemd/v22 v22.1.0 h1:kq/SbG2BCKLkDKkjQf5OWwKWUKj1lgs3lFI4PxnR5lg= github.com/coreos/go-systemd/v22 v22.1.0/go.mod h1:xO0FLkIi5MaZafQlIrOotqXZ90ih+1atmu1JpKERPPk= +github.com/coreos/go-systemd/v22 v22.3.2 h1:D9/bQk5vlXQFZ6Kwuu6zaiXJ9oTPe68++AzAJc1DzSI= +github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= @@ -155,6 +159,8 @@ github.com/go-openapi/swag v0.0.0-20160704191624-1d0bd113de87/go.mod h1:DXUve3Dp github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/godbus/dbus/v5 v5.0.3 h1:ZqHaoEF7TBzh4jzPmqVhE/5A1z9of6orkAe5uHoAeME= github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= +github.com/godbus/dbus/v5 v5.0.4 h1:9349emZab16e7zQvpmsbtjc18ykshndd8y2PG3sgJbA= +github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gofrs/flock v0.8.0 h1:MSdYClljsF3PbENUUEx85nkWfJSGfzYI9yEBZOJz6CY= github.com/gofrs/flock v0.8.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= diff --git a/pkg/test/dockerutil/BUILD b/pkg/test/dockerutil/BUILD index 366f068e3..3a535b0e8 100644 --- a/pkg/test/dockerutil/BUILD +++ b/pkg/test/dockerutil/BUILD @@ -15,6 +15,7 @@ go_library( visibility = ["//:sandbox"], deps = [ "//pkg/test/testutil", + "//runsc/cgroup", "@com_github_docker_docker//api/types:go_default_library", "@com_github_docker_docker//api/types/container:go_default_library", "@com_github_docker_docker//api/types/mount:go_default_library", diff --git a/pkg/test/dockerutil/dockerutil.go b/pkg/test/dockerutil/dockerutil.go index 31216ae9a..313e7f5b4 100644 --- a/pkg/test/dockerutil/dockerutil.go +++ b/pkg/test/dockerutil/dockerutil.go @@ -29,6 +29,7 @@ import ( "time" "gvisor.dev/gvisor/pkg/test/testutil" + "gvisor.dev/gvisor/runsc/cgroup" ) var ( @@ -54,6 +55,11 @@ var ( pprofCPU = flag.Bool("pprof-cpu", false, "enables CPU profiling with runsc debug") pprofHeap = flag.Bool("pprof-heap", false, "enables heap profiling with runsc debug") pprofMutex = flag.Bool("pprof-mutex", false, "enables mutex profiling with runsc debug") + + // This matches the string "native.cgroupdriver=systemd" (including optional + // whitespace), which can be found in a docker daemon configuration file's + // exec-opts field. + useSystemdRgx = regexp.MustCompile("\\s*(native\\.cgroupdriver)\\s*=\\s*(systemd)\\s*") ) // EnsureSupportedDockerVersion checks if correct docker is installed. @@ -92,6 +98,39 @@ func RuntimePath() (string, error) { return p, nil } +// UsingSystemdCgroup returns true if the docker configuration has the +// native.cgroupdriver=systemd option set in "exec-opts", or if the +// system is using cgroupv2, in which case systemd is the default driver. +func UsingSystemdCgroup() (bool, error) { + // Read the configuration data; the file must exist. + configBytes, err := ioutil.ReadFile(*config) + if err != nil { + return false, err + } + // Unmarshal the configuration. + c := make(map[string]interface{}) + if err := json.Unmarshal(configBytes, &c); err != nil { + return false, err + } + // Decode the expected configuration. + e, ok := c["exec-opts"] + if !ok { + // No exec-opts. Default is true on cgroupv2, false otherwise. + return cgroup.IsOnlyV2(), nil + } + eos, ok := e.([]interface{}) + if !ok { + // The exec opts are not an array. + return false, fmt.Errorf("unexpected format: %+v", eos) + } + for _, opt := range eos { + if optStr, ok := opt.(string); ok && useSystemdRgx.MatchString(optStr) { + return true, nil + } + } + return false, nil +} + func runtimeMap() (map[string]interface{}, error) { // Read the configuration data; the file must exist. configBytes, err := ioutil.ReadFile(*config) diff --git a/runsc/cgroup/BUILD b/runsc/cgroup/BUILD index a8a085761..3c135a680 100644 --- a/runsc/cgroup/BUILD +++ b/runsc/cgroup/BUILD @@ -13,6 +13,7 @@ go_library( deps = [ "//pkg/cleanup", "//pkg/log", + "@com_github_bits_and_blooms_bitset//:go_default_library", "@com_github_cenkalti_backoff//:go_default_library", "@com_github_coreos_go_systemd_v22//dbus:go_default_library", "@com_github_godbus_dbus_v5//:go_default_library", diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go index 6ff0ea494..e7140ea53 100644 --- a/runsc/cgroup/cgroup.go +++ b/runsc/cgroup/cgroup.go @@ -332,25 +332,34 @@ type cgroupV1 struct { // NewFromSpec creates a new Cgroup instance if the spec includes a cgroup path. // Returns nil otherwise. Cgroup paths are loaded based on the current process. -func NewFromSpec(spec *specs.Spec) (Cgroup, error) { +// If useSystemd is true, the Cgroup will be created and managed with +// systemd. This requires systemd (>=v244) to be running on the host and the +// cgroup path to be in the form `slice:prefix:name`. +func NewFromSpec(spec *specs.Spec, useSystemd bool) (Cgroup, error) { if spec.Linux == nil || spec.Linux.CgroupsPath == "" { return nil, nil } - return NewFromPath(spec.Linux.CgroupsPath) + return NewFromPath(spec.Linux.CgroupsPath, useSystemd) } // NewFromPath creates a new Cgroup instance from the specified relative path. // Cgroup paths are loaded based on the current process. -func NewFromPath(cgroupsPath string) (Cgroup, error) { - return new("self", cgroupsPath) +// If useSystemd is true, the Cgroup will be created and managed with +// systemd. This requires systemd (>=v244) to be running on the host and the +// cgroup path to be in the form `slice:prefix:name`. +func NewFromPath(cgroupsPath string, useSystemd bool) (Cgroup, error) { + return new("self", cgroupsPath, useSystemd) } // NewFromPid loads cgroup for the given process. -func NewFromPid(pid int) (Cgroup, error) { - return new(strconv.Itoa(pid), "") +// If useSystemd is true, the Cgroup will be created and managed with +// systemd. This requires systemd (>=v244) to be running on the host and the +// cgroup path to be in the form `slice:prefix:name`. +func NewFromPid(pid int, useSystemd bool) (Cgroup, error) { + return new(strconv.Itoa(pid), "", useSystemd) } -func new(pid, cgroupsPath string) (Cgroup, error) { +func new(pid, cgroupsPath string, useSystemd bool) (Cgroup, error) { var ( parents map[string]string err error @@ -367,13 +376,15 @@ func new(pid, cgroupsPath string) (Cgroup, error) { } if IsOnlyV2() { - if p, ok := parents[cgroup2Key]; ok { + // The cgroupsPath is in a special `slice:prefix:name` format for systemd + // that should not be modified. + if p, ok := parents[cgroup2Key]; ok && !useSystemd { // The cgroup of current pid will have tasks in it and we can't use // that, instead, use the its parent which should not have tasks in it. cgroupsPath = filepath.Join(filepath.Dir(p), cgroupsPath) } // Assume that for v2, cgroup is always mounted at cgroupRoot. - cg, err = newCgroupV2(cgroupRoot, cgroupsPath) + cg, err = newCgroupV2(cgroupRoot, cgroupsPath, useSystemd) if err != nil { return nil, err } @@ -390,7 +401,8 @@ func new(pid, cgroupsPath string) (Cgroup, error) { // CgroupJSON is a wrapper for Cgroup that can be encoded to JSON. type CgroupJSON struct { - Cgroup Cgroup `json:"cgroup"` + Cgroup Cgroup `json:"cgroup"` + UseSystemd bool `json:"useSystemd"` } type cgroupJSONv1 struct { @@ -401,8 +413,23 @@ type cgroupJSONv2 struct { Cgroup *cgroupV2 `json:"cgroup"` } +type cgroupJSONSystemd struct { + Cgroup *cgroupSystemd `json:"cgroup"` +} + // UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON func (c *CgroupJSON) UnmarshalJSON(data []byte) error { + if c.UseSystemd { + systemd := cgroupJSONSystemd{} + if err := json.Unmarshal(data, &systemd); err != nil { + return err + } + if systemd.Cgroup != nil { + c.Cgroup = systemd.Cgroup + } + return nil + } + if IsOnlyV2() { v2 := cgroupJSONv2{} err := json.Unmarshal(data, &v2) @@ -426,6 +453,10 @@ func (c *CgroupJSON) MarshalJSON() ([]byte, error) { return json.Marshal(&v1) } if IsOnlyV2() { + if c.UseSystemd { + systemd := cgroupJSONSystemd{Cgroup: c.Cgroup.(*cgroupSystemd)} + return json.Marshal(&systemd) + } v2 := cgroupJSONv2{Cgroup: c.Cgroup.(*cgroupV2)} return json.Marshal(&v2) } diff --git a/runsc/cgroup/cgroup_v2.go b/runsc/cgroup/cgroup_v2.go index 8881e7eb9..b5cc59130 100644 --- a/runsc/cgroup/cgroup_v2.go +++ b/runsc/cgroup/cgroup_v2.go @@ -20,6 +20,7 @@ import ( "bufio" "bytes" "context" + "encoding/binary" "errors" "fmt" "io/ioutil" @@ -30,7 +31,9 @@ import ( "strings" "time" + "github.com/bits-and-blooms/bitset" "github.com/cenkalti/backoff" + "github.com/coreos/go-systemd/v22/dbus" specs "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/cleanup" @@ -51,7 +54,7 @@ var ( ErrInvalidGroupPath = errors.New("cgroup: invalid group path") // controllers2 is the group of all supported cgroupv2 controllers - controllers2 = map[string]controller{ + controllers2 = map[string]controllerv2{ "cpu": &cpu2{}, "cpuset": &cpuset2{}, "io": &io2{}, @@ -73,28 +76,23 @@ type cgroupV2 struct { Own []string `json:"own"` } -func newCgroupV2(mountpoint string, group string) (*cgroupV2, error) { +func newCgroupV2(mountpoint, group string, useSystemd bool) (Cgroup, error) { data, err := ioutil.ReadFile(filepath.Join(mountpoint, "cgroup.controllers")) if err != nil { return nil, err } - - return &cgroupV2{ + cg := &cgroupV2{ Mountpoint: mountpoint, Path: group, Controllers: strings.Fields(string(data)), - }, nil + } + if useSystemd { + return newCgroupV2Systemd(cg) + } + return cg, err } -// Install creates and configures cgroups. -func (c *cgroupV2) Install(res *specs.LinuxResources) error { - log.Debugf("Installing cgroup path %q", c.MakePath("")) - - // Clean up partially created cgroups on error. Errors during cleanup itself - // are ignored. - clean := cleanup.Make(func() { _ = c.Uninstall() }) - defer clean.Clean() - +func (c *cgroupV2) createCgroupPaths() (bool, error) { // setup all known controllers for the current subtree // For example, given path /foo/bar and mount /sys/fs/cgroup, we need to write // the controllers to: @@ -103,14 +101,14 @@ func (c *cgroupV2) Install(res *specs.LinuxResources) error { val := "+" + strings.Join(c.Controllers, " +") elements := strings.Split(c.Path, "/") current := c.Mountpoint + created := false for i, e := range elements { current = filepath.Join(current, e) - created := false if i > 0 { if err := os.Mkdir(current, 0o755); err != nil { if !os.IsExist(err) { - return err + return false, err } } else { created = true @@ -120,33 +118,49 @@ func (c *cgroupV2) Install(res *specs.LinuxResources) error { // enable all known controllers for subtree if i < len(elements)-1 { if err := writeFile(filepath.Join(current, subtreeControl), []byte(val), 0700); err != nil { - return err + return false, err } - } else if created { - // if we created our final cgroup path then we can set the resources - for controllerName, ctrlr := range controllers2 { - // first check if our controller is found in the system - found := false - for _, knownController := range c.Controllers { - if controllerName == knownController { - found = true - } - } + } + } + return created, nil +} - // if we don't have the controller - if found { - if err := ctrlr.set(res, current); err != nil { - return err - } - continue +// Install creates and configures cgroups. +func (c *cgroupV2) Install(res *specs.LinuxResources) error { + log.Debugf("Installing cgroup path %q", c.MakePath("")) + // Clean up partially created cgroups on error. Errors during cleanup itself + // are ignored. + clean := cleanup.Make(func() { _ = c.Uninstall() }) + defer clean.Clean() + + created, err := c.createCgroupPaths() + if err != nil { + return err + } + if created { + // If we created our final cgroup path then we can set the resources. + for controllerName, ctrlr := range controllers2 { + // First check if our controller is found in the system. + found := false + for _, knownController := range c.Controllers { + if controllerName == knownController { + found = true } - if ctrlr.optional() { - if err := ctrlr.skip(res); err != nil { - return err - } - } else { - return fmt.Errorf("mandatory cgroup controller %q is missing for %q", controllerName, current) + } + + // In case we don't have the controller. + if found { + if err := ctrlr.set(res, c.MakePath("")); err != nil { + return err } + continue + } + if ctrlr.optional() { + if err := ctrlr.skip(res); err != nil { + return err + } + } else { + return fmt.Errorf("mandatory cgroup controller %q is missing for %q", controllerName, c.MakePath("")) } } } @@ -303,10 +317,63 @@ func (c *cgroupV2) MakePath(controllerName string) string { return filepath.Join(c.Mountpoint, c.Path) } +type controllerv2 interface { + controller + generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) +} + type cpu2 struct { mandatory } +func (*cpu2) generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) { + props := []dbus.Property{} + if spec == nil || spec.CPU == nil { + return props, nil + } + cpu := spec.CPU + if cpu.Shares != nil { + weight := convertCPUSharesToCgroupV2Value(*cpu.Shares) + if weight != 0 { + props = append(props, newProp("CPUWeight", weight)) + } + } + var ( + period uint64 + quota int64 + ) + if cpu.Period != nil { + period = *cpu.Period + } + if cpu.Quota != nil { + quota = *cpu.Quota + } + if period != 0 { + props = append(props, newProp("CPUQuotaPeriodUSec", period)) + } + if quota != 0 || period != 0 { + // Corresponds to USEC_INFINITY in systemd. + cpuQuotaPerSecUSec := uint64(math.MaxUint64) + if quota > 0 { + if period == 0 { + // Assume the default. + period = defaultPeriod + } + // systemd converts CPUQuotaPerSecUSec (microseconds per CPU second) to + // CPUQuota (integer percentage of CPU) internally. This means that if a + // fractional percent of CPU is indicated by spec.CPU.Quota, we need to + // round up to the nearest 10ms (1% of a second) such that child cgroups + // can set the cpu.cfs_quota_us they expect. + cpuQuotaPerSecUSec = uint64(quota*1000000) / period + if cpuQuotaPerSecUSec%10000 != 0 { + cpuQuotaPerSecUSec = ((cpuQuotaPerSecUSec / 10000) + 1) * 10000 + } + } + props = append(props, newProp("CPUQuotaPerSecUSec", cpuQuotaPerSecUSec)) + } + return props, nil +} + func (*cpu2) set(spec *specs.LinuxResources, path string) error { if spec == nil || spec.CPU == nil { return nil @@ -347,6 +414,34 @@ type cpuset2 struct { mandatory } +func (*cpuset2) generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) { + props := []dbus.Property{} + if spec == nil || spec.CPU == nil { + return props, nil + } + cpu := spec.CPU + if cpu.Cpus == "" && cpu.Mems == "" { + return props, nil + } + cpus := cpu.Cpus + mems := cpu.Mems + if cpus != "" { + bits, err := RangeToBits(cpus) + if err != nil { + return nil, fmt.Errorf("%w: cpus=%q conversion error: %v", ErrBadResourceSpec, cpus, err) + } + props = append(props, newProp("AllowedCPUs", bits)) + } + if mems != "" { + bits, err := RangeToBits(mems) + if err != nil { + return nil, fmt.Errorf("%w: mems=%q conversion error: %v", ErrBadResourceSpec, mems, err) + } + props = append(props, newProp("AllowedMemoryNodes", bits)) + } + return props, nil +} + func (*cpuset2) set(spec *specs.LinuxResources, path string) error { if spec == nil || spec.CPU == nil { return nil @@ -371,6 +466,31 @@ type memory2 struct { mandatory } +func (*memory2) generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) { + props := []dbus.Property{} + if spec == nil || spec.Memory == nil { + return props, nil + } + mem := spec.Memory + if mem.Swap != nil { + if mem.Limit == nil { + return nil, ErrBadResourceSpec + } + swap, err := convertMemorySwapToCgroupV2Value(*mem.Swap, *mem.Limit) + if err != nil { + return nil, err + } + props = append(props, newProp("MemorySwapMax", uint64(swap))) + } + if mem.Limit != nil { + props = append(props, newProp("MemoryMax", uint64(*mem.Limit))) + } + if mem.Reservation != nil { + props = append(props, newProp("MemoryLow", uint64(*mem.Reservation))) + } + return props, nil +} + func (*memory2) set(spec *specs.LinuxResources, path string) error { if spec == nil || spec.Memory == nil { return nil @@ -424,6 +544,13 @@ type pid2 struct { mandatory } +func (*pid2) generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) { + if spec != nil && spec.Pids != nil { + return []dbus.Property{newProp("TasksMax", uint64(spec.Pids.Limit))}, nil + } + return []dbus.Property{}, nil +} + func (*pid2) set(spec *specs.LinuxResources, path string) error { if spec == nil || spec.Pids == nil { return nil @@ -440,6 +567,29 @@ type io2 struct { mandatory } +func (*io2) generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) { + props := []dbus.Property{} + if spec == nil || spec.BlockIO == nil { + return props, nil + } + io := spec.BlockIO + if io != nil { + if io.Weight != nil && *io.Weight != 0 { + ioWeight := convertBlkIOToIOWeightValue(*io.Weight) + props = append(props, newProp("IOWeight", ioWeight)) + } + for _, dev := range io.WeightDevice { + val := fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, *dev.Weight) + props = append(props, newProp("IODeviceWeight", val)) + } + props = addIOProps(props, "IOReadBandwidthMax", io.ThrottleReadBpsDevice) + props = addIOProps(props, "IOWriteBandwidthMax", io.ThrottleWriteBpsDevice) + props = addIOProps(props, "IOReadIOPSMax", io.ThrottleReadIOPSDevice) + props = addIOProps(props, "IOWriteIOPSMax", io.ThrottleWriteIOPSDevice) + } + return props, nil +} + func (*io2) set(spec *specs.LinuxResources, path string) error { if spec == nil || spec.BlockIO == nil { return nil @@ -532,6 +682,10 @@ func (*hugeTLB2) skip(spec *specs.LinuxResources) error { return nil } +func (*hugeTLB2) generateProperties(spec *specs.LinuxResources) ([]dbus.Property, error) { + return nil, nil +} + func (*hugeTLB2) set(spec *specs.LinuxResources, path string) error { if spec == nil { return nil @@ -673,3 +827,59 @@ func parseUint(s string, base, bitSize int) (uint64, error) { return value, nil } + +// RangeToBits converts a text representation of a CPU mask (as written to +// or read from cgroups' cpuset.* files, e.g. "1,3-5") to a slice of bytes +// with the corresponding bits set (as consumed by systemd over dbus as +// AllowedCPUs/AllowedMemoryNodes unit property value). +// Copied from runc. +func RangeToBits(str string) ([]byte, error) { + bits := &bitset.BitSet{} + for _, r := range strings.Split(str, ",") { + // allow extra spaces around + r = strings.TrimSpace(r) + // allow empty elements (extra commas) + if r == "" { + continue + } + ranges := strings.SplitN(r, "-", 2) + if len(ranges) > 1 { + start, err := strconv.ParseUint(ranges[0], 10, 32) + if err != nil { + return nil, err + } + end, err := strconv.ParseUint(ranges[1], 10, 32) + if err != nil { + return nil, err + } + if start > end { + return nil, errors.New("invalid range: " + r) + } + for i := uint(start); i <= uint(end); i++ { + bits.Set(i) + } + } else { + val, err := strconv.ParseUint(ranges[0], 10, 32) + if err != nil { + return nil, err + } + bits.Set(uint(val)) + } + } + + val := bits.Bytes() + if len(val) == 0 { + // do not allow empty values + return nil, errors.New("empty value") + } + ret := make([]byte, len(val)*8) + for i := range val { + // bitset uses BigEndian internally + binary.BigEndian.PutUint64(ret[i*8:], val[len(val)-1-i]) + } + // remove upper all-zero bytes + for ret[0] == 0 { + ret = ret[1:] + } + return ret, nil +} diff --git a/runsc/cgroup/systemd.go b/runsc/cgroup/systemd.go index 8e974324a..68083876b 100644 --- a/runsc/cgroup/systemd.go +++ b/runsc/cgroup/systemd.go @@ -15,50 +15,90 @@ package cgroup import ( + "context" "errors" "fmt" "os" - "path" "path/filepath" + "regexp" "strconv" + "strings" + "time" systemdDbus "github.com/coreos/go-systemd/v22/dbus" dbus "github.com/godbus/dbus/v5" specs "github.com/opencontainers/runtime-spec/specs-go" + "gvisor.dev/gvisor/pkg/cleanup" + "gvisor.dev/gvisor/pkg/log" ) -// ErrBadResourceSpec indicates that a cgroupSystemd function was -// passed a specs.LinuxResources object that is impossible or illegal -// to process. -var ErrBadResourceSpec = errors.New("misconfigured resource spec") +var ( + // ErrBadResourceSpec indicates that a cgroupSystemd function was + // passed a specs.LinuxResources object that is impossible or illegal + // to process. + ErrBadResourceSpec = errors.New("misconfigured resource spec") + // ErrInvalidSlice indicates that the slice name passed via cgroup.Path is + // invalid. + ErrInvalidSlice = errors.New("invalid slice name") -// cgroupSystemd represents a cgroup managed by systemd. + isRunningSystemd = runningSystemd() +) + +// cgroupSystemd represents a cgroupv2 managed by systemd. type cgroupSystemd struct { + cgroupV2 // Name is the name of the of the systemd scope that controls the cgroups. - Name string `json:"name"` - // Mountpoint is the unified mount point of cgroupV2. - Mountpoint string `json:"mountpoint"` - // Path is the relative path to the unified mountpoint. - Path string `json:"path"` - // Controllers is the list of supported controllers. - Controllers []string `json:"controllers"` - // OwnedPaths is the list of owned paths created when installing this cgroup. - OwnedPaths []string `json:"owned_paths"` + Name string + // Parent is the encapsulating slice. + Parent string + // ScopePrefix is the prefix for the scope name. + ScopePrefix string properties []systemdDbus.Property dbusConn *systemdDbus.Conn } -// Install creates and configures a scope unit with the specified resource -// limits. -func (c *cgroupSystemd) Install(res *specs.LinuxResources) error { - slice := path.Base(c.Path) - ext := path.Ext(slice) - if ext != ".slice" { - return fmt.Errorf("invalid systemd path %s does not end in a parent slice: %w", c.Path, ErrInvalidGroupPath) +func newCgroupV2Systemd(cgv2 *cgroupV2) (*cgroupSystemd, error) { + if !isRunningSystemd { + return nil, fmt.Errorf("systemd not running on host") } - c.properties = append(c.properties, systemdDbus.PropSlice(slice)) - c.properties = append(c.properties, systemdDbus.PropDescription("runsc container "+c.Name)) + ctx := context.Background() + cg := &cgroupSystemd{cgroupV2: *cgv2} + // Parse the path from expected "slice:prefix:name" + // for e.g. "system.slice:docker:1234" + parts := strings.Split(cg.Path, ":") + if len(parts) != 3 { + return nil, fmt.Errorf("expected cgroupsPath to be of format \"slice:prefix:name\" for systemd cgroups, got %q instead", cg.Path) + } + cg.Parent = parts[0] + cg.ScopePrefix = parts[1] + cg.Name = parts[2] + if err := validSlice(cg.Parent); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidGroupPath, err) + } + // Rewrite Path so that it is compatible with cgroupv2 methods. + cg.Path = filepath.Join(expandSlice(cg.Parent), cg.unitName()) + conn, err := systemdDbus.NewWithContext(ctx) + if err != nil { + return nil, err + } + var version int + if version, err = systemdVersion(conn); err != nil { + return nil, fmt.Errorf("error parsing systemd version: %v", err) + } + if version < 244 { + return nil, fmt.Errorf("systemd version %d not supported, please upgrade to at least 244", version) + } + cg.dbusConn = conn + return cg, err +} + +// Install configures the properties for a scope unit but does not start the +// unit. +func (c *cgroupSystemd) Install(res *specs.LinuxResources) error { + log.Debugf("Installing systemd cgroup resource controller under %v", c.Parent) + c.properties = append(c.properties, systemdDbus.PropSlice(c.Parent)) + c.properties = append(c.properties, systemdDbus.PropDescription("Secure container "+c.Name)) pid := os.Getpid() c.properties = append(c.properties, systemdDbus.PropPids(uint32(pid))) // We always want proper accounting for the container for reporting resource @@ -69,92 +109,166 @@ func (c *cgroupSystemd) Install(res *specs.LinuxResources) error { c.addProp("IOAccounting", true) // Delegate must be true so that the container can manage its own cgroups. c.addProp("Delegate", true) - return c.genResourceControl(res) -} + // For compatibility with runc. + c.addProp("DefaultDependencies", false) -// MakePath builds a path to the given controller. -func (c *cgroupSystemd) MakePath(string) string { - return filepath.Join(c.Mountpoint, c.Path) -} - -func (c *cgroupSystemd) genResourceControl(res *specs.LinuxResources) error { - if res == nil { - return nil - } - var ( - mem = res.Memory - cpu = res.CPU - io = res.BlockIO - ) - if res.Pids != nil { - c.addProp("TasksMax", res.Pids.Limit) - } - if mem != nil { - if mem.Swap != nil { - if mem.Limit == nil { - return ErrBadResourceSpec + for controllerName, ctrlr := range controllers2 { + // First check if our controller is found in the system. + found := false + for _, knownController := range c.Controllers { + if controllerName == knownController { + found = true } - swap, err := convertMemorySwapToCgroupV2Value(*mem.Swap, *mem.Limit) + } + if found { + props, err := ctrlr.generateProperties(res) if err != nil { return err } - c.addProp("MemorySwapMax", strconv.FormatInt(swap, 10)) + c.properties = append(c.properties, props...) + continue } - if mem.Limit != nil { - c.addProp("MemoryMax", *mem.Limit) - } - if mem.Reservation != nil { - c.addProp("MemoryLow", *mem.Reservation) - } - } - if cpu != nil { - if cpu.Shares != nil { - weight := convertCPUSharesToCgroupV2Value(*cpu.Shares) - if weight != 0 { - c.addProp("CPUShares", weight) + if ctrlr.optional() { + if err := ctrlr.skip(res); err != nil { + return err } - } - - if cpu.Quota != nil && *cpu.Quota > 0 { - c.addProp("CPUQuota", strconv.FormatInt(*cpu.Quota, 10)+"%") - } - var period uint64 - if cpu.Period != nil && *cpu.Period != 0 { - period = *cpu.Period } else { - period = defaultPeriod + return fmt.Errorf("mandatory cgroup controller %q is missing for %q", controllerName, c.Path) } - // period is in microseconds, so we have to divide by 10 to convert - // to the milliseconds that systemd expects. - c.addProp("CPUQuotaPeriodSec", strconv.FormatUint(period/10, 10)+"ms") - if cpu.Cpus != "" { - c.addProp("AllowedCPUs", cpu.Cpus) - } - if cpu.Mems != "" { - c.addProp("AllowedMemoryNodes", cpu.Mems) - } - } - if io != nil { - if io.Weight != nil { - c.addProp("IOWeight", *io.Weight) - } - for _, dev := range io.WeightDevice { - val := fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, *dev.Weight) - c.addProp("IODevice", val) - } - c.addIOProps("IOReadBandwidth", io.ThrottleReadBpsDevice) - c.addIOProps("IOWriteBandwidth", io.ThrottleWriteBpsDevice) - c.addIOProps("IOReadIOPS", io.ThrottleReadIOPSDevice) - c.addIOProps("IOWriteIOPS", io.ThrottleWriteIOPSDevice) } return nil } -func (c *cgroupSystemd) addIOProps(name string, devs []specs.LinuxThrottleDevice) { +func (c *cgroupSystemd) unitName() string { + return fmt.Sprintf("%s-%s.scope", c.ScopePrefix, c.Name) +} + +// MakePath builds a path to the given controller. +func (c *cgroupSystemd) MakePath(string) string { + fullSlicePath := expandSlice(c.Parent) + path := filepath.Join(c.Mountpoint, fullSlicePath, c.unitName()) + return path +} + +// Join implements Cgroup.Join. +func (c *cgroupSystemd) Join() (func(), error) { + log.Debugf("Joining systemd cgroup %v", c.unitName()) + timeout := 30 * time.Second + ctx := context.Background() + // Clean up partially created cgroups on error. Errors during cleanup itself + // are ignored. + clean := cleanup.Make(func() { _ = c.Uninstall() }) + defer clean.Clean() + + conn, err := systemdDbus.NewWithContext(ctx) + if err != nil { + return nil, err + } + c.dbusConn = conn + unitName := c.unitName() + statusChan := make(chan string) + timedCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + if _, err := c.dbusConn.StartTransientUnitContext(timedCtx, unitName, "replace", c.properties, statusChan); err == nil { + s := <-statusChan + close(statusChan) + switch s { + case "done": + // All cases that are not "done" according to the dbus package. + case "cancelled", "timeout", "failed", "dependency", "skipped": + c.dbusConn.ResetFailedUnitContext(ctx, unitName) + return nil, fmt.Errorf("error creating systemd unit `%s`: got %s", unitName, s) + default: + c.dbusConn.ResetFailedUnitContext(ctx, unitName) + return nil, fmt.Errorf("unknown job completion status %q", s) + } + } else { + return nil, fmt.Errorf("systemd error: %v", err) + } + if _, err = c.createCgroupPaths(); err != nil { + return nil, err + } + return clean.Release(), nil +} + +// systemd represents slice hierarchy using `-`, so we need to follow suit when +// generating the path of slice. Essentially, test-a-b.slice becomes +// /test.slice/test-a.slice/test-a-b.slice. +func expandSlice(slice string) string { + var path, prefix string + suffix := ".slice" + sliceName := strings.TrimSuffix(slice, suffix) + // If input was -.slice, we should just return root now. + if sliceName == "-" { + return "/" + } + for _, component := range strings.Split(sliceName, "-") { + // Append the component to the path and to the prefix. + path += "/" + prefix + component + suffix + prefix += component + "-" + } + return path +} + +func validSlice(slice string) error { + suffix := ".slice" + // Name has to end with ".slice", but can't be just ".slice". + if slice == suffix || !strings.HasSuffix(slice, suffix) { + return fmt.Errorf("%w: %s", ErrInvalidSlice, slice) + } + + // Path-separators are not allowed. + if strings.Contains(slice, "/") { + return fmt.Errorf("%w: %s", ErrInvalidSlice, slice) + } + + sliceName := strings.TrimSuffix(slice, suffix) + // If input was -.slice, we should just return root now. + if sliceName == "-" { + return nil + } + for _, component := range strings.Split(sliceName, "-") { + // test--a.slice isn't permitted, nor is -test.slice. + if component == "" { + return fmt.Errorf("%w: %s", ErrInvalidSlice, slice) + } + } + return nil +} + +func runningSystemd() bool { + fi, err := os.Lstat("/run/systemd/system") + return err == nil && fi.IsDir() +} + +func systemdVersion(conn *systemdDbus.Conn) (int, error) { + vStr, err := conn.GetManagerProperty("Version") + if err != nil { + return -1, errors.New("unable to get systemd version") + } + // vStr should be of the form: + // "v245.4-1.fc32", "245", "v245-1.fc32", "245-1.fc32" (without quotes). + // The result for all of the above should be 245. + // Thus, we unconditionally remove the "v" prefix + // and then match on the first integer we can grab. + re := regexp.MustCompile(`v?([0-9]+)`) + matches := re.FindStringSubmatch(vStr) + if len(matches) < 2 { + return -1, fmt.Errorf("can't parse version %q: incorrect number of matches %d", vStr, len(matches)) + } + version, err := strconv.Atoi(matches[1]) + if err != nil { + return -1, fmt.Errorf("%w: can't parse version %q", err, vStr) + } + return version, nil +} + +func addIOProps(props []systemdDbus.Property, name string, devs []specs.LinuxThrottleDevice) []systemdDbus.Property { for _, dev := range devs { val := fmt.Sprintf("%d:%d %d", dev.Major, dev.Minor, dev.Rate) - c.addProp(name, val) + props = append(props, newProp(name, val)) } + return props } func (c *cgroupSystemd) addProp(name string, value interface{}) { diff --git a/runsc/cgroup/systemd_test.go b/runsc/cgroup/systemd_test.go index 0ce39cd04..0b88903c4 100644 --- a/runsc/cgroup/systemd_test.go +++ b/runsc/cgroup/systemd_test.go @@ -18,8 +18,6 @@ package cgroup import ( "errors" - "path/filepath" - "strconv" "testing" systemdDbus "github.com/coreos/go-systemd/v22/dbus" @@ -27,38 +25,85 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" specs "github.com/opencontainers/runtime-spec/specs-go" - "gvisor.dev/gvisor/pkg/test/testutil" ) -var defaultProps = []systemdDbus.Property{} +var ( + defaultProps = []systemdDbus.Property{} + mandatoryControllers = []string{"cpu", "cpuset", "io", "memory", "pids"} +) + +func TestIsValidSlice(t *testing.T) { + for _, tc := range []struct { + name string + slice string + err error + }{ + { + name: "success", + slice: "system.slice", + }, + { + name: "root slice", + slice: "-.slice", + }, + { + name: "path in slice", + slice: "system-child-grandchild.slice", + }, + { + name: "bad suffix", + slice: "system.scope", + err: ErrInvalidSlice, + }, + { + name: "has path seperators", + slice: "systemd.slice/child.slice", + err: ErrInvalidSlice, + }, + { + name: "invalid separator pattern", + slice: "systemd--child.slice", + err: ErrInvalidSlice, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := validSlice(tc.slice) + if !errors.Is(err, tc.err) { + t.Errorf("validSlice(%s) = %v, want %v", tc.slice, err, tc.err) + } + }) + } +} + +func TestExpandSlice(t *testing.T) { + original := "test-a-b.slice" + want := "/test.slice/test-a.slice/test-a-b.slice" + expanded := expandSlice(original) + if expanded != want { + t.Errorf("expandSlice(%q) = %q, want %q", original, expanded, want) + } +} func TestInstall(t *testing.T) { for _, tc := range []struct { - name string - res *specs.LinuxResources - cgroupPath string - wantProps []systemdDbus.Property - err error + name string + res *specs.LinuxResources + wantProps []systemdDbus.Property + err error }{ { - name: "bad parent", - res: nil, - cgroupPath: "not_a_slice", - err: ErrInvalidGroupPath, - }, - { - name: "no limits", + name: "defaults", res: nil, wantProps: []systemdDbus.Property{ {"Slice", dbus.MakeVariant("parent.slice")}, - {Name: "Description", Value: dbus.MakeVariant("runsc container ")}, + {Name: "Description", Value: dbus.MakeVariant("Secure container 123")}, {Name: "MemoryAccounting", Value: dbus.MakeVariant(true)}, {Name: "CPUAccounting", Value: dbus.MakeVariant(true)}, {Name: "TasksAccounting", Value: dbus.MakeVariant(true)}, {Name: "IOAccounting", Value: dbus.MakeVariant(true)}, {Name: "Delegate", Value: dbus.MakeVariant(true)}, + {Name: "DefaultDependencies", Value: dbus.MakeVariant(false)}, }, - cgroupPath: "parent.slice", }, { name: "memory", @@ -69,11 +114,10 @@ func TestInstall(t *testing.T) { Reservation: int64Ptr(3), }, }, - cgroupPath: "parent.slice", wantProps: []systemdDbus.Property{ - {"MemoryMax", dbus.MakeVariant(int64(1))}, - {"MemoryLow", dbus.MakeVariant(int64(3))}, - {"MemorySwapMax", dbus.MakeVariant("1")}, + {"MemoryMax", dbus.MakeVariant(uint64(1))}, + {"MemoryLow", dbus.MakeVariant(uint64(3))}, + {"MemorySwapMax", dbus.MakeVariant(uint64(1))}, }, }, { @@ -83,21 +127,19 @@ func TestInstall(t *testing.T) { Swap: int64Ptr(1), }, }, - err: ErrBadResourceSpec, - cgroupPath: "parent.slice", + err: ErrBadResourceSpec, }, { name: "cpu defaults", res: &specs.LinuxResources{ CPU: &specs.LinuxCPU{ Shares: uint64Ptr(0), - Quota: int64Ptr(0), + Quota: int64Ptr(5), Period: uint64Ptr(0), }, }, - cgroupPath: "parent.slice", wantProps: []systemdDbus.Property{ - {"CPUQuotaPeriodSec", dbus.MakeVariant(strconv.FormatUint(defaultPeriod/10, 10) + "ms")}, + {"CPUQuotaPerSecUSec", dbus.MakeVariant(uint64(10000))}, }, }, { @@ -105,19 +147,31 @@ func TestInstall(t *testing.T) { res: &specs.LinuxResources{ CPU: &specs.LinuxCPU{ Shares: uint64Ptr(1), - Period: uint64Ptr(20), - Quota: int64Ptr(3), + Period: uint64Ptr(20000), + Quota: int64Ptr(300000), Cpus: "4", Mems: "5", }, }, - cgroupPath: "parent.slice", wantProps: []systemdDbus.Property{ - {"CPUShares", dbus.MakeVariant(convertCPUSharesToCgroupV2Value(1))}, - {"CPUQuotaPeriodSec", dbus.MakeVariant("2ms")}, - {"CPUQuota", dbus.MakeVariant("3%")}, - {"AllowedCPUs", dbus.MakeVariant("4")}, - {"AllowedMemoryNodes", dbus.MakeVariant("5")}, + {"CPUWeight", dbus.MakeVariant(convertCPUSharesToCgroupV2Value(1))}, + {"CPUQuotaPeriodUSec", dbus.MakeVariant(uint64(20000))}, + {"CPUQuotaPerSecUSec", dbus.MakeVariant(uint64(15000000))}, + {"AllowedCPUs", dbus.MakeVariant([]byte{1 << 4})}, + {"AllowedMemoryNodes", dbus.MakeVariant([]byte{1 << 5})}, + }, + }, + { + name: "cpuset", + res: &specs.LinuxResources{ + CPU: &specs.LinuxCPU{ + Cpus: "1-3,5", + Mems: "5-8", + }, + }, + wantProps: []systemdDbus.Property{ + {"AllowedCPUs", dbus.MakeVariant([]byte{0b_101110})}, + {"AllowedMemoryNodes", dbus.MakeVariant([]byte{1, 0b_11100000})}, }, }, { @@ -144,29 +198,24 @@ func TestInstall(t *testing.T) { }, }, }, - cgroupPath: "parent.slice", wantProps: []systemdDbus.Property{ - {"IOWeight", dbus.MakeVariant(uint16(1))}, - {"IODevice", dbus.MakeVariant("2:3 4")}, - {"IODevice", dbus.MakeVariant("5:6 7")}, - {"IOReadBandwidth", dbus.MakeVariant("8:9 10")}, - {"IOReadBandwidth", dbus.MakeVariant("11:12 13")}, - {"IOWriteBandwidth", dbus.MakeVariant("14:15 16")}, - {"IOReadIOPS", dbus.MakeVariant("17:18 19")}, - {"IOWriteIOPS", dbus.MakeVariant("20:21 22")}, + {"IOWeight", dbus.MakeVariant(convertBlkIOToIOWeightValue(1))}, + {"IODeviceWeight", dbus.MakeVariant("2:3 4")}, + {"IODeviceWeight", dbus.MakeVariant("5:6 7")}, + {"IOReadBandwidthMax", dbus.MakeVariant("8:9 10")}, + {"IOReadBandwidthMax", dbus.MakeVariant("11:12 13")}, + {"IOWriteBandwidthMax", dbus.MakeVariant("14:15 16")}, + {"IOReadIOPSMax", dbus.MakeVariant("17:18 19")}, + {"IOWriteIOPSMax", dbus.MakeVariant("20:21 22")}, }, }, } { t.Run(tc.name, func(t *testing.T) { - dir := testutil.TmpDir() - testPath := filepath.Join(dir, tc.cgroupPath) - - cg := cgroupSystemd{ - Path: testPath, - } + cg := cgroupSystemd{Name: "123", Parent: "parent.slice"} + cg.Controllers = mandatoryControllers err := cg.Install(tc.res) if !errors.Is(err, tc.err) { - t.Fatalf("Wrong error, got: %s, want: %s", tc.err, err) + t.Fatalf("Wrong error, got: %s, want: %s", err, tc.err) } cmper := cmp.Comparer(func(a dbus.Variant, b dbus.Variant) bool { return a.String() == b.String() diff --git a/runsc/cli/main.go b/runsc/cli/main.go index 9070b8a5c..cccd1cb96 100644 --- a/runsc/cli/main.go +++ b/runsc/cli/main.go @@ -39,11 +39,9 @@ import ( ) var ( - // Although these flags are not part of the OCI spec, they are used by + // Although this flags is not part of the OCI spec, it is used by // Docker, and thus should not be changed. - // TODO(gvisor.dev/issue/193): support systemd cgroups - systemdCgroup = flag.Bool("systemd-cgroup", false, "Use systemd for cgroups. NOT SUPPORTED.") - showVersion = flag.Bool("version", false, "show version and exit.") + showVersion = flag.Bool("version", false, "show version and exit.") // These flags are unique to runsc, and are used to configure parts of the // system that are not covered by the runtime spec. @@ -120,12 +118,6 @@ func Main(version string) { cmd.Fatalf(err.Error()) } - // TODO(gvisor.dev/issue/193): support systemd cgroups - if *systemdCgroup { - fmt.Fprintln(os.Stderr, "systemd cgroup flag passed, but systemd cgroups not supported. See gvisor.dev/issue/193") - os.Exit(1) - } - var errorLogger io.Writer if *logFD > -1 { errorLogger = os.NewFile(uintptr(*logFD), "error log file") @@ -231,6 +223,7 @@ func Main(version string) { log.Infof("\t\tStrace: %t, max size: %d, syscalls: %s", conf.Strace, conf.StraceLogSize, conf.StraceSyscalls) log.Infof("\t\tVFS2 enabled: %t, LISAFS: %t", conf.VFS2, conf.Lisafs) log.Infof("\t\tDebug: %v", conf.Debug) + log.Infof("\t\tSystemd: %v", conf.SystemdCgroup) log.Infof("***************************") if conf.TestOnlyAllowRunAsCurrentUserWithoutChroot { diff --git a/runsc/config/config.go b/runsc/config/config.go index 426fe5e81..2b32134b1 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -226,6 +226,9 @@ type Config struct { // Don't configure cgroups. IgnoreCgroups bool `flag:"ignore-cgroups"` + // Use systemd to configure cgroups. + SystemdCgroup bool `flag:"systemd-cgroup"` + // TestOnlyAllowRunAsCurrentUserWithoutChroot should only be used in // tests. It allows runsc to start the sandbox process as the current // user, and without chrooting the sandbox process. This can be diff --git a/runsc/config/flags.go b/runsc/config/flags.go index ac4e5c7c8..d72d6597f 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -34,6 +34,7 @@ func RegisterFlags(flagSet *flag.FlagSet) { flagSet.String("log", "", "file path where internal debug information is written, default is stdout.") flagSet.String("log-format", "text", "log format: text (default), json, or json-k8s.") flagSet.Bool("debug", false, "enable debug logging.") + flagSet.Bool("systemd-cgroup", false, "EXPERIMENTAL. Use systemd for cgroups.") // These flags are unique to runsc, and are used to configure parts of the // system that are not covered by the runtime spec. diff --git a/runsc/container/container.go b/runsc/container/container.go index ec606e34a..b7e96fce4 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -1226,13 +1226,13 @@ func (c *Container) setupCgroupForRoot(conf *config.Config, spec *specs.Spec) (c var parentCgroup cgroup.Cgroup if parentPath, ok := spec.Annotations[cgroupParentAnnotation]; ok { var err error - parentCgroup, err = cgroup.NewFromPath(parentPath) + parentCgroup, err = cgroup.NewFromPath(parentPath, conf.SystemdCgroup) if err != nil { return nil, nil, err } } else { var err error - parentCgroup, err = cgroup.NewFromSpec(spec) + parentCgroup, err = cgroup.NewFromSpec(spec, conf.SystemdCgroup) if parentCgroup == nil || err != nil { return nil, nil, err } @@ -1263,7 +1263,7 @@ func (c *Container) setupCgroupForSubcontainer(conf *config.Config, spec *specs. } } - cg, err := cgroup.NewFromSpec(spec) + cg, err := cgroup.NewFromSpec(spec, conf.SystemdCgroup) if cg == nil || err != nil { return nil, err } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 8e47603db..4eaeeefcf 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -173,10 +173,13 @@ type Args struct { // sandbox. func New(conf *config.Config, args *Args) (*Sandbox, error) { s := &Sandbox{ - ID: args.ID, - CgroupJSON: cgroup.CgroupJSON{Cgroup: args.Cgroup}, - UID: -1, // prevent usage before it's set. - GID: -1, // prevent usage before it's set. + ID: args.ID, + CgroupJSON: cgroup.CgroupJSON{ + Cgroup: args.Cgroup, + UseSystemd: conf.SystemdCgroup, + }, + UID: -1, // prevent usage before it's set. + GID: -1, // prevent usage before it's set. } // The Cleanup object cleans up partially created sandboxes when an error // occurs. Any errors occurring during cleanup itself are ignored. @@ -373,7 +376,7 @@ func (s *Sandbox) Processes(cid string) ([]*control.Process, error) { // NewCGroup returns the sandbox's Cgroup, or an error if it does not have one. func (s *Sandbox) NewCGroup() (cgroup.Cgroup, error) { - return cgroup.NewFromPid(s.Pid.load()) + return cgroup.NewFromPid(s.Pid.load(), false /* useSystemd */) } // Execute runs the specified command in the container. It returns the PID of diff --git a/test/README.md b/test/README.md index 15b0f4c33..cda307d03 100644 --- a/test/README.md +++ b/test/README.md @@ -28,7 +28,7 @@ To run image and integration tests, run: To run root tests, run: -`make root-tests` +`make sudo TARGETS=test/root:root_test` There are a few other interesting variations for image and integration tests: diff --git a/test/root/cgroup_test.go b/test/root/cgroup_test.go index 975f7438b..6ba0e270e 100644 --- a/test/root/cgroup_test.go +++ b/test/root/cgroup_test.go @@ -77,6 +77,10 @@ func TestMemCgroup(t *testing.T) { gid := d.ID() t.Logf("cgroup ID: %s", gid) + useSystemd, err := dockerutil.UsingSystemdCgroup() + if err != nil { + t.Fatalf("docker run failed: %v", err) + } // Wait when the container will allocate memory. memUsage := 0 start := time.Now() @@ -86,13 +90,13 @@ func TestMemCgroup(t *testing.T) { // or after looping below (so the application can start). time.Sleep(100 * time.Millisecond) - var path string - // Read the cgroup memory limit. + path := filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.limit_in_bytes") if cgroup.IsOnlyV2() { path = filepath.Join("/sys/fs/cgroup/docker", gid, "memory.max") - } else { - path = filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.limit_in_bytes") + if useSystemd { + path = filepath.Join("/sys/fs/cgroup/system.slice/docker-"+gid+".scope", "memory.max") + } } // Read the cgroup memory limit. outRaw, err := ioutil.ReadFile(path) @@ -110,12 +114,14 @@ func TestMemCgroup(t *testing.T) { continue } + path = filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.max_usage_in_bytes") if cgroup.IsOnlyV2() { // v2 does not have max_usage_in_bytes equivalent, so memory.current is the // next best thing that we can use path = filepath.Join("/sys/fs/cgroup/docker", gid, "memory.current") - } else { - path = filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.max_usage_in_bytes") + if useSystemd { + path = filepath.Join("/sys/fs/cgroup/system.slice/docker-"+gid+".scope", "memory.current") + } } // Read the cgroup memory usage. outRaw, err = ioutil.ReadFile(path) @@ -333,6 +339,7 @@ func TestCgroupV2(t *testing.T) { ctx := context.Background() d := dockerutil.MakeContainer(ctx, t) defer d.CleanUp(ctx) + defaultTestCPUQuota := int64(100000) // This is not a comprehensive list of attributes. // @@ -357,7 +364,7 @@ func TestCgroupV2(t *testing.T) { field: "cpu-period", value: 2000, file: "cpu.max", - want: "max 2000", + want: fmt.Sprintf("%d 2000", defaultTestCPUQuota), }, { field: "memory", @@ -394,6 +401,14 @@ func TestCgroupV2(t *testing.T) { }, } + useSystemd, err := dockerutil.UsingSystemdCgroup() + if err != nil { + t.Fatalf("docker run failed: %v", err) + } + baseCgroupPath := "/sys/fs/cgroup/docker" + if useSystemd { + baseCgroupPath = fmt.Sprintf("/sys/fs/cgroup/system.slice") + } // Make configs. conf, hostconf, _ := d.ConfigsFrom(dockerutil.RunOpts{ Image: "basic/alpine", @@ -406,6 +421,8 @@ func TestCgroupV2(t *testing.T) { hostconf.Resources.CPUShares = attr.value case "cpu-period": hostconf.Resources.CPUPeriod = attr.value + // systemd throws out the period if quota is not also set. + hostconf.Resources.CPUQuota = defaultTestCPUQuota case "cpu-quota": hostconf.Resources.CPUQuota = attr.value case "kernel-memory": @@ -421,7 +438,7 @@ func TestCgroupV2(t *testing.T) { hostconf.Resources.MemorySwappiness = &val case "blkio-weight": // detect existence of io.bfq.weight as this is not always loaded - _, err := ioutil.ReadFile(filepath.Join("/sys/fs/cgroup/docker", attr.file)) + _, err := ioutil.ReadFile(filepath.Join(baseCgroupPath, attr.file)) if err == nil || !attr.skipIfNotFound { hostconf.Resources.BlkioWeight = uint16(attr.value) } @@ -448,6 +465,9 @@ func TestCgroupV2(t *testing.T) { // Check list of attributes defined above. for _, attr := range attrs { path := filepath.Join("/sys/fs/cgroup/docker", gid, attr.file) + if useSystemd { + path = filepath.Join(baseCgroupPath, "docker-"+gid+".scope", attr.file) + } out, err := ioutil.ReadFile(path) if err != nil { if os.IsNotExist(err) && attr.skipIfNotFound { @@ -467,6 +487,9 @@ func TestCgroupV2(t *testing.T) { t.Fatalf("SandboxPid: %v", err) } path := filepath.Join("/sys/fs/cgroup/docker", gid, "cgroup.procs") + if useSystemd { + path = filepath.Join(baseCgroupPath, "docker-"+gid+".scope", "cgroup.procs") + } if err := verifyPid(pid, path); err != nil { t.Errorf("cgroup control processes: %v", err) } @@ -478,9 +501,16 @@ func TestCgroupParent(t *testing.T) { ctx := context.Background() d := dockerutil.MakeContainer(ctx, t) defer d.CleanUp(ctx) + useSystemd, err := dockerutil.UsingSystemdCgroup() + if err != nil { + t.Fatalf("docker run failed: %v", err) + } // Construct a known cgroup name. parent := testutil.RandomID("runsc-") + if useSystemd { + parent = "system-runsc.slice" + } conf, hostconf, _ := d.ConfigsFrom(dockerutil.RunOpts{ Image: "basic/alpine", }, "sleep", "10000") @@ -515,11 +545,14 @@ func TestCgroupParent(t *testing.T) { if err != nil { t.Fatalf("invalid PID (%s): %v", ppidStr, err) } - cgroups, err := cgroup.NewFromPid(ppid) + cgroups, err := cgroup.NewFromPid(ppid, false /* useSystemd */) if err != nil { t.Fatalf("cgroup.NewFromPid(%d): %v", ppid, err) } path := filepath.Join(cgroups.MakePath("cpuacct"), parent, gid, "cgroup.procs") + if useSystemd { + path = filepath.Join(cgroups.MakePath("cpuacct"), parent, "docker-"+gid+".scope", "cgroup.procs") + } if err := verifyPid(pid, path); err != nil { t.Errorf("cgroup control %q processes: %v", "memory", err) } diff --git a/test/root/main_test.go b/test/root/main_test.go index cae850bc4..600b8607d 100644 --- a/test/root/main_test.go +++ b/test/root/main_test.go @@ -31,7 +31,9 @@ import ( // path for runsc. func TestMain(m *testing.M) { config.RegisterFlags(flag.CommandLine) - flag.Parse() + if !flag.CommandLine.Parsed() { + flag.Parse() + } if !specutils.HasCapabilities(capability.CAP_SYS_ADMIN, capability.CAP_DAC_OVERRIDE) { fmt.Println("Test requires sysadmin privileges to run. Try again with sudo.")