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

Stop timeout isn't respected at shutdown/reboot #77873

Closed
mrunalp opened this issue May 14, 2019 · 23 comments · Fixed by opencontainers/runc#2224
Closed

Stop timeout isn't respected at shutdown/reboot #77873

mrunalp opened this issue May 14, 2019 · 23 comments · Fixed by opencontainers/runc#2224
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@mrunalp
Copy link
Contributor

mrunalp commented May 14, 2019

What happened:
Containers are getting terminated by systemd without respecting the terminationGracePeriodSeconds set in the pod yaml on reboot or shutdown of a node.

What you expected to happen:
terminationGracePeriodSeconds is respected by systemd when using systemd as the cgroup manager.

How to reproduce it (as minimally and precisely as possible):

  1. Use systemd as the cgroup manager in your container runtime.
  2. Create a pod yaml with terminationGracePeriodSeconds set to 120 seconds.
  3. Reboot the node.
  4. You will notice that the containers get SIGTERM, followed by systemd default stop timeout (typically 90 seconds) and then they are SIGKILLed.

Anything else we need to know?:
This can be fixed by passing the stop timeout to the containers as part of the CreateContainer CRI API. This will allow the container runtimes to set the systemd property for the scope to override the default stop timeout to the value set through terminationGracePeriodSeconds.
This needs changes across the stack as runc doesn't currently provide a way to set the TimeoutStopUSec for the systemd scope created for a container.
The behavior with cgroupfs cgroup manager will need further investigation.

Environment:

  • Kubernetes version (use kubectl version): All versions.
@mrunalp mrunalp added the kind/bug Categorizes issue or PR as related to a bug. label May 14, 2019
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 14, 2019
@mrunalp
Copy link
Contributor Author

mrunalp commented May 14, 2019

cc: @derekwaynecarr @dchen1107

@derekwaynecarr
Copy link
Member

while this impacts all pods, it is particularly an issue with static pods or daemon set backed pods which typically are not drained before a maintenance action.

/milestone v1.15

@k8s-ci-robot k8s-ci-robot added this to the v1.15 milestone May 29, 2019
@yujuhong
Copy link
Contributor

@derekwaynecarr IIUC, this is a feature request, since kubernetes has never supported this. Is there a reason why this is marked milestone 1.15 when we are already past enhancement freeze and almost reaching code freeze?

@Random-Liu
Copy link
Member

Random-Liu commented May 30, 2019

I'm not sure whether we want the user configured grace period to block node reboot.

Do people only reboot node after draining pods?

If not, say I have a 10min grace period pod, will it block the node from reboot for 10min? Is it fair to the other pods which have 10s grace period? It seems that they may have a 10min downtime or rescheduled to other nodes unnecessarily?

@mrunalp
Copy link
Contributor Author

mrunalp commented May 30, 2019 via email

@Random-Liu
Copy link
Member

Random-Liu commented May 30, 2019

We can restrict it similar to how privileged is restricted. This is useful for daemon sets and static pods that are system owned.

If that is the case, it seems that we need to define Kubernetes api for that. Kubelet doesn't know whether a pod belongs to a daemonset or not.

BTW, maybe it makes sense to block node reboot for that long, I'm not sure, either. Just thinking out loud.

@mattjmcnaughton
Copy link
Contributor

/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 30, 2019
@soggiest
Copy link

soggiest commented Jun 7, 2019

Is this issue release blocking or can we move it to 1.16?

@liggitt
Copy link
Member

liggitt commented Jun 8, 2019

If this is not a regression in 1.15, I'd expect it can be moved

@soggiest
Copy link

soggiest commented Jun 9, 2019

/milestone v1.16

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.15, v1.16 Jun 9, 2019
@josiahbjorgaard
Copy link
Contributor

Hi all, code freeze for v1.16 is coming up on Aug. 29 (in just 6 days). This issue is set for milestone v1.16, will it be solved by then? It will need to have a PR merged beforehand. If not, we will remove the v1.16 milestone.

