Merge pull request containers#10234 from giuseppe/fix-cgroupfs-pod

cgroup: fix rootless --cgroup-parent with pods

libpod/container_internal_linux.go

@@ -2216,6 +2216,17 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) {
 	return passwdPath, groupPath, nil
 }
 
+func isRootlessCgroupSet(cgroup string) bool {
+	// 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.
+	// Check both that the cgroup or its parent is set to the default value (used by pods).
+	return cgroup != CgroupfsDefaultCgroupParent && filepath.Dir(cgroup) != CgroupfsDefaultCgroupParent
+}
+
 // Get cgroup path in a format suitable for the OCI spec
 func (c *Container) getOCICgroupPath() (string, error) {
 	unified, err := cgroups.IsCgroup2UnifiedMode()
@@ -2227,13 +2238,7 @@ func (c *Container) getOCICgroupPath() (string, error) {
 	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.
+		if !isRootlessCgroupSet(c.config.CgroupParent) {
 			return "", nil
 		}
 		return c.config.CgroupParent, nil

libpod/pod_internal.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v3/libpod/define"
+	"github.com/containers/podman/v3/pkg/rootless"
 	"github.com/containers/storage/pkg/stringid"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -74,9 +75,11 @@ func (p *Pod) refresh() error {
 			}
 			p.state.CgroupPath = cgroupPath
 		case config.CgroupfsCgroupsManager:
-			p.state.CgroupPath = filepath.Join(p.config.CgroupParent, p.ID())
+			if rootless.IsRootless() && isRootlessCgroupSet(p.config.CgroupParent) {
+				p.state.CgroupPath = filepath.Join(p.config.CgroupParent, p.ID())
 
-			logrus.Debugf("setting pod cgroup to %s", p.state.CgroupPath)
+				logrus.Debugf("setting pod cgroup to %s", p.state.CgroupPath)
+			}
 		default:
 			return errors.Wrapf(define.ErrInvalidArg, "unknown cgroups manager %s specified", p.runtime.config.Engine.CgroupManager)
 		}

libpod/runtime_ctr.go

@@ -295,7 +295,10 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 					if podCgroup == "" {
 						return nil, errors.Wrapf(define.ErrInternal, "pod %s cgroup is not set", pod.ID())
 					}
-					ctr.config.CgroupParent = podCgroup
+					canUseCgroup := !rootless.IsRootless() || isRootlessCgroupSet(podCgroup)
+					if canUseCgroup {
+						ctr.config.CgroupParent = podCgroup
+					}
 				} else if !rootless.IsRootless() {
 					ctr.config.CgroupParent = CgroupfsDefaultCgroupParent
 				}

libpod/runtime_pod_linux.go

@@ -75,17 +75,20 @@ func (r *Runtime) NewPod(ctx context.Context, options ...PodCreateOption) (_ *Po
 	// Check CGroup parent sanity, and set it if it was not set
 	switch r.config.Engine.CgroupManager {
 	case config.CgroupfsCgroupsManager:
-		if pod.config.CgroupParent == "" {
-			pod.config.CgroupParent = CgroupfsDefaultCgroupParent
-		} else if strings.HasSuffix(path.Base(pod.config.CgroupParent), ".slice") {
-			return nil, errors.Wrapf(define.ErrInvalidArg, "systemd slice received as cgroup parent when using cgroupfs")
-		}
-		// If we are set to use pod cgroups, set the cgroup parent that
-		// all containers in the pod will share
-		// No need to create it with cgroupfs - the first container to
-		// launch should do it for us
-		if pod.config.UsePodCgroup {
-			pod.state.CgroupPath = filepath.Join(pod.config.CgroupParent, pod.ID())
+		canUseCgroup := !rootless.IsRootless() || isRootlessCgroupSet(pod.config.CgroupParent)
+		if canUseCgroup {
+			if pod.config.CgroupParent == "" {
+				pod.config.CgroupParent = CgroupfsDefaultCgroupParent
+			} else if strings.HasSuffix(path.Base(pod.config.CgroupParent), ".slice") {
+				return nil, errors.Wrapf(define.ErrInvalidArg, "systemd slice received as cgroup parent when using cgroupfs")
+			}
+			// If we are set to use pod cgroups, set the cgroup parent that
+			// all containers in the pod will share
+			// No need to create it with cgroupfs - the first container to
+			// launch should do it for us
+			if pod.config.UsePodCgroup {
+				pod.state.CgroupPath = filepath.Join(pod.config.CgroupParent, pod.ID())
+			}
 		}
 	case config.SystemdCgroupsManager:
 		if pod.config.CgroupParent == "" {