Remove dockershim from crictl

[shannonxtreme]

/kind feature /kind deprecation

What this PR does / why we need it:

Removes dockershim from the crictl docs and main_unix / main_windows in preparation for the 1.24 deprecation of dockershim.

• Remove dockershim.sock from default runtime endpoints for unix and windows
• Change dockershim.sock to containerd.sock in the default description for --runtime-endpoints.
• Remove dockershim from the crictl docs (including code examples in the docs)

This PR doesn't address critest mentions of dockershim as I'm not sure how to do that (but I'm happy to learn).

Which issue(s) this PR fixes:

Fixes #870

Special notes for your reviewer:

Does this PR introduce a user-facing change?

`dockershim` is no longer one of the runtime endpoints for `crictl`. 

[k8s-ci-robot]

Welcome @shannonxtreme!

It looks like this is your first PR to kubernetes-sigs/cri-tools 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cri-tools has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[dims]

looks like all the bases are covered. thanks @shannonxtreme

https://cs.k8s.io/?q=dockershim.sock&i=nope&files=&excludeFiles=&repos=kubernetes-sigs/cri-tools

/assign @adisky

one thing though ... for hack/run-dockershim-critest.sh we could add some comments that i can only work with https://github.com/Mirantis/cri-dockerd and not with any other CRI implementation. (either in this PR or in a follow up, i don't mind)

[k8s-ci-robot]

@dims: GitHub didn't allow me to assign the following users: adisky.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to this:

looks like all the bases are covered. thanks @shannonxtreme

https://cs.k8s.io/?q=dockershim.sock&i=nope&files=&excludeFiles=&repos=kubernetes-sigs/cri-tools

/assign @adisky

one thing though ... for hack/run-dockershim-critest.sh we could add some comments that i can only work with https://github.com/Mirantis/cri-dockerd and not with any other CRI implementation. (either in this PR or in a follow up, i don't mind)

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.

[SergeyKanzhelev]

Do we want crictl to be able to talk to earlier versions of k8s clusters while they are still supported? Once crictl is moved to use v1 of CRI API only, this will be totally reasonable PR. Before that, I'd suggest we keep it.

[SergeyKanzhelev]

Do we want crictl to be able to talk to earlier versions of k8s clusters while they are still supported?

One thing is that the matrix is not up to date: https://github.com/kubernetes-sigs/cri-tools#current-status

[adisky]

Do we want crictl to be able to talk to earlier versions of k8s clusters while they are still supported? Once crictl is moved to use v1 of CRI API only, this will be totally reasonable PR. Before that, I'd suggest we keep it.

@SergeyKanzhelev doesn't users expected to use matching version of crictl & k8s, if K8s 1.24 removed dockershim crictl 1.24 also expected to do the same? possiblty we should release a 1.23 tag for crictl then merge this PR?

Also this isn't essentially removing any support, only removing dockershim from default endpoints users can still configure unix:///var/run/dockershim.sock manually

[SergeyKanzhelev]

@SergeyKanzhelev doesn't users expected to use matching version of crictl & k8s

as a general statement, this wouldn't be ideal. crictl may be used alongside the containerd. For instance, it is installed on COS. And it works independently on the version of k8s installed on that COS image.

Since the dockershim.sock is only exposed by k8s itself, this statement has more sense. Also, defaults are deprecated and customers are encouraged to specify the socket explicitly (#599). So even if we will break customer scenario here, where the fallback to default is expected, it will not be completely unexpected. It also will be a good notification for customers to migrate :-).

[saschagrunert]

Let's discuss the release related bits in https://github.com/kubernetes-sigs/cri-tools/issues/856

[afbjorklund]

We would have hate having to use different versions of crictl in minikube, for different versions of kubernetes.

Removing dockershim support just for the sake of it seems useless (or premature), if it is not there it is not there ?

---

EDIT: Just to be clear, minikube does set up the crictl.yaml. Even for docker, where it not even used or required

But still using crictl 1.21 for k8s 1.23, so requiring a closer match would still be a big change compared with today

[afbjorklund]

Note that the packages are still installing crictl 1.19.0. (never mind 1.23.0)

 cri-tools |  1.19.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages
 cri-tools |  1.13.0-01 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages
 cri-tools |  1.13.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages
 cri-tools |  1.12.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages
 cri-tools |  1.11.1-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages
 cri-tools |  1.11.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages
 cri-tools | 1.0.0-beta.1-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages

https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/install-kubeadm/#installing-kubeadm-kubelet-and-kubectl

[shannonxtreme]

Since the defaults are deprecated anyway, isn't this loss of functionality more of an incentive to move off relying on just the defaults, and instead specifying explicit endpoints?

Idk, someone from sig-node probably knows better than me

On Thu., Jan. 13, 2022, 16:16 Anders Björklund, @.***> wrote:

Note that the packages are still installing crictl 1.19.0. (never mind 1.23.0)

cri-tools | 1.19.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages cri-tools | 1.13.0-01 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages cri-tools | 1.13.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages cri-tools | 1.12.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages cri-tools | 1.11.1-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages cri-tools | 1.11.0-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages cri-tools | 1.0.0-beta.1-00 | https://apt.kubernetes.io kubernetes-xenial/main amd64 Packages

https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/install-kubeadm/#installing-kubeadm-kubelet-and-kubectl

— Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/cri-tools/pull/871#issuecomment-1012083703, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHH4EFSP7AMCGZB5I7ADO5TUV27DDANCNFSM5LY6AQUA . You are receiving this because you were mentioned.Message ID: @.***>

[afbjorklund]

Since the defaults are deprecated anyway, isn't this loss of functionality more of an incentive to move off relying on just the defaults, and instead specifying explicit endpoints?

But then you could remove all endpoints, instead of just one.

[shannonxtreme]

I'm not sure why that hasn't happened. Maybe a deprecation policy thing?

I guess a middle ground could be to move dockershim.sock to last on the list so that it isn't the first endpoint tried by default

On Thu., Jan. 13, 2022, 16:25 Anders Björklund, @.***> wrote:

Since the defaults are deprecated anyway, isn't this loss of functionality more of an incentive to move off relying on just the defaults, and instead specifying explicit endpoints?

But then you could remove all endpoints, instead of just one.

— Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/cri-tools/pull/871#issuecomment-1012090276, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHH4EFWOBNMOIF5BNFD6BJTUV3AEBANCNFSM5LY6AQUA . You are receiving this because you were mentioned.Message ID: @.***>

[afbjorklund]

I'm not sure why that hasn't happened. Maybe a deprecation policy thing?

The majority of users have not even installled (much less configured) these still undocumented debugging tools, since it was not recommended for their runtime. So I guess it made sense to have it try with dockershim, if they dared to install the tool ?

But now that is required by all container runtimes, maybe it will end up being documented under "container runtimes" and not just under "node debugging" and it will see some better installations - such as packaging and configuring (setting up crictl.yaml)

• https://kubernetes.io/docs/setup/production-environment/container-runtimes/

• https://kubernetes.io/docs/tasks/debug-application-cluster/crictl/

[afbjorklund]

as a general statement, this wouldn't be ideal. crictl may be used alongside the containerd. For instance, it is installed on COS. And it works independently on the version of k8s installed on that COS image.

Same thing for minikube, currently using a random version 1.21.0 for the crictl installation in the OS

If it will be versioned with kubernetes, we need to move it to runtime bootstrapping installation instead.

[afbjorklund]

It also will be a good notification for customers to migrate :-).

They don't have to "migrate", they just have to start using CRI

[shannonxtreme]

Is it that the defaults currently do not work? I thought the case is that the defaults are deprecated and will be removed at some point, but still work for now

On Thu., Jan. 13, 2022, 17:27 Anders Björklund, @.***> wrote:

@.**** commented on this pull request.

In docs/benchmark.md https://github.com/kubernetes-sigs/cri-tools/pull/871#discussion_r783955520 :

Additional options

• -ginkgo.focus: Only run the tests that match the regular expression.
• -image-endpoint: Set the endpoint of image service. Same with runtime-endpoint if not specified. -- -runtime-endpoint: Set the endpoint of runtime service. Default to Unix: unix:///var/run/dockershim.sock or Windows: tcp://localhost:3735. +- -runtime-endpoint: Set the endpoint of runtime service. Default to Unix: unix:///var/run/containerd.sock or Windows: tcp://localhost:3735.

there is no default: #597 https://github.com/kubernetes-sigs/cri-tools/issues/597

— Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/cri-tools/pull/871#discussion_r783955520, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHH4EFXUWCWAMZP76JGPB4DUV3HMHANCNFSM5LY6AQUA . You are receiving this because you were mentioned.Message ID: @.***>

[afbjorklund]

Is it that the defaults currently do not work?

The project doesn't dare to pick a new favorite, so recommends using any container runtime that follows the CRI

"Items on this page refer to third party products or projects that provide functionality required by Kubernetes. The Kubernetes project authors aren't responsible for those third-party products or projects."

And if crictl needs to try all the possible locations, that introduces a latency when checking the various sockets...

But checking all 4 locations "works", I thought.

• https://github.com/kubernetes-sigs/cri-tools/pull/869

[SergeyKanzhelev]

It also will be a good notification for customers to migrate :-).

