Ibotta / sopstool

SOPS multi-file wrapper
Apache License 2.0
38 stars 4 forks source link

Allow graceful shutdown by pass signals to child process #36

Closed ibottamike closed 4 years ago

ibottamike commented 5 years ago

Story/Issue Link

DI-4716

Background

The entrypoint command runs the command as a child process but does not pass Unix signals. When used in a container in k8s, this prevents the running process from receiving the sigterm and sigkill signals to allow a graceful shutdown. During graceful shutdown, services can stop receiving requests (HTTP or from a queue) and finish the existing requests before shutting down. This is required for PWI. Using entrypoint with the -e can be used as a workaround to not run the command as a child process. This allows the same support to the standard command.

Implementation is based on https://kevin.burke.dev/kevin/proxying-to-a-subcommand-with-go/

Versioning

Additional Requests to Reviewers

Tasks

/cc

ibottamike commented 5 years ago

The pattern being used in docker container is to run entrypoint using -e to replace the process instead of having the complexity of passing signals to a child process.

onyxraven commented 5 years ago

I'm not opposed to still adding this though, I think it is the correct behavior we would want by default.

ibottamike commented 5 years ago

I'm not opposed to still adding this though, I think it is the correct behavior we would want by default.

I can reopen the PR. As pointed out, it is difficult to unit test. For testing, instead of trying to unit test I would test through the method. For example, have a long live process that can handle the signals with success or error. I was having difficulty locally getting this to work in the current dev env (go 1.9).
Two options are: have PR with the current version without signal tests or first have a PR to upgrade the go version then add functionality. I have built and manually tested directly and in a docker container.