connhelper: use ssh multiplexing

[tonistiigi]

Add multiplexing to ssh connection helper so commands that do multiple requests share the connection. Environment variables for opting out and controlling the persist time for even better connection sharing.

Before:

 # time docker run alpine true
docker run alpine true  0.04s user 0.03s system 1% cpu 4.373 total
 # time docker build . 2>/dev/null
docker build . 2> /dev/null  0.05s user 0.03s system 3% cpu 2.312 total

After

 # time docker run alpine true
docker run alpine true  0.04s user 0.03s system 2% cpu 2.376 total
 # time docker build . 2>/dev/null
docker build . 2> /dev/null  0.04s user 0.03s system 5% cpu 1.295 total

@AkihiroSuda @cpuguy83 @tiborvass

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

[AkihiroSuda]

SGTM, but IIRC we intentionally avoided having socket files, for security purpose

[AkihiroSuda]

That's why we are using stdio instead of ssh -L

[AkihiroSuda]

What happens if .ssh/config already has this?

[tonistiigi]

It overwrites afaik. This is the main reason I left a way to opt-out. If we want to be conservative about it, one way would be to use a different helper prefix.

[AkihiroSuda]

Probably we should create a new directory, because when ~/.docker already exists with other permission bits, we can't guarantee the sockets are created under a directory with 0700.
On my Macintosh, ~/.docker was somehow created with 0755.

But it is more likely to hit UNIX_PATH_MAX (108)... 😢

[AkihiroSuda]

Probably we can let commandconn change working directory for avoiding 108 limit

[tonistiigi]

The socket itself is created with 0600. This mkdir is to avoid the case where .docker does not exist.

[AkihiroSuda]

@justincormack PTAL?

[justincormack]

Is there any reason not to have a Docker config option for this as well?

[tonistiigi]

If you would ever set them they would probably be more dependent on the actual DOCKER_HOST value, not a global client config. I'm not even sure we even want to document these, it should be very advanced usage if you want to set them.

[thaJeztah]

ping @justincormack pal ^

[AkihiroSuda]

ping @tonistiigi

[thaJeztah]

@AkihiroSuda ^ looking good for you, or still concerns / changes needed?

[thaJeztah]

can you document the new environment variables in https://github.com/docker/cli/blob/master/docs/reference/commandline/cli.md#environment-variables ?

[thaJeztah]

LGTM