Add plugins support #4
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
damdo
commented
Sep 7, 2023
Comment on lines
+108
to
117
| log.Print("requesting reboot") | ||
| // TODO: change call from target.Reboot to something like target.ScheduleReboot | ||
| // which can asyncronouly perform the task instead of waiting | ||
| // for all the services to shutdown and then ack the reboot, | ||
| // othewise this causes a deadlock as the selfupdate service won't SIGTERM cleanly | ||
| // until the reboot ack is received, but won't receive the ack until all services shut down, | ||
| // causing a delayed reboot until SIGKILL kicks in. | ||
| if err := target.Reboot(); err != nil { | ||
| return fmt.Errorf("reboot: %v", err) | ||
| } |
There was a problem hiding this comment.
This one will need some thought
There was a problem hiding this comment.
Hmm, why will selfupdate not SIGTERM cleanly in this state? I would have expected it to. SIGINT and SIGTERM are hooked up to context cancellation in selfupdate’s main function.
damdo
commented
Sep 7, 2023
| ticker := time.NewTicker(frequency) | ||
|
|
||
| for { | ||
| for c := time.Tick(frequency); ; { |
There was a problem hiding this comment.
Changing this so then we get an immediate first check without waiting an extra frequency interval.
Also changing the skipWaiting to only skip the Jitter once, the first time.
These two changes combined still achieve the same initially defined behaviour for skipWaiting, but improve the first check also in cases where skipWaiting is set to false
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the update system efforts, we are introducing the ability to specify plugins that allow to push and pull updates from many different sources (e.g. OCI, S3, ...) in addition to the natively supported HTTP file server source.
As such in gokrazy/gokrazy#173 we want to give
selfupdatethe ability to understand payload links that differ from HTTP and may be coming from a GUS update response.To be able to do this, selfupdate must give the user the ability to specify a plugin name (which will be used by
selfupdateto reference the plugin binary) and some credentials that the plugin will use to fetch the update from the source registries (think for example at the credentials necessary to fetch an OCI update payload from an OCI registry or at the AWS credentials necessary to pull a file from AWS S3).This PR aims at implementing this ability for
selfupdate.Note:
With this approach
selfupdatewill be able to load only one plugin at a time in addition to the already natively supported HTTP fetching mechanism.