-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Make the default container runtime dynamic #13251
Conversation
c21f216
to
96b8857
Compare
Since the dockershim removal, there is no longer a constant default container runtime provided by upstream. Only CRI.
96b8857
to
c4800a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this PR mean, if minikube default kubernete is 1.24+
minikube start
without any args will start with containerd ?
You don't know. It is dynamic and random, depending on k8s version. // defaultRuntime returns the default container runtime
func defaultRuntime(k8sVersion string) string It might be docker, it might be containerd. Something following the CRI. If you want a special one, you have to use But no changes yet, the minikube default CRI is the same as it was before. People should still start to write --container-runtime=docker if they need it. For instance, it is required if you plan on using |
so this might not be what some of our embedded users expect like "skaffold" who would expect docker runtime by default, this would make them have to explicitly add the flag.... I assume this means for 1.24+ k8s versions the dynamic will choose containerd ? |
they will need to start doing that, since only CRI is guaranteed in future k8s versions all the special hacks and privileges of "assuming" a docker daemon, are no longer true
I gave up on that concept, since skaffold is dragging their legs and since docker is still the most popular anyway, which container runtime should be selected by default was moved into a different issue once skaffold is on track again, and there are lots of requests, the default could be changed. |
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 48.0s 44.1s 44.2s 44.0s 43.9s Times for minikube ingress: 29.5s 30.0s 41.5s 31.1s 31.0s docker driver with docker runtime
Times for minikube (PR 13251) start: 25.9s 25.0s 26.0s 25.9s 26.6s Times for minikube ingress: 24.5s 26.0s 26.9s 23.9s 71.9s docker driver with containerd runtime
Times for minikube start: 33.3s 40.2s 40.5s 41.6s 41.3s Times for minikube ingress: 31.4s 25.5s 25.4s 25.4s 23.4s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
the DownloadOnly tests are failing because we're not always passing in container-runtime into our tests suites (and therefore into our minikube start calls). this is usually fine since we default back to docker in a normal codepath, but these tests call download.PreloadExists directly, and then passes and an empty string for containerRuntime (since ContainerRuntime() in main_test.go defaults to constants.DefaultContainerRuntime. the solution i think is to change one line (main_test.go at L165) that to constants.Docker. |
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 54.3s 53.4s 52.6s 52.9s 53.0s Times for minikube ingress: 26.7s 30.6s 30.1s 30.1s 32.0s docker driver with docker runtime
Times for minikube ingress: 26.4s 23.9s 22.4s 22.5s 22.9s Times for minikube start: 26.8s 26.3s 26.7s 27.4s 27.1s docker driver with containerd runtime
Times for minikube start: 36.4s 45.9s 45.5s 45.3s 42.1s Times for minikube ingress: 20.0s 20.9s 19.4s 18.9s 23.4s |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afbjorklund, sharifelgamal The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
Since the dockershim removal, there is no longer a constant
default container runtime provided by upstream. Only CRI.
Closes #13250