-
Notifications
You must be signed in to change notification settings - Fork 92
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
systemd: enable bootc-status-updated.target on startup #1041
Conversation
Actually it's not even firstboot, for example after doing a |
d1b0440
to
37830ab
Compare
In the general case, don't we actually want to trigger |
Hmm yeah probably. In a perfect world we don't want to trigger it if nothing has changed (you rebooted the system without any sort of update/switch/rollback queued), but I think that's better than it not triggering when the status did change. |
This heavily relates to a discussion elsewhere for a metadata flag of "is this the first boot of a deployment"? If we had that, then we could key off it in our systemd generator and only enqueue the change notification if we found that. But for now I think it's actually OK to run on boot by default - most tools that want to hook into this should be idempotent anyways, i.e. they need to handle the case where nothing changed. Not that anything will run by default except for the subman case today anyways. |
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.
Marking requested-changes per #1041 (comment)
37830ab
to
42c7aa3
Compare
Updated to static enable the |
@@ -2,3 +2,6 @@ | |||
Description=Target for bootc status changes | |||
Documentation=man:bootc-status-updated.target(8) | |||
StopWhenUnneeded=true |
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.
Since we're now enabling this default; I think we should probably also add here ConditionPathExists=/run/ostree-booted
? In practice it's probably effectively the same, but the difference is that everyone who isn't using bootc will I think see "skipped as condition not met" vs "reached target bootc-status-updated" and I think the former makes more sense.
Alternatively: We could just dynamically enable this from our generator in that condition.
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.
👍 good catch, updated
Any services that react to status changes should be idempotent, and should run on boot. It is likely (but not guaranteed) that during boot we are in the process of switching from a previous deployment to a new one, and thus what was previously "staged" is now "booted" and what was previously "booted" is now "rollback", so anything that cares about status is going to want to handle that. Signed-off-by: John Eckersberg <[email protected]>
42c7aa3
to
edcdf9c
Compare
Any services that react to status changes should be idempotent, and
should run on boot. It is likely (but not guaranteed) that during
boot we are in the process of switching from a previous deployment to
a new one, and thus what was previously "staged" is now "booted" and
what was previously "booted" is now "rollback", so anything that cares
about status is going to want to handle that.
Signed-off-by: John Eckersberg [email protected]