-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop systemd service #663
Drop systemd service #663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks like a good start! But per my comment in the other thread we do need to at least maintain some mechanism for "locking".
Ostree for example does this https://github.com/ostreedev/ostree/blob/64a09da0eb5ca4dade83125a2ebc47b52c60c5a5/src/libostree/ostree-sysroot.c#L1684
with a classic unix lockfile.
I would lean towards continuing to run under systemd under a well-known unit name, which also acts as a form of "lock" because only one instance a unit can be active at a time.
b741034
to
8269c57
Compare
Thanks Colin a lot for the detailed info, copy the comment here: Basically we detect if we're running in systemd; if we're not, we re-exec ourselves via systemd-run. Then we can just directly run code in what is now the daemon. I think an important aspect of this is that we retain something like |
8269c57
to
085e127
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This looks good to me.
2727f83
to
8885b6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for working on this!
6d217ac
to
f241cec
Compare
Seems the
Change to temp bootupd1.service to do some testing, find 2 issues:
|
Yeah, I don't think we need to worry about it.
Right, need to pass
Right, this one is tricky; what we probably need to do is detect the need to re-exec before parsing the args with clap, then we can directly pass through |
259bc30
to
0065833
Compare
Agree, it will not be the problem if using rpm. @ravanelli has the workaround to add it in fedora-coreos.yaml.
SGTM, thanks! |
Also check the selinux avc denied logs, should we revert the bootupd policy fedora-selinux/selinux-policy#1598? avc logs
|
I think so yes. It was clearly never tested. |
0065833
to
108aa24
Compare
108aa24
to
bc7185b
Compare
Fixes coreos#551 Get hints by coreos#551 (comment), and copy the comment here: Basically we detect if we're running in systemd; if we're not, we re-exec ourselves via systemd-run. Then we can just directly run code in what is now the daemon. I think an important aspect of this is that we retain something like `--unit bootupd` which acts as a lock - only one unit with that name can run at a time to avoid two concurrent invocations breaking things.
bc7185b
to
261fb5e
Compare
Thanks! I've not tested the PR but looks good overall. |
If for whatever reason a bootupd command fails, it will leave the systemd service unit in a failed state and systemd will then refuse to run a unit under the same name with `systemd-run` again until the failure is cleared. Thus systematically call `systemctl reset-failed` before calling `systemd-run` to clear any potential failures from previous calls. See: coreos#707 See: coreos#663
If for whatever reason a bootupd command fails, it will leave the systemd service unit in a failed state and systemd will then refuse to run a unit under the same name with `systemd-run` again until the failure is cleared. Thus systematically call `systemctl reset-failed` before calling `systemd-run` to clear any potential failures from previous calls. See: coreos#707 See: coreos#663
If for whatever reason a bootupd command fails, it will leave the systemd service unit in a failed state and systemd will then refuse to run a unit under the same name with `systemd-run` again until the failure is cleared. Thus systematically call `systemctl reset-failed` before calling `systemd-run` to clear any potential failures from previous calls. Do not check the return code of the systemctl command on purpose as it may fail if the unit does not exists yet, i.e. if no bootupctl command has been run yet. Also ignore stdout/stderr to avoid showing unexpected errors messages to users. See: coreos#707 See: coreos#663
Fixes #551