contrib/systemd: use multi-user.target instead of default.target

[hnez]

The systemd documentation says the following about using default.target as WantedBy= in service files:

For typical unit files please set "WantedBy=" to a regular target (like multi-user.target or graphical.target), instead of default.target, since such a service will also be run on special boots like on system update, emergency boot ...

The mentioned "system update" special boots refer to systemd.offline-updates a mechanism that enables doing a special minimalistic boot to run e.g. migration scripts after installing an update (using a package manager or an image based updater like e.g. RAUC) and before the first boot of the updated system.

To prevent conflicts between normal services on the system and migration scripts, normal services should not be started for these special boots.

For this to work normal services may not use WantedBy=default.target, because according to the documentation the system-update.target becomes the default.target for system-update boots:

3. Very early in the new boot systemd-system-update-generator(8) checks whether /system-update or /etc/system-update exists. If so, it (temporarily and for this boot only) redirects (i.e. symlinks) default.target to system-update.target, a special target that pulls in the base system (i.e. sysinit.target, so that all file systems are mounted but little else) and the system update units.

Use WantedBy=multi-user.target target instead of default.target to enable the use of system-update.target.

Does this PR introduce a user-facing change?

The podman systemd units are now part of the `multi-user.target` instead of the `default.target`

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hnez
Once this PR has been reviewed and has the lgtm label, please assign mheon for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

LGTM however podman generate systemd still generates files with default.target, that command is deprecated but may still be worth to fix the generator?

For the replacement quadlet, user are responsible to set the WantedBy line so it is not a problem there. However the docs use default.target so they should be updated as well.https://docs.podman.io/en/latest/markdown/podman-systemd.unit.5.html#enabling-unit-files

[hnez]

Hi @Luap99, thanks for the quick feedback!

I am not too familiar with the podman codebase or even podman from a users point of view, so I am more or less sed -i "s/default.target/multi-user.target/"-ing blindly here and leaving the compile and runtime testing to the CI pipeline.

Feel free to point out any mistakes I made along the way or things that are still missing.

[Luap99]

Yeah I think some more investigation is needed, looking at the history I found 9a10e21

So it seems simple replace is incorrect and will break all rootless users

$ systemctl --user status multi-user.target
Unit multi-user.target could not be found.
$ sudo systemctl status multi-user.target
● multi-user.target - Multi-User System
...

[Luap99]

Neither multi-user.target or graphical.target exists for a user session it seems so I don't think this change can be correct.

$ ls -l /usr/lib/systemd/system/default.target
lrwxrwxrwx. 1 root root 16 Oct 11 02:00 /usr/lib/systemd/system/default.target -> graphical.target
$ ls -l /usr/lib/systemd/user/default.target
-rw-r--r--. 1 root root 471 Oct  9 17:18 /usr/lib/systemd/user/default.target

So there is a real default.target unit for the user session and not just a symlink to another unit as described in the docs.

As such I think the systemd docs are misleading as they only consider the root case.

We assume that we can use the same unit files for root and rootless, if have to do different things in both cases that will complicate things massively. We do not have a good way to do that as of today, I hacked up a work around for another similar problem recently 6b8e8cb

[hnez]

Hi,

I've had some time again to look into this.

LGTM however podman generate systemd still generates files with default.target, that command is deprecated but may still be worth to fix the generator?

I've changed the generator to always output multi-user.target.

It would be possible to e.g. add a --user flag that generates a systemd unit suitable for a user session (e.g. using default.target and maybe some other tweaks for rootless usage).

But I think it is not worth it for a command marked as deprecated. Or is it?

---

For the replacement quadlet, user are responsible to set the WantedBy= line so it is not a problem there. However the docs use default.target so they should be updated as well. https://docs.podman.io/en/latest/markdown/podman-systemd.unit.5.html#enabling-unit-files

I've now read up on quadlets (using a systemd generator to create ad-hoc service files there is a neat design. I will have to remember that for later) and have updated the docs to distinguish between user/rootless and system/rootful *.container files.

---

So it seems simple replace is incorrect and will break all rootless users.

We assume that we can use the same unit files for root and rootless, if have to do different things in both cases that will complicate things massively. We do not have a good way to do that as of today, I hacked up a work around for another similar problem recently 6b8e8cb

It took me a bit to figure out that the files in contrib/systemd/user/ are just symlinks to the files in contrib/systemd/system/.

I am not a huge fan of having symlinks in git repositories to be honest because the changes to the pointed to files do not appear as diffs in the location where the symlink lives. A call to git log -- contrib/systemd/user/ for example will be misleading about the changes that have happened to the user services.

I think in this case it is better to just live with the duplication (I've updated the PR to do so) or de-duplicate some other way, e.g. by deriving the user service files from the system service files in the Makefile.

[Luap99]

Duplication is bad, there are far to many things I saw getting patched only in one place and then forgot to update the other places as well. It is extremely hard for maintainers here to catch these things as we have different people reviewing different things and not anyone is aware of all the duplicated places.
I do acknowledge that problem can exists in the same way the other direction (a change that should have only be done in that one place) but this is much easier to catch as I can just look from where this is called or used to know all the places and then make a informed decision on that.

At this point for the static podman- units it might be much better to use some form of template engine to have proper support to add variable conditions. I think this whole change might need some more design discussion (and/or input from other maintainers cc @containers/podman-maintainers )

LGTM however podman generate systemd still generates files with default.target, that command is deprecated but may still be worth to fix the generator?

I've changed the generator to always output multi-user.target.

It would be possible to e.g. add a --user flag that generates a systemd unit suitable for a user session (e.g. using default.target and maybe some other tweaks for rootless usage).

Chaining the behaviour like this is a breaking change for anyone calling this from automation for user services so we cannot change that. I am fine with not fixing this command because it is detracted but then it MUST keep the current behavior.

[vrothberg]

Just skimming the conversation and the git log which revealed commit 220f9a7 that seems to conflict with the intentions of this PR. I may be off since I don't have the time to dive deeper into the issue.

[hnez]

I just found another flaw in my current version of this pull request.
The systemd unit in contrib/systemd/user that I have modified are not actually used in the Makefile that should install them:

podman/Makefile

Lines 996 to 1003 in 4edd7e1

	install.systemd: $(PODMAN_GENERATED_UNIT_FILES)
	install ${SELINUXOPT} -m 755 -d $(DESTDIR)${SYSTEMDDIR} $(DESTDIR)${USERSYSTEMDDIR}
	for unit in $^ \
	contrib/systemd/system/podman-auto-update.timer \
	contrib/systemd/system/podman.socket; do \
	install ${SELINUXOPT} -m 644 $$unit $(DESTDIR)${USERSYSTEMDDIR}/$$(basename $$unit); \
	install ${SELINUXOPT} -m 644 $$unit $(DESTDIR)${SYSTEMDDIR}/$$(basename $$unit); \
	done

Instead the ones in contrib/systemd/system are used for both. I will update the PR if the design decisions turn out in favor of the current approach.

---

In the meantime my colleague @jluebbe has suggested another approach: add a podman.target (or containers.target -the name is up for discussion) that all services and quadlets can use in their WantedBy= and have two versions of this podman.target.
One with WantedBy=multi-user.target that is installed for systemd system sessions and one with WantedBy=default.target for systemd user sessions.
That way there would be no duplication of the units themselves, just two different .target files.

[Luap99]

In the meantime my colleague @jluebbe has suggested another approach: add a podman.target (or containers.target -the name is up for discussion) that all services and quadlets can use in their WantedBy= and have two versions of this podman.target.
One with WantedBy=multi-user.target that is installed for systemd system sessions and one with WantedBy=default.target for systemd user sessions.
That way there would be no duplication of the units themselves, just two different .target files.

That could certainly work but then we need to deal with yet another unit and then need to documented how this works which I guess might be harder for users to understand.

---

I have a similar issue with After= in #24637
Given we already replace podman with a rather hacky sed line in the Makefile I think it would be much better to write our own templates. I could write a small binary that just parse two arguments for the podman and user session and then just use the golang test/template for that. Similar to how we did it for podman generate systemd.