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.

Sign up for GitHub

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

Jump to bottom

Add support for custom stop signal to send to containers #30051

Open
thockin opened this issue Aug 4, 2016 · 14 comments
Open

Add support for custom stop signal to send to containers #30051

thockin opened this issue Aug 4, 2016 · 14 comments
Labels
area/api Indicates an issue on api area. area/kubelet area/pod-lifecycle Issues or PRs related to Pod lifecycle kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@thockin
Copy link
Member

thockin commented Aug 4, 2016

Docker added STOPSIGNAL to let container images change which signal is delivered to kill the container. Should we support that? It would be bad if an image expected SIGUSR1 and we sent SIGTERM.

They support ints or strings. I think we should only support strings.
@thockin thockin added help-wanted area/kubelet area/api Indicates an issue on api area. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Aug 4, 2016
@yujuhong
Copy link
Contributor

yujuhong commented Aug 4, 2016

If STOPSIGNAL is already in the image, shouldn't the docker daemon kill the container with the right signal automatically? It'd only be broken if we bypass docker daemon or use a non-docker runtime.
Or are you talking about adding a new "stop signal" feature in the kubernetes API?

@thockin
Copy link
Member Author

thockin commented Aug 4, 2016

When we signal a container, do we just 'docker kill' or do we explicitly
send a specific signal? I know we send SIGTERM for graceful term. I don't
know if STOPSIGNAL is intended to be "kill" or "soft kill" (e.g. should we
send STOPSIGNAL instead of SIGTERM or instead of SIGKILL.

On Thu, Aug 4, 2016 at 12:51 PM, Yu-Ju Hong notifications@github.com
wrote:

If STOPSIGNAL is already in the image, shouldn't the docker daemon kill
the container with the right signal automatically? It'd only be broken if
we bypass docker daemon or use a non-docker runtime.
Or are you talking about adding a new "stop signal" feature in the
kubernetes API?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#30051 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFVgVN8P5hrCYCdc3k7Wm2iBymNwPY_dks5qckKtgaJpZM4JcVn8
.

@yujuhong
Copy link
Contributor

yujuhong commented Aug 4, 2016

We use docker stop, which by default sends SIGTERM to the containers.
If you start a container with docker run ---stop-signal=SOMETHINGELSE or has STOPSIGNAL built-in your image, docker will send that signal on docker stop

docker stop itself doesn't even provide flag for callers to specify the signal...

@thockin thockin closed this as completed Aug 4, 2016
@thockin
Copy link
Member Author

thockin commented Aug 4, 2016

ACK, then I think we can close this. thanks!

On Thu, Aug 4, 2016 at 2:32 PM, Yu-Ju Hong notifications@github.com wrote:

We use docker stop, which by default sends SIGTERM to the containers.
If you start a container with docker run ---stop-signal=SOMETHINGELSE or
has STOPSIGNAL built-in your image, docker will send that signal on docker
stop

docker stop itself doesn't even provide flag for callers to specify the
signal...

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#30051 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFVgVBHGysPoonGBmawqQhkwrRpKugCPks5qclpugaJpZM4JcVn8
.

@yujuhong yujuhong mentioned this issue Aug 4, 2016
Closed
@soumyalj soumyalj mentioned this issue Oct 9, 2017
Closed
@neomantra neomantra mentioned this issue May 6, 2019
Merged
@zerowidth zerowidth mentioned this issue Jul 17, 2019
Merged
@thockin thockin reopened this Feb 6, 2020
@thockin
Copy link
Member Author

thockin commented Feb 6, 2020

This continues to be a problem for people who use pre-built images that handle SIGTERM by closing their listening socket. If they could simply change the signal to SIGUSR1, they would not have to deal with this.

@thockin thockin mentioned this issue Feb 7, 2020
Closed
@thockin
Copy link
Member Author

thockin commented Mar 18, 2020

I realized that this request is not as clear as it could be.

Dockerfile supports STOPSIGNAL and we support that transparently.

However, not everyone builds their own images, and telling folks they have to build a custome image just to set this is pretty hostile.

Docker supports --stop-signal as a flag to docker run. Kubernetes has no equivalent as part of Pod or Container APIs. I posit that we should have something.

@bobbypage bobbypage mentioned this issue May 26, 2020
Closed
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 16, 2020
@thockin
Copy link
Member Author

thockin commented Jun 16, 2020

/lifecycle frozen
/remove-lifecycle stale
/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 16, 2020
@mwaggonerzam mwaggonerzam mentioned this issue Jul 18, 2020
Closed
pierrebeaucamp pushed a commit to gravitational/teleport that referenced this issue Dec 1, 2020
Graceful stops in Docker
429021b 
This commit changes the default behaviour when stopping a Docker
container from fast shutdowns to graceful ones. It is a potentially
breaking change for some users and warrants a discussion.

The background for this is that Kubernetes doesn't currently allow
configuring the termination signal it sends to pods. For more on
that, see kubernetes/kubernetes#30051

While this change increases the time it takes for a container to stop,
Docker still sends an additional `SIGKILL` ten seconds after triggering
the termination. This means that the default behaviour still ensures
that the container is terminated _relatively_ quickly.

There are potential workarounds if we decide against changing the
Dockerfile. We could have a seperate image, use `PreStop` hooks, or or
work around the issue in Teleport itself. However, this change is by far
the simplest.
@pierrebeaucamp pierrebeaucamp mentioned this issue Dec 1, 2020
Closed
@mmiranda96
Copy link
Contributor

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 25, 2021
@withinboredom withinboredom mentioned this issue Jul 5, 2021
Closed
@glennpratt
Copy link

@thockin it seems like this ticket is now misnamed. Perhaps you or someone could rename it so as not to confuse those who stumble upon it "Open".

@sftim
Copy link
Contributor

sftim commented Jun 27, 2022

/retitle Add support for custom stop signal to send to containers

@k8s-ci-robot k8s-ci-robot changed the title Support Docker STOPSIGNAL Add support for custom stop signal to send to containers Jun 27, 2022
@thockin thockin mentioned this issue Feb 9, 2023
Closed
@dgeissl dgeissl mentioned this issue Jul 10, 2023
Open
@KeithTt
Copy link

KeithTt commented Aug 17, 2023

How to get current stopsignal? And how to change the value?

@joebowbeer
Copy link

@thockin Close this?

dockerd is removed and kubelet reads the STOPSIGNAL from the image config:

cri-o/cri-o#306

@thockin
Copy link
Member Author

thockin commented Aug 31, 2023

#30051 (comment)

"Kubernetes has no equivalent as part of Pod or Container APIs. I posit that we should have something."

@joebowbeer joebowbeer mentioned this issue Aug 31, 2023
Open
@thockin thockin added the area/pod-lifecycle Issues or PRs related to Pod lifecycle label Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/api Indicates an issue on api area. area/kubelet area/pod-lifecycle Issues or PRs related to Pod lifecycle kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@joebowbeer @glennpratt @thockin @yujuhong @KeithTt @mmiranda96 @k8s-ci-robot @sftim @fejta-bot