cgroup: always honor --cgroup-parent with cgroupfs

if --cgroup-parent is specified, always honor it without doing any detection whether cgroups are supported or not. Closes: containers#10173 Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe · May 3, 2021 · 17ce567 · 17ce567
1 parent 98a84ff
commit 17ce567
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 2 deletions.
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
@@ -2224,8 +2224,19 @@ func (c *Container) getOCICgroupPath() (string, error) {
 	}
 	cgroupManager := c.CgroupManager()
 	switch {
-	case (rootless.IsRootless() && (cgroupManager == config.CgroupfsCgroupsManager || !unified)) || c.config.NoCgroups:
+	case c.config.NoCgroups:
 		return "", nil
+	case (rootless.IsRootless() && (cgroupManager == config.CgroupfsCgroupsManager || !unified)):
+		if c.config.CgroupParent == CgroupfsDefaultCgroupParent {
+			// old versions of podman were setting the CgroupParent to CgroupfsDefaultCgroupParent
+			// by default.  Avoid breaking these versions and check whether the cgroup parent is
+			// set to the default and in this case enable the old behavior.  It should not be a real
+			// problem because the default CgroupParent is usually owned by root so rootless users
+			// cannot access it.
+			// This check might be lifted in a future version of Podman.
+			return "", nil
+		}
+		return c.config.CgroupParent, nil
 	case c.config.CgroupsMode == cgroupSplit:
 		if c.config.CgroupParent != "" {
 			return c.config.CgroupParent, nil

diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
@@ -296,7 +296,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 						return nil, errors.Wrapf(define.ErrInternal, "pod %s cgroup is not set", pod.ID())
 					}
 					ctr.config.CgroupParent = podCgroup
-				} else {
+				} else if !rootless.IsRootless() {
 					ctr.config.CgroupParent = CgroupfsDefaultCgroupParent
 				}
 			} else if strings.HasSuffix(path.Base(ctr.config.CgroupParent), ".slice") {

diff --git a/test/e2e/run_cgroup_parent_test.go b/test/e2e/run_cgroup_parent_test.go
@@ -1,7 +1,10 @@
 package integration
 
 import (
+	"fmt"
 	"os"
+	"path/filepath"
+	"strings"
 
 	. "github.com/containers/podman/v3/test/utils"
 	. "github.com/onsi/ginkgo"
@@ -58,6 +61,38 @@ var _ = Describe("Podman run with --cgroup-parent", func() {
 		Expect(ok).To(BeTrue())
 	})
 
+	Specify("always honor --cgroup-parent", func() {
+		SkipIfCgroupV1("test not supported in cgroups v1")
+		if Containerized() || podmanTest.CgroupManager == "cgroupfs" {
+			Skip("Requires Systemd cgroup manager support")
+		}
+		if IsRemote() {
+			Skip("Not supported for remote")
+		}
+
+		run := podmanTest.Podman([]string{"run", "-d", "--cgroupns=host", fedoraMinimal, "sleep", "100"})
+		run.WaitWithDefaultTimeout()
+		Expect(run.ExitCode()).To(Equal(0))
+		cid := run.OutputToString()
+
+		exec := podmanTest.Podman([]string{"exec", cid, "cat", "/proc/self/cgroup"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(Equal(0))
+
+		cgroup := filepath.Dir(strings.TrimRight(strings.Replace(exec.OutputToString(), "0::", "", -1), "\n"))
+
+		run = podmanTest.Podman([]string{"--cgroup-manager=cgroupfs", "run", "-d", fmt.Sprintf("--cgroup-parent=%s", cgroup), fedoraMinimal, "sleep", "100"})
+		run.WaitWithDefaultTimeout()
+		Expect(run.ExitCode()).To(Equal(0))
+
+		exec = podmanTest.Podman([]string{"exec", cid, "cat", "/proc/self/cgroup"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(Equal(0))
+		cgroupEffective := filepath.Dir(strings.TrimRight(strings.Replace(exec.OutputToString(), "0::", "", -1), "\n"))
+
+		Expect(cgroupEffective).To(Equal(cgroup))
+	})
+
 	Specify("valid --cgroup-parent using slice", func() {
 		if Containerized() || podmanTest.CgroupManager == "cgroupfs" {
 			Skip("Requires Systemd cgroup manager support")