They don't have to "migrate", they just have to start using CRI

I meant migrate away from using dockershim.

[SergeyKanzhelev]

Is it that the defaults currently do not work? I thought the case is that the defaults are deprecated and will be removed at some point, but still work for now

yes, this is correct. I think the confusing here is around the language "default". Perhaps we can call it "well-known fallback options". And using fallback options is deprecated according to the message we output. In comment https://github.com/kubernetes-sigs/cri-tools/pull/871#discussion_r783673151 I suggested to reflect this by saying you MUSt specify the socket, but if you haven't, crictl will attempt to use well-known fallback options.

[shannonxtreme]

@SergeyKanzhelev and @afbjorklund in this case, should we leave dockershim.sock in the list of fallback options? If we keep it, we should mention in the docs that the endpoint won't work in 1.24+. The main changes for this PR would then just be to update the language around "default" behaviour in the warnings.

WDYT?

[afbjorklund]

@shannonxtreme The question is: can you use crictl 1.24 to control a cluster running kubernetes 1.23 ?

We currently install crictl as part of the OS (together with the container runtime). So it is the same crictl that is being used for all versions of Kubernetes, later on.

We do set the /etc/crictl.yaml (as instructed), so if it is "only" the list of default sockets...

docker@minikube:~$ cat /etc/crictl.yaml 
runtime-endpoint: unix:///var/run/dockershim.sock
image-endpoint: unix:///var/run/dockershim.sock
docker@minikube:~$ crictl --version
crictl version v1.21.0

