More robust bollard interface for http client

[Chrisss93]

The experimental http client's new method is promoted to public which allows the user to select the appropriate inner bollard implementation for its docker client. In the dev branch the experimental client can only talk to to daemons over tcp, but this can be easily widened. This PR allows the experimental client to work both with local/remote docker daemons exposed as tcp endpoints, unix domain sockets or windows named pipes.

The experimental http client uses the DOCKER_HOST environment variable to choose the right bollard interface to talk to the docker http daemon (npipe, unix, tcp, tcp+tls).

[thomaseizinger]

What do you need to configure? We purposely don't expose the client to avoid bollard leaking into our public API.

[Chrisss93]

I see. The motivation here is to allow the crate's http client to talk to the daemon http API over unix sockets and windows named pipes, not just TCP. This currently is not possible based on the instantiation of the docker client (bollard crate does not seem to provide a single unified method to instantiate a client for tcp or unix or npipe). From my experience a dev machine is much more likely to have access to the daemon through socket/pipe while CI environments generally rely on TCP to talk to remote daemons. Does that make sense? Without this I've had to resort to a convoluted indirection of conditionally switching back and forth between the sync Cli client and async Http client depending on the executing environment and DOCKER_HOST env-var.

I understand you don't want to infect the public API of this crate with the bollard dependency. Do you think it would make sense for the Http API to continue to opaquely instantiate its internal bollard client, but via the default_bollard_client method in this PR?

[thomaseizinger]

The motivation here is to allow the crate's http client to talk to the daemon http API over unix sockets and windows named pipes, not just TCP. This currently is not possible based on the instantiation of the docker client (bollard crate does not seem to provide a single unified method to instantiate a client for tcp or unix or npipe).

Thank you for explaining your usecase!

I am open to addressing this problem. Can we just make the constructor smarter and attempt to connecting to the daemon over the different protocols?

1. Try to connect via socket
2. Try to connect via HTTP

At a later point, we can introduce an ENV var that allows overriding this.

Would that work for you?

[Chrisss93]

Thanks @thomaseizinger !

I am open to addressing this problem. Can we just make the constructor smarter and attempt to connecting to the daemon over the different protocols?

This would be a good first step but I don't believe we can implement some try-ish fallback within the constructor because bollard doesn't actually try to connect to the daemon when we instantiate its client. So we won't know if that protocol-specific client is right for the given environment until we start calling methods on the inner client. I could try to ping the daemon within the constructor but that would require new to be async which would be very odd.

[thomaseizinger]

Thanks!

Two comments.

[thomaseizinger]

I'd rather not add an additional feature-flag. Can we just always activate this?

[thomaseizinger]

We should connect based on the presence of DOCKER_CERT_PATH and not, whether the feature is enabled.

Features are additive so any dependency on your dependency tree can activate it which could break, how you are connecting to the docker daemon.

[thomaseizinger]

Looks good to me! Did you manage to test this manually that it works in all cases?

[tidal-gaute]

Hi! This is a great improvement, for us it'll simply running tests locally and in CI (which is a docker in docker setup).

Out of curiosity, is there a reason why using Bollard's Docker::connect_with_local_defaults isn't a good default?

e.g.

Http {
    inner: Arc::new(Client {
        command: env::command::<env::Os>().unwrap_or_default(),
        bollard: Docker::connect_with_local_defaults().unwrap(),
        created_networks: RwLock::new(Vec::new()),
    }),
}

[DDtKey]

It's gonna be addressed very soon as part of #563.

We gonna have several possible ways of configuration. Very similar to Go and Java implementations in terms of configuration.