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

Order containerd.service before kubelet.service #6873

Closed
wants to merge 1 commit into from

Conversation

dvzrv
Copy link

@dvzrv dvzrv commented Apr 28, 2022

containerd.service:
Order containerd.service before kubelet.service, as this way it is
started before a kubelet (if that unit is enabled) and allows for
garbage collection to work (see
cri-o/cri-o#4437 for comparison).
As docker.service in
https://github.com/moby/moby/blob/1f8d44babf18811ff9020de667bf6fda8d3c4401/contrib/init/systemd/docker.service#L4
is ordered after containerd.service this should suffice to ensure
garbage collection to work with docker as kubernetes container runtime.

Signed-off-by: David Runge dave@sleepmap.de

containerd.service:
Order containerd.service before kubelet.service, as this way it is
started before a kubelet (if that unit is enabled) and allows for
garbage collection to work (see
cri-o/cri-o#4437 for comparison).
As docker.service in
https://github.com/moby/moby/blob/1f8d44babf18811ff9020de667bf6fda8d3c4401/contrib/init/systemd/docker.service#L4
is ordered after containerd.service this should suffice to ensure
garbage collection to work with docker as kubernetes container runtime.

Signed-off-by: David Runge <dave@sleepmap.de>
@k8s-ci-robot
Copy link

Hi @dvzrv. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@mikebrow
Copy link
Member

/ok-to-test
cc @dims @thaJeztah

@thaJeztah
Copy link
Member

Curious; wouldn't adding After=containerd.service on kubelet do the same?

@thaJeztah
Copy link
Member

Basically; it feels a bit odd to define "before <some service that may depend on me>" (which may be "many" (and unknown which)) whereas "after <service I depend on and want to be started before myself>" feels a lot more logical.

Perhaps I'm wrong though, so interested to hear if there's a specific reason

@dvzrv
Copy link
Author

dvzrv commented Apr 28, 2022

@thaJeztah Yes. However, I am not aware of systemd services provided by upstream kubernetes. Adding it to containerd, every downstream facilitating systemd profits.

@thaJeztah
Copy link
Member

Right, so in that case it's even more tricky, because if all kubelet systemd units are hand-crafted by anyone running them; they may name those service anything they like?

If people run those as a systemd unit; do they use a canonical example as starting point? or how do they create the service?

@mikebrow
Copy link
Member

@dvzrv when you say gc, are you talking about kubelet's gc that happens through the cri api, or containerd's gc, or docker's gc?

@dvzrv
Copy link
Author

dvzrv commented Apr 28, 2022

If people run those as a systemd unit; do they use a canonical example as starting point? or how do they create the service?

I am not aware of a canonical naming for this (as in provided by Kubernetes), but if you look at how Canonical (no pun intended) names it for Ubuntu, this is the same as e.g. Fedora/RHEL and also Arch Linux names it.

@dvzrv when you say gc, are you talking about kubelet's gc that happens through the cri api, or containerd's gc, or docker's gc?

This relates to the kubelet garbage collection, that fails due to docker being started to late (see downstream ticket for Arch Linux that led me here: https://bugs.archlinux.org/task/74397). I was under the impression, that this was caused by containerd being started too late, but after confirmation by the affected user it seems, that in fact docker is not started early enough.

If you believe that this will have no benefit for containerd in the kubernetes node context, feel free to close this PR.

@dvzrv
Copy link
Author

dvzrv commented Apr 29, 2022

Closing due to archlinux/svntogit-community@4f57655

@dvzrv dvzrv closed this Apr 29, 2022
@thaJeztah
Copy link
Member

Thanks! Yes, I think fixing this in the unit files that depend on containerd is a better place to fix it

Perhaps Kubernetes should have some docs / reference implementation of a systemd unit to illustrate this (/cc @dims)

@dvzrv
Copy link
Author

dvzrv commented Apr 29, 2022

Perhaps Kubernetes should have some docs / reference implementation of a systemd unit to illustrate this (/cc @dims)

I agree. I'm not entirely happy about there being so many different (but essentially very similar) approaches in the various downstream distributions. It's easy to miss subtleties such as ordering and it would be much nicer if there was a central place for this.

I'll try to upstream the units I created for Arch Linux, but they'll need some adaption to work for e.g. different config directories.

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

Successfully merging this pull request may close these issues.

None yet

4 participants