Skip to content

Commit 6806b83

Browse files
committed
ConfigureTransport: enable compression for non unix/npipe protos
`ConfigureTransport` is used by the official Docker client to set up its `http.Transport`: * [`NewClientWithOpts()`][1] starts by calling `defaultHTTPClient()` which creates a new `http.Transport` and call `ConfigureTransport` with either the default unix socket address, or the default npipe socket address (see [here][2]). This first call to `ConfigureTransport` will set `DisableCompression` to false. * Then, when [`WithHost()`][3] is called, `ConfigureTransport` is called once again with the same `http.Transport` instance. If that hostURL uses tcp proto, we fall in `ConfigureTransport`'s default switch case, which doesn't reset `DisableCompression`. This effectively means that (presumably) **the Docker client has never been using HTTP compression for TCP hosts.** Note that `WithHost` is called whenever the `WithHostFromEnv` or `FromEnv` option funcs are used. We think it _does_ make sense to keep compression disabled for unix / npipe sockets (it would just make the CPU needlessly spin more if we enable it), and we think it generally also makes sense to enable it for HTTP over TCP. We discussed creating a new `http.Transport` in Docker client codebase whenever `ConfigureTransport` is called, but we can't do that without introducing major side-effects (see [here][4]). So we still have to leave in a world where multiple calls to `ConfigureTransport` might be made for the same `http.Transport`. As such, this commit make sure compression is enabled for non unix / npipe protos. If `ConfigureTransport` callers really want to make sure compression is disabled, they should set `DisableCompression` _only after_ all subsequent calls to `ConfigureTransport` have been made. The comment on that function has also been updated to reflect that. For the record, `DisableCompression = false` was first introduced in this repo by 88d5fb2, which can be tracked down to this initial change in moby/moby: moby/moby@fb7ceeb [1]: https://github.com/moby/moby/blob/e9efc0a361de39903d47fe12f628c92fc095397b/client/client.go#L181-L197 [2]: https://github.com/moby/moby/blob/e9efc0a361de39903d47fe12f628c92fc095397b/client/client.go#L237-L238 [3]: https://github.com/moby/moby/blob/e9efc0a361de39903d47fe12f628c92fc095397b/client/options.go#L71-L73 [4]: docker#103 (comment) Signed-off-by: Albin Kerouanton <[email protected]>
1 parent 12f72d4 commit 6806b83

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

sockets/sockets.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@ const defaultTimeout = 10 * time.Second
1313
// ErrProtocolNotAvailable is returned when a given transport protocol is not provided by the operating system.
1414
var ErrProtocolNotAvailable = errors.New("protocol not available")
1515

16-
// ConfigureTransport configures the specified Transport according to the
17-
// specified proto and addr.
18-
// If the proto is unix (using a unix socket to communicate) or npipe the
19-
// compression is disabled.
16+
// ConfigureTransport configures the specified [http.Transport] according to the specified proto
17+
// and addr.
18+
//
19+
// If the proto is unix (using a unix socket to communicate) or npipe the compression is disabled.
20+
// For other protos, compression is enabled. If you want to manually enable/disable compression,
21+
// make sure you do it _after_ any subsequent calls to ConfigureTransport is made against the same
22+
// [http.Transport].
2023
func ConfigureTransport(tr *http.Transport, proto, addr string) error {
2124
switch proto {
2225
case "unix":
@@ -25,6 +28,7 @@ func ConfigureTransport(tr *http.Transport, proto, addr string) error {
2528
return configureNpipeTransport(tr, proto, addr)
2629
default:
2730
tr.Proxy = http.ProxyFromEnvironment
31+
tr.DisableCompression = false
2832
tr.DialContext = (&net.Dialer{
2933
Timeout: defaultTimeout,
3034
}).DialContext

0 commit comments

Comments
 (0)