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

Add runc_nosd build tag to opt out of systemd support #4547

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

kolyshkin
Copy link
Contributor

This is a carry of #3959, please see details in there.

The changes from #3959 are:

  • all merge conflicts resolved
  • build tag renamed from no_systemd to runc_nosd

Using this makes runc binary size ~10% smaller (of which half is github.com/godbus/dbus/v5).

For the unstripped binary:

│ -9.91%  │ runc                                        │ 15 MB    │ 14 MB    │ -1.5 MB │
│         │ runc-nosd                                   │          │          │         │

For stripped binary:

│ -10.32% │ runc-stripped                               │ 11 MB    │ 9.5 MB   │ -1.1 MB │
│         │ runc-nosd-stripped                          │          │          │         │

@kolyshkin kolyshkin changed the title Submit/no systemd 2 Add a build tag to opt out of systemd support Dec 6, 2024
@kolyshkin kolyshkin force-pushed the submit/no-systemd-2 branch from 7451783 to b10a80b Compare December 6, 2024 02:46
@kolyshkin kolyshkin changed the title Add a build tag to opt out of systemd support Add runc_nosd build tag to opt out of systemd support Dec 6, 2024
- name: compile with no build tags
run: make BUILDTAGS=""
- name: compile with runc_nosd tag added
run: make BUILDTAGS=""
Copy link
Member

Choose a reason for hiding this comment

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

This is missing runc_nosd.

@akhilerm
Copy link
Contributor

akhilerm commented Dec 6, 2024

A quick question. Isnt systemd required if you are using runc with cgroups v2? Or is it only a recommendation to use the systemd manager for cgroupv2?

@cyphar
Copy link
Member

cyphar commented Dec 6, 2024

@akhilerm You can in principle use runc without systemd on cgroupv2 systems, but it is very fragile because systemd can rewrite or change your cgroups if you don't register your cgroups with it and ask it to delegate them to you. I wouldn't recommend it, the kinds of bugs you run into are really frustrating to debug and can lead to containers having their cgroup limits entirely removed by systemd. But it's not (yet) strictly required.

@akhilerm
Copy link
Contributor

akhilerm commented Dec 6, 2024

@akhilerm You can in principle use runc without systemd on cgroupv2 systems, but it is very fragile because systemd can rewrite or change your cgroups if you don't register your cgroups with it and ask it to delegate them to you. I wouldn't recommend it, the kinds of bugs you run into are really frustrating to debug and can lead to containers having their cgroup limits entirely removed by systemd. But it's not (yet) strictly required.

If thats the case, will this change have an impact on the users. As most of them will be on cgroupsv2 or are in the process of migrating to cgroupv2. Which means, they will need the systemd support.

@cyphar
Copy link
Member

cyphar commented Dec 6, 2024

@akhilerm We aren't going to ship binaries with this flag set, and it won't be the default build option. I'm not even sure if @kolyshkin intends for this to be merged or is still expeirmenting.

At best it'll be an option for users that really want to disable this feature, for whatever reason. Personally I think the dislike of systemd that motivated the original PR ("Lennartix") kind of sullies the patch set somewhat, as it's not really clear to me why removing some DBus calls (that are no-ops on the vanishingly small number of non-systemd systems anyway) is that important.

@kolyshkin kolyshkin force-pushed the submit/no-systemd-2 branch from b10a80b to eea9ab2 Compare December 6, 2024 17:40
@cyphar
Copy link
Member

cyphar commented Dec 7, 2024

@kolyshkin Can you reword the original commit messages to not use the term "lennartix"?

@thaJeztah
Copy link
Member

Yeah, I'm not sure if we should take this change. I know the original contributor opened pull requests in various projects, but my fear is that it'll add the risk of considerable complexity; from experience, I know that such build-tags can start as "easy", but with more tags, the matrix can expand rapidly, which may result in more and more code having to be differentiated. For some packages that could be "ok" (e.g. I'm much more comfortable with the no_criu tag).

For this case specifically, we know running without systemd isn't "ideal", and I'm not sure how much test-coverage we get, more so if none of the active maintainers will be running in such a configuration. So I'd rather not take on that complexity unless there's a code-owner that can commit to maintaining.

metux and others added 8 commits December 12, 2024 08:06
Simplify the branching between systemd and pure cgroups.

Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
Factor out all references to systemd-specific dbus property struct into
some alias. That's helpful in making it possible to build it w/o systemd
dependencies.

[@kolyshkin: wording]
Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Factor out an init function specific to systemd, so subsequent patches
can easily make building systemd support build-time optional.

[@kolyshkin: wording]
Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Running under systemd requires lots of special code that contributes
to ca. 10 percent (ca. 1 MB) to the binary size. This is only needed on
targets that might run systemd - there're dozens of distros, let alone
embedded/edge devices or special images (eg. cluster worker nodes) that
do not and never will run systemd, thus do not need that code at all.

It's not just about reducing memory consumption, but also having over
10.000 lines of code less to audit.

In order not to change default behaviour, introducing an inverse build tag,
runc_nosd, for explicitly opting out from systemd special handlings.

Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
Co-authored-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Fix lint-extra err:

    Warning: var-naming: func addCpuQuota should be addCPUQuota (revive)

Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants