This commit replaces `os.Setenv` with `t.Setenv` in tests. The
environment variable is automatically restored to its original value
when the test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.Setenv
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Just a small clean-up (there's more endpoints to do this for, but
I was working on changes in this area on the CLI when I noticed we
were setting this query-parameter unconditionally.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was added in 93c3e6c91e, at which time only
some basic handling of non-succesful status codes was present;
93c3e6c91e/api/client/utils.go (L112-L121)
Given that since 38e6d474af non-successful status-
codes are already handled, and a 204 ("no content") status should not be an error,
this special case should no longer be needed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
both -1 and 0 are accepted as "no limit", so don't send the
limit option if no limit was set. For simplicity, we're ignoring
values <= 0.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The wrapResponseError() utility converted some specific errors, but in
doing so, could hide the actual error message returned by the daemon.
In addition, starting with 38e6d474af,
HTTP status codes were already mapped to their corresponding errdefs
types on the client-side, making this conversion redundant.
This patch removes the wrapResponseError() utility; it's worth noting
that some error-messages will change slightly (as they now return the
error as returned by the daemon), but may cointain more details as
before, and in some cases prevents hiding the actual error.
Before this change:
docker container rm nosuchcontainer
Error: No such container: nosuchcontainer
docker container cp mycontainer:/no/such/path .
Error: No such container:path: mycontainer:/no/such/path
docker container cp ./Dockerfile mycontainer:/no/such/path
Error: No such container:path: mycontainer:/no/such
docker image rm nosuchimage
Error: No such image: nosuchimage
docker network rm nosuchnetwork
Error: No such network: nosuchnetwork
docker volume rm nosuchvolume
Error: No such volume: nosuchvolume
docker plugin rm nosuchplugin
Error: No such plugin: nosuchplugin
docker checkpoint rm nosuchcontainer nosuchcheckpoint
Error response from daemon: No such container: nosuchcontainer
docker checkpoint rm mycontainer nosuchcheckpoint
Error response from daemon: checkpoint nosuchcheckpoint does not exist for container mycontainer
docker service rm nosuchservice
Error: No such service: nosuchservice
docker node rm nosuchnode
Error: No such node: nosuchnode
docker config rm nosuschconfig
Error: No such config: nosuschconfig
docker secret rm nosuchsecret
Error: No such secret: nosuchsecret
After this change:
docker container rm nosuchcontainer
Error response from daemon: No such container: nosuchcontainer
docker container cp mycontainer:/no/such/path .
Error response from daemon: Could not find the file /no/such/path in container mycontainer
docker container cp ./Dockerfile mycontainer:/no/such/path
Error response from daemon: Could not find the file /no/such in container mycontainer
docker image rm nosuchimage
Error response from daemon: No such image: nosuchimage:latest
docker network rm nosuchnetwork
Error response from daemon: network nosuchnetwork not found
docker volume rm nosuchvolume
Error response from daemon: get nosuchvolume: no such volume
docker plugin rm nosuchplugin
Error response from daemon: plugin "nosuchplugin" not found
docker checkpoint rm nosuchcontainer nosuchcheckpoint
Error response from daemon: No such container: nosuchcontainer
docker checkpoint rm mycontainer nosuchcheckpoint
Error response from daemon: checkpoint nosuchcheckpoint does not exist for container mycontainer
docker service rm nosuchservice
Error response from daemon: service nosuchservice not found
docker node rm nosuchnode
Error response from daemon: node nosuchnode not found
docker config rm nosuchconfig
Error response from daemon: config nosuchconfig not found
docker secret rm nosuchsecret
Error response from daemon: secret nosuchsecret not found
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The API defines a default limit for searches, but when removing the
default from the cli, the client still sends "0" as a limit, which
is not allowed by existing versions of the API:
docker search --limit=0 busybox
Error response from daemon: Limit 0 is outside the range of [1, 100]
This patch changes the client so that no limit is sent if none was set ("0"),
allowing the daemon to use its (or the registry's) default.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This adds consts for the environment variables that are supported
by the client. These environment variables are unlikely to change,
or at least, unlikely to be removed, but having consts allows for
them to be documented.
I did not change all occurrences of these variables to use the const,
as they're used in various tests, and it's ok to use a fixture for
those, but it's nice to have a const available for (external) consumers
of the client package, and to have their purpose (and caveats)
documented in the code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Improve documentation of various functions to better describe their behavior.
- Rename some variables to be more descriptive (as this is client code, used
by external consumers, it's nice to be a bit more explicit).
- Remove a redundant check in `WithVersionFromEnv()`, as `WithVersion()`
already checks for empty values.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was setting the non-exported `Client.basePath` directly, however,
it was setting it to a value that would never realistically happen, because
`NewClientWithOpts()` initializes the Client with the default API version:
ea5b4765d9/client/client.go (L119-L130)
Which is used by `getAPIPath()` to construct the URL/path:
ea5b4765d9/client/client.go (L176-L190)
While this didn't render the test "invalid", using a Client that's constructed
in the usual way, makes it more representative.
Given that we deprecated (but still support) the non-versioned API paths, with
the exception of the `/_ping` API endpoint, we should probably change `getAPIPath()`
to default to the "current version", instead of allowing it to use an empty string.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- avoid accessing non-exported fields where possible, and test using accessors
instead, so that we're closer to how it's actually used.
- use a variable or const for "expected" in some tests, so that "expected" is
printed as part of the test-failure output (instead of just a "value").
- swap the order of "actual" and "expected" for consistency, and to make it
easier to see what the "expected" value is in some cases ("expected" on the
right, so that it reads `val (actual) != val (expected)`).
- don't set fields in the Ping response that are not relevant to the test.
- rename some variables for consistency.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
I think this was there for historic reasons (may have been goimports expected
this, and we used to have a linter that wanted it), but it's not needed, so
let's remove it (to make my IDE less complaining about unneeded aliases).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The client would always send a value, even if no `condition` was set;
Calling POST /v1.41/containers/foo/wait?condition=
This patch changes the client to not send the parameter if it's empty (and the
API default value should be used).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As caught by gosimple:
client/client.go:138:14: S1040: type assertion to the same type: c.client.Transport already has type http.RoundTripper (gosimple)
if _, ok := c.client.Transport.(http.RoundTripper); !ok {
^
This check was originally added in dc9f5c2ca3, to
check if the passed option was a `http.Transport`, and later changed in
e345cd12f9 to check for `http.RoundTripper` instead.
Client.client is a http.Client, for which the Transport field is a RoundTripper,
so this check is redundant.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's deprecated in Go 1.18:
client/request.go:157:8: SA1019: err.Temporary is deprecated: Temporary errors are not well-defined. Most "temporary" errors are timeouts, and the few exceptions are surprising. Do not use this method. (staticcheck)
if !err.Temporary() {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Unify the NetworkInspect tests to remove some boilerplating
Before this change:
go test -v -run TestNetworkInspect ./client/
=== RUN TestNetworkInspectError
--- PASS: TestNetworkInspectError (0.00s)
=== RUN TestNetworkInspectNotFoundError
--- PASS: TestNetworkInspectNotFoundError (0.00s)
=== RUN TestNetworkInspectWithEmptyID
--- PASS: TestNetworkInspectWithEmptyID (0.00s)
=== RUN TestNetworkInspect
--- PASS: TestNetworkInspect (0.00s)
PASS
ok github.com/docker/docker/client 0.010s
With this change:
go test -v -run TestNetworkInspect ./client/
=== RUN TestNetworkInspect
=== RUN TestNetworkInspect/empty_ID
=== RUN TestNetworkInspect/no_options
=== RUN TestNetworkInspect/verbose
=== RUN TestNetworkInspect/global_scope
=== RUN TestNetworkInspect/unknown_network
=== RUN TestNetworkInspect/server_error
--- PASS: TestNetworkInspect (0.00s)
--- PASS: TestNetworkInspect/empty_ID (0.00s)
--- PASS: TestNetworkInspect/no_options (0.00s)
--- PASS: TestNetworkInspect/verbose (0.00s)
--- PASS: TestNetworkInspect/global_scope (0.00s)
--- PASS: TestNetworkInspect/unknown_network (0.00s)
--- PASS: TestNetworkInspect/server_error (0.00s)
PASS
ok github.com/docker/docker/client 0.012s
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This function was added in a754d89b40, but not
used. Currently, the only consumer of this function I could find was docker/cli,
which used it in a unit-test (this test has already been updated to not depend
on this function); https://grep.app/search?q=.CustomHTTPHeaders%28%29&filter[lang][0]=Go
Given that commit a68ae4a2d9 deprecated the
corresponding client.SetCustomHTTPHeaders() function, and because there is no
active use for this function, it should be ok to deprecate.
We can include this in a patch-release (to be sure nobody else is depending on
it, and (if someone is) to notify them of the deprecation.
As a follow-up to this commit, I'll remove both functions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Let clients choose object types to compute disk usage of.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
client/request.go:245:2: S1031: unnecessary nil check around range (gosimple)
if headers != nil {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Using "/go/" redirects for some topics, which allows us to
redirect to new locations if topics are moved around in the
documentation.
- Updated some old URLs to their new location.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Changes certain words and adds punctuation to the comments of functions in the client package, which end up in the GoDoc documentation. Areas where only periods were needed were ignored to prevent excessive code churn.
Signed-off-by: Levi Harrison <levisamuelharrison@gmail.com>
As `NewEnvClient` is deprecated in favor of `NewClientWithOpts`, the main package documentation should reflect this. This is also the text that appears on godoc.org so it's quite important that it is correct (for newbies like me)
Signed-off-by: Tobias Pfandzelter <pfandzelter@campus.tu-berlin.de>
- `ContainerSpec` and `PluginSpec` are mutually exclusive, so instead of using
two separate if-statements, combine them in a switch.
- Use local variables (at cost of some slight duplication)
- Fix a potential NPE if image-digest resolution failed for a `PluginSpec`.
The code was always using `ContainerSpec.Image` to create a `digestWarning`,
but in case we're resoling the digest for a `PluginSpec`, `ContainerSpec`
will be `nil` (as they're mutually exclusive). This issue was introduced in
72c3bcf2a5, where the new `PluginSpec` path
was added.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This enables image lookup when creating a container to fail when the
reference exists but it is for the wrong platform. This prevents trying
to run an image for the wrong platform, as can be the case with, for
example binfmt_misc+qemu.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Metrics collectors generally don't need the daemon to prime the stats
with something to compare since they already have something to compare
with.
Before this change, the API does 2 collection cycles (which takes
roughly 2s) in order to provide comparison for CPU usage over 1s. This
was primarily added so that `docker stats --no-stream` had something to
compare against.
Really the CLI should have just made a 2nd call and done the comparison
itself rather than forcing it on all API consumers.
That ship has long sailed, though.
With this change, clients can set an option to just pull a single stat,
which is *at least* a full second faster:
Old:
```
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=false > /dev/null
2>&1
real0m1.864s
user0m0.005s
sys0m0.007s
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=false > /dev/null
2>&1
real0m1.173s
user0m0.010s
sys0m0.006s
```
New:
```
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=true > /dev/null
2>&1
real0m0.680s
user0m0.008s
sys0m0.004s
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=true > /dev/null
2>&1
real0m0.156s
user0m0.007s
sys0m0.007s
```
This fixes issues with downstreams ability to use the stats API to
collect metrics.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The `docker push` command up until docker v0.9.1 always pushed all tags of a given
image, so `docker push foo/bar` would push (e.g.) all of `foo/bar:latest`, `foo:/bar:v1`,
and `foo/bar:v1.0.0`.
Pushing all tags of an image was not desirable in many case, so docker v0.10.0
enhanced `docker push` to optionally specify a tag to push (`docker push foo/bar:v1`)
(see issue 3411 and PR 4948 (commit e648a186d6).
This behavior exists up until today, and is confusing, because unlike other commands,
`docker push` does not default to use the `:latest` tag when omitted, but instead
makes it push "all tags of the image".
`docker pull` had a similar behavior, but PR 7759 (9c08364a41)
changed the behavior to default to the `:latest` tag, and added a `--all-tags` flag
to the CLI to optionally pull all images.
This patch implements the API client changes to make `docker push` match the behavior
of `docker pull`, and default to pull a single image, unless the `all` option is passed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The old nolint comment didn't seem to work anymore;
```
client/container_list.go:39:22: SA1019: filters.ToParamWithVersion is deprecated: do not use in any new code; use ToJSON instead (staticcheck)
client/events.go:94:22: SA1019: filters.ToParamWithVersion is deprecated: do not use in any new code; use ToJSON instead (staticcheck)
client/image_list.go:28:22: SA1019: filters.ToParamWithVersion is deprecated: do not use in any new code; use ToJSON instead (staticcheck)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adds a new ServiceStatus field to the Service object, which includes the
running and desired task counts. This new field is gated behind a
"status" query parameter.
Signed-off-by: Drew Erny <drew.erny@docker.com>
These checks were redundant, as we were not expecting
a specific string, just that a server-error or authentication
error was returned.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Keeping this code for now to allow connecting to old daemons, but we might
want to remove this at some point
```
client/hijack.go:90:12: SA1019: httputil.ErrPersistEOF is deprecated: No longer used. (staticcheck)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add annotations to suppress warnings like this one:
> client/container_list.go:38:22: SA1019: filters.ToParamWithVersion is deprecated: Use ToJSON (staticcheck)
> filterJSON, err := filters.ToParamWithVersion(cli.version, options.Filters)
> ^
Modify the deprecation notice to specify it is applicable to new code
only.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Format the source according to latest goimports.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Improve the readability of the connection error displayed to the user on
Windows when running docker commands fails by checking if the client is
privileged. If so then display the actual error wrapped in a generic
error "This error may indicate that the docker daemon is not running."
If not that display the actual error wrapped in a more specific error:
"In the default daemon configuration on Windows, the docker client must
be run with elevated privileges to connect."
Signed-off-by: Nick Adcock <nick.adcock@docker.com>
```
14:26:43 client/client.go:255:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
14:26:43 return &*cli.client
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When we see an `ECONNREFUSED` (or equivalent) from an attempted `HEAD` on the
`/_ping` endpoint there is no point in trying again with `GET` since the server
is not responding/available at all.
Once vendored into the cli this will partially mitigate https://github.com/docker/cli/issues/1739
("Docker commands take 1 minute to timeout if context endpoint is unreachable")
by cutting the effective timeout in half.
Signed-off-by: Ian Campbell <ijc@docker.com>
WithAPIVersionNegotiation enables automatic API version negotiation for the client.
With this option enabled, the client automatically negotiates the API version
to use when making requests. API version negotiation is performed on the first
request; subsequent requests will not re-negotiate.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit 3d72963ab8 fixed
situations where a version negotiation could override
the version, even though a client was initialized with a
fixed version.
In situations where the "fixed" version is empty, we
should ignore the option, and treat the client as
"not having a fixed version", so that API version
negotiation can still be performed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Minor improvement, but makes defining a list of options
a bit cleaner, and more descriptive;
Before:
opts := make([]func(*client.Client) error, 0)
After:
opts := make([]client.Opt, 0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Unlike a plain `net/http/client.Do()`, requests made through client/request
use the `sendRequest` function, which parses the server response, and may
convert non-transport errors into errors (through `cli.checkResponseErr()`).
This means that we cannot assume that no reader was opened if an error is
returned.
This patch changes various locations where `ensureReaderClosed` was only
called in the non-error situation, and uses a `defer` to make sure it's
always called.
`ensureReaderClosed` itself already checks if the response's body was set,
so in situations where the error was due to a transport error, calling
`ensureReaderClosed` should be a no-op.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
looks like we don't need this handling
Before this patch:
Error: No such image: nosuchimage
After this patch:
Error response from daemon: No such image: nosuchimage:latest
"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Monitoring systems and load balancers are usually configured to use HEAD
requests for health monitoring. The /_ping endpoint currently does not
support this type of request, which means that those systems have fallback
to GET requests.
This patch adds support for HEAD requests on the /_ping endpoint.
Although optional, this patch also returns `Content-Type` and `Content-Length`
headers in case of a HEAD request; Refering to RFC 7231, section 4.3.2:
The HEAD method is identical to GET except that the server MUST NOT
send a message body in the response (i.e., the response terminates at
the end of the header section). The server SHOULD send the same
header fields in response to a HEAD request as it would have sent if
the request had been a GET, except that the payload header fields
(Section 3.3) MAY be omitted. This method can be used for obtaining
metadata about the selected representation without transferring the
representation data and is often used for testing hypertext links for
validity, accessibility, and recent modification.
A payload within a HEAD request message has no defined semantics;
sending a payload body on a HEAD request might cause some existing
implementations to reject the request.
The response to a HEAD request is cacheable; a cache MAY use it to
satisfy subsequent HEAD requests unless otherwise indicated by the
Cache-Control header field (Section 5.2 of [RFC7234]). A HEAD
response might also have an effect on previously cached responses to
GET; see Section 4.3.5 of [RFC7234].
With this patch applied, either `GET` or `HEAD` requests work; the only
difference is that the body is empty in case of a `HEAD` request;
curl -i --unix-socket /var/run/docker.sock http://localhost/_ping
HTTP/1.1 200 OK
Api-Version: 1.40
Cache-Control: no-cache, no-store, must-revalidate
Docker-Experimental: false
Ostype: linux
Pragma: no-cache
Server: Docker/dev (linux)
Date: Mon, 14 Jan 2019 12:35:16 GMT
Content-Length: 2
Content-Type: text/plain; charset=utf-8
OK
curl --head -i --unix-socket /var/run/docker.sock http://localhost/_ping
HTTP/1.1 200 OK
Api-Version: 1.40
Cache-Control: no-cache, no-store, must-revalidate
Content-Length: 0
Content-Type: text/plain; charset=utf-8
Docker-Experimental: false
Ostype: linux
Pragma: no-cache
Server: Docker/dev (linux)
Date: Mon, 14 Jan 2019 12:34:15 GMT
The client is also updated to use `HEAD` by default, but fallback to `GET`
if the daemon does not support this method.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
