Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

add to actually delete all config files when the service is stopped - #96

Merged
mmou merged 1 commit into
masterfrom
mmou/PICNIC-1151-rm-configs
Apr 28, 2020
Merged

mmou merged 1 commit into
masterfrom
mmou/PICNIC-1151-rm-configs

Conversation

@mmou

@mmou mmou commented Apr 28, 2020 •

Copy link
Copy Markdown
Contributor

I don't know if this ever worked, but my kssh config files were not getting deleted on docker kill of the keybaseca container. Issue #86 mentions the same problem. (Note my files do get properly deleted if I'm in the container and only kill the keybaseca process.)

This is because docker kill sends a SIGKILL only to the process with pid1, and that process wasn't forwarding the signal to the keybaseca process. Adding --init to docker run makes it such that an init process based on tini is run as pid 1, that will properly reap zombie processes. But the problem here is that even if we are able to properly propagate the signal to keybaseca, keybaseca needs kbfsfuse to stay alive long enough to actually delete the config files. So at this point...I decided to try another approach of just starting another docker container to delete all config files, before killing all ca-related containers.

make clean-kssh will delete all config files that the CA Keybase account can find. This will only mess you up if for some reason, you have another CA bot simultaneously running that is listening on and signing keys for the same teams -- this seems really unlikely, and if you are in this situation, you can just restart your other CA bot to rewrite the kssh files.

@@ -42,7 +42,7 @@ func main() {
cli.BoolFlag{
Name: "wipe-all-configs",
Hidden: true,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i kept this hidden, but lmk if you feel strongly otherwise

@mmou
mmou requested a review from ddworken April 28, 2020 00:36
@ddworken

Copy link
Copy Markdown
Contributor

LGTM 👍

I suspect you're right that the config file clearing didn't work correctly all along, so thanks for fixing it!

@mmou
mmou merged commit 3fc2ba1 into master Apr 28, 2020
@mmou mmou mentioned this pull request Apr 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants