handle non-existing systemd units deactivation

[tormath1]

Feature Request

Hello, between systemd-250 and systemd-252, it seems there is a change in the codebase that makes the systemd --root... disable my-unit.service output different. I highly suspect this commit: systemd/systemd@7a6c73d but it would require more investigation.

On Flatcar, between two releases that includes a systemd bump from 250 to 252, we have the following:

• new stable-3510.2.0:

bash-5.1# systemctl --root=/sysroot disable docker.socket
Failed to disable unit, unit docker.socket does not exist.
bash-5.1# echo $?
1

• previous stable-3374.2.5:

bash-5.1# systemctl --root=/sysroot disable docker.socket
Failed to disable unit, unit docker.socket does not exist.
bash-5.1# echo $?
0

As Ignition relies on the return code of systemctl command (only when disabling units):

ignition/internal/exec/util/unit.go

Lines 163 to 166 in 6e54286

	args := []string{"--root", ut.DestDir, "disable", disabledUnit}
	if output, err := exec.Command(distro.SystemctlCmd(), args...).CombinedOutput(); err != nil {
	return fmt.Errorf("cannot remove symlink(s) for %s: %v: %q", disabledUnit, err, string(output))
	}

It leads to a boot failure.

Environment

What hardware/cloud provider/hypervisor is being used to run Ignition? Flatcar on Qemu and OpenStack

Desired Feature

I think Ignition should provide a best effort to disable symlinks but it should let the machine booting even if the symlinks do not exist to at least write the preset value.

This issue is limited to Flatcar with Torcx as docker.{service,socket} is provided by Torcx but we can think about a user that would extend the OS with systemd-sysext images for examples.

Other Information

• Initial Flatcar issue: Fresh rebuild of flatcar VM fails to boot because docker.socket is not found by ignition flatcar/Flatcar#1004

[bgilbert]

Interestingly, on systemd 251, systemctl disable enoent.service exits 1 but systemctl --root=/ disable enoent.service exits 0.

On the one hand, Ignition is supposed to fail whenever it can't do something requested by the config. On the other hand, Ignition has never worked this way for systemd units (because Ignition just writes out a preset and trusts systemd to handle the enablement later in boot, #587). It makes some sense that disabling an already-disabled unit would be a no-op, and indeed the original PR added a blackbox test to ensure that (#1352 (comment)). After CI switched to Fedora 38, this test started failing in CI.

I think we should treat this as a regression for now. We might eventually want to change the semantics, but I don't think it makes sense to do that only for this single case on specific systemd releases.

[prestist]

@bgilbert What were we fishing for with the error check previously? since it would return 0 in this case?

While more feedback is nice, it will break previously working configs. So I suspect to keep things good we can check the return error and if its error is "...does not exist" then we can ignore it ?

[bgilbert]

I don't think there was a specific error we were concerned about. We're just checking for errors because it's the right thing to do.

It's not a good practice to compare error strings (they might change, or in other circumstances might be localized into another language), so we should try to avoid that if there's any possible alternative. In this case, we could probably run a systemctl command to check whether the unit exists, and only try to disable it if so.

[prestist]

Ah yeah that makes waaay more sense. Thank you for the feedback.