From 51f39a1204134f7edf0fc2563ee628968f0609e5 Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Thu, 4 Feb 2021 13:53:48 +0000 Subject: [PATCH 1/9] cgroupv2: skip tests on containerd < 1.4 containerd < 1.4 does not support cgroupv2, so we adjust the Make targets and installer scripts to skip test run on those versions. Signed-off-by: Daniel Dao --- Makefile | 17 ++++++++++++++++- tools/install_containerd.sh | 6 ++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3b3c7ef64..32d159380 100644 --- a/Makefile +++ b/Makefile @@ -113,6 +113,12 @@ RUNTIME_BIN := $(RUNTIME_DIR)/runsc RUNTIME_LOG_DIR := $(RUNTIME_DIR)/logs RUNTIME_LOGS := $(RUNTIME_LOG_DIR)/runsc.log.%TEST%.%TIMESTAMP%.%COMMAND% +ifeq ($(shell stat -f -c "%T" /sys/fs/cgroup 2>/dev/null),cgroup2fs) +CGROUPV2 := true +else +CGROUPV2 := false +endif + $(RUNTIME_BIN): # See below. @mkdir -p "$(RUNTIME_DIR)" ifeq (,$(STAGED_BINARIES)) @@ -204,7 +210,7 @@ tests: unit-tests nogo-tests container-tests syscall-tests integration-tests: ## Run all standard integration tests. integration-tests: docker-tests overlay-tests hostnet-tests swgso-tests -integration-tests: do-tests kvm-tests containerd-test-1.3.9 +integration-tests: do-tests kvm-tests containerd-tests-min .PHONY: integration-tests network-tests: ## Run all networking integration tests. @@ -320,10 +326,19 @@ else endif @$(call sudo,test/root:root_test,--runtime=$(RUNTIME) -test.v) +ifeq ($(CGROUPV2),false) +containerd-tests-min: containerd-test-1.3.9 +else +containerd-tests-min: containerd-test-1.4.3 +endif + # The shim builds with containerd 1.3.9 and it's not backward compatible. Test # with 1.3.9 and newer versions. +# When run under cgroupv2 environment, skip 1.3.9 as it does not support cgroupv2 containerd-tests: ## Runs all supported containerd version tests. +ifeq ($(CGROUPV2),false) containerd-tests: containerd-test-1.3.9 +endif containerd-tests: containerd-test-1.4.3 containerd-tests: containerd-test-1.5.4 diff --git a/tools/install_containerd.sh b/tools/install_containerd.sh index be54f494c..171f4def0 100755 --- a/tools/install_containerd.sh +++ b/tools/install_containerd.sh @@ -26,6 +26,12 @@ if [[ "${CONTAINERD_MAJOR}" -eq 1 ]] && [[ "${CONTAINERD_MINOR}" -le 4 ]]; then export GO111MODULE=off fi +# containerd < 1.4 doesn't work with cgroupv2 setup, so we check for that here +if [[ "$(stat -f -c %T /sys/fs/cgroup 2>/dev/null)" -eq "cgroup2fs" && "${CONTAINERD_MAJOR}" -eq 1 && "${CONTAINERD_MINOR}" -lt 4 ]]; then + echo "containerd < 1.4 does not work with cgroup2" + exit 1 +fi + # Helper for Go packages below. install_helper() { declare -r PACKAGE="${1}" From 881a271ff729e5c1e236c02cb44b5b1c22e0afdf Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Tue, 2 Nov 2021 15:20:58 +0000 Subject: [PATCH 2/9] runsc: Add cgroup v2 implementation Adds support for cgroupv2 based on the common cgroup interface. The cgroupv2 implementation mostly mirrors the structure of cgroupv1, with many helper functions derived from containerd/cgroups and opencontainers/runc implementations. We implemented the following controllers: cpu, cpuset, memory, io, pids, hugetlb. In order to avoid upgrading containerd dependency (to get oom poller implementation), we copied the oom poller implementation for cgroupv2 into shim/oom_v2.go. This requires containerd/cgroups dependency to have cgroupv2 support which we already have. Signed-off-by: Daniel Dao --- pkg/shim/BUILD | 2 + pkg/shim/epoll.go | 6 +- pkg/shim/oom_v2.go | 112 ++++++ pkg/shim/service.go | 36 +- runsc/cgroup/BUILD | 4 +- runsc/cgroup/cgroup.go | 97 +++-- runsc/cgroup/cgroup_test.go | 2 +- runsc/cgroup/cgroup_v2.go | 673 +++++++++++++++++++++++++++++++++ runsc/cgroup/cgroup_v2_test.go | 199 ++++++++++ runsc/container/container.go | 7 - test/root/cgroup_test.go | 170 ++++++++- 11 files changed, 1264 insertions(+), 44 deletions(-) create mode 100644 pkg/shim/oom_v2.go create mode 100644 runsc/cgroup/cgroup_v2.go create mode 100644 runsc/cgroup/cgroup_v2_test.go diff --git a/pkg/shim/BUILD b/pkg/shim/BUILD index c426da16c..8c0b1f1f0 100644 --- a/pkg/shim/BUILD +++ b/pkg/shim/BUILD @@ -8,6 +8,7 @@ go_library( "api.go", "debug.go", "epoll.go", + "oom_v2.go", "options.go", "service.go", "service_linux.go", @@ -24,6 +25,7 @@ go_library( "//runsc/specutils", "@com_github_burntsushi_toml//:go_default_library", "@com_github_containerd_cgroups//:go_default_library", + "@com_github_containerd_cgroups//v2:go_default_library", "@com_github_containerd_cgroups//stats/v1:go_default_library", "@com_github_containerd_console//:go_default_library", "@com_github_containerd_containerd//api/events:go_default_library", diff --git a/pkg/shim/epoll.go b/pkg/shim/epoll.go index 463e11a84..5d29c8e28 100644 --- a/pkg/shim/epoll.go +++ b/pkg/shim/epoll.go @@ -81,9 +81,13 @@ func (e *epoller) run(ctx context.Context) { } } -func (e *epoller) add(id string, cg cgroups.Cgroup) error { +func (e *epoller) add(id string, cgx interface{}) error { e.mu.Lock() defer e.mu.Unlock() + cg, ok := cgx.(cgroups.Cgroup) + if !ok { + return fmt.Errorf("expected cgroups.Cgroup, got: %T", cgx) + } fd, err := cg.OOMEventFD() if err != nil { return err diff --git a/pkg/shim/oom_v2.go b/pkg/shim/oom_v2.go new file mode 100644 index 000000000..a2b7308bf --- /dev/null +++ b/pkg/shim/oom_v2.go @@ -0,0 +1,112 @@ +// Copyright The containerd Authors. +// Copyright 2021 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build linux +// +build linux + +package shim + +import ( + "context" + "fmt" + + cgroupsv2 "github.com/containerd/cgroups/v2" + "github.com/containerd/containerd/runtime" + "github.com/containerd/containerd/runtime/v2/shim" + "github.com/sirupsen/logrus" +) + +// newOOMv2Epoller returns an implementation that listens to OOM events +// from a container's cgroups v2. This is copied from containerd to avoid +// having to upgrade containerd package just to get it +func newOOMv2Poller(publisher shim.Publisher) (oomPoller, error) { + return &watcherV2{ + itemCh: make(chan itemV2), + publisher: publisher, + }, nil +} + +// watcher implementation for handling OOM events from a container's cgroup +type watcherV2 struct { + itemCh chan itemV2 + publisher shim.Publisher +} + +type itemV2 struct { + id string + ev cgroupsv2.Event + err error +} + +// Close closes the watcher +func (w *watcherV2) Close() error { + return nil +} + +// Run the loop +func (w *watcherV2) run(ctx context.Context) { + lastOOMMap := make(map[string]uint64) // key: id, value: ev.OOM + for { + select { + case <-ctx.Done(): + w.Close() + return + case i := <-w.itemCh: + if i.err != nil { + delete(lastOOMMap, i.id) + continue + } + lastOOM := lastOOMMap[i.id] + if i.ev.OOM > lastOOM { + if err := w.publisher.Publish(ctx, runtime.TaskOOMEventTopic, &TaskOOM{ + ContainerID: i.id, + }); err != nil { + logrus.WithError(err).Error("publish OOM event") + } + } + if i.ev.OOM > 0 { + lastOOMMap[i.id] = i.ev.OOM + } + } + } +} + +// Add cgroups.Cgroup to the epoll monitor +func (w *watcherV2) add(id string, cgx interface{}) error { + cg, ok := cgx.(*cgroupsv2.Manager) + if !ok { + return fmt.Errorf("expected *cgroupsv2.Manager, got: %T", cgx) + } + // NOTE: containerd/cgroups/v2 does not support closing eventCh routine currently. + // The routine shuts down when an error happens, mostly when the cgroup is deleted. + eventCh, errCh := cg.EventChan() + go func() { + for { + i := itemV2{id: id} + select { + case ev := <-eventCh: + i.ev = ev + w.itemCh <- i + case err := <-errCh: + i.err = err + w.itemCh <- i + // we no longer get any event/err when we got an err + logrus.WithError(err).Warn("error from eventChan") + return + } + } + }() + return nil +} diff --git a/pkg/shim/service.go b/pkg/shim/service.go index 8e5aea739..68966afdf 100644 --- a/pkg/shim/service.go +++ b/pkg/shim/service.go @@ -29,6 +29,7 @@ import ( "github.com/BurntSushi/toml" "github.com/containerd/cgroups" cgroupsstats "github.com/containerd/cgroups/stats/v1" + cgroupsv2 "github.com/containerd/cgroups/v2" "github.com/containerd/console" "github.com/containerd/containerd/api/events" "github.com/containerd/containerd/api/types/task" @@ -82,6 +83,15 @@ const ( cgroupParentAnnotation = "dev.gvisor.spec.cgroup-parent" ) +type oomPoller interface { + io.Closer + // add adds `cg` cgroup to oom poller. `cg` is cgroups.Cgroup in v1 and + // `cgroupsv2.Manager` in v2 + add(id string, cg interface{}) error + // run monitors oom event and notifies the shim about them + run(ctx context.Context) +} + // New returns a new shim service that can be used via GRPC. func New(ctx context.Context, id string, publisher shim.Publisher, cancel func()) (shim.Shim, error) { var opts shim.Opts @@ -89,7 +99,15 @@ func New(ctx context.Context, id string, publisher shim.Publisher, cancel func() opts = ctxOpts.(shim.Opts) } - ep, err := newOOMEpoller(publisher) + var ( + ep oomPoller + err error + ) + if cgroups.Mode() == cgroups.Unified { + ep, err = newOOMv2Poller(publisher) + } else { + ep, err = newOOMEpoller(publisher) + } if err != nil { return nil, err } @@ -161,7 +179,7 @@ type service struct { ec chan proc.Exit // oomPoller monitors the sandbox's cgroup for OOM notifications. - oomPoller *epoller + oomPoller oomPoller // cancel is a function that needs to be called before the shim stops. The // function is provided by the caller to New(). @@ -473,7 +491,19 @@ func (s *service) Create(ctx context.Context, r *taskAPI.CreateTaskRequest) (*ta // sandbox create since the sandbox process will be created here. pid := process.Pid() if pid > 0 { - cg, err := cgroups.Load(cgroups.V1, cgroups.PidPath(pid)) + var ( + cg interface{} + err error + ) + if cgroups.Mode() == cgroups.Unified { + var cgPath string + cgPath, err = cgroupsv2.PidGroupPath(pid) + if err == nil { + cg, err = cgroupsv2.LoadManager("/sys/fs/cgroup", cgPath) + } + } else { + cg, err = cgroups.Load(cgroups.V1, cgroups.PidPath(pid)) + } if err != nil { return nil, fmt.Errorf("loading cgroup for %d: %w", pid, err) } diff --git a/runsc/cgroup/BUILD b/runsc/cgroup/BUILD index 4dcbc285a..4cd19b6c3 100644 --- a/runsc/cgroup/BUILD +++ b/runsc/cgroup/BUILD @@ -4,7 +4,7 @@ package(licenses = ["notice"]) go_library( name = "cgroup", - srcs = ["cgroup.go"], + srcs = ["cgroup.go", "cgroup_v2.go"], visibility = ["//:sandbox"], deps = [ "//pkg/cleanup", @@ -19,7 +19,7 @@ go_library( go_test( name = "cgroup_test", size = "small", - srcs = ["cgroup_test.go"], + srcs = ["cgroup_test.go", "cgroup_v2_test.go"], library = ":cgroup", tags = ["local"], deps = [ diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go index 3adf6bd83..0e3ce8398 100644 --- a/runsc/cgroup/cgroup.go +++ b/runsc/cgroup/cgroup.go @@ -217,10 +217,10 @@ func loadPaths(pid string) (map[string]string, error) { } defer mountinfo.Close() - return loadPathsHelper(procCgroup, mountinfo) + return loadPathsHelper(procCgroup, mountinfo, IsOnlyV2()) } -func loadPathsHelper(cgroup, mountinfo io.Reader) (map[string]string, error) { +func loadPathsHelper(cgroup, mountinfo io.Reader, unified bool) (map[string]string, error) { paths := make(map[string]string) scanner := bufio.NewScanner(cgroup) @@ -231,7 +231,8 @@ func loadPathsHelper(cgroup, mountinfo io.Reader) (map[string]string, error) { if len(tokens) != 3 { return nil, fmt.Errorf("invalid cgroups file, line: %q", scanner.Text()) } - if len(tokens[1]) == 0 { + if len(tokens[1]) == 0 && unified { + paths[cgroup2Key] = tokens[2] continue } for _, ctrlr := range strings.Split(tokens[1], ",") { @@ -255,30 +256,42 @@ func loadPathsHelper(cgroup, mountinfo io.Reader) (map[string]string, error) { // Format: ID parent major:minor root mount-point options opt-fields - fs-type source super-options // Example: 39 32 0:34 / /sys/fs/cgroup/devices rw,noexec shared:18 - cgroup cgroup rw,devices fields := strings.Fields(mountScanner.Text()) - if len(fields) < 9 || fields[len(fields)-3] != "cgroup" { + if len(fields) < 9 { // Skip mounts that are not cgroup mounts. continue } - // Cgroup controller type is in the super-options field. - superOptions := strings.Split(fields[len(fields)-1], ",") - for _, opt := range superOptions { - // Remove prefix for cgroups with no controller, eg. systemd. - opt = strings.TrimPrefix(opt, "name=") + switch fields[len(fields)-3] { + case "cgroup": + // Cgroup controller type is in the super-options field. + superOptions := strings.Split(fields[len(fields)-1], ",") + for _, opt := range superOptions { + // Remove prefix for cgroups with no controller, eg. systemd. + opt = strings.TrimPrefix(opt, "name=") - // Only considers cgroup controllers that are registered, and skip other - // irrelevant options, e.g. rw. - if cgroupPath, ok := paths[opt]; ok { - rootDir := fields[3] - if rootDir != "/" { - // When cgroup is in submount, remove repeated path components from - // cgroup path to avoid duplicating them. - relCgroupPath, err := filepath.Rel(rootDir, cgroupPath) - if err != nil { - return nil, err + // Only considers cgroup controllers that are registered, and skip other + // irrelevant options, e.g. rw. + if cgroupPath, ok := paths[opt]; ok { + rootDir := fields[3] + if rootDir != "/" { + // When cgroup is in submount, remove repeated path components from + // cgroup path to avoid duplicating them. + relCgroupPath, err := filepath.Rel(rootDir, cgroupPath) + if err != nil { + return nil, err + } + paths[opt] = relCgroupPath } - paths[opt] = relCgroupPath } } + case "cgroup2": + if cgroupPath, ok := paths[cgroup2Key]; ok { + root := fields[3] + relCgroupPath, err := filepath.Rel(root, cgroupPath) + if err != nil { + return nil, err + } + paths[cgroup2Key] = relCgroupPath + } } } if err := mountScanner.Err(); err != nil { @@ -335,24 +348,38 @@ func NewFromPid(pid int) (Cgroup, error) { } func new(pid, cgroupsPath string) (Cgroup, error) { - var parents map[string]string + var ( + parents map[string]string + err error + cg Cgroup + ) // If path is relative, load cgroup paths for the process to build the // relative paths. if !filepath.IsAbs(cgroupsPath) { - var err error parents, err = loadPaths(pid) if err != nil { return nil, fmt.Errorf("finding current cgroups: %w", err) } } - cg := &cgroupV1{ - Name: cgroupsPath, - Parents: parents, - Own: make(map[string]bool), + + if IsOnlyV2() { + if p, ok := parents[cgroup2Key]; ok { + // 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) + } else { + cg = &cgroupV1{ + Name: cgroupsPath, + Parents: parents, + Own: make(map[string]bool), + } } log.Debugf("New cgroup for pid: %s, %+v", pid, cg) - return cg, nil + return cg, err } // CgroupJSON is a wrapper for Cgroup that can be encoded to JSON. @@ -364,8 +391,20 @@ type cgroupJSONv1 struct { Cgroup *cgroupV1 `json:"cgroup"` } +type cgroupJSONv2 struct { + Cgroup *cgroupV2 `json:"cgroup"` +} + // UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON func (c *CgroupJSON) UnmarshalJSON(data []byte) error { + if IsOnlyV2() { + v2 := cgroupJSONv2{} + err := json.Unmarshal(data, &v2) + if v2.Cgroup != nil { + c.Cgroup = v2.Cgroup + } + return err + } v1 := cgroupJSONv1{} err := json.Unmarshal(data, &v1) if v1.Cgroup != nil { @@ -380,6 +419,10 @@ func (c *CgroupJSON) MarshalJSON() ([]byte, error) { v1 := cgroupJSONv1{} return json.Marshal(&v1) } + if IsOnlyV2() { + v2 := cgroupJSONv2{Cgroup: c.Cgroup.(*cgroupV2)} + return json.Marshal(&v2) + } v1 := cgroupJSONv1{Cgroup: c.Cgroup.(*cgroupV1)} return json.Marshal(&v1) } diff --git a/runsc/cgroup/cgroup_test.go b/runsc/cgroup/cgroup_test.go index abd10756b..06b61a881 100644 --- a/runsc/cgroup/cgroup_test.go +++ b/runsc/cgroup/cgroup_test.go @@ -831,7 +831,7 @@ func TestLoadPaths(t *testing.T) { t.Run(tc.name, func(t *testing.T) { r := strings.NewReader(tc.cgroups) mountinfo := strings.NewReader(tc.mountinfo) - got, err := loadPathsHelper(r, mountinfo) + got, err := loadPathsHelper(r, mountinfo, false) if len(tc.err) == 0 { if err != nil { t.Fatalf("Unexpected error: %v", err) diff --git a/runsc/cgroup/cgroup_v2.go b/runsc/cgroup/cgroup_v2.go new file mode 100644 index 000000000..36ea4360f --- /dev/null +++ b/runsc/cgroup/cgroup_v2.go @@ -0,0 +1,673 @@ +// Copyright The runc Authors. +// Copyright The containerd Authors. +// Copyright 2021 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package cgroup + +import ( + "bufio" + "bytes" + "context" + "errors" + "fmt" + "io/ioutil" + "math" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/cenkalti/backoff" + specs "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/cleanup" + "gvisor.dev/gvisor/pkg/log" +) + +const ( + subtreeControl = "cgroup.subtree_control" + controllersFile = "cgroup.controllers" + cgroup2Key = "cgroup2" + + // https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html + defaultPeriod = 100000 +) + +var ( + ErrInvalidFormat = errors.New("cgroup: parsing file with invalid format failed") + ErrInvalidGroupPath = errors.New("cgroup: invalid group path") + + // controllers2 is the group of all supported cgroupv2 controllers + controllers2 = map[string]controller{ + "cpu": &cpu2{}, + "cpuset": &cpuset2{}, + "io": &io2{}, + "memory": &memory2{}, + "pids": &pid2{}, + "hugetlb": &hugeTLB2{}, + } +) + +// cgroupV2 represents a cgroup inside supported all cgroupV2 controllers +type cgroupV2 struct { + // 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"` + // Own is the list of owned path created when install this cgroup + Own []string `json:"own"` +} + +func newCgroupV2(mountpoint string, group string) (*cgroupV2, error) { + cg := &cgroupV2{ + Mountpoint: mountpoint, + Path: group, + } + err := cg.setupControllers() + return cg, err +} + +// setupControllers setup all supported controllers based on cgroup.controllers +// in the unified cgroup mount point +func (c *cgroupV2) setupControllers() error { + if c.Controllers != nil { + return nil + } + + data, err := ioutil.ReadFile(filepath.Join(c.Mountpoint, "cgroup.controllers")) + if err != nil { + return err + } + fields := strings.Fields(string(data)) + c.Controllers = fields + + return nil +} + +// 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() + + // 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: + // * /sys/fs/cgroup/cgroup.subtree_control + // * /sys/fs/cgroup/foo/cgroup.subtree_control + val := "+" + strings.Join(c.Controllers, " +") + elements := strings.Split(c.Path, "/") + current := c.Mountpoint + + 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 + } + } else { + created = true + c.Own = append(c.Own, current) + } + } + // 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 + } + } 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 + } + } + + // if we don't have the controller + if !found { + 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) + } + } else { + if err := ctrlr.set(res, current); err != nil { + return err + } + } + } + } + } + + clean.Release() + return nil +} + +// Uninstall removes the settings done in Install(). If cgroup path already +// existed when Install() was called, Uninstall is a noop. +func (c *cgroupV2) Uninstall() error { + log.Debugf("Deleting cgroup %q", c.MakePath("")) + + // If we try to remove the cgroup too soon after killing the sandbox we + // might get EBUSY, so we retry for a few seconds until it succeeds. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + b := backoff.WithContext(backoff.NewConstantBackOff(100*time.Millisecond), ctx) + + // delete last entry in owned first + for i := len(c.Own) - 1; i >= 0; i-- { + current := c.Own[i] + log.Debugf("Removing cgroup for path=%q", current) + + fn := func() error { + err := unix.Rmdir(current) + if os.IsNotExist(err) { + return nil + } + return err + } + if err := backoff.Retry(fn, b); err != nil { + return fmt.Errorf("removing cgroup path %q: %w", current, err) + } + } + + return nil +} + +// Join adds the current process to the all controllers. Returns function that +// restores cgroup to the original state. +func (c *cgroupV2) Join() (func(), error) { + // First save the current state so it can be restored. + paths, err := loadPaths("self") + if err != nil { + return nil, err + } + // since this is unified, get the first path of current process's cgroup is enough + undoPath := filepath.Join(c.Mountpoint, paths[cgroup2Key]) + + cu := cleanup.Make(func() { + log.Debugf("Restoring cgroup %q", undoPath) + // Writing the value 0 to a cgroup.procs file causes + // the writing process to be moved to the corresponding + // cgroup. - cgroups(7). + if err := setValue(undoPath, "cgroup.procs", "0"); err != nil { + log.Warningf("Error restoring cgroup %q: %v", undoPath, err) + } + }) + defer cu.Clean() + + // now join the cgroup + if err := setValue(c.MakePath(""), "cgroup.procs", "0"); err != nil { + return nil, err + } + + return cu.Release(), nil +} + +// CPUQuota returns the CFS CPU quota. +func (c *cgroupV2) CPUQuota() (float64, error) { + cpuMax, err := getValue(c.MakePath(""), "cpu.max") + if err != nil { + return -1, err + } + data := strings.SplitN(cpuMax, " ", 2) + if len(data) != 2 { + return -1, fmt.Errorf("invalid cpu.max data %q", cpuMax) + } + + quota, err := strconv.ParseInt(data[0], 10, 64) + if err != nil { + return -1, err + } + + period, err := strconv.ParseInt(data[1], 10, 64) + if err != nil { + return -1, err + } + + if quota <= 0 || period <= 0 { + return -1, err + } + return float64(quota) / float64(period), nil +} + +// CPUUsage returns the total CPU usage of the cgroup. +func (c *cgroupV2) CPUUsage() (uint64, error) { + cpuStat, err := getValue(c.MakePath(""), "cpu.stat") + if err != nil { + return 0, err + } + + sc := bufio.NewScanner(strings.NewReader(cpuStat)) + for sc.Scan() { + key, value, err := parseKeyValue(sc.Text()) + if err != nil { + return 0, err + } + if key == "usage_usec" { + return value, nil + } + } + + return 0, nil +} + +// NumCPU returns the number of CPUs configured in 'cpuset/cpuset.cpus'. +func (c *cgroupV2) NumCPU() (int, error) { + cpuset, err := getValue(c.MakePath(""), "cpuset.cpus.effective") + if err != nil { + return 0, err + } + return countCpuset(strings.TrimSpace(cpuset)) +} + +// MemoryLimit returns the memory limit. +func (c *cgroupV2) MemoryLimit() (uint64, error) { + limStr, err := getValue(c.MakePath(""), "memory.max") + if err != nil { + return 0, err + } + limStr = strings.TrimSpace(limStr) + if limStr == "max" { + return math.MaxUint64, nil + } + return strconv.ParseUint(limStr, 10, 64) +} + +// MakePath builds a path to the given controller. +func (c *cgroupV2) MakePath(controllerName string) string { + return filepath.Join(c.Mountpoint, c.Path) +} + +type cpu2 struct { + mandatory +} + +func (*cpu2) set(spec *specs.LinuxResources, path string) error { + if spec == nil || spec.CPU == nil { + return nil + } + + if spec.CPU.Shares != nil { + weight := convertCPUSharesToCgroupV2Value(*spec.CPU.Shares) + if weight != 0 { + if err := setValue(path, "cpu.weight", strconv.FormatUint(weight, 10)); err != nil { + return err + } + } + } + + if spec.CPU.Period != nil || spec.CPU.Quota != nil { + v := "max" + if spec.CPU.Quota != nil && *spec.CPU.Quota > 0 { + v = strconv.FormatInt(*spec.CPU.Quota, 10) + } + + var period uint64 + if spec.CPU.Period != nil && *spec.CPU.Period != 0 { + period = *spec.CPU.Period + } else { + period = defaultPeriod + } + + v += " " + strconv.FormatUint(period, 10) + if err := setValue(path, "cpu.max", v); err != nil { + return err + } + } + + return nil +} + +type cpuset2 struct { + mandatory +} + +func (*cpuset2) set(spec *specs.LinuxResources, path string) error { + if spec == nil || spec.CPU == nil { + return nil + } + + if spec.CPU.Cpus != "" { + if err := setValue(path, "cpuset.cpus", spec.CPU.Cpus); err != nil { + return err + } + } + + if spec.CPU.Mems != "" { + if err := setValue(path, "cpuset.mems", spec.CPU.Mems); err != nil { + return err + } + } + + return nil +} + +type memory2 struct { + mandatory +} + +func (*memory2) set(spec *specs.LinuxResources, path string) error { + if spec == nil || spec.Memory == nil { + return nil + } + + if spec.Memory.Swap != nil { + // in cgroup v2, we set memory and swap separately, but the spec specifies + // Swap field as memory+swap, so we need memory limit here to be set in order + // to get the correct swap value + if spec.Memory.Limit == nil { + return errors.New("cgroup: Memory.Swap is set without Memory.Limit") + } + + swap, err := convertMemorySwapToCgroupV2Value(*spec.Memory.Swap, *spec.Memory.Limit) + if err != nil { + return nil + } + swapStr := numToStr(swap) + // memory and memorySwap set to the same value -- disable swap + if swapStr == "" && swap == 0 && *spec.Memory.Swap > 0 { + swapStr = "0" + } + // never write empty string to `memory.swap.max`, it means set to 0. + if swapStr != "" { + if err := setValue(path, "memory.swap.max", swapStr); err != nil { + return err + } + } + } + + if spec.Memory.Limit != nil { + if val := numToStr(*spec.Memory.Limit); val != "" { + if err := setValue(path, "memory.max", val); err != nil { + return err + } + } + } + + if spec.Memory.Reservation != nil { + if val := numToStr(*spec.Memory.Reservation); val != "" { + if err := setValue(path, "memory.low", val); err != nil { + return err + } + } + } + + return nil +} + +type pid2 struct { + mandatory +} + +func (*pid2) set(spec *specs.LinuxResources, path string) error { + if spec == nil || spec.Pids == nil { + return nil + } + + if val := numToStr(spec.Pids.Limit); val != "" { + return setValue(path, "pids.max", val) + } + + return nil +} + +type io2 struct { + mandatory +} + +func (*io2) set(spec *specs.LinuxResources, path string) error { + if spec == nil || spec.BlockIO == nil { + return nil + } + blkio := spec.BlockIO + + var ( + err error + bfq *os.File + ) + + // If BFQ IO scheduler is available, use it. + if blkio.Weight != nil || len(blkio.WeightDevice) > 0 { + bfq, err = os.Open(filepath.Join(path, "io.bfq.weight")) + if err == nil { + defer bfq.Close() + } else if !os.IsNotExist(err) { + return err + } + + } + + if blkio.Weight != nil && *blkio.Weight != 0 { + if bfq != nil { + if _, err := bfq.WriteString(strconv.FormatUint(uint64(*blkio.Weight), 10)); err != nil { + return err + } + } else { + // bfq io scheduler is not available, fallback to io.weight with + // a conversion scheme + ioWeight := convertBlkIOToIOWeightValue(*blkio.Weight) + if err = setValue(path, "io.weight", strconv.FormatUint(ioWeight, 10)); err != nil { + return err + } + } + } + + if bfqDeviceWeightSupported(bfq) { + // ignore leaf weight, does not apply to cgroupv2 + for _, dev := range blkio.WeightDevice { + if dev.Weight != nil { + val := fmt.Sprintf("%d:%d %d\n", dev.Major, dev.Minor, *dev.Weight) + if _, err := bfq.WriteString(val); err != nil { + return fmt.Errorf("failed to set device weight %q: %w", val, err) + } + } + } + } + + if err := setThrottle2(path, "rbps", blkio.ThrottleReadBpsDevice); err != nil { + return err + } + + if err := setThrottle2(path, "wbps", blkio.ThrottleWriteBpsDevice); err != nil { + return err + } + + if err := setThrottle2(path, "riops", blkio.ThrottleReadIOPSDevice); err != nil { + return err + } + + if err := setThrottle2(path, "riops", blkio.ThrottleWriteIOPSDevice); err != nil { + return err + } + + return nil +} + +func setThrottle2(path, name string, devs []specs.LinuxThrottleDevice) error { + for _, dev := range devs { + val := fmt.Sprintf("%d:%d %s=%d", dev.Major, dev.Minor, name, dev.Rate) + if err := setValue(path, "io.max", val); err != nil { + return err + } + } + return nil +} + +type hugeTLB2 struct { +} + +func (*hugeTLB2) optional() bool { + return true +} + +func (*hugeTLB2) skip(spec *specs.LinuxResources) error { + if spec != nil && len(spec.HugepageLimits) > 0 { + return fmt.Errorf("HugepageLimits set but hugetlb cgroup controller not found") + } + return nil +} + +func (*hugeTLB2) set(spec *specs.LinuxResources, path string) error { + if spec == nil { + return nil + } + 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 +} + +// Since the OCI spec is designed for cgroup v1, in some cases +// there is need to convert from the cgroup v1 configuration to cgroup v2 +// the formula for cpuShares is y = (1 + ((x - 2) * 9999) / 262142) +// convert from [2-262144] to [1-10000] +// 262144 comes from Linux kernel definition "#define MAX_SHARES (1UL << 18)" +func convertCPUSharesToCgroupV2Value(cpuShares uint64) uint64 { + if cpuShares == 0 { + return 0 + } + return (1 + ((cpuShares-2)*9999)/262142) +} + +// convertMemorySwapToCgroupV2Value converts MemorySwap value from OCI spec +// for use by cgroup v2 drivers. A conversion is needed since Resources.MemorySwap +// is defined as memory+swap combined, while in cgroup v2 swap is a separate value. +func convertMemorySwapToCgroupV2Value(memorySwap, memory int64) (int64, error) { + // for compatibility with cgroup1 controller, set swap to unlimited in + // case the memory is set to unlimited, and swap is not explicitly set, + // treating the request as "set both memory and swap to unlimited". + if memory == -1 && memorySwap == 0 { + return -1, nil + } + if memorySwap == -1 || memorySwap == 0 { + // -1 is "max", 0 is "unset", so treat as is + return memorySwap, nil + } + // sanity checks + if memory == 0 || memory == -1 { + return 0, errors.New("unable to set swap limit without memory limit") + } + if memory < 0 { + return 0, fmt.Errorf("invalid memory value: %d", memory) + } + if memorySwap < memory { + return 0, errors.New("memory+swap limit should be >= memory limit") + } + + return memorySwap - memory, nil +} + +// Since the OCI spec is designed for cgroup v1, in some cases +// there is need to convert from the cgroup v1 configuration to cgroup v2 +// the formula for BlkIOWeight to IOWeight is y = (1 + (x - 10) * 9999 / 990) +// convert linearly from [10-1000] to [1-10000] +func convertBlkIOToIOWeightValue(blkIoWeight uint16) uint64 { + if blkIoWeight == 0 { + return 0 + } + return 1 + (uint64(blkIoWeight)-10)*9999/990 +} + +// numToStr converts an int64 value to a string for writing to a +// cgroupv2 files with .min, .max, .low, or .high suffix. +// The value of -1 is converted to "max" for cgroupv1 compatibility +// (which used to write -1 to remove the limit). +func numToStr(value int64) (ret string) { + switch { + case value == 0: + ret = "" + case value == -1: + ret = "max" + default: + ret = strconv.FormatInt(value, 10) + } + return ret +} + +// bfqDeviceWeightSupported checks for per-device BFQ weight support (added +// in kernel v5.4, commit 795fe54c2a8) by reading from "io.bfq.weight". +func bfqDeviceWeightSupported(bfq *os.File) bool { + if bfq == nil { + return false + } + + if _, err := bfq.Seek(0, 0); err != nil { + return false + } + + buf := make([]byte, 32) + if _, err := bfq.Read(buf); err != nil { + return false + } + // If only a single number (default weight) if read back, we have older kernel. + _, err := strconv.ParseInt(string(bytes.TrimSpace(buf)), 10, 64) + return err != nil +} + +// parseKeyValue parses a space-separated "name value" kind of cgroup +// parameter and returns its key as a string, and its value as uint64 +// (ParseUint is used to convert the value). For example, +// "io_service_bytes 1234" will be returned as "io_service_bytes", 1234. +func parseKeyValue(t string) (string, uint64, error) { + parts := strings.SplitN(t, " ", 3) + if len(parts) != 2 { + return "", 0, fmt.Errorf("line %q is not in key value format", t) + } + + value, err := parseUint(parts[1], 10, 64) + if err != nil { + return "", 0, err + } + + return parts[0], value, nil +} + +// parseUint converts a string to an uint64 integer. +// Negative values are returned at zero as, due to kernel bugs, +// some of the memory cgroup stats can be negative. +func parseUint(s string, base, bitSize int) (uint64, error) { + value, err := strconv.ParseUint(s, base, bitSize) + if err != nil { + intValue, intErr := strconv.ParseInt(s, base, bitSize) + // 1. Handle negative values greater than MinInt64 (and) + // 2. Handle negative values lesser than MinInt64 + if intErr == nil && intValue < 0 { + return 0, nil + } else if errors.Is(intErr, strconv.ErrRange) && intValue < 0 { + return 0, nil + } + + return value, err + } + + return value, nil +} diff --git a/runsc/cgroup/cgroup_v2_test.go b/runsc/cgroup/cgroup_v2_test.go new file mode 100644 index 000000000..20d32609a --- /dev/null +++ b/runsc/cgroup/cgroup_v2_test.go @@ -0,0 +1,199 @@ +// Copyright The runc Authors. +// Copyright 2021 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package cgroup + +import ( + "strings" + "testing" +) + +var cgroupv2MountInfo = `29 22 0:26 / /sys/fs/cgroup rw shared:4 - cgroup2 cgroup2 rw,seclabel,nsdelegate` + +func TestLoadPathsCgroupv2(t *testing.T) { + for _, tc := range []struct { + name string + cgroups string + mountinfo string + want map[string]string + err string + }{ + { + name: "cgroupv2", + cgroups: "0::/docker/123", + mountinfo: cgroupv2MountInfo, + want: map[string]string{ + "cgroup2": "docker/123", + }, + }, + + { + name: "cgroupv2-nested", + cgroups: "0::/", + mountinfo: cgroupv2MountInfo, + want: map[string]string{ + "cgroup2": ".", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + r := strings.NewReader(tc.cgroups) + mountinfo := strings.NewReader(tc.mountinfo) + got, err := loadPathsHelper(r, mountinfo, true) + if len(tc.err) == 0 { + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } else if !strings.Contains(err.Error(), tc.err) { + t.Fatalf("Wrong error message, want: *%s*, got: %v", tc.err, err) + } + for key, vWant := range tc.want { + vGot, ok := got[key] + if !ok { + t.Errorf("Missing controller %q", key) + } + if vWant != vGot { + t.Errorf("Wrong controller %q value, want: %q, got: %q", key, vWant, vGot) + } + delete(got, key) + } + for k, v := range got { + t.Errorf("Unexpected controller %q: %q", k, v) + } + }) + } +} + +func TestNumToStr(t *testing.T) { + cases := map[int64]string{ + 0: "", + -1: "max", + 10: "10", + } + for i, expected := range cases { + got := numToStr(i) + if got != expected { + t.Errorf("expected numToStr(%d) to be %q, got %q", i, expected, got) + } + } +} + +func TestConvertBlkIOToIOWeightValue(t *testing.T) { + cases := map[uint16]uint64{ + 0: 0, + 10: 1, + 1000: 10000, + } + for i, expected := range cases { + got := convertBlkIOToIOWeightValue(i) + if got != expected { + t.Errorf("expected ConvertBlkIOToIOWeightValue(%d) to be %d, got %d", i, expected, got) + } + } +} + +func TestConvertCPUSharesToCgroupV2Value(t *testing.T) { + cases := map[uint64]uint64{ + 0: 0, + 2: 1, + 262144: 10000, + } + for i, expected := range cases { + got := convertCPUSharesToCgroupV2Value(i) + if got != expected { + t.Errorf("expected ConvertCPUSharesToCgroupV2Value(%d) to be %d, got %d", i, expected, got) + } + } +} + +func TestConvertMemorySwapToCgroupV2Value(t *testing.T) { + cases := []struct { + memswap, memory int64 + expected int64 + expErr bool + }{ + { + memswap: 0, + memory: 0, + expected: 0, + }, + { + memswap: -1, + memory: 0, + expected: -1, + }, + { + memswap: -1, + memory: -1, + expected: -1, + }, + { + memswap: -2, + memory: 0, + expErr: true, + }, + { + memswap: -1, + memory: 1000, + expected: -1, + }, + { + memswap: 1000, + memory: 1000, + expected: 0, + }, + { + memswap: 500, + memory: 200, + expected: 300, + }, + { + memswap: 300, + memory: 400, + expErr: true, + }, + { + memswap: 300, + memory: 0, + expErr: true, + }, + { + memswap: 300, + memory: -300, + expErr: true, + }, + { + memswap: 300, + memory: -1, + expErr: true, + }, + } + + for _, c := range cases { + swap, err := convertMemorySwapToCgroupV2Value(c.memswap, c.memory) + if c.expErr { + if err == nil { + t.Errorf("memswap: %d, memory %d, expected error, got %d, nil", c.memswap, c.memory, swap) + } + // no more checks + continue + } + if err != nil { + t.Errorf("memswap: %d, memory %d, expected success, got error %s", c.memswap, c.memory, err) + } + if swap != c.expected { + t.Errorf("memswap: %d, memory %d, expected %d, got %d", c.memswap, c.memory, c.expected, swap) + } + } +} diff --git a/runsc/container/container.go b/runsc/container/container.go index bee37e1b3..f1d92f12e 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -1277,13 +1277,6 @@ func (c *Container) setupCgroupForSubcontainer(conf *config.Config, spec *specs. // error is suppressed and a nil cgroups instance is returned to indicate that // no cgroups was configured. func cgroupInstall(conf *config.Config, cg cgroup.Cgroup, res *specs.LinuxResources) (cgroup.Cgroup, error) { - // TODO(gvisor.dev/issue/3481): Remove when cgroups v2 is supported. - if cgroup.IsOnlyV2() { - if conf.Rootless { - return nil, nil - } - return nil, fmt.Errorf("cgroups V2 is not yet supported. Enable cgroups V1 and retry") - } if err := cg.Install(res); err != nil { switch { case errors.Is(err, unix.EACCES) && conf.Rootless: diff --git a/test/root/cgroup_test.go b/test/root/cgroup_test.go index 39e838582..cd375f082 100644 --- a/test/root/cgroup_test.go +++ b/test/root/cgroup_test.go @@ -86,8 +86,15 @@ 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. + 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") + } // Read the cgroup memory limit. - path := filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.limit_in_bytes") outRaw, err := ioutil.ReadFile(path) if err != nil { // It's possible that the container does not exist yet. @@ -103,8 +110,14 @@ func TestMemCgroup(t *testing.T) { continue } + 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") + } // Read the cgroup memory usage. - path = filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.max_usage_in_bytes") outRaw, err = ioutil.ReadFile(path) if err != nil { t.Fatalf("error reading usage: %v", err) @@ -126,7 +139,10 @@ func TestMemCgroup(t *testing.T) { } // TestCgroup sets cgroup options and checks that cgroup was properly configured. -func TestCgroup(t *testing.T) { +func TestCgroupV1(t *testing.T) { + if cgroup.IsOnlyV2() { + t.Skip("skipping cgroupv1 attribute testing in cgroupv2 setup") + } ctx := context.Background() d := dockerutil.MakeContainer(ctx, t) defer d.CleanUp(ctx) @@ -308,6 +324,154 @@ func TestCgroup(t *testing.T) { } } +// TestCgroupV2 sets cgroup options and checks that cgroup was properly configured with +// cgroupv2 setup +func TestCgroupV2(t *testing.T) { + if !cgroup.IsOnlyV2() { + t.Skip("skipping cgroupv2 attribute testing in cgroupv1 setup") + } + ctx := context.Background() + d := dockerutil.MakeContainer(ctx, t) + defer d.CleanUp(ctx) + + // This is not a comprehensive list of attributes. + // + // Note that we are specifically missing cpusets, which fail if specified. + // In any case, it's unclear if cpusets can be reliably tested here: these + // are often run on a single core virtual machine, and there is only a single + // CPU available in our current set, and every container's set. + attrs := []struct { + field string + value int64 + file string + want string + skipIfNotFound bool + }{ + { + field: "cpu-shares", + value: 3333, + file: "cpu.weight", + want: "128", + }, + { + field: "cpu-period", + value: 2000, + file: "cpu.max", + want: "max 2000", + }, + { + field: "memory", + value: 1 << 30, + file: "memory.max", + want: "1073741824", + }, + { + field: "memory-reservation", + value: 500 << 20, + file: "memory.low", + want: "524288000", + }, + { + field: "memory-swap", + value: 1 << 31, + file: "memory.swap.max", + // memory.swap.max is only the swap value, unlike cgroupv1 + want: fmt.Sprintf("%d", 1<<31-1<<30), + skipIfNotFound: true, // swap may be disabled on the machine. + }, + { + field: "blkio-weight", + value: 750, + file: "io.bfq.weight", + want: fmt.Sprintf("default %d", 750), + skipIfNotFound: true, // blkio groups may not be available. + }, + { + field: "pids-limit", + value: 1000, + file: "pids.max", + want: "1000", + }, + } + + // Make configs. + conf, hostconf, _ := d.ConfigsFrom(dockerutil.RunOpts{ + Image: "basic/alpine", + }, "sleep", "10000") + + // Add Cgroup arguments to configs. + for _, attr := range attrs { + switch attr.field { + case "cpu-shares": + hostconf.Resources.CPUShares = attr.value + case "cpu-period": + hostconf.Resources.CPUPeriod = attr.value + case "cpu-quota": + hostconf.Resources.CPUQuota = attr.value + case "kernel-memory": + hostconf.Resources.KernelMemory = attr.value + case "memory": + hostconf.Resources.Memory = attr.value + case "memory-reservation": + hostconf.Resources.MemoryReservation = attr.value + case "memory-swap": + hostconf.Resources.MemorySwap = attr.value + case "memory-swappiness": + val := attr.value + 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)) + if err == nil || !attr.skipIfNotFound { + hostconf.Resources.BlkioWeight = uint16(attr.value) + } + case "pids-limit": + val := attr.value + hostconf.Resources.PidsLimit = &val + } + } + + // Create container. + if err := d.CreateFrom(ctx, "basic/alpine", conf, hostconf, nil); err != nil { + t.Fatalf("create failed with: %v", err) + } + + // Start container. + if err := d.Start(ctx); err != nil { + t.Fatalf("start failed with: %v", err) + } + + // Lookup the relevant cgroup ID. + gid := d.ID() + t.Logf("cgroup ID: %s", gid) + + // Check list of attributes defined above. + for _, attr := range attrs { + path := filepath.Join("/sys/fs/cgroup", "docker", gid, attr.file) + out, err := ioutil.ReadFile(path) + if err != nil { + if os.IsNotExist(err) && attr.skipIfNotFound { + t.Logf("skipped %s", attr.file) + continue + } + t.Fatalf("failed to read %q: %v", path, err) + } + if got := strings.TrimSpace(string(out)); got != attr.want { + t.Errorf("field: %q, cgroup attribute %s, got: %q, want: %q", attr.field, attr.file, got, attr.want) + } + } + + // Check that sandbox is inside cgroup. + pid, err := d.SandboxPid(ctx) + if err != nil { + t.Fatalf("SandboxPid: %v", err) + } + path := filepath.Join("/sys/fs/cgroup", "docker", gid, "cgroup.procs") + if err := verifyPid(pid, path); err != nil { + t.Errorf("cgroup control processes: %v", err) + } +} + // TestCgroupParent sets the "CgroupParent" option and checks that the child and // parent's cgroups are created correctly relative to each other. func TestCgroupParent(t *testing.T) { From caf6f8d1527007eea582c54e5a52a7a8445790a5 Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Tue, 9 Nov 2021 11:51:44 +0000 Subject: [PATCH 3/9] cgroupv2: fix CPUQuota parsing CPUQuota can return "max PERIOD", in this case, we detect "max" and return `-1, nil`, which for the current usecase of detecting cpu-num from quota should be sufficient. Signed-off-by: Daniel Dao --- runsc/cgroup/cgroup_v2.go | 13 +++++++++- runsc/cgroup/cgroup_v2_test.go | 43 ++++++++++++++++++++++++++++++++++ runsc/sandbox/sandbox.go | 2 +- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/runsc/cgroup/cgroup_v2.go b/runsc/cgroup/cgroup_v2.go index 36ea4360f..5e6158ea7 100644 --- a/runsc/cgroup/cgroup_v2.go +++ b/runsc/cgroup/cgroup_v2.go @@ -234,11 +234,21 @@ func (c *cgroupV2) CPUQuota() (float64, error) { if err != nil { return -1, err } - data := strings.SplitN(cpuMax, " ", 2) + + return parseCPUQuota(cpuMax) +} + +func parseCPUQuota(cpuMax string) (float64, error) { + data := strings.SplitN(strings.TrimSpace(cpuMax), " ", 2) if len(data) != 2 { return -1, fmt.Errorf("invalid cpu.max data %q", cpuMax) } + // no cpu limit if quota is max + if data[0] == "max" { + return -1, nil + } + quota, err := strconv.ParseInt(data[0], 10, 64) if err != nil { return -1, err @@ -253,6 +263,7 @@ func (c *cgroupV2) CPUQuota() (float64, error) { return -1, err } return float64(quota) / float64(period), nil + } // CPUUsage returns the total CPU usage of the cgroup. diff --git a/runsc/cgroup/cgroup_v2_test.go b/runsc/cgroup/cgroup_v2_test.go index 20d32609a..62ed8f2be 100644 --- a/runsc/cgroup/cgroup_v2_test.go +++ b/runsc/cgroup/cgroup_v2_test.go @@ -197,3 +197,46 @@ func TestConvertMemorySwapToCgroupV2Value(t *testing.T) { } } } + +func TestParseCPUQuota(t *testing.T) { + cases := []struct { + quota string + expected float64 + expErr bool + }{ + { + quota: "max 100000\n", + expected: -1, + }, + { + quota: "10000 100000", + expected: 0.1, + }, + { + quota: "20000 100000\n", + expected: 0.2, + }, + + { + quota: "-1", + expected: -1, + expErr: true, + }, + } + + for _, c := range cases { + res, err := parseCPUQuota(c.quota) + if c.expErr { + if err == nil { + t.Errorf("quota: %q, expected error, got %.2f, nil", c.quota, res) + } + continue + } + if err != nil { + t.Errorf("quota: %q, expected success, got error %s", c.quota, err) + } + if res != c.expected { + t.Errorf("quota: %q, expected %.2f, got error %.2f", c.quota, c.expected, res) + } + } +} diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 8f7022fa1..2605b7cd3 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -777,7 +777,7 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn quota, err := s.CgroupJSON.Cgroup.CPUQuota() if err != nil { - return fmt.Errorf("getting cpu qouta from cgroups: %v", err) + return fmt.Errorf("getting cpu quota from cgroups: %v", err) } if n := int(math.Ceil(quota)); n > 0 { if n < minCPUs { From 60456ecc502da63e6957d2145b14ebbb79fd803c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 23 Nov 2021 13:04:23 -0800 Subject: [PATCH 4/9] buildkite: runsc tests on cgroupv2 Signed-off-by: Andrei Vagin --- .buildkite/pipeline.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index 3457ac676..26a981afd 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -111,9 +111,19 @@ steps: - <<: *common label: ":test_tube: Unit tests" command: make unit-tests + - <<: *common + label: ":test_tube: Unit tests (cgroupv2)" + command: make unit-tests + agents: + queue: "cgroupv2" - <<: *common label: ":test_tube: Container tests" command: make container-tests + - <<: *common + label: ":test_tube: Container tests (cgroupv2)" + command: make container-tests + agents: + queue: "cgroupv2" # All system call tests. - <<: *common @@ -125,6 +135,11 @@ steps: - <<: *common label: ":docker: Docker tests" command: make docker-tests + - <<: *common + label: ":docker: Docker tests (cgroupv2)" + command: make docker-tests + agents: + queue: "cgroupv2" - <<: *common label: ":goggles: Overlay tests" command: make overlay-tests @@ -152,6 +167,11 @@ steps: - <<: *common label: ":docker: Containerd 1.5.4 tests" command: make containerd-test-1.5.4 + - <<: *common + label: ":docker: Containerd 1.5.4 tests (cgroupv2)" + command: make containerd-test-1.5.4 + agents: + queue: "cgroupv2" # Check the website builds. - <<: *common From 11700f409f816ae1964013342377eea2417ff19b Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 23 Nov 2021 13:32:20 -0800 Subject: [PATCH 5/9] buildkite: set DEBIAN_FRONTEND=noninteractive --- .buildkite/hooks/pre-command | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.buildkite/hooks/pre-command b/.buildkite/hooks/pre-command index baaf853ca..68bf14ef9 100644 --- a/.buildkite/hooks/pre-command +++ b/.buildkite/hooks/pre-command @@ -1,8 +1,9 @@ # Install packages we need. Docker must be installed and configured, # as should Go itself. We just install some extra bits and pieces. function install_pkgs() { + export DEBIAN_FRONTEND=noninteractive while true; do - if sudo apt-get update && sudo apt-get install -y "$@"; then + if sudo -E apt-get update && sudo -E apt-get install -y "$@"; then break fi done From 1f9d74e1bba7b59e0ecc5736ce03d6aea84025ef Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 23 Nov 2021 13:55:59 -0800 Subject: [PATCH 6/9] Set native.cgroupdriver=cgroupfs for docker --- .buildkite/hooks/pre-command | 5 +++-- runsc/cmd/install.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.buildkite/hooks/pre-command b/.buildkite/hooks/pre-command index 68bf14ef9..600e1fb1c 100644 --- a/.buildkite/hooks/pre-command +++ b/.buildkite/hooks/pre-command @@ -9,7 +9,8 @@ function install_pkgs() { done } install_pkgs make linux-libc-dev graphviz jq curl binutils gnupg gnupg-agent \ - gcc pkg-config apt-transport-https ca-certificates software-properties-common + gcc pkg-config apt-transport-https ca-certificates software-properties-common \ + jq # Install headers, only if available. if test -n "$(apt-cache search --names-only "^linux-headers-$(uname -r)$")"; then @@ -24,7 +25,7 @@ export TOTAL_PARTITIONS=${BUILDKITE_PARALLEL_JOB_COUNT:-1} # Ensure Docker has experimental enabled. EXPERIMENTAL=$(sudo docker version --format='{{.Server.Experimental}}') if test "${EXPERIMENTAL}" != "true"; then - make sudo TARGETS=//runsc:runsc ARGS="install --experimental=true" + make sudo TARGETS=//runsc:runsc ARGS="install --experimental=true --cgroupdriver=cgroupfs" sudo systemctl restart docker fi diff --git a/runsc/cmd/install.go b/runsc/cmd/install.go index dc9e01d95..43f8bd7d7 100644 --- a/runsc/cmd/install.go +++ b/runsc/cmd/install.go @@ -32,6 +32,7 @@ type Install struct { ConfigFile string Runtime string Experimental bool + CgroupDriver string } // Name implements subcommands.Command.Name. @@ -55,6 +56,7 @@ func (i *Install) SetFlags(fs *flag.FlagSet) { fs.StringVar(&i.ConfigFile, "config_file", "/etc/docker/daemon.json", "path to Docker daemon config file") fs.StringVar(&i.Runtime, "runtime", "runsc", "runtime name") fs.BoolVar(&i.Experimental, "experimental", false, "enable experimental features") + fs.StringVar(&i.CgroupDriver, "cgroupdriver", "", "docker cgroup driver") } // Execute implements subcommands.Command.Execute. @@ -95,6 +97,17 @@ func (i *Install) Execute(_ context.Context, f *flag.FlagSet, _ ...interface{}) c["experimental"] = true } + if i.CgroupDriver != "" { + v, ok := c["exec-opts"] + if ok { + log.Printf("%v", v) + opts := v.([]interface{}) + c["exec-opts"] = append(opts, fmt.Sprintf("native.cgroupdriver=%s", i.CgroupDriver)) + } else { + c["exec-opts"] = []string{fmt.Sprintf("native.cgroupdriver=%s", i.CgroupDriver)} + } + } + // Write out the runtime. if err := writeConfig(c, i.ConfigFile); err != nil { log.Fatalf("Error writing config file %q: %v", i.ConfigFile, err) From 70ae38ab761c77f0c207f80694f94b6b8658c3e1 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 23 Nov 2021 16:23:28 -0800 Subject: [PATCH 7/9] buildkite: CgroupDriver has to be cgroupfs --- .buildkite/hooks/pre-command | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.buildkite/hooks/pre-command b/.buildkite/hooks/pre-command index 600e1fb1c..ce7c2cff8 100644 --- a/.buildkite/hooks/pre-command +++ b/.buildkite/hooks/pre-command @@ -24,7 +24,8 @@ export TOTAL_PARTITIONS=${BUILDKITE_PARALLEL_JOB_COUNT:-1} # Ensure Docker has experimental enabled. EXPERIMENTAL=$(sudo docker version --format='{{.Server.Experimental}}') -if test "${EXPERIMENTAL}" != "true"; then +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" sudo systemctl restart docker fi From 102a00ff5e8b2482d148795f1ad3bb1320093bca Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 23 Nov 2021 15:01:01 -0800 Subject: [PATCH 8/9] Add an empty line between header and package --- runsc/cgroup/cgroup_v2.go | 1 + 1 file changed, 1 insertion(+) diff --git a/runsc/cgroup/cgroup_v2.go b/runsc/cgroup/cgroup_v2.go index 5e6158ea7..13da3ec6d 100644 --- a/runsc/cgroup/cgroup_v2.go +++ b/runsc/cgroup/cgroup_v2.go @@ -13,6 +13,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package cgroup import ( From 32205d7a9490059b5c1405191384708863a26ac6 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 23 Nov 2021 15:28:29 -0800 Subject: [PATCH 9/9] tools/install_containerd.sh: compare strings properly --- tools/install_containerd.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/install_containerd.sh b/tools/install_containerd.sh index 171f4def0..e5c6131b3 100755 --- a/tools/install_containerd.sh +++ b/tools/install_containerd.sh @@ -27,7 +27,7 @@ if [[ "${CONTAINERD_MAJOR}" -eq 1 ]] && [[ "${CONTAINERD_MINOR}" -le 4 ]]; then fi # containerd < 1.4 doesn't work with cgroupv2 setup, so we check for that here -if [[ "$(stat -f -c %T /sys/fs/cgroup 2>/dev/null)" -eq "cgroup2fs" && "${CONTAINERD_MAJOR}" -eq 1 && "${CONTAINERD_MINOR}" -lt 4 ]]; then +if [[ "$(stat -f -c %T /sys/fs/cgroup 2>/dev/null)" == "cgroup2fs" && "${CONTAINERD_MAJOR}" -eq 1 && "${CONTAINERD_MINOR}" -lt 4 ]]; then echo "containerd < 1.4 does not work with cgroup2" exit 1 fi