But once you start changing the CRI version, this rule will become more important. Unless it is always backward compatible, so that the new tool can talk to old clusters ?

https://github.com/kubernetes-sigs/cri-tools/pull/871#issuecomment-1011495359

[afbjorklund]

And like I mentioned elsewhere, currently crictl 1.19 is being packaged and required...

So there is no mapping between crictl version and kubernetes version, in requirements.

Kubernetes Version	cri-tools Version	cri-tools branch
≥ 1.16.x	≥ 1.16.x	master

cri-tools/kubernetes-xenial 1.19.0-00 amd64

[shannonxtreme]

@afbjorklund I'm honestly not sure about the versioning thing, sorry. I'd assume if we left the list of fallback endpoints as-is, the behaviour wouldn't change, but I'm not positive.

Maybe someone from SIG node can confirm, or @SergeyKanzhelev.

[adisky]

I still think this is a nice workaround from @shannonxtreme

I guess a middle ground could be to move dockershim.sock to last on the
list so that it isn't the first endpoint tried by default

[shannonxtreme]

I made a few changes @afbjorklund and @SergeyKanzhelev:

• Add dockershim.sock as the last item in the list of "default" endpoints
• In the docs, position setting the endpoint manually as the recommended approach
• For the description of default endpoints in docs, add "unsupported in Kubernetes 1.25" to dockershim.sock

This should keep crictl working for previous versions of Kubernetes. We should remove the default endpoints at some point, preferable in a version release of crictl.

I'd like to get this change merged. This no longer breaks users on previous versions of K8s, makes the removal of dockershim in 1.25 clear, and doesn't require crictl to be versioned to work.

[afbjorklund]

makes the removal of dockershim in 1.25 clear,

was it moved again ? It was supposed to be 1.24

---

Otherwise it seems like a reasonable compromise, to emphasize the requirement to set up /etc/crictl.yaml.

Even better if this could be dragged out of the debugging shadow and into the installation light, in the docs ?

• https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/troubleshooting-kubeadm/

• https://kubernetes.io/docs/setup/production-environment/container-runtimes/

---

And to emphasize: there is no default endpoint (anymore)

There is no Dana, only Zuul

[shannonxtreme]

Lol it was not moved again, I just apparently had a brain fart. Thank you. That's also why I am trying to change the wording to be more "the fallback (or backup) endpoints are deprecated".

[shannonxtreme]

IDK why VS Code is adding the extra line 1 in main_unix.go and main_windows.go (//go:build !windows). Will it affect the file? I can't save the file without it being auto added

[afbjorklund]

IDK why VS Code is adding the extra line 1 in main_unix.go and main_windows.go (//go:build !windows). Will it affect the file? I can't save the file without it being auto added

If your gofmt is "too new", it will update the line to the new syntax (you can remove the old syntax)

https://go.googlesource.com/proposal/+/master/design/draft-gobuild.md

If you want to keep your code compiling with go1.16 as well, you can keep go1.17 but use gofmt1.16