From 1a4509dd06454dc765aed7f62cb26531416ffce3 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Tue, 4 May 2021 20:35:36 -0700 Subject: [PATCH 01/19] Use /proc/partitions to get device names Signed-off-by: Alexey Ivanov --- blkio.go | 9 ++++++--- blkio_test.go | 44 +++++++++++++++++++++++++------------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/blkio.go b/blkio.go index a837e19f..f59c75bb 100644 --- a/blkio.go +++ b/blkio.go @@ -130,7 +130,7 @@ func (b *blkioController) Stat(path string, stats *v1.Metrics) error { } } - f, err := os.Open(filepath.Join(b.procRoot, "diskstats")) + f, err := os.Open(filepath.Join(b.procRoot, "partitions")) if err != nil { return err } @@ -335,7 +335,10 @@ func getDevices(r io.Reader) (map[deviceKey]string, error) { s = bufio.NewScanner(r) devices = make(map[deviceKey]string) ) - for s.Scan() { + for i := 0; s.Scan(); i++ { + if i < 2 { + continue + } fields := strings.Fields(s.Text()) major, err := strconv.Atoi(fields[0]) if err != nil { @@ -352,7 +355,7 @@ func getDevices(r io.Reader) (map[deviceKey]string, error) { if _, ok := devices[key]; ok { continue } - devices[key] = filepath.Join("/dev", fields[2]) + devices[key] = filepath.Join("/dev", fields[3]) } return devices, s.Err() } diff --git a/blkio_test.go b/blkio_test.go index 36b08957..07b2a967 100644 --- a/blkio_test.go +++ b/blkio_test.go @@ -24,18 +24,19 @@ import ( v1 "github.com/containerd/cgroups/stats/v1" ) -const data = ` 7 0 loop0 0 0 0 0 0 0 0 0 0 0 0 - 7 1 loop1 0 0 0 0 0 0 0 0 0 0 0 - 7 2 loop2 0 0 0 0 0 0 0 0 0 0 0 - 7 3 loop3 0 0 0 0 0 0 0 0 0 0 0 - 7 4 loop4 0 0 0 0 0 0 0 0 0 0 0 - 7 5 loop5 0 0 0 0 0 0 0 0 0 0 0 - 7 6 loop6 0 0 0 0 0 0 0 0 0 0 0 - 7 7 loop7 0 0 0 0 0 0 0 0 0 0 0 - 8 0 sda 1892042 187697 63489222 1246284 1389086 2887005 134903104 11390608 1 1068060 12692228 - 8 1 sda1 1762875 37086 61241570 1200512 1270037 2444415 131214808 11152764 1 882624 12409308 - 8 2 sda2 2 0 4 0 0 0 0 0 0 0 0 - 8 5 sda5 129102 150611 2244440 45716 18447 442590 3688296 67268 0 62584 112984` +const data = `major minor #blocks name + + 7 0 4 loop0 + 7 1 163456 loop1 + 7 2 149616 loop2 + 7 3 147684 loop3 + 7 4 122572 loop4 + 7 5 8936 loop5 + 7 6 31464 loop6 + 7 7 182432 loop7 + 259 0 937692504 nvme0n1 + 259 1 31744 nvme0n1p1 +` func TestGetDevices(t *testing.T) { r := strings.NewReader(data) @@ -43,13 +44,18 @@ func TestGetDevices(t *testing.T) { if err != nil { t.Fatal(err) } - name, ok := devices[deviceKey{8, 0}] - if !ok { - t.Fatal("no device found for 8,0") - } - const expected = "/dev/sda" - if name != expected { - t.Fatalf("expected device name %q but received %q", expected, name) + for dev, expected := range map[deviceKey]string{ + deviceKey{7, 0}: "/dev/loop0", + deviceKey{259, 0}: "/dev/nvme0n1", + deviceKey{259, 1}: "/dev/nvme0n1p1", + } { + name, ok := devices[dev] + if !ok { + t.Fatalf("no device found for %d:%d", dev.major, dev.minor) + } + if name != expected { + t.Fatalf("expected device name %q but received %q", expected, name) + } } } From 23b51209bf7bd11941d5613119309b11029e8373 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 5 Jun 2021 16:37:00 +0200 Subject: [PATCH 02/19] utils: export ParseCgroupFile() this exports the ParseCgroupFile() function, so that external consumers (such as moby) can use this function, instead of the implementation in runc's libcontainer (which is not intended for external consumers). The GoDoc documentation was borrowed from runc: https://github.com/opencontainers/runc/commit/d244b4058ee81ca43182bffe536d63ec70326904 Co-authored-by: Kir Kolyshkin Signed-off-by: Sebastiaan van Stijn --- paths.go | 4 ++-- paths_test.go | 4 ++-- utils.go | 11 ++++++++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/paths.go b/paths.go index 27197eca..ff50c95d 100644 --- a/paths.go +++ b/paths.go @@ -39,7 +39,7 @@ func StaticPath(path string) Path { // NestedPath will nest the cgroups based on the calling processes cgroup // placing its child processes inside its own path func NestedPath(suffix string) Path { - paths, err := parseCgroupFile("/proc/self/cgroup") + paths, err := ParseCgroupFile("/proc/self/cgroup") if err != nil { return errorPath(err) } @@ -50,7 +50,7 @@ func NestedPath(suffix string) Path { // This is commonly used for the Load function to restore an existing container func PidPath(pid int) Path { p := fmt.Sprintf("/proc/%d/cgroup", pid) - paths, err := parseCgroupFile(p) + paths, err := ParseCgroupFile(p) if err != nil { return errorPath(errors.Wrapf(err, "parse cgroup file %s", p)) } diff --git a/paths_test.go b/paths_test.go index 6860dee0..9dd2898d 100644 --- a/paths_test.go +++ b/paths_test.go @@ -41,7 +41,7 @@ func TestSelfPath(t *testing.T) { } else if err != nil { t.Fatal(err) } - paths, err := parseCgroupFile("/proc/self/cgroup") + paths, err := ParseCgroupFile("/proc/self/cgroup") if err != nil { t.Fatal(err) } @@ -63,7 +63,7 @@ func TestPidPath(t *testing.T) { } else if err != nil { t.Fatal(err) } - paths, err := parseCgroupFile("/proc/self/cgroup") + paths, err := ParseCgroupFile("/proc/self/cgroup") if err != nil { t.Fatal(err) } diff --git a/utils.go b/utils.go index ed894b3e..ac7a0d49 100644 --- a/utils.go +++ b/utils.go @@ -285,7 +285,16 @@ func parseKV(raw string) (string, uint64, error) { } } -func parseCgroupFile(path string) (map[string]string, error) { +// ParseCgroupFile parses the given cgroup file, typically /proc/self/cgroup +// or /proc//cgroup, into a map of subsystems to cgroup paths, e.g. +// "cpu": "/user.slice/user-1000.slice" +// "pids": "/user.slice/user-1000.slice" +// etc. +// +// Note that for cgroup v2 unified hierarchy, there are no per-controller +// cgroup paths, so the resulting map will have a single element where the key +// is empty string ("") and the value is the cgroup path the is in. +func ParseCgroupFile(path string) (map[string]string, error) { f, err := os.Open(path) if err != nil { return nil, err From f8918cf2d923b7003970ce99bb29545f2f34b531 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 20 Apr 2021 14:24:52 +0200 Subject: [PATCH 03/19] go.mod: coreos/go-systemd/v22 v22.3.2 to prepare for deprecations I noticed that some containerd dependencies updated to the latest version of github.com/coreos/go-systemd/v22, which has deprecated various functions. Updating the version here as well to prevent using functions that have been deprecated. v22.3.2 also contains a fix for leaked sockets after successful probe. Signed-off-by: Sebastiaan van Stijn --- go.mod | 4 ++-- go.sum | 8 ++++---- systemd.go | 15 +++++++++------ v2/manager.go | 11 +++++++---- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index eed71ffb..5bd39c72 100644 --- a/go.mod +++ b/go.mod @@ -4,10 +4,10 @@ go 1.13 require ( github.com/cilium/ebpf v0.4.0 - github.com/coreos/go-systemd/v22 v22.1.0 + github.com/coreos/go-systemd/v22 v22.3.2 github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect github.com/docker/go-units v0.4.0 - github.com/godbus/dbus/v5 v5.0.3 + github.com/godbus/dbus/v5 v5.0.4 github.com/gogo/protobuf v1.3.2 github.com/opencontainers/runtime-spec v1.0.2 github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index f0861023..2e6161e6 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/cilium/ebpf v0.4.0 h1:QlHdikaxALkqWasW8hAC1mfR0jdmvbfaBdBPFmRSglA= github.com/cilium/ebpf v0.4.0/go.mod h1:4tRaxcgiL706VnOzHOdBlY8IEAIdxINsQBcU4xJJXRs= -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/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 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng4PGlyM= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= @@ -13,8 +13,8 @@ github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= github.com/frankban/quicktest v1.11.3 h1:8sXhOn0uLys67V8EsXLc6eszDs8VXWxL3iRvebPhedY= github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k= -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/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= diff --git a/systemd.go b/systemd.go index c17f34a6..4da57cb4 100644 --- a/systemd.go +++ b/systemd.go @@ -17,6 +17,7 @@ package cgroups import ( + "context" "path/filepath" "strings" "sync" @@ -78,7 +79,8 @@ func (s *SystemdController) Name() Name { } func (s *SystemdController) Create(path string, _ *specs.LinuxResources) error { - conn, err := systemdDbus.New() + ctx := context.TODO() + conn, err := systemdDbus.NewWithContext(ctx) if err != nil { return err } @@ -90,7 +92,7 @@ func (s *SystemdController) Create(path string, _ *specs.LinuxResources) error { checkDelegate := func() { canDelegate = true dlSlice := newProperty("Delegate", true) - if _, err := conn.StartTransientUnit(slice, "testdelegate", []systemdDbus.Property{dlSlice}, nil); err != nil { + if _, err := conn.StartTransientUnitContext(ctx, slice, "testdelegate", []systemdDbus.Property{dlSlice}, nil); err != nil { if dbusError, ok := err.(dbus.Error); ok { // Starting with systemd v237, Delegate is not even a property of slices anymore, // so the D-Bus call fails with "InvalidArgs" error. @@ -100,7 +102,7 @@ func (s *SystemdController) Create(path string, _ *specs.LinuxResources) error { } } - conn.StopUnit(slice, "testDelegate", nil) + _, _ = conn.StopUnitContext(ctx, slice, "testDelegate", nil) } once.Do(checkDelegate) properties := []systemdDbus.Property{ @@ -118,7 +120,7 @@ func (s *SystemdController) Create(path string, _ *specs.LinuxResources) error { } ch := make(chan string) - _, err = conn.StartTransientUnit(name, "replace", properties, ch) + _, err = conn.StartTransientUnitContext(ctx, name, "replace", properties, ch) if err != nil { return err } @@ -127,14 +129,15 @@ func (s *SystemdController) Create(path string, _ *specs.LinuxResources) error { } func (s *SystemdController) Delete(path string) error { - conn, err := systemdDbus.New() + ctx := context.TODO() + conn, err := systemdDbus.NewWithContext(ctx) if err != nil { return err } defer conn.Close() _, name := splitName(path) ch := make(chan string) - _, err = conn.StopUnit(name, "replace", ch) + _, err = conn.StopUnitContext(ctx, name, "replace", ch) if err != nil { return err } diff --git a/v2/manager.go b/v2/manager.go index 3bb546cb..4554a11e 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -18,6 +18,7 @@ package v2 import ( "bufio" + "context" "io/ioutil" "math" "os" @@ -662,8 +663,9 @@ func NewSystemd(slice, group string, pid int, resources *Resources) (*Manager, e if slice == "" { slice = defaultSlice } + ctx := context.TODO() path := filepath.Join(defaultCgroup2Path, slice, group) - conn, err := systemdDbus.New() + conn, err := systemdDbus.NewWithContext(ctx) if err != nil { return &Manager{}, err } @@ -733,7 +735,7 @@ func NewSystemd(slice, group string, pid int, resources *Resources) (*Manager, e } statusChan := make(chan string, 1) - if _, err := conn.StartTransientUnit(group, "replace", properties, statusChan); err == nil { + if _, err := conn.StartTransientUnitContext(ctx, group, "replace", properties, statusChan); err == nil { select { case <-statusChan: case <-time.After(time.Second): @@ -759,14 +761,15 @@ func LoadSystemd(slice, group string) (*Manager, error) { } func (c *Manager) DeleteSystemd() error { - conn, err := systemdDbus.New() + ctx := context.TODO() + conn, err := systemdDbus.NewWithContext(ctx) if err != nil { return err } defer conn.Close() group := systemdUnitFromPath(c.path) ch := make(chan string) - _, err = conn.StopUnit(group, "replace", ch) + _, err = conn.StopUnitContext(ctx, group, "replace", ch) if err != nil { return err } From 7254c1242cee46d1c37bf3da2151eed571879b5c Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Sat, 19 Jun 2021 00:20:56 -0700 Subject: [PATCH 04/19] Rename branch from master to main Signed-off-by: Derek McGowan --- .github/workflows/ci.yml | 4 ++-- README.md | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cab5a41d..05b5191b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,11 +2,11 @@ name: CI on: push: branches: - - master + - main - 'release/**' pull_request: branches: - - master + - main - 'release/**' jobs: diff --git a/README.md b/README.md index d4b09f3d..fc2c7a9b 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # cgroups [![Build Status](https://github.com/containerd/cgroups/workflows/CI/badge.svg)](https://github.com/containerd/cgroups/actions?query=workflow%3ACI) -[![codecov](https://codecov.io/gh/containerd/cgroups/branch/master/graph/badge.svg)](https://codecov.io/gh/containerd/cgroups) +[![codecov](https://codecov.io/gh/containerd/cgroups/branch/main/graph/badge.svg)](https://codecov.io/gh/containerd/cgroups) [![GoDoc](https://godoc.org/github.com/containerd/cgroups?status.svg)](https://godoc.org/github.com/containerd/cgroups) [![Go Report Card](https://goreportcard.com/badge/github.com/containerd/cgroups)](https://goreportcard.com/report/github.com/containerd/cgroups) @@ -142,8 +142,8 @@ All static path should not include `/sys/fs/cgroup/` prefix, it should start wit Cgroups is a containerd sub-project, licensed under the [Apache 2.0 license](./LICENSE). As a containerd sub-project, you will find the: - * [Project governance](https://github.com/containerd/project/blob/master/GOVERNANCE.md), - * [Maintainers](https://github.com/containerd/project/blob/master/MAINTAINERS), - * and [Contributing guidelines](https://github.com/containerd/project/blob/master/CONTRIBUTING.md) + * [Project governance](https://github.com/containerd/project/blob/main/GOVERNANCE.md), + * [Maintainers](https://github.com/containerd/project/blob/main/MAINTAINERS), + * and [Contributing guidelines](https://github.com/containerd/project/blob/main/CONTRIBUTING.md) information in our [`containerd/project`](https://github.com/containerd/project) repository. From 66590933215fb7b0c9f9ffd39e9946ab510d7679 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jul 2021 00:34:01 +0200 Subject: [PATCH 05/19] cgroup v1: implement AddProc() This implements AddProc() for cgroups v1 to make the API a closer match with the cgroupsv2 API. Signed-off-by: Sebastiaan van Stijn --- cgroup.go | 10 ++++++++++ control.go | 2 ++ 2 files changed, 12 insertions(+) diff --git a/cgroup.go b/cgroup.go index e0e014b2..caf0ad9c 100644 --- a/cgroup.go +++ b/cgroup.go @@ -162,6 +162,16 @@ func (c *cgroup) Add(process Process) error { return c.add(process) } +// AddProc moves the provided process id into the new cgroup +func (c *cgroup) AddProc(pid uint64) error { + c.mu.Lock() + defer c.mu.Unlock() + if c.err != nil { + return c.err + } + return c.Add(Process{Pid: int(pid)}) +} + func (c *cgroup) add(process Process) error { for _, s := range pathers(c.subsystems) { p, err := c.path(s.Name()) diff --git a/control.go b/control.go index a4cb9b83..fe684b2c 100644 --- a/control.go +++ b/control.go @@ -61,6 +61,8 @@ type Cgroup interface { New(string, *specs.LinuxResources) (Cgroup, error) // Add adds a process to the cgroup (cgroup.procs) Add(Process) error + // AddProc adds the process with the given id to the cgroup (cgroup.procs) + AddProc(pid uint64) error // AddTask adds a process to the cgroup (tasks) AddTask(Process) error // Delete removes the cgroup as a whole From b19a60d64d89b858b3c2bee4a04fb7f70642ffce Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jul 2021 11:14:18 +0200 Subject: [PATCH 06/19] v2: remove errors that are never returned Unlike v1, the v2 package never returns these errors. This patch removes them to prevent consumers of this package from assuming they may be returned, and writing special handling around them. NOTE: this is a breaking change, so (following SemVer) would require a major version update. However, consumers should not use these errors. Alternatively, we could "deprecate" them first. Making it a breaking change makes it more visible (compile time error), which may help consumers to find incorrect handling of these errors. Signed-off-by: Sebastiaan van Stijn --- v2/errors.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/v2/errors.go b/v2/errors.go index dfab548e..7cc86cc2 100644 --- a/v2/errors.go +++ b/v2/errors.go @@ -22,16 +22,8 @@ import ( ) var ( - ErrInvalidPid = errors.New("cgroups: pid must be greater than 0") - ErrMountPointNotExist = errors.New("cgroups: cgroup mountpoint does not exist") - ErrInvalidFormat = errors.New("cgroups: parsing file with invalid format failed") - ErrFreezerNotSupported = errors.New("cgroups: freezer cgroup (v2) not supported on this system") - ErrMemoryNotSupported = errors.New("cgroups: memory cgroup (v2) not supported on this system") - ErrPidsNotSupported = errors.New("cgroups: pids cgroup (v2) not supported on this system") - ErrCPUNotSupported = errors.New("cgroups: cpu cgroup (v2) not supported on this system") - ErrCgroupDeleted = errors.New("cgroups: cgroup deleted") - ErrNoCgroupMountDestination = errors.New("cgroups: cannot find cgroup mount destination") - ErrInvalidGroupPath = errors.New("cgroups: invalid group path") + ErrInvalidFormat = errors.New("cgroups: parsing file with invalid format failed") + ErrInvalidGroupPath = errors.New("cgroups: invalid group path") ) // ErrorHandler is a function that handles and acts on errors From db173a86bea843700360e7061c6f8d3869d13363 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jul 2021 11:26:36 +0200 Subject: [PATCH 07/19] v2: remove ErrorHandler and IgnoreNotExist as they are not implemented Unlike v1, these are not used/implemented in the v2 code, so removing these. Signed-off-by: Sebastiaan van Stijn --- v2/errors.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/v2/errors.go b/v2/errors.go index 7cc86cc2..eeae362b 100644 --- a/v2/errors.go +++ b/v2/errors.go @@ -18,21 +18,9 @@ package v2 import ( "errors" - "os" ) var ( ErrInvalidFormat = errors.New("cgroups: parsing file with invalid format failed") ErrInvalidGroupPath = errors.New("cgroups: invalid group path") ) - -// ErrorHandler is a function that handles and acts on errors -type ErrorHandler func(err error) error - -// IgnoreNotExist ignores any errors that are for not existing files -func IgnoreNotExist(err error) error { - if os.IsNotExist(err) { - return nil - } - return err -} From 4fe70f3edc256fc2345d5f8f8a54e2f4e96f271e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jul 2021 09:32:50 +0200 Subject: [PATCH 08/19] v1: reduce duplicated code Code for handling Tasks and Processes, except for "cgroup.procs" vs "tasks", and Tasks being a different type. This patch: - Makes the Task type an alias for Process (as they're identical otherwise) - Adds a `processType` type for the `cgroupProcs` and `cgroupTasks` consts. This type is an alias for "string", and mostly for clarity (indicating it's an 'enum'). - Merges the `cgroup.add()` and `cgroup.addTask()` functions, adding a `pType` argument. - Merges the `cgroup.processes()` and `cgroup.tasks()` functions, adding a `pType` argument. - Merges the `readPids()` and `readTasksPids()` utilities, adding a `pType` argument. - Move locking and validation into `cgroup.add()`. All places using `cgroup.add()` were taking a lock and doing this validation, so looks we can move this code into `cgroup.add()` itself. Signed-off-by: Sebastiaan van Stijn --- cgroup.go | 99 +++++++++++------------------------------------------- control.go | 23 +++++-------- utils.go | 36 ++------------------ 3 files changed, 31 insertions(+), 127 deletions(-) diff --git a/cgroup.go b/cgroup.go index caf0ad9c..35499a78 100644 --- a/cgroup.go +++ b/cgroup.go @@ -151,46 +151,20 @@ func (c *cgroup) Subsystems() []Subsystem { // Add moves the provided process into the new cgroup func (c *cgroup) Add(process Process) error { - if process.Pid <= 0 { - return ErrInvalidPid - } - c.mu.Lock() - defer c.mu.Unlock() - if c.err != nil { - return c.err - } - return c.add(process) + return c.add(process, cgroupProcs) } // AddProc moves the provided process id into the new cgroup func (c *cgroup) AddProc(pid uint64) error { - c.mu.Lock() - defer c.mu.Unlock() - if c.err != nil { - return c.err - } - return c.Add(Process{Pid: int(pid)}) -} - -func (c *cgroup) add(process Process) error { - for _, s := range pathers(c.subsystems) { - p, err := c.path(s.Name()) - if err != nil { - return err - } - if err := retryingWriteFile( - filepath.Join(s.Path(p), cgroupProcs), - []byte(strconv.Itoa(process.Pid)), - defaultFilePerm, - ); err != nil { - return err - } - } - return nil + return c.add(Process{Pid: int(pid)}, cgroupProcs) } // AddTask moves the provided tasks (threads) into the new cgroup func (c *cgroup) AddTask(process Process) error { + return c.add(process, cgroupTasks) +} + +func (c *cgroup) add(process Process, pType procType) error { if process.Pid <= 0 { return ErrInvalidPid } @@ -199,20 +173,17 @@ func (c *cgroup) AddTask(process Process) error { if c.err != nil { return c.err } - return c.addTask(process) -} - -func (c *cgroup) addTask(process Process) error { for _, s := range pathers(c.subsystems) { p, err := c.path(s.Name()) if err != nil { return err } - if err := retryingWriteFile( - filepath.Join(s.Path(p), cgroupTasks), + err = retryingWriteFile( + filepath.Join(s.Path(p), pType), []byte(strconv.Itoa(process.Pid)), defaultFilePerm, - ); err != nil { + ) + if err != nil { return err } } @@ -336,39 +307,7 @@ func (c *cgroup) Processes(subsystem Name, recursive bool) ([]Process, error) { if c.err != nil { return nil, c.err } - return c.processes(subsystem, recursive) -} - -func (c *cgroup) processes(subsystem Name, recursive bool) ([]Process, error) { - s := c.getSubsystem(subsystem) - sp, err := c.path(subsystem) - if err != nil { - return nil, err - } - path := s.(pather).Path(sp) - var processes []Process - err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if !recursive && info.IsDir() { - if p == path { - return nil - } - return filepath.SkipDir - } - dir, name := filepath.Split(p) - if name != cgroupProcs { - return nil - } - procs, err := readPids(dir, subsystem) - if err != nil { - return err - } - processes = append(processes, procs...) - return nil - }) - return processes, err + return c.processes(subsystem, recursive, cgroupProcs) } // Tasks returns the tasks running inside the cgroup along @@ -379,17 +318,17 @@ func (c *cgroup) Tasks(subsystem Name, recursive bool) ([]Task, error) { if c.err != nil { return nil, c.err } - return c.tasks(subsystem, recursive) + return c.processes(subsystem, recursive, cgroupTasks) } -func (c *cgroup) tasks(subsystem Name, recursive bool) ([]Task, error) { +func (c *cgroup) processes(subsystem Name, recursive bool, pType procType) ([]Process, error) { s := c.getSubsystem(subsystem) sp, err := c.path(subsystem) if err != nil { return nil, err } path := s.(pather).Path(sp) - var tasks []Task + var processes []Process err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error { if err != nil { return err @@ -401,17 +340,17 @@ func (c *cgroup) tasks(subsystem Name, recursive bool) ([]Task, error) { return filepath.SkipDir } dir, name := filepath.Split(p) - if name != cgroupTasks { + if name != pType { return nil } - procs, err := readTasksPids(dir, subsystem) + procs, err := readPids(dir, subsystem, pType) if err != nil { return err } - tasks = append(tasks, procs...) + processes = append(processes, procs...) return nil }) - return tasks, err + return processes, err } // Freeze freezes the entire cgroup and all the processes inside it @@ -521,7 +460,7 @@ func (c *cgroup) MoveTo(destination Cgroup) error { return c.err } for _, s := range c.subsystems { - processes, err := c.processes(s.Name(), true) + processes, err := c.processes(s.Name(), true, cgroupProcs) if err != nil { return err } diff --git a/control.go b/control.go index fe684b2c..8cd14e63 100644 --- a/control.go +++ b/control.go @@ -23,10 +23,12 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" ) +type procType = string + const ( - cgroupProcs = "cgroup.procs" - cgroupTasks = "tasks" - defaultDirPerm = 0755 + cgroupProcs procType = "cgroup.procs" + cgroupTasks procType = "tasks" + defaultDirPerm = 0755 ) // defaultFilePerm is a var so that the test framework can change the filemode @@ -37,22 +39,15 @@ const ( var defaultFilePerm = os.FileMode(0) type Process struct { - // Subsystem is the name of the subsystem that the process is in + // Subsystem is the name of the subsystem that the process / task is in. Subsystem Name - // Pid is the process id of the process + // Pid is the process id of the process / task. Pid int - // Path is the full path of the subsystem and location that the process is in + // Path is the full path of the subsystem and location that the process / task is in. Path string } -type Task struct { - // Subsystem is the name of the subsystem that the task is in - Subsystem Name - // Pid is the process id of the task - Pid int - // Path is the full path of the subsystem and location that the task is in - Path string -} +type Task = Process // Cgroup handles interactions with the individual groups to perform // actions on them as them main interface to this cgroup package diff --git a/utils.go b/utils.go index ac7a0d49..2297980d 100644 --- a/utils.go +++ b/utils.go @@ -164,9 +164,9 @@ func remove(path string) error { return fmt.Errorf("cgroups: unable to remove path %q", path) } -// readPids will read all the pids of processes in a cgroup by the provided path -func readPids(path string, subsystem Name) ([]Process, error) { - f, err := os.Open(filepath.Join(path, cgroupProcs)) +// readPids will read all the pids of processes or tasks in a cgroup by the provided path +func readPids(path string, subsystem Name, pType procType) ([]Process, error) { + f, err := os.Open(filepath.Join(path, pType)) if err != nil { return nil, err } @@ -195,36 +195,6 @@ func readPids(path string, subsystem Name) ([]Process, error) { return out, nil } -// readTasksPids will read all the pids of tasks in a cgroup by the provided path -func readTasksPids(path string, subsystem Name) ([]Task, error) { - f, err := os.Open(filepath.Join(path, cgroupTasks)) - if err != nil { - return nil, err - } - defer f.Close() - var ( - out []Task - s = bufio.NewScanner(f) - ) - for s.Scan() { - if t := s.Text(); t != "" { - pid, err := strconv.Atoi(t) - if err != nil { - return nil, err - } - out = append(out, Task{ - Pid: pid, - Subsystem: subsystem, - Path: path, - }) - } - } - if err := s.Err(); err != nil { - return nil, err - } - return out, nil -} - func hugePageSizes() ([]string, error) { var ( pageSizes []string From 2ca92c515038c7a30c1998dbe29a87873b98a3b3 Mon Sep 17 00:00:00 2001 From: Miao Wang Date: Mon, 26 Jul 2021 16:10:09 +0800 Subject: [PATCH 09/19] cgroupv2: enable controllers before setting resources in NewChild() Signed-off-by: Miao Wang --- v2/manager.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/v2/manager.go b/v2/manager.go index 4554a11e..1daa0780 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -301,15 +301,23 @@ func (c *Manager) NewChild(name string, resources *Resources) (*Manager, error) if err := os.MkdirAll(path, defaultDirPerm); err != nil { return nil, err } + m := Manager{ + unifiedMountpoint: c.unifiedMountpoint, + path: path, + } + if resources != nil { + if err := m.ToggleControllers(resources.EnabledControllers(), Enable); err != nil { + // clean up cgroup dir on failure + os.Remove(path) + return nil, err + } + } if err := setResources(path, resources); err != nil { // clean up cgroup dir on failure os.Remove(path) return nil, err } - return &Manager{ - unifiedMountpoint: c.unifiedMountpoint, - path: path, - }, nil + return &m, nil } func (c *Manager) AddProc(pid uint64) error { From 73a8516e7f0f45fe9931d02cc57ff6059202f553 Mon Sep 17 00:00:00 2001 From: Miao Wang Date: Mon, 26 Jul 2021 18:29:20 +0800 Subject: [PATCH 10/19] cgroupv2: reset lastErr to nil when subtree control is successfully written As stated originally in the comment, ``the user may face EPERM on parent groups", it should not be an error when writing to grand-parent groups fails but writing to the last one succeeds. Signed-off-by: Miao Wang --- v2/manager.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/v2/manager.go b/v2/manager.go index 1daa0780..c5196bbd 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -272,6 +272,8 @@ func (c *Manager) ToggleControllers(controllers []string, t ControllerToggle) er // controller is already written. // So we only return the last error. lastErr = errors.Wrapf(err, "failed to write subtree controllers %+v to %q", controllers, filePath) + } else { + lastErr = nil } } return lastErr From d55de5d2af9a3616eb758e393a2170a1421155c1 Mon Sep 17 00:00:00 2001 From: linrunlong Date: Thu, 9 Sep 2021 20:43:14 +0800 Subject: [PATCH 11/19] cgroup.go: avoid panic on nil interface Signed-off-by: linrl3 --- cgroup.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cgroup.go b/cgroup.go index 35499a78..3b77815d 100644 --- a/cgroup.go +++ b/cgroup.go @@ -327,6 +327,9 @@ func (c *cgroup) processes(subsystem Name, recursive bool, pType procType) ([]Pr if err != nil { return nil, err } + if s == nil { + return nil, fmt.Errorf("cgroups: %s doesn't exist in %s subsystem", sp, subsystem) + } path := s.(pather).Path(sp) var processes []Process err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error { From 0072297e4547d623094fc68b0df41eef364d1747 Mon Sep 17 00:00:00 2001 From: zounengren Date: Sat, 25 Sep 2021 23:24:30 +0800 Subject: [PATCH 12/19] replace pkg/errors from vendor Signed-off-by: Zou Nengren --- .github/workflows/ci.yml | 8 ++++---- cgroup.go | 7 ++++--- go.mod | 5 ++--- go.sum | 6 ++---- opts.go | 2 +- paths.go | 5 ++--- v2/devicefilter.go | 11 ++++++----- v2/ebpf.go | 7 ++++--- v2/manager.go | 24 +++++++++++++----------- v2/utils.go | 4 ++-- 10 files changed, 40 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05b5191b..7382d096 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,9 +23,9 @@ jobs: # Install Go # - name: Install Go - uses: actions/setup-go@v1 + uses: actions/setup-go@v2 with: - go-version: '1.15' + go-version: '1.16.x' - name: Set env shell: bash @@ -88,9 +88,9 @@ jobs: steps: - name: Install Go - uses: actions/setup-go@v1 + uses: actions/setup-go@v2 with: - go-version: '1.15' + go-version: '1.16.x' - name: Set env shell: bash diff --git a/cgroup.go b/cgroup.go index 3b77815d..5b70f40e 100644 --- a/cgroup.go +++ b/cgroup.go @@ -17,6 +17,7 @@ package cgroups import ( + "errors" "fmt" "os" "path/filepath" @@ -25,8 +26,8 @@ import ( "sync" v1 "github.com/containerd/cgroups/stats/v1" - specs "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pkg/errors" + + "github.com/opencontainers/runtime-spec/specs-go" ) // New returns a new control via the cgroup cgroups interface @@ -83,7 +84,7 @@ func Load(hierarchy Hierarchy, path Path, opts ...InitOpts) (Cgroup, error) { for _, s := range pathers(subsystems) { p, err := path(s.Name()) if err != nil { - if os.IsNotExist(errors.Cause(err)) { + if errors.Is(err, os.ErrNotExist) { return nil, ErrCgroupDeleted } if err == ErrControllerNotActive { diff --git a/go.mod b/go.mod index 5bd39c72..b3df917d 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/containerd/cgroups -go 1.13 +go 1.16 require ( github.com/cilium/ebpf v0.4.0 @@ -10,8 +10,7 @@ require ( github.com/godbus/dbus/v5 v5.0.4 github.com/gogo/protobuf v1.3.2 github.com/opencontainers/runtime-spec v1.0.2 - github.com/pkg/errors v0.9.1 - github.com/sirupsen/logrus v1.7.0 + github.com/sirupsen/logrus v1.8.1 github.com/stretchr/testify v1.6.1 github.com/urfave/cli v1.22.2 golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c diff --git a/go.sum b/go.sum index 2e6161e6..20ed896a 100644 --- a/go.sum +++ b/go.sum @@ -28,16 +28,14 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/opencontainers/runtime-spec v1.0.2 h1:UfAcuLBJB9Coz72x1hgl8O5RVzTdNiaglX6v2DM6FI0= github.com/opencontainers/runtime-spec v1.0.2/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= -github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= -github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM= -github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= +github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE= +github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= diff --git a/opts.go b/opts.go index a1449e29..1e428d04 100644 --- a/opts.go +++ b/opts.go @@ -17,7 +17,7 @@ package cgroups import ( - "github.com/pkg/errors" + "errors" ) var ( diff --git a/paths.go b/paths.go index ff50c95d..bddc4e9c 100644 --- a/paths.go +++ b/paths.go @@ -17,10 +17,9 @@ package cgroups import ( + "errors" "fmt" "path/filepath" - - "github.com/pkg/errors" ) type Path func(subsystem Name) (string, error) @@ -52,7 +51,7 @@ func PidPath(pid int) Path { p := fmt.Sprintf("/proc/%d/cgroup", pid) paths, err := ParseCgroupFile(p) if err != nil { - return errorPath(errors.Wrapf(err, "parse cgroup file %s", p)) + return errorPath(fmt.Errorf("parse cgroup file %s: %w", p, err)) } return existingPath(paths, "") } diff --git a/v2/devicefilter.go b/v2/devicefilter.go index 4b8c32be..0882036c 100644 --- a/v2/devicefilter.go +++ b/v2/devicefilter.go @@ -23,15 +23,16 @@ // // This particular Go implementation based on runc version // https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go + package v2 import ( + "errors" "fmt" "math" "github.com/cilium/ebpf/asm" "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pkg/errors" "golang.org/x/sys/unix" ) @@ -106,13 +107,13 @@ func (p *program) appendDevice(dev specs.LinuxDeviceCgroup) error { hasType = false default: // if not specified in OCI json, typ is set to DeviceTypeAll - return errors.Errorf("invalid DeviceType %q", dev.Type) + return fmt.Errorf("invalid DeviceType %q", dev.Type) } if *dev.Major > math.MaxUint32 { - return errors.Errorf("invalid major %d", *dev.Major) + return fmt.Errorf("invalid major %d", *dev.Major) } if *dev.Minor > math.MaxUint32 { - return errors.Errorf("invalid minor %d", *dev.Major) + return fmt.Errorf("invalid minor %d", *dev.Major) } hasMajor := *dev.Major >= 0 // if not specified in OCI json, major is set to -1 hasMinor := *dev.Minor >= 0 @@ -126,7 +127,7 @@ func (p *program) appendDevice(dev specs.LinuxDeviceCgroup) error { case 'm': bpfAccess |= unix.BPF_DEVCG_ACC_MKNOD default: - return errors.Errorf("unknown device access %v", r) + return fmt.Errorf("unknown device access %v", r) } } // If the access is rwm, skip the check. diff --git a/v2/ebpf.go b/v2/ebpf.go index bd384818..45bf5f99 100644 --- a/v2/ebpf.go +++ b/v2/ebpf.go @@ -17,11 +17,12 @@ package v2 import ( + "fmt" + "github.com/cilium/ebpf" "github.com/cilium/ebpf/asm" "github.com/cilium/ebpf/link" "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pkg/errors" "golang.org/x/sys/unix" ) @@ -50,7 +51,7 @@ func LoadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFD Flags: unix.BPF_F_ALLOW_MULTI, }) if err != nil { - return nilCloser, errors.Wrap(err, "failed to call BPF_PROG_ATTACH (BPF_CGROUP_DEVICE, BPF_F_ALLOW_MULTI)") + return nilCloser, fmt.Errorf("failed to call BPF_PROG_ATTACH (BPF_CGROUP_DEVICE, BPF_F_ALLOW_MULTI): %w", err) } closer := func() error { err = link.RawDetachProgram(link.RawDetachProgramOptions{ @@ -59,7 +60,7 @@ func LoadAttachCgroupDeviceFilter(insts asm.Instructions, license string, dirFD Attach: ebpf.AttachCGroupDevice, }) if err != nil { - return errors.Wrap(err, "failed to call BPF_PROG_DETACH (BPF_CGROUP_DEVICE)") + return fmt.Errorf("failed to call BPF_PROG_DETACH (BPF_CGROUP_DEVICE): %w", err) } return nil } diff --git a/v2/manager.go b/v2/manager.go index c5196bbd..203b0272 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -19,6 +19,8 @@ package v2 import ( "bufio" "context" + "errors" + "fmt" "io/ioutil" "math" "os" @@ -29,10 +31,10 @@ import ( "time" "github.com/containerd/cgroups/v2/stats" + systemdDbus "github.com/coreos/go-systemd/v22/dbus" "github.com/godbus/dbus/v5" "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -271,7 +273,7 @@ func (c *Manager) ToggleControllers(controllers []string, t ControllerToggle) er // When running as rootless, the user may face EPERM on parent groups, but it is neglible when the // controller is already written. // So we only return the last error. - lastErr = errors.Wrapf(err, "failed to write subtree controllers %+v to %q", controllers, filePath) + lastErr = fmt.Errorf("failed to write subtree controllers %+v to %q: %w", controllers, filePath, err) } else { lastErr = nil } @@ -526,7 +528,7 @@ func readKVStatsFile(path string, file string, out map[string]interface{}) error for s.Scan() { name, value, err := parseKV(s.Text()) if err != nil { - return errors.Wrapf(err, "error while parsing %s (line=%q)", filepath.Join(path, file), s.Text()) + return fmt.Errorf("error while parsing %s (line=%q): %w", filepath.Join(path, file), s.Text(), err) } out[name] = value } @@ -563,12 +565,12 @@ func (c *Manager) MemoryEventFD() (int, uint32, error) { fpath := filepath.Join(c.path, "memory.events") fd, err := syscall.InotifyInit() if err != nil { - return 0, 0, errors.Errorf("Failed to create inotify fd") + return 0, 0, errors.New("failed to create inotify fd") } wd, err := syscall.InotifyAddWatch(fd, fpath, unix.IN_MODIFY) if wd < 0 { syscall.Close(fd) - return 0, 0, errors.Errorf("Failed to add inotify watch for %q", fpath) + return 0, 0, fmt.Errorf("failed to add inotify watch for %q", fpath) } return fd, uint32(wd), nil @@ -607,35 +609,35 @@ func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) { if v, ok := out["high"]; ok { e.High, ok = v.(uint64) if !ok { - errCh <- errors.Errorf("cannot convert high to uint64: %+v", v) + errCh <- fmt.Errorf("cannot convert high to uint64: %+v", v) return } } if v, ok := out["low"]; ok { e.Low, ok = v.(uint64) if !ok { - errCh <- errors.Errorf("cannot convert low to uint64: %+v", v) + errCh <- fmt.Errorf("cannot convert low to uint64: %+v", v) return } } if v, ok := out["max"]; ok { e.Max, ok = v.(uint64) if !ok { - errCh <- errors.Errorf("cannot convert max to uint64: %+v", v) + errCh <- fmt.Errorf("cannot convert max to uint64: %+v", v) return } } if v, ok := out["oom"]; ok { e.OOM, ok = v.(uint64) if !ok { - errCh <- errors.Errorf("cannot convert oom to uint64: %+v", v) + errCh <- fmt.Errorf("cannot convert oom to uint64: %+v", v) return } } if v, ok := out["oom_kill"]; ok { e.OOMKill, ok = v.(uint64) if !ok { - errCh <- errors.Errorf("cannot convert oom_kill to uint64: %+v", v) + errCh <- fmt.Errorf("cannot convert oom_kill to uint64: %+v", v) return } } @@ -658,7 +660,7 @@ func setDevices(path string, devices []specs.LinuxDeviceCgroup) error { } dirFD, err := unix.Open(path, unix.O_DIRECTORY|unix.O_RDONLY, 0600) if err != nil { - return errors.Errorf("cannot get dir FD for %s", path) + return fmt.Errorf("cannot get dir FD for %s", path) } defer unix.Close(dirFD) if _, err := LoadAttachCgroupDeviceFilter(insts, license, dirFD); err != nil { diff --git a/v2/utils.go b/v2/utils.go index 8b8d654d..902466f5 100644 --- a/v2/utils.go +++ b/v2/utils.go @@ -29,9 +29,9 @@ import ( "time" "github.com/containerd/cgroups/v2/stats" + "github.com/godbus/dbus/v5" "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -61,7 +61,7 @@ func remove(path string) error { return nil } } - return errors.Wrapf(err, "cgroups: unable to remove path %q", path) + return fmt.Errorf("cgroups: unable to remove path %q: %w", path, err) } // parseCgroupProcsFile parses /sys/fs/cgroup/$GROUPPATH/cgroup.procs From 80a7821536fc82757f390f8d65693074bd3ef8e4 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Sun, 18 Jul 2021 02:05:31 +0200 Subject: [PATCH 13/19] cgroup: Optionally add process and task to a subsystems subset In some cases, one may want to add processes and task only to certain subsystems in a cgroup. For example, when a cgroup is created to only limit a given subsystem usage, but leaves all other subsystems unconstrained, there may be a need to move a process to only the contrained cgroup subsystem while leaving it in another cgroup for other subsystems. This commit makes the Control interface process and task addition functions variadic, in order to pass an optional list of subsystems that behave as s subsystems filter. Signed-off-by: Samuel Ortiz --- cgroup.go | 50 +++++++++++++++++++------ cgroup_test.go | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++ control.go | 22 ++++++++--- 3 files changed, 154 insertions(+), 17 deletions(-) diff --git a/cgroup.go b/cgroup.go index 5b70f40e..0fab1cec 100644 --- a/cgroup.go +++ b/cgroup.go @@ -150,22 +150,50 @@ func (c *cgroup) Subsystems() []Subsystem { return c.subsystems } -// Add moves the provided process into the new cgroup -func (c *cgroup) Add(process Process) error { - return c.add(process, cgroupProcs) +func (c *cgroup) subsystemsFilter(subsystems ...Name) []Subsystem { + if len(subsystems) == 0 { + return c.subsystems + } + + var filteredSubsystems = []Subsystem{} + for _, s := range c.subsystems { + for _, f := range subsystems { + if s.Name() == f { + filteredSubsystems = append(filteredSubsystems, s) + break + } + } + } + + return filteredSubsystems } -// AddProc moves the provided process id into the new cgroup -func (c *cgroup) AddProc(pid uint64) error { - return c.add(Process{Pid: int(pid)}, cgroupProcs) +// Add moves the provided process into the new cgroup. +// Without additional arguments, the process is added to all the cgroup subsystems. +// When giving Add a list of subsystem names, the process is only added to those +// subsystems, provided that they are active in the targeted cgroup. +func (c *cgroup) Add(process Process, subsystems ...Name) error { + return c.add(process, cgroupProcs, subsystems...) } -// AddTask moves the provided tasks (threads) into the new cgroup -func (c *cgroup) AddTask(process Process) error { - return c.add(process, cgroupTasks) +// AddProc moves the provided process id into the new cgroup. +// Without additional arguments, the process with the given id is added to all +// the cgroup subsystems. When giving AddProc a list of subsystem names, the process +// id is only added to those subsystems, provided that they are active in the targeted +// cgroup. +func (c *cgroup) AddProc(pid uint64, subsystems ...Name) error { + return c.add(Process{Pid: int(pid)}, cgroupProcs, subsystems...) } -func (c *cgroup) add(process Process, pType procType) error { +// AddTask moves the provided tasks (threads) into the new cgroup. +// Without additional arguments, the task is added to all the cgroup subsystems. +// When giving AddTask a list of subsystem names, the task is only added to those +// subsystems, provided that they are active in the targeted cgroup. +func (c *cgroup) AddTask(process Process, subsystems ...Name) error { + return c.add(process, cgroupTasks, subsystems...) +} + +func (c *cgroup) add(process Process, pType procType, subsystems ...Name) error { if process.Pid <= 0 { return ErrInvalidPid } @@ -174,7 +202,7 @@ func (c *cgroup) add(process Process, pType procType) error { if c.err != nil { return c.err } - for _, s := range pathers(c.subsystems) { + for _, s := range pathers(c.subsystemsFilter(subsystems...)) { p, err := c.path(s.Name()) if err != nil { return err diff --git a/cgroup_test.go b/cgroup_test.go index b8971055..1b170fdb 100644 --- a/cgroup_test.go +++ b/cgroup_test.go @@ -101,6 +101,63 @@ func TestAdd(t *testing.T) { } } +func TestAddFilteredSubsystems(t *testing.T) { + mock, err := newMock() + if err != nil { + t.Fatal(err) + } + defer mock.delete() + control, err := New(mock.hierarchy, StaticPath("test"), &specs.LinuxResources{}) + if err != nil { + t.Error(err) + return + } + + filteredSubsystems := []Name{"memory", "cpu"} + if err := control.Add(Process{Pid: 1234}, filteredSubsystems...); err != nil { + t.Error(err) + return + } + + for _, s := range filteredSubsystems { + if err := checkPid(mock, filepath.Join(string(s), "test"), 1234); err != nil { + t.Error(err) + return + } + } + + if err := checkPid(mock, filepath.Join("devices", "test"), 1234); err == nil { + t.Error("Pid should not be added to the devices subsystem") + return + } + + bogusSubsystems := append(filteredSubsystems, "bogus") + if err := control.Add(Process{Pid: 5678}, bogusSubsystems...); err != nil { + t.Error(err) + return + } + + for _, s := range filteredSubsystems { + if err := checkPid(mock, filepath.Join(string(s), "test"), 5678); err != nil { + t.Error(err) + return + } + } + + nilSubsystems := []Name{} + if err := control.Add(Process{Pid: 9012}, nilSubsystems...); err != nil { + t.Error(err) + return + } + + for _, s := range Subsystems() { + if err := checkPid(mock, filepath.Join(string(s), "test"), 9012); err != nil { + t.Error(err) + return + } + } +} + func TestAddTask(t *testing.T) { mock, err := newMock() if err != nil { @@ -124,6 +181,48 @@ func TestAddTask(t *testing.T) { } } +func TestAddTaskFilteredSubsystems(t *testing.T) { + mock, err := newMock() + if err != nil { + t.Fatal(err) + } + defer mock.delete() + control, err := New(mock.hierarchy, StaticPath("test"), &specs.LinuxResources{}) + if err != nil { + t.Error(err) + return + } + filteredSubsystems := []Name{"memory", "cpu"} + if err := control.AddTask(Process{Pid: 1234}, filteredSubsystems...); err != nil { + t.Error(err) + return + } + for _, s := range filteredSubsystems { + if err := checkTaskid(mock, filepath.Join(string(s), "test"), 1234); err != nil { + t.Error(err) + return + } + } + + if err := checkTaskid(mock, filepath.Join("devices", "test"), 1234); err == nil { + t.Error("Task should not be added to the devices subsystem") + return + } + + bogusSubsystems := append(filteredSubsystems, "bogus") + if err := control.AddTask(Process{Pid: 5678}, bogusSubsystems...); err != nil { + t.Error(err) + return + } + + for _, s := range filteredSubsystems { + if err := checkTaskid(mock, filepath.Join(string(s), "test"), 5678); err != nil { + t.Error(err) + return + } + } +} + func TestListPids(t *testing.T) { mock, err := newMock() if err != nil { diff --git a/control.go b/control.go index 8cd14e63..5fcaac6d 100644 --- a/control.go +++ b/control.go @@ -54,12 +54,22 @@ type Task = Process type Cgroup interface { // New creates a new cgroup under the calling cgroup New(string, *specs.LinuxResources) (Cgroup, error) - // Add adds a process to the cgroup (cgroup.procs) - Add(Process) error - // AddProc adds the process with the given id to the cgroup (cgroup.procs) - AddProc(pid uint64) error - // AddTask adds a process to the cgroup (tasks) - AddTask(Process) error + // Add adds a process to the cgroup (cgroup.procs). Without additional arguments, + // the process is added to all the cgroup subsystems. When giving Add a list of + // subsystem names, the process is only added to those subsystems, provided that + // they are active in the targeted cgroup. + Add(Process, ...Name) error + // AddProc adds the process with the given id to the cgroup (cgroup.procs). + // Without additional arguments, the process with the given id is added to all + // the cgroup subsystems. When giving AddProc a list of subsystem names, the process + // id is only added to those subsystems, provided that they are active in the targeted + // cgroup. + AddProc(uint64, ...Name) error + // AddTask adds a process to the cgroup (tasks). Without additional arguments, the + // task is added to all the cgroup subsystems. When giving AddTask a list of subsystem + // names, the task is only added to those subsystems, provided that they are active in + // the targeted cgroup. + AddTask(Process, ...Name) error // Delete removes the cgroup as a whole Delete() error // MoveTo moves all the processes under the calling cgroup to the provided one From 17fece81870ef8aa1a31f05210b8f425e37038a0 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 19 Nov 2021 23:13:25 +0000 Subject: [PATCH 14/19] Fix potential dirfd leak. Make sure we don't accidentally leak this dir fd by using O_CLOEXEC. Signed-off-by: Brian Goff --- v2/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v2/manager.go b/v2/manager.go index 203b0272..b56989bb 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -658,7 +658,7 @@ func setDevices(path string, devices []specs.LinuxDeviceCgroup) error { if err != nil { return err } - dirFD, err := unix.Open(path, unix.O_DIRECTORY|unix.O_RDONLY, 0600) + dirFD, err := unix.Open(path, unix.O_DIRECTORY|unix.O_RDONLY|unix.O_CLOEXEC, 0600) if err != nil { return fmt.Errorf("cannot get dir FD for %s", path) } From 182c3afa53b8cccce0611cca9dee46410c4f82f7 Mon Sep 17 00:00:00 2001 From: ningmingxiao Date: Mon, 6 Dec 2021 22:08:20 +0800 Subject: [PATCH 15/19] fix Implicit memory aliasing in for loop Signed-off-by: ningmingxiao --- rdma.go | 1 + 1 file changed, 1 insertion(+) diff --git a/rdma.go b/rdma.go index b6f0d416..3b59b107 100644 --- a/rdma.go +++ b/rdma.go @@ -67,6 +67,7 @@ func (p *rdmaController) Create(path string, resources *specs.LinuxResources) er for device, limit := range resources.Rdma { if device != "" && (limit.HcaHandles != nil || limit.HcaObjects != nil) { + limit := limit return retryingWriteFile( filepath.Join(p.Path(path), "rdma.max"), []byte(createCmdString(device, &limit)), From 35b5b55c686080de64facf127d6d6a5ca9a0fe6b Mon Sep 17 00:00:00 2001 From: Jeremi Piotrowski Date: Fri, 3 Dec 2021 13:51:17 +0000 Subject: [PATCH 16/19] v2: Fix inotify leak when cgroup is deleted When running on cgroup2, we currently leak the inotify instance (and goroutine blocked on read) used to monitor 'memory.events' on any container exit. This is highly problematic when containers are automatically restarted because we will exhaust either the fd limit or system-wide inotify instance limit. When a process exits, there is no memory event and even when the cgroup is deleted, the inotify read is also not unblocked. This is not the case when containerd is running on cgroup (v1) because that uses a different mechanism for notification and detects cgroup deletion. Fulfill the contract on cgroup2 by additionally monitoring cgroup.events for process exit. When the last process exits the kernel signals an event on 'cgroup.events'. For robustness we check both 'cgroup.events' and 'memory.events' on any notification and also handle ENOENT/ENODEV errors from read/open of 'memory.events'. We signal exit up the stack by closing the error channel. Strangely, the error channel was not previously being returned to the caller. Signed-off-by: Jeremi Piotrowski --- v2/manager.go | 45 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/v2/manager.go b/v2/manager.go index b56989bb..c8cf0e93 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -560,6 +560,22 @@ func (c *Manager) freeze(path string, state State) error { } } +func (c *Manager) isCgroupEmpty() bool { + // In case of any error we return true so that we exit and don't leak resources + out := make(map[string]interface{}) + if err := readKVStatsFile(c.path, "cgroup.events", out); err != nil { + return true + } + if v, ok := out["populated"]; ok { + populated, ok := v.(uint64) + if !ok { + return true + } + return populated == 0 + } + return true +} + // MemoryEventFD returns inotify file descriptor and 'memory.events' inotify watch descriptor func (c *Manager) MemoryEventFD() (int, uint32, error) { fpath := filepath.Join(c.path, "memory.events") @@ -568,9 +584,15 @@ func (c *Manager) MemoryEventFD() (int, uint32, error) { return 0, 0, errors.New("failed to create inotify fd") } wd, err := syscall.InotifyAddWatch(fd, fpath, unix.IN_MODIFY) - if wd < 0 { + if err != nil { syscall.Close(fd) - return 0, 0, fmt.Errorf("failed to add inotify watch for %q", fpath) + return 0, 0, fmt.Errorf("failed to add inotify watch for %q: %w", fpath, err) + } + // monitor to detect process exit/cgroup deletion + evpath := filepath.Join(c.path, "cgroup.events") + if _, err = syscall.InotifyAddWatch(fd, evpath, unix.IN_MODIFY); err != nil { + syscall.Close(fd) + return 0, 0, fmt.Errorf("failed to add inotify watch for %q: %w", evpath, err) } return fd, uint32(wd), nil @@ -578,22 +600,21 @@ func (c *Manager) MemoryEventFD() (int, uint32, error) { func (c *Manager) EventChan() (<-chan Event, <-chan error) { ec := make(chan Event) - errCh := make(chan error) + errCh := make(chan error, 1) go c.waitForEvents(ec, errCh) - return ec, nil + return ec, errCh } func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) { - fd, wd, err := c.MemoryEventFD() - - defer syscall.InotifyRmWatch(fd, wd) - defer syscall.Close(fd) + defer close(errCh) + fd, _, err := c.MemoryEventFD() if err != nil { errCh <- err return } + defer syscall.Close(fd) for { buffer := make([]byte, syscall.SizeofInotifyEvent*10) @@ -643,7 +664,13 @@ func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) { } ec <- e } else { - errCh <- err + // When cgroup is deleted read may return -ENODEV instead of -ENOENT from open. + if _, statErr := os.Lstat(filepath.Join(c.path, "memory.events")); !os.IsNotExist(statErr) { + errCh <- err + } + return + } + if c.isCgroupEmpty() { return } } From 6a46df25065d551a8d998495c34263354c84f2c1 Mon Sep 17 00:00:00 2001 From: Jeremi Piotrowski Date: Mon, 31 Jan 2022 14:25:00 +0100 Subject: [PATCH 17/19] v2: manager: factor out memory.events parsing This makes waitForEvents() more shorter and more readable. Signed-off-by: Jeremi Piotrowski --- v2/manager.go | 74 +++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/v2/manager.go b/v2/manager.go index c8cf0e93..28655f8e 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -606,6 +606,41 @@ func (c *Manager) EventChan() (<-chan Event, <-chan error) { return ec, errCh } +func parseMemoryEvents(out map[string]interface{}) (Event, error) { + e := Event{} + if v, ok := out["high"]; ok { + e.High, ok = v.(uint64) + if !ok { + return Event{}, fmt.Errorf("cannot convert high to uint64: %+v", v) + } + } + if v, ok := out["low"]; ok { + e.Low, ok = v.(uint64) + if !ok { + return Event{}, fmt.Errorf("cannot convert low to uint64: %+v", v) + } + } + if v, ok := out["max"]; ok { + e.Max, ok = v.(uint64) + if !ok { + return Event{}, fmt.Errorf("cannot convert max to uint64: %+v", v) + } + } + if v, ok := out["oom"]; ok { + e.OOM, ok = v.(uint64) + if !ok { + return Event{}, fmt.Errorf("cannot convert oom to uint64: %+v", v) + } + } + if v, ok := out["oom_kill"]; ok { + e.OOMKill, ok = v.(uint64) + if !ok { + return Event{}, fmt.Errorf("cannot convert oom_kill to uint64: %+v", v) + } + } + return e, nil +} + func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) { defer close(errCh) @@ -626,41 +661,10 @@ func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) { if bytesRead >= syscall.SizeofInotifyEvent { out := make(map[string]interface{}) if err := readKVStatsFile(c.path, "memory.events", out); err == nil { - e := Event{} - if v, ok := out["high"]; ok { - e.High, ok = v.(uint64) - if !ok { - errCh <- fmt.Errorf("cannot convert high to uint64: %+v", v) - return - } - } - if v, ok := out["low"]; ok { - e.Low, ok = v.(uint64) - if !ok { - errCh <- fmt.Errorf("cannot convert low to uint64: %+v", v) - return - } - } - if v, ok := out["max"]; ok { - e.Max, ok = v.(uint64) - if !ok { - errCh <- fmt.Errorf("cannot convert max to uint64: %+v", v) - return - } - } - if v, ok := out["oom"]; ok { - e.OOM, ok = v.(uint64) - if !ok { - errCh <- fmt.Errorf("cannot convert oom to uint64: %+v", v) - return - } - } - if v, ok := out["oom_kill"]; ok { - e.OOMKill, ok = v.(uint64) - if !ok { - errCh <- fmt.Errorf("cannot convert oom_kill to uint64: %+v", v) - return - } + e, err := parseMemoryEvents(out) + if err != nil { + errCh <- err + return } ec <- e } else { From cf1f978b93bf784118d3ab7dec6a47b8204918c2 Mon Sep 17 00:00:00 2001 From: Jeremi Piotrowski Date: Thu, 27 Jan 2022 14:04:06 +0000 Subject: [PATCH 18/19] v2: flip error handling for readKVStat("memory.events") to reduce indentation Signed-off-by: Jeremi Piotrowski --- v2/manager.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/v2/manager.go b/v2/manager.go index 28655f8e..afed14c6 100644 --- a/v2/manager.go +++ b/v2/manager.go @@ -660,20 +660,19 @@ func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) { } if bytesRead >= syscall.SizeofInotifyEvent { out := make(map[string]interface{}) - if err := readKVStatsFile(c.path, "memory.events", out); err == nil { - e, err := parseMemoryEvents(out) - if err != nil { - errCh <- err - return - } - ec <- e - } else { + if err := readKVStatsFile(c.path, "memory.events", out); err != nil { // When cgroup is deleted read may return -ENODEV instead of -ENOENT from open. if _, statErr := os.Lstat(filepath.Join(c.path, "memory.events")); !os.IsNotExist(statErr) { errCh <- err } return } + e, err := parseMemoryEvents(out) + if err != nil { + errCh <- err + return + } + ec <- e if c.isCgroupEmpty() { return } From a7d6888aa30218c8aff15d979eb3f6aec0b7979c Mon Sep 17 00:00:00 2001 From: Jeremi Piotrowski Date: Thu, 27 Jan 2022 11:04:36 +0000 Subject: [PATCH 19/19] v2: add test case for Manager.EventChan() behavior This test case demonstrates that now the oom monitor goroutine terminates when a cgroup is removed. Systemd is responsible for cgroup removal so a "cat" process is spawned in a scope, with stdin attached to a pipe. When the pipe is closed the process terminates, systemd notices and removes the cgroup, which results in the test case finishing. Signed-off-by: Jeremi Piotrowski --- go.mod | 5 +-- go.sum | 26 +++++++++++++--- v2/manager_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 v2/manager_test.go diff --git a/go.mod b/go.mod index b3df917d..80d3f6ea 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,8 @@ require ( github.com/gogo/protobuf v1.3.2 github.com/opencontainers/runtime-spec v1.0.2 github.com/sirupsen/logrus v1.8.1 - github.com/stretchr/testify v1.6.1 + github.com/stretchr/testify v1.7.0 github.com/urfave/cli v1.22.2 - golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c + go.uber.org/goleak v1.1.12 + golang.org/x/sys v0.0.0-20210510120138-977fb7262007 ) diff --git a/go.sum b/go.sum index 20ed896a..cda30b49 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,7 @@ github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -38,43 +39,60 @@ github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= -github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= -github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/urfave/cli v1.22.2 h1:gsqYFH8bb9ekPA12kRo0hfjngWQjkJPlN9R0N78BoUo= github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +go.uber.org/goleak v1.1.12 h1:gZAh5/EyT/HQwlpkCy6wTpqfH9H8Lz8zbm3dZh+OyzA= +go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c h1:VwygUrnw9jn88c4u8GD3rZQbqrP/tgas88tPUbBxQrk= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= +golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/v2/manager_test.go b/v2/manager_test.go new file mode 100644 index 00000000..35d104ca --- /dev/null +++ b/v2/manager_test.go @@ -0,0 +1,76 @@ +/* + Copyright The containerd 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 + + http://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 v2 + +import ( + "fmt" + "os/exec" + "testing" + "time" + + "go.uber.org/goleak" +) + +func TestEventChanCleanupOnCgroupRemoval(t *testing.T) { + checkCgroupMode(t) + + cmd := exec.Command("cat") + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatalf("Failed to create cat process: %v", err) + } + if err := cmd.Start(); err != nil { + t.Fatalf("Failed to start cat process: %v", err) + } + proc := cmd.Process + if proc == nil { + t.Fatal("Process is nil") + } + + group := fmt.Sprintf("testing-watcher-%d.scope", proc.Pid) + c, err := NewSystemd("", group, proc.Pid, &Resources{}) + if err != nil { + t.Fatalf("Failed to init new cgroup manager: %v", err) + } + + evCh, errCh := c.EventChan() + + // give event goroutine a chance to start + time.Sleep(500 * time.Millisecond) + + if err := stdin.Close(); err != nil { + t.Fatalf("Failed closing stdin: %v", err) + } + if err := cmd.Wait(); err != nil { + t.Fatalf("Failed waiting for cmd: %v", err) + } + + done := false + for !done { + select { + case <-evCh: + case err := <-errCh: + if err != nil { + t.Fatalf("Unexpected error on error channel: %v", err) + } + done = true + case <-time.After(5 * time.Second): + t.Fatal("Timed out") + } + } + goleak.VerifyNone(t) +}