From 39bf07694c5716cce3cb7e25c6d01b441b6b4e01 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 10 Nov 2020 13:31:19 +0100 Subject: use container cgroups path When looking up a container's cgroup path, parse /proc/[PID]/cgroup. This will work across all cgroup managers and configurations and is supported on cgroups v1 and v2. Fixes: #8265 Signed-off-by: Valentin Rothberg --- libpod/container.go | 55 +++++++++++--------------------------- libpod/container_internal_linux.go | 5 +--- test/e2e/stats_test.go | 41 ++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 44 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index 580fa7b3d..333e1d848 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -1,22 +1,18 @@ package libpod import ( + "bytes" "fmt" "io/ioutil" "net" "os" - "path/filepath" - "strings" "time" "github.com/containernetworking/cni/pkg/types" cnitypes "github.com/containernetworking/cni/pkg/types/current" - "github.com/containers/common/pkg/config" "github.com/containers/image/v5/manifest" "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/libpod/lock" - "github.com/containers/podman/v2/pkg/rootless" - "github.com/containers/podman/v2/utils" "github.com/containers/storage" "github.com/cri-o/ocicni/pkg/ocicni" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -912,44 +908,23 @@ func (c *Container) CgroupManager() string { // CGroupPath returns a cgroups "path" for a given container. func (c *Container) CGroupPath() (string, error) { - cgroupManager := c.CgroupManager() - - switch { - case c.config.NoCgroups || c.config.CgroupsMode == "disabled": + if c.config.NoCgroups || c.config.CgroupsMode == "disabled" { return "", errors.Wrapf(define.ErrNoCgroups, "this container is not creating cgroups") - case c.config.CgroupsMode == cgroupSplit: - if c.config.CgroupParent != "" { - return "", errors.Errorf("cannot specify cgroup-parent with cgroup-mode %q", cgroupSplit) - } - cg, err := utils.GetCgroupProcess(c.state.ConmonPID) - if err != nil { - return "", err - } - // Use the conmon cgroup for two reasons: we validate the container - // delegation was correct, and the conmon cgroup doesn't change at runtime - // while we are not sure about the container that can create sub cgroups. - if !strings.HasSuffix(cg, "supervisor") { - return "", errors.Errorf("invalid cgroup for conmon %q", cg) - } - return strings.TrimSuffix(cg, "/supervisor") + "/container", nil - case cgroupManager == config.CgroupfsCgroupsManager: - return filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID())), nil - case cgroupManager == config.SystemdCgroupsManager: - if rootless.IsRootless() { - uid := rootless.GetRootlessUID() - parts := strings.SplitN(c.config.CgroupParent, "/", 2) - - dir := "" - if len(parts) > 1 { - dir = parts[1] - } + } - return filepath.Join(parts[0], fmt.Sprintf("user-%d.slice/user@%d.service/user.slice/%s", uid, uid, dir), createUnitName("libpod", c.ID())), nil - } - return filepath.Join(c.config.CgroupParent, createUnitName("libpod", c.ID())), nil - default: - return "", errors.Wrapf(define.ErrInvalidArg, "unsupported CGroup manager %s in use", cgroupManager) + // Read /proc/[PID]/cgroup and look at the first line. cgroups(7) + // nails it down to three fields with the 3rd pointing to the cgroup's + // path which works both on v1 and v2. + procPath := fmt.Sprintf("/proc/%d/cgroup", c.state.PID) + lines, err := ioutil.ReadFile(procPath) + if err != nil { + return "", err + } + fields := bytes.Split(bytes.Split(lines, []byte("\n"))[0], []byte(":")) + if len(fields) != 3 { + return "", errors.Errorf("expected 3 fields but got %d: %s", len(fields), procPath) } + return string(fields[2]), nil } // RootFsSize returns the root FS size of the container diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 83d5c20cb..7754a65c5 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -2091,10 +2091,7 @@ func (c *Container) getOCICgroupPath() (string, error) { logrus.Debugf("Setting CGroups for container %s to %s", c.ID(), systemdCgroups) return systemdCgroups, nil case cgroupManager == config.CgroupfsCgroupsManager: - cgroupPath, err := c.CGroupPath() - if err != nil { - return "", err - } + cgroupPath := filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID())) logrus.Debugf("Setting CGroup path for container %s to %s", c.ID(), cgroupPath) return cgroupPath, nil default: diff --git a/test/e2e/stats_test.go b/test/e2e/stats_test.go index c8f5efa9d..5e8a7a3d0 100644 --- a/test/e2e/stats_test.go +++ b/test/e2e/stats_test.go @@ -5,6 +5,7 @@ package integration import ( "fmt" "os" + "strconv" "time" . "github.com/containers/podman/v2/test/utils" @@ -126,4 +127,44 @@ var _ = Describe("Podman stats", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) }) + + // Regression test for #8265 + It("podman stats with custom memory limits", func() { + // Run thre containers. One with a memory limit. Make sure + // that the limits are different and the limited one has a + // lower limit. + ctrNoLimit0 := "no-limit-0" + ctrNoLimit1 := "no-limit-1" + ctrWithLimit := "with-limit" + + session := podmanTest.Podman([]string{"run", "-d", "--name", ctrNoLimit0, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"run", "-d", "--name", ctrNoLimit1, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"run", "-d", "--name", ctrWithLimit, "--memory", "50m", ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"stats", "--no-stream", "--format", "{{.MemLimit}}", ctrNoLimit0, ctrNoLimit1, ctrWithLimit}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // We have three containers. The unlimited ones need to have + // the same limit, the limited one a lower one. + limits := session.OutputToStringArray() + Expect(len(limits)).To(BeNumerically("==", 3)) + Expect(limits[0]).To(Equal(limits[1])) + Expect(limits[0]).ToNot(Equal(limits[2])) + + defaultLimit, err := strconv.Atoi(limits[0]) + Expect(err).To(BeNil()) + customLimit, err := strconv.Atoi(limits[2]) + Expect(err).To(BeNil()) + + Expect(customLimit).To(BeNumerically("<", defaultLimit)) + }) }) -- cgit v1.2.3-54-g00ecf