testutil: copy slim version of the docker client into testutil#5731
testutil: copy slim version of the docker client into testutil#5731tonistiigi merged 1 commit intomoby:masterfrom
Conversation
723c3dc to
85a8590
Compare
85a8590 to
2d26d6c
Compare
|
@jsternberg Can you open a draft PR from this branch in moby so we can see the tests with |
2d26d6c to
35d84a6
Compare
|
This is passing in the draft PR. I had to rebase this PR to be based on v0.19 though to make it work because of unrelated API changes. I'll rebase it back and resolve the conflicts. |
35d84a6 to
2cea406
Compare
2cea406 to
f81eeb4
Compare
f81eeb4 to
0c8625f
Compare
|
@tonistiigi applied the feedback. |
tonistiigi
left a comment
There was a problem hiding this comment.
Looks good. Rerun the Moby PR after updating as well.
| c.proto = hostURL.Scheme | ||
| c.addr = hostURL.Host | ||
| c.basePath = hostURL.Path | ||
| if tr, ok := c.client.Transport.(*http.Transport); ok { |
There was a problem hiding this comment.
This doesn't seem to do anything based on host, so can be in the default constructor.
There was a problem hiding this comment.
I tried changing this but it does seem to matter. It does things based on the proto and host and has been the cause of the new failures so I'm going to revert this back to what's in the original client.
0b451d4 to
418b39a
Compare
tonistiigi
left a comment
There was a problem hiding this comment.
Code looks ok, but seems latest PR in moby has some issues. I see errors from ping there.
| return nil, errors.New("No host URL specified in client") | ||
| } | ||
|
|
||
| if tr, ok := c.client.Transport.(*http.Transport); ok { |
There was a problem hiding this comment.
Not a big issue, but this type-cast still seems unnecessary. *http.Transport comes directly from defaultHTTPClient so you could just call defaultHTTPTransport instead to get it with the correct type.
418b39a to
414325d
Compare
|
@jsternberg Check the Moby PR |
0b6bebd to
15decde
Compare
15decde to
2dee7fe
Compare
Copies a slim version of the docker client with only the necessary methods so we can break our dependency on the client in moby. This client is only used in an integration test so it's not really needed and we don't really actively need updates or to be on the most recent API version since we just do an unversioned ping and then call the hijack method. This was created by copying the package into `testutil` and then deleting unused sections of code. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
2dee7fe to
0a7f949
Compare
|
In follow-up we should remove the |
Copies a slim version of the docker client with only the necessary methods so we can break our dependency on the client in moby. This client is only used in an integration test so it's not really needed and we don't really actively need updates or to be on the most recent API version since we just do an unversioned ping and then call the hijack method.
This was created by copying the package into
testutiland then deleting unused sections of code.Fixes #3355.