Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Remove OnFailure= from targets, put in services #188

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented May 27, 2020

In systemd v245, which is in f32, the OnFailure= directive in target
units stopped working. This seems to be by design though:

systemd/systemd#14142

If target units were never meant to handle this, this might be why we've
had issues like boot loops on failure in the past (see #115).

Anyway, let's just pepper our services with the directive instead.

In systemd v245, which is in f32, the `OnFailure=` directive in target
units stopped working. This seems to be by design though:

systemd/systemd#14142

If target units were never meant to handle this, this might be why we've
had issues like boot loops on failure in the past (see #115).

Anyway, let's just pepper our services with the directive instead.
@jlebon
Copy link
Member Author

jlebon commented May 27, 2020

I asked if there's a cleaner way to do this now: systemd/systemd#14142 (comment)

@jlebon
Copy link
Member Author

jlebon commented Jun 1, 2020

OK, I think we should really get this in. Right now on f32 FCOS, Ignition failures do not cause a failed boot. (I thought we had a test for that, but I guess not.)

@jlebon jlebon merged commit 8f5d1ec into coreos:master Jun 1, 2020
@jlebon jlebon deleted the pr/on-failure-systemd branch June 1, 2020 18:19
@dustymabe
Copy link
Member

I think this is safe but it's worth asking the question: If this change were to make its way into an f31 based FCOS (or another distro at the older version of systemd) is it still safe to make?

@jlebon
Copy link
Member Author

jlebon commented Jun 1, 2020

If this change were to make its way into an f31 based FCOS (or another distro at the older version of systemd) is it still safe to make?

Yes, this is a backwards compatible change. I've just tested this on top of current FCOS stable (which is still on f31) and sanity-checked that it works fine in both the failure and success path.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants