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

k3s: Service starts up before network; and doesn't shutdown properly #103158

Closed
ThinkChaos opened this issue Nov 8, 2020 · 16 comments · Fixed by #223823
Closed

k3s: Service starts up before network; and doesn't shutdown properly #103158

ThinkChaos opened this issue Nov 8, 2020 · 16 comments · Fixed by #223823

Comments

@ThinkChaos
Copy link
Contributor

Describe the bug

  1. systemd.services.k3s doesn't depend on network-online.target, which introduces a race and causes spurious k3s.service errors.

  2. Stopping the service doesn't actually stop the containers, so when shutting the system down we get systemd-shutdown[1]: Waiting for: containerd-shim. This causes the shutdown time to go through the roof.

Additional context
Kind of related to #98090 because k3s-killall.sh could be useful for 2.

Notify maintainers
@euank


Nonetheless, thanks for maintaining this!

@freezeboy
Copy link
Contributor

freezeboy commented Nov 8, 2020

For number 2, would not a KillMode=control-group be enough in the service unit?

@ThinkChaos
Copy link
Contributor Author

systemd.service.k3s was most likely copied from rancher/k3s/k3s.service and because control-group is the default (according to docs) I believe it's there for a reason.
I still gave it a shot: defined my own k3s_custom service using control-group. Instead of systemd-shutdown complaining about containerd-shim this gives us the standard systemd 1m30s timeout countdown.

I'll make a PR with the network part for 1 as that does work.

@euank
Copy link
Member

euank commented Nov 9, 2020

Yup, it was copied from k3s.service.

I think the reason it's there is that often, if you just do something like systemctl restart k3s.service, it's desirable for the containers to keep running since k8s can "re-adopt" those orphaned containers.

It does sound like it interacts poorly with shutdowns, but for things like k3s upgrades, it seems desirable.
I'm not sure the right solution there.

For the networking thing, I definitely think we want an after=network-online.target. Probably a simple miss on my part.

I use the module with k3s.docker = true, which sets after=docker.service, which as a side-effect orders it after network.target, which is why I think I haven't been impacted by that yet.

One other thing that sorta falls under this issue: k3s should also be ordered after firewalld.service on nixos to ensure the kube-proxy iptables rules don't race with the nixos fw rules on bootup. This is something I realized a few days ago, and hadn't gotten around to fixing.

@ThinkChaos
Copy link
Contributor Author

It does sound like it interacts poorly with shutdowns, but for things like k3s upgrades, it seems desirable.

I agree, I noticed when I reverted back to process just now, nixos-rebuild switch waited for the containers to stop.

k3s should also be ordered after firewalld.service

I added that to the PR. I only saw your comment after submitting it.

I'm not sure the right solution there.

How about adding k3s-killall.sh and using it in ExecStopPost (ExecStop is called for restarts too)?
It's not perfect because this means one more file to keep in sync with upstream, but I don't see a better solution.

@jnetod
Copy link
Contributor

jnetod commented Jun 3, 2021

2 is affecting docker too on 21.05.

containerd/containerd#5502

@wamserma
Copy link
Member

2 is affecting docker too on 21.05.

containerd/containerd#5502

Yes, this is pretty annoying. I wish that issue would get more attention (by the containerd folks).

@codygman
Copy link
Contributor

codygman commented Nov 5, 2021

2 is affecting docker too on 21.05.

containerd/containerd#5502

Came here to link this, it costs 20-30s per reboot 😔

@rgoulter
Copy link

rgoulter commented Nov 5, 2021

I ran into the "shutdown blocked on containerd-shim" message by way of forgetting about containers I'd previously run.

I had some a container which I'd run and then forgot about. IIRC, even having stopped containers means that containerd-shim will time out on shutdown/reboot.

In my case, I didn't need to keep the container around between reboots. So, I was able to work around the timeout by just removing the containers. -- But if you've forgotten about it, you wouldn't know to look.

@Baughn
Copy link
Contributor

Baughn commented Dec 7, 2021

I ran into the "shutdown blocked on containerd-shim" message by way of forgetting about containers I'd previously run.

I ran into it by having a server I intended to reboot get stuck, waiting for containerd-shim forever. It did not time out. This happened after the network had been torn down, so there was no way to fix it until I got back from vacation. RIP all my planning.

Destroying containers prior to shutdown isn't a good fix. The user might forget, or -- quite possibly -- might still want those containers to exist after the reboot.

This arguably is indeed a bug in the docker service, but also in however reboots are configured. Nothing should be able to block a reboot indefinitely.

@mohe2015
Copy link
Contributor

mohe2015 commented Dec 7, 2021

I assume containerd/containerd#5828 will fix this?

@wamserma
Copy link
Member

wamserma commented Dec 7, 2021

I'm wondering if this gets merged to containerd before we release 22.05. ;)

@dustinlacewell
Copy link

I'm wondering if this gets merged to containerd before we release 22.05. ;)

The second workaround in this SO answer is just a systemd unit so I wonder if we could integrate that if they do not get this merged by then.

https://unix.stackexchange.com/questions/666963/docker-20-10-x-keeps-system-waiting-for-several-minutes-before-shutdown-or-reboo

@wamserma
Copy link
Member

I'm wondering if this gets merged to containerd before we release 22.05. ;)

Merged! containerd/containerd#5828

Unfortunately the diff does not apply cleanly to 1.5.9.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 30, 2022
@Scrumplex
Copy link
Member

Looks like this has been backported to the 1.5 branch: containerd/containerd#6509

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 29, 2022
@wamserma
Copy link
Member

At least the shutdown issue seems gone on 22.05. The other issue seems to be solved by #103228 - closing.

@Scrumplex
Copy link
Member

From what I can tell, I still have this issue. Running on 22.05.3891.7269939a5d5 (Quokka). I can confirm that my k3s.service file is the same as in #103228

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

Successfully merging a pull request may close this issue.