Ignition shouldn't use systemd presets to enable units

[bgilbert]

Issue Report

Environment

Any

Desired Feature

Ignition should create the requisite unit symlinks itself, rather than using systemd presets.

Other Information

This would solve multiple problems: #2209, #2241, #2243.

[lucab]

I'd need to dig into this a bit more, but I think this would not work with services provided via torcx as the service units do not yet exist when ignition is run. Moreover I think the decision to using presets came after lessons learned from cloudinit issues, but I don't have the specific details at hand.

[euank]

To duplicate some discussion with @bgilbert:

It seems plausible that we could split enable into two different forms of enabling things: things that exist and things that don't.

In the case of "things that exist", we can create symlinks. This includes aliases and templated units.

For things that don't, we could use a preset + log a warning. This would include torcx units and other units not written by Ignition directly.

Perhaps in the long term torcx should have an initrd component which handles unit enablement in its own way, and somehow coordinates with Ignition's intent to let user's override it?

[bgilbert]

A torcx initrd component seems to add significant complexity. In principle any unit written but not enabled by a generator would need to be enabled via preset (i.e. it's not necessarily just a torcx issue) but I don't know if that's common.

Logging a warning would address #2243, and as an additional notification mechanism we could write a unit whose sole job is to fail if any of the units enabled via preset weren't actually enabled.

[cgwalters]

Just to be sure I understand so far:

diff --git a/internal/exec/util/unit.go b/internal/exec/util/unit.go
index a3e05af..ace06fc 100644
--- a/internal/exec/util/unit.go
+++ b/internal/exec/util/unit.go
@@ -91,11 +91,11 @@ func (u Util) MaskUnit(unit types.Unit) error {
 }
 
 func (u Util) EnableUnit(unit types.Unit) error {
-	return u.appendLineToPreset(fmt.Sprintf("enable %s", unit.Name))
+	return chroot(fmt.Sprintf("systemctl enable %s", unit.Name))
 }
 
 func (u Util) DisableUnit(unit types.Unit) error {
-	return u.appendLineToPreset(fmt.Sprintf("disable %s", unit.Name))
+	return chroot(fmt.Sprintf("systemctl mask %s", unit.Name))
 }
 
 func (u Util) appendLineToPreset(data string) error {

would break torcx?

[ajeddeloh]

IIRC systemctl talks to systemd over dbus and does not create the links manually, right? systemctl also refuses to run in a chroot (at least in the CL SDK that's the case)

[cgwalters]

See systemd/systemd#7631 for systemctl and dbus.

[cgwalters]

In the end systemctl enable X is generally a really complicated way to invoke ln -s, so TL;DR: if we set SYSTEMCTL_OFFLINE=1 in the environment now you'll consistently have it just do that and not try to do DBus.

[cgwalters]

I see a few paths here. One is to enhance systemd in some way to make this nicer. It could be enhancing the preset file. For #2243, it could be as simple as:

enable-required foo.service

Which would be like enable but fail if foo.service doesn't exist. For #2209...maybe just define enable-required to also work with aliases, it's not clear to me why it wouldn't. (I don't understand offhand why presets explicitly ban aliases) Lennart replied on #2241 - probably best to continue that conversation.

That said, we could likely kill all of these issues at once if (as this issue suggests) we don't use presets, but instead write out a small systemd service that runs in early boot that does basically:

#!/bin/bash
set -euo pipefail
systemctl enable foo.service bar@blah.service
systemctl daemon-reload

[bgilbert]

One of the major lessons from coreos-cloudinit is that services shouldn't be fiddling with the boot process. We could use a generator, though.

[cgwalters]

We could use a generator, though.

The tricky thing there is generators are designed to be transient. So Ignition would have to write out something like /etc/ignition/default-units.json and install a generator from that. Which would feel weird versus just directly creating the multi-user.target.wants/ symlinks in /etc - we can do it at the same time as generators.

[ajeddeloh]

It sounds like we need to figure out what failure in the real root looks like. We generally want failures to take the machine down entirely (e.g. Ignition failures prevent the switch-root from ever happening). Failing in the real root is less "bulletproof" as failing in the initramfs. Users can work around it and it doesn't set off all sorts of alarms like the initramfs failures.

[coreosbot]

Moved to coreos/ignition#588.