@liggitt
Copy link
Member

liggitt commented Sep 3, 2019

I'm not aware of any activity on this issue targeting 1.16

@liggitt liggitt removed this from the v1.16 milestone Sep 3, 2019
@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 Dec 2, 2019
@dtolmachev
Copy link

@mrunalp
I've found this issue trying to fix the same problem as it is described in "How to reproduce it" section.

We run static pod with Docker container. Also we have kubelet and containerd on VM.

The problem: I want to make graceful shutdown with 60s duration for container. But currently, we have only 10 seconds. That's because Docker receives SIGTERM and it is interpreted as docker stop with default 10s timeout
It is possible to set --stop-timeout for docker run but we run container with kubelet and terminationGracePeriodSeconds in pod.yaml doesn't change the shutdown duration.
The pull-request in containerd project is still open.

Can I help with finishing this PR? What job needs to be done?

Thanks a lot for supporting Kubernetes and containerd!

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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 rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 5, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mrunalp
Copy link
Contributor Author

mrunalp commented Feb 24, 2020

Can this be reopened? I am opening a PR to address this.

@dims
Copy link
Member

dims commented Feb 24, 2020

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Feb 24, 2020
@k8s-ci-robot
Copy link
Contributor

@dims: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mrunalp
Copy link
Contributor Author

mrunalp commented Feb 24, 2020

@dims Thanks :)

kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Feb 25, 2020
Enable passing of sandbox's termination grace period
down to OCI runtime, as an annotation for systemd.

This is a glue between
* kubernetes/kubernetes#88495
and
* opencontainers/runc#2224
  (or containers/crun#266)
that is a part of the fix for
* kubernetes/kubernetes#77873

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Feb 26, 2020
Enable passing of kubernetes termination grace period
down to OCI runtime, as an annotation for systemd.

This builds on top of

* opencontainers/runc#2224
  (or containers/crun#266)

and is part of the fix for

* kubernetes/kubernetes#77873

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Mar 7, 2020
Enable passing of kubernetes termination grace period
down to OCI runtime, as an annotation for systemd.

This builds on top of

* opencontainers/runc#2224
  (or containers/crun#266)

and is part of the fix for

* kubernetes/kubernetes#77873

(cherry picked from commit 1f85692)

Conflicts: a minor conflict in server/container_create_linux.go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Mar 7, 2020
Enable passing of kubernetes termination grace period
down to OCI runtime, as an annotation for systemd.

This builds on top of

* opencontainers/runc#2224
  (or containers/crun#266)

and is part of the fix for

* kubernetes/kubernetes#77873

(cherry picked from commit 1f85692)

Conflicts: a minor conflict in server/container_create_linux.go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

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

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Apr 22, 2020
Enable passing of kubernetes termination grace period
down to OCI runtime, as an annotation for systemd.

This builds on top of

* opencontainers/runc#2224
  (or containers/crun#266)

and is part of the fix for

* kubernetes/kubernetes#77873

(cherry picked from commit 1f85692)

Conflicts: a minor conflict in server/container_create_linux.go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Apr 22, 2020
Enable passing of kubernetes termination grace period
down to OCI runtime, as an annotation for systemd.

This builds on top of

* opencontainers/runc#2224
  (or containers/crun#266)

and is part of the fix for

* kubernetes/kubernetes#77873

(cherry picked from commit 1f85692)

Conflicts: a minor conflict in server/container_create_linux.go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/cri-o that referenced this issue Jul 30, 2020
Enable passing of kubernetes termination grace period
down to OCI runtime, as an annotation for systemd.

This builds on top of

* opencontainers/runc#2224
  (or containers/crun#266)

and is part of the fix for

* kubernetes/kubernetes#77873

(cherry picked from commit 1f85692)

Conflicts: a minor conflict in server/container_create_linux.go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet