From 27ac750c7d949fc5922c4a11bf3e8e4606dd2a04 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 5 May 2021 21:58:35 +0200 Subject: cgroup: fix rootless --cgroup-parent with pods extend to pods the existing check whether the cgroup is usable when running as rootless with cgroupfs. commit 17ce567c6827abdcd517699bc07e82ccf48f7619 introduced the regression. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 19 ++++++++++++------- libpod/pod_internal.go | 7 +++++-- libpod/runtime_ctr.go | 5 ++++- libpod/runtime_pod_linux.go | 25 ++++++++++++++----------- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index f87e845cb..f0608e2b2 100644 --- a/libpod/container_internal_linux.go +++ b/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 diff --git a/libpod/pod_internal.go b/libpod/pod_internal.go index 31b4ba443..e81bd7b16 100644 --- a/libpod/pod_internal.go +++ b/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) } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 328f47c12..7d31e392f 100644 --- a/libpod/runtime_ctr.go +++ b/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 } diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index cf48a9453..4ede23cac 100644 --- a/libpod/runtime_pod_linux.go +++ b/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 == "" { -- cgit v1.2.3-54-g00ecf