Skip to content
This repository has been archived by the owner on Sep 18, 2020. It is now read-only.

systemd: run After update_engine #132

Merged
merged 1 commit into from Oct 11, 2017
Merged

systemd: run After update_engine #132

merged 1 commit into from Oct 11, 2017

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Oct 6, 2017

d01a4c3 dropped a Requires/After on update-engine.service, since those would prevent systemd from restarting locksmithd if update-engine failed. However, that change created a race in which locksmithd would start before update_engine, fail with

Cannot get update engine status: The name com.coreos.update1 was not provided by any .service files

and be restarted by systemd 10 seconds later.

Ideally update_engine would support bus activation, but enabling that would require users to take an additional step when disabling update_engine, breaking backward compatibility. Instead, add an After
to fix the race, but do not pull in update_engine. In principle we could also add a Wants, but that would change behavior in a corner case (restarting locksmithd with update_engine stopped).

d01a4c3 dropped a Requires/After on
update-engine.service, since those would prevent systemd from restarting
locksmithd if update-engine failed. However, that change created a
race in which locksmithd would start before update_engine, fail with

    Cannot get update engine status: The name com.coreos.update1 was not provided by any .service files

and be restarted by systemd 10 seconds later.

Ideally update_engine would support bus activation, but enabling that
would require users to take an additional step when disabling
update_engine, breaking backward compatibility. Instead, add an After
to fix the race, but do not pull in update_engine. In principle we
could also add a Wants, but that would change behavior in a corner case
(restarting locksmithd with update_engine stopped).
Copy link
Contributor

@euank euank left a comment

Choose a reason for hiding this comment

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

I agree that this change should be backwards compatible; LGTM

@bgilbert
Copy link
Contributor Author

Good to merge.

@cgonyeo cgonyeo merged commit e7cd0bb into coreos:master Oct 11, 2017
@bgilbert bgilbert deleted the race branch October 13, 2017 22:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants