-
Notifications
You must be signed in to change notification settings - Fork 54
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
feat(daemon): introduce reboot cleanup function #496
Conversation
This reboot mode ensures that the machine is not restarted during daemon.Stop() when a restart has been requested. Instead, it signals the daemon.Stop() caller that further actions to ensure a restart are required through a new exported error daemon.ErrRestartExternal. This is required in scenarios where further actions are required before the machine can be safely rebooted, such as syncing and unmounting filesystems.
This PR adds another low level restart handler without changing the internal restart logic (i.e. restart manager) of the Daemon. In a previous change, we (with the approval of CTO) added the syscall reboot handler. However, further work on a derivative system is proving that reboot request invocation inside the daemon is too limiting for our use-case. It either requires system knowledge passed into the daemon for cleanup, or the daemon can stay mostly system agnostic, but request cleanup + reboot to be dealt with by the caller.
Since the default reboot handler has to be explicitly changed, this has no impact on existing use cases. |
internals/daemon/daemon.go
Outdated
if rebootDelay > 0 { | ||
time.Sleep(rebootDelay) | ||
} |
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.
doing this sleep inside HandleRestart seems wrong, you might have to distinguish the calls from HandleRestart vs doReboot now
Regarding the timeout comment - inside Ben was asking if we should not just bypass the daemon here (which is what we have done). So I think we just revisit this to make sure out approach makes sense. |
Closing for now, since it clearly needs more thought than I put into it initially. Let's work on this during the next cycle! Thanks for your feedback |
No description provided.