-
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
feat: direct image build on containerd via docker-env #15452
Conversation
Hi @ComradeProgrammer. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
Can one of the admins verify this patch? |
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.
thank you for this PR, do you mind adding in the PR description an example of using it ? with the output
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.
I was imagining the nerdctl support to be much more similar to the podman support
d53ebd2
to
0c67a19
Compare
By the way could you please give me a more specific hint about how podman is provisioned with the node ? I didn't find that. So I just moved the downloading procedure to the 'minikube start' phase, after the node is provisioned |
https://github.com/kubernetes/minikube/blob/v1.28.0/deploy/kicbase/Dockerfile#L188 Eventually it is supposed to be done by the provisioner (in libmachine), but that is not working at the moment. So eventually, it would be nice if minikube knew how to install a container runtime on a node (a.k.a. provisioning) But at the moment, users of the "none" driver (or the generic "ssh" driver) will just have to install it themselves. Since the issues were written, the CRI and CNI components have moved to be included with Kubernetes: |
0c67a19
to
70c396d
Compare
@afbjorklund Thanks for your guidance. Now nerdctld will be started when minikube starts, and now nerdctld listen on nerdctld instead of docker.sock. Is there any other suggestions? Or revisions I need to make? |
can u plz update the PR description with the example output of using it? |
Of course, I will update it very soon. |
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.
lets mark this as beta feature, and print to user that this might not be perfect.
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.
lets add a case for containerd in DockerEnv test inside functional tests. if it is containerd , it should run it with this new feature
70c396d
to
815007c
Compare
a test case has been added |
Anything I need to do next? |
Will take another look, if you feel that it is done. |
815007c
to
cf9cfed
Compare
3ded273
to
3a1686e
Compare
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 51.5s 52.3s 52.4s 52.9s 49.8s Times for minikube ingress: 27.7s 29.3s 28.2s 27.3s 28.2s docker driver with docker runtime
Times for minikube start: 25.4s 22.1s 24.8s 25.7s 23.4s Times for minikube ingress: 48.3s 48.4s 51.3s 49.9s 48.4s docker driver with containerd runtime
Times for minikube start: 23.6s 22.6s 21.2s 24.3s 21.0s Times for minikube ingress: 32.4s 31.4s 31.4s 31.3s 31.4s |
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
Now the PR has be rebased and modified, and now we no longer require the user to execute something like The new usage information has been updated in the PR's explanation. Please have a look at it again if you have time. Thanks. |
ae624d9
to
2e4912e
Compare
2e4912e
to
a814542
Compare
kvm2 driver with docker runtime |
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
ok-to-build-image |
Hi @ComradeProgrammer, we have updated your PR with the reference to newly built kicbase image. Pull the changes locally if you want to test with them or update your PR further. |
kvm2 driver with docker runtime
Times for minikube ingress: 28.2s 28.2s 28.2s 28.2s 29.7s Times for minikube start: 50.5s 52.5s 51.9s 51.1s 51.7s docker driver with docker runtime
Times for minikube ingress: 48.8s 49.3s 48.3s 48.9s 48.3s Times for minikube start: 21.7s 24.3s 25.4s 24.7s 22.3s docker driver with containerd runtime
Times for minikube start: 24.6s 23.4s 20.3s 23.8s 22.7s Times for minikube ingress: 31.4s 30.3s 31.4s 30.3s 31.4s |
kvm2 driver with docker runtime
Times for minikube start: 51.1s 48.2s 50.3s 52.5s 52.3s Times for minikube ingress: 27.2s 28.2s 27.2s 26.8s 26.7s docker driver with docker runtime
Times for minikube start: 24.4s 22.5s 24.7s 21.8s 23.1s Times for minikube (PR 15452) ingress: 48.9s 27.9s 32.9s 28.9s 49.4s docker driver with containerd runtime
Times for minikube ingress: 31.4s 31.4s 19.4s 32.4s 31.3s Times for minikube start: 24.3s 23.2s 20.9s 20.8s 23.0s |
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. |
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. |
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.
thank you for delivering this great feature @ComradeProgrammer and also thank you @spowelljr for the SSH agent that was required for this PR and follow up fixes
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ComradeProgrammer, medyagh, spowelljr 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 |
fixes #15389
This pr enable the use of
minikube docker-env
command, so that the minikube can be used as docker, even when the runtime is containerdBefore:
After:
$ minikube docker-env --ssh-host --ssh-add
stdout:
stderr:
$ DOCKER_HOST="ssh://docker@127.0.0.1:49157" docker version
Note: when you execute
docker ps
, and see this outputThen just copy the ssh-keygen command in the output and execute it.
then execute
minikube ssh-host --append-known
minikube docker-env --ssh-host --ssh-add
again.It will work