Skip to content
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

Don't restart key units until all keys have been uploaded #177

Open
cprussin opened this issue Sep 25, 2023 · 1 comment
Open

Don't restart key units until all keys have been uploaded #177

cprussin opened this issue Sep 25, 2023 · 1 comment

Comments

@cprussin
Copy link
Contributor

cprussin commented Sep 25, 2023

Sort of an extension of #174 , it would also be really nice if the temp key files weren't moved into their final locations until all keys have been uploaded, and then were moved as a batch at roughly the same time. The reason for this is that if you have services that depend on multiple keys, then those services end up getting bounced repeatedly as keys are uploaded. This isn't so painful for some services, but for other stuff -- e.g. wireguard -- this can be super obnoxious.

A solution I'd propose here is the following:

  1. Upload keys to a temporary destination (removing the ones that haven't changed, as per Only restart key unit if keyfile has actually changed #174)
  2. Run a script after all keys have been uploaded which
    1. Stops all key units for keys that have changed
    2. Moves the changed keys to their final location
    3. Removes the temporary upload dir
    4. Starts the key units for the changed keys all at once

This would also allow us to do away with the weird flappy service implementation here and the inotify watches which were mostly written that way to avoid issues with systemd's reliability when using path units (see #119), because I think we can safely assume that keys will only be touched via colemana itself so we don't have to account for other direct changes on the filesystem (although we would need to account for persistent keys that are available on boot if we went this route).

@cprussin
Copy link
Contributor Author

hey @zhaofengli ! Just curious if you had a chance to see this ticket and think about my proposal? It's probably my only real pain point with colemana and I'd love to put a fix in for it if you're open to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant