Docker: restore "from env" behavior, and perform API version negotiation #270
Conversation
testing/docker.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| c.NegotiateAPIVersion(context.TODO()) |
There was a problem hiding this comment.
This could be passed as an option to NewClientWithOpts after updating to a more recent client that has the WithAPIVersionNegotiation option (added in moby/moby#39032), but that's only a small improvement, and I wanted to keep the diff small
There was a problem hiding this comment.
Let's use the dockerclient.WithAPIVersionNegotiation option instead if it's not too much work to update the docker client version we're currently pinned to.
testing/docker.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| c.NegotiateAPIVersion(context.TODO()) |
There was a problem hiding this comment.
Let's use the dockerclient.WithAPIVersionNegotiation option instead if it's not too much work to update the docker client version we're currently pinned to.
thaJeztah
force-pushed
the
docker_api_version_negotiation
branch
from
dfc35e9 to
841161a
Compare
|
Updated 👍 |
|
oh, looks like I need to rebase, as it's showing as outdated |
commit c31948c replaced the deprecated `dockerclient.NewEnvClient` with `dockerclient.NewClientWithOpts`, but did not add the `FromEnv` option to keep the old behavior. Signed-off-by: Sebastiaan van Stijn <[email protected]>
This helps in situations where the daemon is older than the API client. API version negotiation checks the maximum supported API version by the daemon, and downgrades to that API version if needed. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
docker_api_version_negotiation
branch
from
841161a to
014d1ba
Compare
|
done |
|
FYI, these changes won't take affect since they're against a deprecated unused package. The package has been left around since it was publicly available. I'll still merge the PR since it's still useful as a reference 😉 |
|
Builds have been fix, could you rebase/merge? Edit: Apparently I have the ability to merge to your repo/branch, so I took the liberty to do so |
Pull Request Test Coverage Report for Build 515
💛 - Coveralls |
|
Thanks! |
This should help with #254 (and #252), where CI failed because the travis was running an older Docker version.