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.

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

(#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit #397

Merged

Conversation

dtardon
Copy link
Member

@dtardon dtardon commented Jun 20, 2023

Resolves: #2215925

@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jun 20, 2023
@github-actions
Copy link

github-actions bot commented Jun 20, 2023

Tracker - 2215925

The following commits meet all requirements

commit upstream
2172f90 - core: when Delegate=yes is set for a unit, run ExecStartPre= and frien… systemd/systemd@78f9320
systemd/systemd-stable@78f9320
2fc2acc - man: link Delegate= documentation up with the markdown docs systemd/systemd@077c40b
systemd/systemd-stable@077c40b

@systemd-rhel-bot systemd-rhel-bot added the pr/needs-review Formerly needs-review label Jun 20, 2023
@systemd-rhel-bot systemd-rhel-bot changed the title When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit Jun 20, 2023
@systemd-rhel-bot systemd-rhel-bot added the tracker/unapproved Formerly needs-acks label Jun 20, 2023
@msekletar
Copy link
Member

@dtardon CentOS CI is very unhappy, in results I see failures all over the place and but I am not sure if they can be attributed to this change. Can you please force push to retrigger the CI?

@systemd-rhel-bot systemd-rhel-bot removed the tracker/unapproved Formerly needs-acks label Jul 13, 2023
…nds in a subcgroup of the unit

Otherwise we might conflict with the "no-processes-in-inner-cgroup" rule
of cgroupsv2. Consider nspawn starting up and initializing its cgroup
hierarchy with "supervisor/" and "payload/" as subcgroup, with itself
moved into the former and the payload into the latter. Now, if an
ExecStartPre= is run right after it cannot be placed in the main cgroup,
because that is now in inner cgroup with populated children.

Hence, let's run these helpers in another sub-cgroup .control/ below it.

This is somewhat ugly since it weakens the clear separation of
ownership, but given that this is an explicit contract, and double opt-in should be acceptable.

Fixes: #10482
(cherry picked from commit 78f9320)

Resolves: #2215925
(cherry picked from commit 077c40b)

Related: #2215925
@dtardon dtardon force-pushed the bz2215925-Delegate+ExecStopPost branch from adea707 to 2fc2acc Compare July 13, 2023 13:07
@mrc0mmand
Copy link
Member

The fail is not related to this PR, I've seen it in other PRs as well. I'll try to figure out what's going on.

@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Jul 13, 2023
@systemd-rhel-bot systemd-rhel-bot changed the title (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit (#2215925) (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit Aug 1, 2023
@systemd-rhel-bot systemd-rhel-bot changed the title (#2215925) (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit (#2215925) (#2215925) (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit Aug 8, 2023
@dtardon dtardon changed the title (#2215925) (#2215925) (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit (#2215925) When Delegate=yes is set for a unit, run ExecStartPre= and friends in a subcgroup of the unit Aug 16, 2023
Copy link
Member

@msekletar msekletar left a comment

Choose a reason for hiding this comment

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

LGTM

@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-review Formerly needs-review label Aug 22, 2023
@systemd-rhel-bot systemd-rhel-bot merged commit 0c1f962 into redhat-plumbers:main Aug 22, 2023
10 checks passed
@dtardon dtardon deleted the bz2215925-Delegate+ExecStopPost branch August 23, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants