[Feature] Support docker over ssh (#324, @ekristen & @inercia) #324
Conversation
ekristen
commented
Aug 8, 2020
•
ekristen
commented
- switches to using docker/cli @ 19.03
- introduces helper function for getting the docker client so docker over SSH can be supported
- also attempt to use remote docker host for API host info for kubeconfig
pkg/cluster/node.go
Outdated
| @@ -297,7 +297,13 @@ func patchServerSpec(node *k3d.Node) error { | |||
| node.Labels[k3d.LabelServerAPIHost] = node.ServerOpts.ExposeAPI.Host | |||
| node.Labels[k3d.LabelServerAPIPort] = node.ServerOpts.ExposeAPI.Port | |||
|
|
|||
| node.Args = append(node.Args, "--tls-san", node.ServerOpts.ExposeAPI.Host) // add TLS SAN for non default host name | |||
| // If the runtime is docker, attempt to use the docker host | |||
| if runtime.ID() == "docker" && runtime.GetHost() != "" { | |||
There was a problem hiding this comment.
| if runtime.ID() == "docker" && runtime.GetHost() != "" { | |
| if runtime.GetHost() != "" { |
If runtimes which don't support this just return an empty string, we can drop this runtime-specific check here, right?
pkg/cluster/node.go
Outdated
| node.Labels[k3d.LabelServerAPIHost] = node.ServerOpts.ExposeAPI.Host | ||
| node.Labels[k3d.LabelServerAPIPort] = node.ServerOpts.ExposeAPI.Port | ||
|
|
||
| node.Args = append(node.Args, "--tls-san", node.ServerOpts.ExposeAPI.Host) // add TLS SAN for non default host name | ||
| // If the runtime is docker, attempt to use the docker host | ||
| if runtime.ID() == "docker" && runtime.GetHost() != "" { | ||
| node.Labels[k3d.LabelServerAPIHostIP] = runtime.GetHost() | ||
| node.Labels[k3d.LabelServerAPIHost] = runtime.GetHost() | ||
| } | ||
|
|
||
| node.Args = append(node.Args, "--tls-san", node.Labels[k3d.LabelServerAPIHost]) // add TLS SAN for non default host name |
There was a problem hiding this comment.
Shouldn't we then already error out if a different Host was set for the ExposeAPI.Host?
There was a problem hiding this comment.
I don't believe so, but this is also where things get a bit wonky. From what I could tell the APIHost and APIHostIP almost exclusively assumed local host and 0.0.0.0 was going to be the value.
I originally tried just overriding APIHostIP globally at run-time via a --host parameter but that severely broke things in a number of ways, to include docker port forwarding.
That's when I found the TODO note a few lines up stating maybe this should be the IP of the docker host, that's when I added the runtime.ID() and runtime.GetHost() so that if we are on a remote host we set it on the label.
There was a problem hiding this comment.
Yep, what I meant was that we should throw an error or a warning when a user uses a remote docker connection (i.e. has the DOCKER_HOST env var set) but also tries to override the APIHost in the --api-port flag.
There was a problem hiding this comment.
I see, yes that could be a problem.
|
Hey @ekristen , are you still working on this? :) |
|
@iwilltry42 I got it working locally, but didn't have the time to see to the requested changes just yet. Thanks for bringing it back to my attention. I'll see if I can find some time to come back to this. |
|
Any progress on this? |
inercia
force-pushed
the
support-docker-over-ssh
branch
from
e025bd8
to
321ba91
Compare
|
I've just rebased this PR on top of main. It seems to be working locally for me. @iwilltry42 could you take a look when you have some time? |
inercia
force-pushed
the
support-docker-over-ssh
branch
from
40d93f1
to
edc1f70
Compare
Co-authored-by: Thorsten Klein <thorsten.klein.95@googlemail.com>
inercia
force-pushed
the
support-docker-over-ssh
branch
from
edc1f70
to
7356c95
Compare
iwilltry42
changed the title
|
@iwilltry42 thanks for finishing it up and merging. Apologies for my lack of response. I've been extraordinarily busy and have been playing catch up on email this week. |
|
All thanks to @inercia here 👍 |
