Optionally use ssh for docker-env instead of tcp #9548
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afbjorklund 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 |
k8s-ci-robot
added
size/M
Codecov Report
@@ Coverage Diff @@
## master #9548 +/- ##
==========================================
+ Coverage 29.05% 29.11% +0.05%
==========================================
Files 172 172
Lines 10482 10510 +28
==========================================
+ Hits 3046 3060 +14
- Misses 7011 7025 +14
Partials 425 425
|
Since Docker 18.09, it supports ssh: as well as tcp: for connecting to the docker daemon (in $DOCKER_HOST). This avoids setting up certificates, but leaves key handling (host keys and identity keys) to ssh config.
afbjorklund
force-pushed
the
docker-env-ssh
branch
from
90581ab
to
7841aea
Compare
|
this is a cool PR, I look forward to see if this will fix some docker-env cert issues |
|
/ok-to-test do. youmind adding an integeration test for this , maybe under funcationl test |
k8s-ci-robot
added
the
ok-to-test
|
kvm2 Driver |
medyagh
requested changes
Oct 28, 2020
For some reason we always need to clean up Docker's mess (undocumented DOCKER_HOST) https://docs.docker.com/engine/reference/commandline/dockerd/
|
|
We could export the value in a variable, but it seems a bit misleading if Docker is not using it ? Currently we are using a special So we need to add the identity key with ssh-add cmd, and the host keys to known_hosts file |
|
Probably they will use the ssh tunnel rather than the main ip, but same difference... Most likely we want to only show the That way there is only one host key to worry about, the comment (#) only goes on stderr: $ ssh-keyscan -t rsa -p 35717 127.0.0.1
# 127.0.0.1:35717 SSH-2.0-OpenSSH_8.1
[127.0.0.1]:35717 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCvysZKhUslF/dc3yz0d1anVgIRYuQ9rofRhx1UwTWVTlNqpVDW47Bf1NSigEn59xgt9srfoTQLg2Zd0obPCOSw+1teMMVKl+C887lZ+VtfggzOdzT9dRiy37pvmYkBhMctS1G/wqn/hKUL4YWgBQ68ZTLKDww0Caiy+YA5qaF2qhlaxKPpsiQ/icoxPFGa/JD/1nTGYvs9rO+GLiXdmv3QlvzRo8i+gC4+4h4k7XXVEK/9ihRvs9zIou09mOLogM0hiPSVNK+yu1PW7YphBmGFMT07CsfJnfwO8+Cg1lW2MFbypgbwLzShxdmklugCI5qCrpNmvORS4ullq4Hxc/rSaj1WTyTW0J6YOrB1YmVL8CiaM1TrH/kOvmB8ZaBmqG8tSYoPpJN90s4e6Fla9NTIpwnxG0mwSv416/2q8/tmEsdmQEnCbAUghwCEeQkcodfFQYUCPRHenIHV/CLlnkRvOr7tR/DYYCkgppRZqqNCJNLgIeI8e3fYY/00gkHbRqc=
|
|
One weird thing is that it doesn't actually talk directly to the docker socket, it starts a
So the client will need a
|
k8s-ci-robot
added
size/L
|
kvm2 Driver Times for Minikube (PR 9548): 64.5s 62.5s 64.6s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 30.0s 29.5s 28.6s Averages Time Per Log |
|
@medyagh now there is a $ eval $(minikube docker-env --ssh-host --ssh-add) |
Travis tests have failedHey @afbjorklund, 1st Buildmake testTravisBuddy Request Identifier: 2f172e40-21ce-11eb-a04c-b76cbd6ddda3 |
|
kvm2 Driver Times for Minikube (PR 9548): 56.6s 55.5s 54.7s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 27.0s 26.8s 27.1s Averages Time Per Log |
afbjorklund
force-pushed
the
docker-env-ssh
branch
from
da09588
to
88b8843
Compare
Travis tests have failedHey @afbjorklund, 1st Buildmake testTravisBuddy Request Identifier: d0576f30-21cf-11eb-a04c-b76cbd6ddda3 |
|
kvm2 Driver Times for Minikube (PR 9548): 68.6s 71.1s 70.0s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 42.2s 42.8s 42.0s Averages Time Per Log |
|
we will do this PR after the release and also after the non-defaut docker PR is merged to ensure they work well together |
1.15.0 Release
|
kvm2 Driver Times for Minikube (PR 9548): 71.0s 70.4s 73.7s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 44.1s 39.5s 42.7s Averages Time Per Log |
k8s-ci-robot
added
the
needs-rebase
1.15.1 Release
|
kvm2 Driver Times for Minikube (PR 9548): 22.7s 56.8s 59.4s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 29.4s 26.9s 28.2s Averages Time Per Log |
k8s-ci-robot
removed
the
needs-rebase
|
kvm2 Driver Times for Minikube (PR 9548): 69.4s 65.1s 63.1s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 31.3s 32.0s 32.4s Averages Time Per Log |
k8s-ci-robot
added
the
needs-rebase
k8s-ci-robot
removed
the
needs-rebase
|
kvm2 Driver Times for Minikube (PR 9548): 54.7s 54.7s 61.2s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 28.6s 28.4s 27.7s Averages Time Per Log |
|
kvm2 Driver Times for Minikube (PR 9548): 69.7s 60.5s 63.1s Averages Time Per Log docker Driver Times for Minikube (PR 9548): 30.4s 29.7s 30.1s Averages Time Per Log |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since Docker 18.09, it supports ssh: as well as tcp:
for connecting to the docker daemon (in $DOCKER_HOST).
This avoids setting up certificates, but leaves key
handling (host keys and identity keys) to ssh config.
See #9229
Once this is default, we only have to set up ssh...
And it will use
/var/run/docker.sock, over ssh.