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

Properly handle operation as init process #4086

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

brandond
Copy link
Member

@brandond brandond commented Sep 27, 2021

Proposed Changes

Properly handle operation as init process. This includes both vacating the root cgroup on cgroupv2 systems, and reaping child processes since containerd doesn't clean up after its own shims.

This is a Go adaptation of the behavior proposed in #3237; we rejected doing this in shell and K3d has been using a workaround ever since.

Types of Changes

bugfix

Verification

  • Run K3s in Docker on a system with cgroup v2; note that it works
  • Note that there are no <defunct> processes inside the Docker container

Linked Issues

User-Facing Change

The K3s docker image now works on cgroup v2 systems, and properly reaps terminated containerd shim processes.

Further Comments

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
@brandond brandond requested a review from a team as a code owner September 27, 2021 22:56
@brandond brandond merged commit a16105b into k3s-io:master Sep 28, 2021
@iwilltry42
Copy link
Collaborator

So does this supersede #3242 ?

@brandond
Copy link
Member Author

@iwilltry42 I believe so yes. It doesn't do everything that your PR did but it doesn't appear to be necessary to get things working?

@iwilltry42
Copy link
Collaborator

@brandond I'm not sure.. I think the evacuation is the most important bit and that's the same in both.
I think you just put it in a different spot and I changed some other cgroup related things with it back then.
I guess the codebase also changed quite a bit since then.
If it works with your changes in k3d with the entrypoint disabled, then I'm happy already 😁

@brandond
Copy link
Member Author

Is there an easy way to test k3d without the entrypoint?

@iwilltry42
Copy link
Collaborator

Not 100% sure right now (on my phone) but using export K3D_FIX_CGROUPV2=0 should work

@brandond
Copy link
Member Author

brandond commented Sep 30, 2021

No, it doesn't work with K3D_FIX_CGROUPV2=0 because /sbin/docker-init is pid 1. The K3s containers need to be started without the docker --init flag for our fix to take effect; is there a way to do that? Kinda looks like it's hardcoded:

https://github.com/rancher/k3d/blob/6c94d7db3d4116d8add1d9af1bcec0b185783694/pkg/runtimes/docker/translate.go#L48

@iwilltry42
Copy link
Collaborator

Yeah, it's hardcoded right now, but I can add an env var switch for that, if needed and drop it in the next v5 pre-release?

@AkihiroSuda
Copy link
Contributor

This commit seems to have broken rootless mode
https://github.com/k3s-io/k3s/runs/3726354551

@brandond
Copy link
Member Author

brandond commented Oct 1, 2021

I believe it's been failing since before that commit. I can still run rootless locally so I think it's something with the CI configuration. Unfortunately the cgroup v2 tests don't output any logs and I'm not sure how to execute them locally so it's been difficult to troubleshoot.

@AkihiroSuda
Copy link
Contributor

Looking at https://github.com/k3s-io/k3s/commits/master , the CI was green ✅ for the previous commit (“ set transport to skip verify if se skip flag passed ”)

@brandond
Copy link
Member Author

brandond commented Oct 1, 2021

Ah, indeed. Looks like it is legit just broken:

brandond@dev01:/var/lib/repos/go/src/github.com/k3s-io/k3s$ systemd-run --user -p Delegate=yes --tty ./dist/artifacts/k3s server --data-dir /var/lib/repos/k3s/ --rootless
Running as unit: run-u3.service
Press ^] three times within 1s to disconnect TTY.
INFO[0000] Acquiring lock file /var/lib/repos/k3s/data/.lock
INFO[0000] Preparing data dir /var/lib/repos/k3s/data/ffe439099bcec803a4a982d914444fc964d4e58050884e9ab0015ae0fe8956ad
FATA[0000] failed to evacuate root cgroup: mkdir /sys/fs/cgroup/init.scope/init: permission denied

Do you have a suggestion as to how we could better surface errors from the rootless cgroup v2 CI? It has a history of flaking so I do not personally look too closely at the results.

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

Successfully merging this pull request may close these issues.

5 participants