diff options
-rw-r--r-- | pkg/cgroups/blkio.go | 2 | ||||
-rw-r--r-- | pkg/cgroups/cgroups.go | 41 | ||||
-rw-r--r-- | pkg/cgroups/cpu.go | 2 | ||||
-rw-r--r-- | pkg/cgroups/cpuset.go | 3 | ||||
-rw-r--r-- | pkg/cgroups/memory.go | 3 | ||||
-rw-r--r-- | pkg/cgroups/pids.go | 3 | ||||
-rw-r--r-- | test/e2e/pod_rm_test.go | 17 |
7 files changed, 62 insertions, 9 deletions
diff --git a/pkg/cgroups/blkio.go b/pkg/cgroups/blkio.go index 9c2a811d9..bacd4eb93 100644 --- a/pkg/cgroups/blkio.go +++ b/pkg/cgroups/blkio.go @@ -37,7 +37,7 @@ func (c *blkioHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *blkioHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Blkio)) + return rmDirRecursively(ctr.getCgroupv1Path(Blkio)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/cgroups.go b/pkg/cgroups/cgroups.go index 1dad45d7f..081db772f 100644 --- a/pkg/cgroups/cgroups.go +++ b/pkg/cgroups/cgroups.go @@ -328,6 +328,13 @@ func Load(path string) (*CgroupControl, error) { systemd: false, } if !cgroup2 { + controllers, err := getAvailableControllers(handlers, false) + if err != nil { + return nil, err + } + control.additionalControllers = controllers + } + if !cgroup2 { for name := range handlers { p := control.getCgroupv1Path(name) if _, err := os.Stat(p); err != nil { @@ -355,11 +362,40 @@ func (c *CgroupControl) Delete() error { return c.DeleteByPath(c.path) } +// rmDirRecursively delete recursively a cgroup directory. +// It differs from os.RemoveAll as it doesn't attempt to unlink files. +// On cgroupfs we are allowed only to rmdir empty directories. +func rmDirRecursively(path string) error { + if err := os.Remove(path); err == nil || os.IsNotExist(err) { + return nil + } + entries, err := ioutil.ReadDir(path) + if err != nil { + return errors.Wrapf(err, "read %s", path) + } + for _, i := range entries { + if i.IsDir() { + if err := rmDirRecursively(filepath.Join(path, i.Name())); err != nil { + return err + } + } + } + if os.Remove(path); err != nil { + if !os.IsNotExist(err) { + return errors.Wrapf(err, "remove %s", path) + } + } + return nil +} + // DeleteByPath deletes the specified cgroup path func (c *CgroupControl) DeleteByPath(path string) error { if c.systemd { return systemdDestroy(path) } + if c.cgroup2 { + return rmDirRecursively(filepath.Join(cgroupRoot, c.path)) + } var lastError error for _, h := range handlers { if err := h.Destroy(c); err != nil { @@ -368,8 +404,11 @@ func (c *CgroupControl) DeleteByPath(path string) error { } for _, ctr := range c.additionalControllers { + if ctr.symlink { + continue + } p := c.getCgroupv1Path(ctr.name) - if err := os.Remove(p); err != nil { + if err := rmDirRecursively(p); err != nil { lastError = errors.Wrapf(err, "remove %s", p) } } diff --git a/pkg/cgroups/cpu.go b/pkg/cgroups/cpu.go index 1c8610cc4..03677f1ef 100644 --- a/pkg/cgroups/cpu.go +++ b/pkg/cgroups/cpu.go @@ -68,7 +68,7 @@ func (c *cpuHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *cpuHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(CPU)) + return rmDirRecursively(ctr.getCgroupv1Path(CPU)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/cpuset.go b/pkg/cgroups/cpuset.go index 25d2f7f76..46d0484f2 100644 --- a/pkg/cgroups/cpuset.go +++ b/pkg/cgroups/cpuset.go @@ -3,7 +3,6 @@ package cgroups import ( "fmt" "io/ioutil" - "os" "path/filepath" "strings" @@ -77,7 +76,7 @@ func (c *cpusetHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *cpusetHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(CPUset)) + return rmDirRecursively(ctr.getCgroupv1Path(CPUset)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/memory.go b/pkg/cgroups/memory.go index 80e88d17c..b3991f7e3 100644 --- a/pkg/cgroups/memory.go +++ b/pkg/cgroups/memory.go @@ -2,7 +2,6 @@ package cgroups import ( "fmt" - "os" "path/filepath" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -33,7 +32,7 @@ func (c *memHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *memHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Memory)) + return rmDirRecursively(ctr.getCgroupv1Path(Memory)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/pkg/cgroups/pids.go b/pkg/cgroups/pids.go index ffbde100d..65b9b5b34 100644 --- a/pkg/cgroups/pids.go +++ b/pkg/cgroups/pids.go @@ -3,7 +3,6 @@ package cgroups import ( "fmt" "io/ioutil" - "os" "path/filepath" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -40,7 +39,7 @@ func (c *pidHandler) Create(ctr *CgroupControl) (bool, error) { // Destroy the cgroup func (c *pidHandler) Destroy(ctr *CgroupControl) error { - return os.Remove(ctr.getCgroupv1Path(Pids)) + return rmDirRecursively(ctr.getCgroupv1Path(Pids)) } // Stat fills a metrics structure with usage stats for the controller diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go index 0d3f47f30..f0689f152 100644 --- a/test/e2e/pod_rm_test.go +++ b/test/e2e/pod_rm_test.go @@ -3,6 +3,8 @@ package integration import ( "fmt" "os" + "path/filepath" + "strings" . "github.com/containers/libpod/test/utils" . "github.com/onsi/ginkgo" @@ -40,6 +42,21 @@ var _ = Describe("Podman pod rm", func() { result := podmanTest.Podman([]string{"pod", "rm", podid}) result.WaitWithDefaultTimeout() Expect(result.ExitCode()).To(Equal(0)) + + // Also check that we don't leak cgroups + err := filepath.Walk("/sys/fs/cgroup", func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() { + Expect(err).To(BeNil()) + } + if strings.Contains(info.Name(), podid) { + return fmt.Errorf("leaking cgroup path %s", path) + } + return nil + }) + Expect(err).To(BeNil()) }) It("podman pod rm latest pod", func() { |