From 72c1cc9a71ab169ee9946e454097650117e133c5 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Sun, 5 Jul 2020 12:37:32 +0200 Subject: [PATCH 1/2] cgroup, systemd: cleanup cgroups some hierarchies were created directly by .Apply() on top of systemd managed cgroups. systemd doesn't manage these and as a result we leak these cgroups. Cherry-picked-by: Peter Hunt Signed-off-by: Giuseppe Scrivano --- libcontainer/cgroups/systemd/v1.go | 11 +++++++++-- tests/integration/delete.bats | 20 ++++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 8bac7ca23..5e8e9dc02 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -212,11 +212,18 @@ func (m *LegacyManager) Destroy() error { } m.mu.Lock() defer m.mu.Unlock() - unitName := getUnitName(m.Cgroups) - if err := stopUnit(unitName); err != nil { + + err := stopUnit(unitName) + // Both on success and on error, cleanup all the cgroups we are aware of. + // Some of them were created directly by Apply() and are not managed by systemd. + if err2 := cgroups.RemovePaths(m.Paths); err2 != nil { + return err2 + } + if err != nil { return err } + m.Paths = make(map[string]string) return nil } diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats index c5ed215d1..af872144c 100644 --- a/tests/integration/delete.bats +++ b/tests/integration/delete.bats @@ -12,24 +12,24 @@ function teardown() { } @test "runc delete" { - # run busybox detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox + runc run -d --console-socket $CONSOLE_SOCKET testbusyboxdelete [ "$status" -eq 0 ] - # check state - testcontainer test_busybox running + testcontainer testbusyboxdelete running - runc kill test_busybox KILL + runc kill testbusyboxdelete KILL [ "$status" -eq 0 ] - # wait for busybox to be in the destroyed state - retry 10 1 eval "__runc state test_busybox | grep -q 'stopped'" + retry 10 1 eval "__runc state testbusyboxdelete | grep -q 'stopped'" - # delete test_busybox - runc delete test_busybox + runc delete testbusyboxdelete [ "$status" -eq 0 ] - runc state test_busybox + runc state testbusyboxdelete [ "$status" -ne 0 ] + + run find /sys/fs/cgroup -wholename '*testbusyboxdelete*' -type d + [ "$status" -eq 0 ] + [ "$output" = "" ] || fail "cgroup not cleaned up correctly: $output" } @test "runc delete --force" { From ec29d6b054c1c5052e33c9c4be1de1ab420a5a9c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 6 Jul 2020 15:44:17 -0700 Subject: [PATCH 2/2] libc/cgroups: empty map in RemovePaths RemovePaths() deletes elements from the paths map for paths that has been successfully removed. Although, it does not empty the map itself (which is needed that AFAIK Go garbage collector does not shrink the map), but all its callers do. Move this operation from callers to RemovePaths. No functional change, except the old map should be garbage collected now. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/apply_raw.go | 6 +----- libcontainer/cgroups/systemd/v1.go | 7 +------ libcontainer/cgroups/utils.go | 1 + 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index 59ce5ea6b..849b15ffa 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -194,11 +194,7 @@ func (m *Manager) Destroy() error { } m.mu.Lock() defer m.mu.Unlock() - if err := cgroups.RemovePaths(m.Paths); err != nil { - return err - } - m.Paths = make(map[string]string) - return nil + return cgroups.RemovePaths(m.Paths) } func (m *Manager) GetPaths() map[string]string { diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 5e8e9dc02..c7b3863e1 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -220,12 +220,7 @@ func (m *LegacyManager) Destroy() error { if err2 := cgroups.RemovePaths(m.Paths); err2 != nil { return err2 } - if err != nil { - return err - } - - m.Paths = make(map[string]string) - return nil + return err } func (m *LegacyManager) GetPaths() map[string]string { diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 8cdd6b7ed..a967f1497 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -488,6 +488,7 @@ func RemovePaths(paths map[string]string) (err error) { } } if len(paths) == 0 { + paths = make(map[string]string) return nil } }