Skip to content

Commit

Permalink
cgroup: fix rootless --cgroup-parent with pods
Browse files Browse the repository at this point in the history
extend to pods the existing check whether the cgroup is usable when
running as rootless with cgroupfs.

commit 17ce567 introduced the
regression.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
  • Loading branch information
giuseppe committed May 6, 2021
1 parent 9b9bd9e commit 27ac750
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 21 deletions.
19 changes: 12 additions & 7 deletions libpod/container_internal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions libpod/pod_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
25 changes: 14 additions & 11 deletions libpod/runtime_pod_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down

0 comments on commit 27ac750

Please sign in to comment.