Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

stage1: support hybrid cgroup hierarchy #3784

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

iaguis
Copy link
Member

@iaguis iaguis commented Sep 1, 2017

systemd introduced the hybrid cgroup hierarchy that mounts a cgroup v1
and v2 hierarchy in parallel. After systemd v233 the cgroup v2 hierarchy
is mounted in /sys/fs/cgroup/unified. This breaks rkt (specifically
flavor host) because we mount /sys/fs/cgroup ourselves and were not
creating the unified directory.

Let's create /sys/fs/cgroup/unified in CreateCgroup() to support the
hybrid cgroup hierarchy. If the host system (or the stage1) doesn't use
the hybrid hierarchy, having this directory created doesn't hurt.

Fixes #3741

systemd introduced the hybrid cgroup hierarchy that mounts a cgroup v1
and v2 hierarchy in parallel. After systemd v233 the cgroup v2 hierarchy
is mounted in `/sys/fs/cgroup/unified`. This breaks rkt (specifically
flavor host) because we mount `/sys/fs/cgroup` ourselves and were not
creating the `unified` directory.

Let's create `/sys/fs/cgroup/unified` in CreateCgroup() to support the
hybrid cgroup hierarchy. If the system or stage1 doesn't use the hybrid
hierarchy, having this directory created doesn't hurt.
@iaguis
Copy link
Member Author

iaguis commented Sep 1, 2017

I tried other implementations before deciding to submit this one, you can read them in git history form here: https://github.com/kinvolk/rkt/commits/iaguis/hybrid-hierarchy-story

@alban
Copy link
Member

alban commented Sep 1, 2017

@iaguis It looks good but...

CreateCgroups() is called twice:

This double usage already non-trivial error management when mounting sysfs.

And this new mkdir is only required on the future root of the pod and not on the root of the host. I wonder if we can reorganise the code to avoid the mkdir on the host.

@iaguis
Copy link
Member Author

iaguis commented Sep 1, 2017

I thought about that and we could create the dir only if if root != "/" so we don't pollute the host fs.

However, note that we're creating the directory in atmpfs inside a mount ns that's different from the host's so I decided it's not worth it.

Copy link
Member

@alban alban left a comment

Choose a reason for hiding this comment

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

LGTM if green

@iaguis iaguis merged commit 779db3f into rkt:master Sep 1, 2017
@iaguis iaguis deleted the iaguis/hybrid-hierarchy branch January 9, 2018 12:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants