Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgroupns: private cgroupns on cgroupv1 breaks --systemd #17736

Merged
merged 2 commits into from
Mar 14, 2023
Merged

cgroupns: private cgroupns on cgroupv1 breaks --systemd #17736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01fd5bc
libpod: remove error stutter
giuseppe Mar 10, 2023
2d1f4a8
cgroupns: private cgroupns on cgroupv1 breaks --systemd
giuseppe Mar 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 46 additions & 40 deletions libpod/container_internal_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,17 +242,17 @@ func (c *Container) setupSystemd(mounts []spec.Mount, g generate.Generator) erro
return err
}

hasCgroupNs := false
for _, ns := range c.config.Spec.Linux.Namespaces {
if ns.Type == spec.CgroupNamespace {
hasCgroupNs = true
break
}
}

if unified {
g.RemoveMount("/sys/fs/cgroup")

hasCgroupNs := false
for _, ns := range c.config.Spec.Linux.Namespaces {
if ns.Type == spec.CgroupNamespace {
hasCgroupNs = true
break
}
}

var systemdMnt spec.Mount
if hasCgroupNs {
systemdMnt = spec.Mount{
Expand All @@ -271,40 +271,46 @@ func (c *Container) setupSystemd(mounts []spec.Mount, g generate.Generator) erro
}
g.AddMount(systemdMnt)
} else {
hasSystemdMount := MountExists(mounts, "/sys/fs/cgroup/systemd")
if hasCgroupNs && !hasSystemdMount {
return errors.New("cgroup namespace is not supported with cgroup v1 and systemd mode")
}
mountOptions := []string{"bind", "rprivate"}
skipMount := false

var statfs unix.Statfs_t
if err := unix.Statfs("/sys/fs/cgroup/systemd", &statfs); err != nil {
if errors.Is(err, os.ErrNotExist) {
// If the mount is missing on the host, we cannot bind mount it so
// just skip it.
skipMount = true
}
mountOptions = append(mountOptions, "nodev", "noexec", "nosuid")
} else {
if statfs.Flags&unix.MS_NODEV == unix.MS_NODEV {
mountOptions = append(mountOptions, "nodev")
}
if statfs.Flags&unix.MS_NOEXEC == unix.MS_NOEXEC {
mountOptions = append(mountOptions, "noexec")
}
if statfs.Flags&unix.MS_NOSUID == unix.MS_NOSUID {
mountOptions = append(mountOptions, "nosuid")
}
if statfs.Flags&unix.MS_RDONLY == unix.MS_RDONLY {
mountOptions = append(mountOptions, "ro")

if !hasSystemdMount {
skipMount := hasSystemdMount
var statfs unix.Statfs_t
if err := unix.Statfs("/sys/fs/cgroup/systemd", &statfs); err != nil {
if errors.Is(err, os.ErrNotExist) {
// If the mount is missing on the host, we cannot bind mount it so
// just skip it.
skipMount = true
}
mountOptions = append(mountOptions, "nodev", "noexec", "nosuid")
} else {
if statfs.Flags&unix.MS_NODEV == unix.MS_NODEV {
mountOptions = append(mountOptions, "nodev")
}
if statfs.Flags&unix.MS_NOEXEC == unix.MS_NOEXEC {
mountOptions = append(mountOptions, "noexec")
}
if statfs.Flags&unix.MS_NOSUID == unix.MS_NOSUID {
mountOptions = append(mountOptions, "nosuid")
}
if statfs.Flags&unix.MS_RDONLY == unix.MS_RDONLY {
mountOptions = append(mountOptions, "ro")
}
}
}
if !skipMount {
systemdMnt := spec.Mount{
Destination: "/sys/fs/cgroup/systemd",
Type: "bind",
Source: "/sys/fs/cgroup/systemd",
Options: mountOptions,
if !skipMount {
systemdMnt := spec.Mount{
Destination: "/sys/fs/cgroup/systemd",
Type: "bind",
Source: "/sys/fs/cgroup/systemd",
Options: mountOptions,
}
g.AddMount(systemdMnt)
g.AddLinuxMaskedPaths("/sys/fs/cgroup/systemd/release_agent")
}
g.AddMount(systemdMnt)
g.AddLinuxMaskedPaths("/sys/fs/cgroup/systemd/release_agent")
}
}

Expand Down Expand Up @@ -446,7 +452,7 @@ func (c *Container) addNetworkNamespace(g *generate.Generator) error {
func (c *Container) addSystemdMounts(g *generate.Generator) error {
if c.Systemd() {
if err := c.setupSystemd(g.Mounts(), *g); err != nil {
return fmt.Errorf("adding systemd-specific mounts: %w", err)
return err
}
}
return nil
Expand Down
7 changes: 7 additions & 0 deletions test/system/250-systemd.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,13 @@ LISTEN_FDNAMES=listen_fdnames" | sort)
is "${container_uuid}" "${output:0:32}" "UUID should be first 32 chars of Container id"
}

@test "podman --systemd fails on cgroup v1 with a private cgroupns" {
skip_if_cgroupsv2

run_podman 126 run --systemd=always --cgroupns=private $IMAGE true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add assert "$output" =~ "blah blah blah blah". 126 can happen for many reasons, it is important to verify that it's happening here for the expected reason.

assert "$output" =~ ".*cgroup namespace is not supported with cgroup v1 and systemd mode"
}

# /~https://github.com/containers/podman/issues/13153
@test "podman rootless-netns slirp4netns process should be in different cgroup" {
is_rootless || skip "only meaningful for rootless"
Expand Down
9 changes: 9 additions & 0 deletions test/system/helpers.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,15 @@ function skip_if_cgroupsv1() {
fi
}

#######################
# skip_if_cgroupsv2 # ...with an optional message
#######################
function skip_if_cgroupsv2() {
if is_cgroupsv2; then
skip "${1:-test requires cgroupsv1}"
fi
}

######################
# skip_if_rootless_cgroupsv1 # ...with an optional message
######################
Expand Down