Stop using the old github.com/docker/docker package paths#6692
Conversation
dosubot
Bot
added
the
size:M
|
Ephemeral COPR build failed. @containers/packit-build please check. |
|
Apparently I broke something, looking… |
|
|
||
| // connect to dockerd using go-dockerclient | ||
| client, err := docker.NewVersionedClientFromEnv(dockerapi.DefaultVersion) | ||
| client, err := docker.NewVersionedClientFromEnv(dockerdockerclient.MaxAPIVersion) |
There was a problem hiding this comment.
This is newer than the version of dockerd our conformance tests are talking to knows about.
There was a problem hiding this comment.
Yes. What was the motivation for 8d6f3fc ?
I don’t understand using fsouza/go-dockerclient with “the latest version supported by docker/docker/client”; the two are not obviously related. (And I can’t see any API to see what is the latest version supported by fsouza/go-dockerclient.)
AFAICT the daemon treats API requests with no version specified as "the server’s current version”, so the previous version of that was already using “whatever is newest” and assuming that fsouza/go-dockerclient can handle it.
So I’m going to try reverting 8d6f3fc .
Alternatively, we could hard-code 1.51 here to exactly preserve today’s behavior.
There was a problem hiding this comment.
… OK I see why that was necessary. Hard-coding 1.51 then, and adding a comment outlining the reasoning.
There was a problem hiding this comment.
When not specified, an API version is set in BuildImage() based on which fields we set in the build options structure that we pass to it. For the conformance tests, this often ends up landing on 1.39 or 1.25, depending on which fields we specify.
The version of the daemon in our debian VMs only understands 1.50, so tests were failing when attempting to use 1.51.
There was a problem hiding this comment.
Hmm, I must be mistaken about the version of dockerd in the VM - I guess that was a limitation only for imagebuilder.
mtrmac
force-pushed
the
docker-client
branch
2 times, most recently
from
29ba708 to
12bd50e
Compare
|
(I have also added a commit to rename the package references, to be hopefully less confusing in the far future.) |
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac
force-pushed
the
docker-client
branch
from
12bd50e to
508e415
Compare
|
Tests are passing now. |
lsm5
left a comment
lsm5
left a comment
There was a problem hiding this comment.
LGTM
@containers/buildah-maintainers PTAL
3 participants
What type of PR is this?
What this PR does / why we need it:
Update the Docker client to a newer version and the updated module paths / API.
Some indirect references remain in
github.com/fsouza/go-dockerclientandgithub.com/openshift/imagebuilder.How to verify it
Existing tests
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
This is a subset of #6691 , useful there to limit the scope of the audit. And I think it needs to happen either way.
Does this PR introduce a user-facing change?