Don't change the behavior for older clients and keep the same behavior.
Otherwise client can't opt-out (because `ReadOnlyNonRecursive` is
unsupported before 1.44).
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Commit e6907243af applied a fix for situations
where the client was configured with API-version negotiation, but did not yet
negotiate a version.
However, the checkVersion() function that was implemented copied the semantics
of cli.NegotiateAPIVersion, which ignored connection failures with the
assumption that connection errors would still surface further down.
However, when using the result of a failed negotiation for NewVersionError,
an API version mismatch error would be produced, masking the actual connection
error.
This patch changes the signature of checkVersion to return unexpected errors,
including failures to connect to the API.
Before this patch:
docker -H unix:///no/such/socket.sock secret ls
"secret list" requires API version 1.25, but the Docker daemon API version is 1.24
With this patch applied:
docker -H unix:///no/such/socket.sock secret ls
Cannot connect to the Docker daemon at unix:///no/such/socket.sock. Is the docker daemon running?
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function has various errors that are returned when failing to make a
connection (due to permission issues, TLS mis-configuration, or failing to
resolve the TCP address).
The errConnectionFailed error is currently used as a special case when
processing Ping responses. The current code did not consistently treat
connection errors, and because of that could either absorb the error,
or process the empty response.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
NegotiateAPIVersion was ignoring errors returned by Ping. The intent here
was to handle API responses from a daemon that may be in an unhealthy state,
however this case is already handled by Ping itself.
Ping only returns an error when either failing to connect to the API (daemon
not running or permissions errors), or when failing to parse the API response.
Neither of those should be ignored in this code, or considered a successful
"ping", so update the code to return
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was added in 27ef09a46f, which changed
the Ping handling to ignore internal server errors. That case is tested in
TestPingFail, which verifies that we accept the Ping response if a 500
status code was received.
The TestPingWithError test was added to verify behavior if a protocol
(connection) error occurred; however the mock-client returned both a
response, and an error; the error returned would only happen if a connection
error occurred, which means that the server would not provide a reply.
Running the test also shows that returning a response is unexpected, and
ignored:
=== RUN TestPingWithError
2024/02/23 14:16:49 RoundTripper returned a response & error; ignoring response
2024/02/23 14:16:49 RoundTripper returned a response & error; ignoring response
--- PASS: TestPingWithError (0.00s)
PASS
This patch updates the test to remove the response.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't error out when mount source doesn't exist and mounts has
`CreateMountpoint` option enabled.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Any PR that is labeled with any `impact/*` label should have a
description for the changelog and an `area/*` label.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
A common pattern in libnetwork is to delete an object using
`DeleteAtomic`, ie. to check the optimistic lock, but put in a retry
loop to refresh the data and the version index used by the optimistic
lock.
This commit introduces a new `Delete` method to delete without
checking the optimistic lock. It focuses only on the few places where
it's obvious the calling code doesn't rely on the side-effects of the
retry loop (ie. refreshing the object to be deleted).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
I noticed that this log didn't use structured logs;
[resolver] failed to query DNS server: 10.115.11.146:53, query: ;google.com.\tIN\t A" error="read udp 172.19.0.2:46361->10.115.11.146:53: i/o timeout
[resolver] failed to query DNS server: 10.44.139.225:53, query: ;google.com.\tIN\t A" error="read udp 172.19.0.2:53991->10.44.139.225:53: i/o timeout
But other logs did;
DEBU[2024-02-20T15:48:51.026704088Z] [resolver] forwarding query client-addr="udp:172.19.0.2:39661" dns-server="udp:192.168.65.7:53" question=";google.com.\tIN\t A"
DEBU[2024-02-20T15:48:51.028331088Z] [resolver] forwarding query client-addr="udp:172.19.0.2:35163" dns-server="udp:192.168.65.7:53" question=";google.com.\tIN\t AAAA"
DEBU[2024-02-20T15:48:51.057329755Z] [resolver] received AAAA record "2a00:1450:400e:801::200e" for "google.com." from udp:192.168.65.7
DEBU[2024-02-20T15:48:51.057666880Z] [resolver] received A record "142.251.36.14" for "google.com." from udp:192.168.65.7
As we're already constructing a logger with these fields, we may as well use it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Allow to override the PAGER/GIT_PAGER variables inside the container.
Use `cat` as pager when running in Github Actions (to avoid things like
`git diff` stalling the CI).
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Don't use OTEL tracing in this test because we're testing the HTTP proxy
behavior here and we don't want OTEL to interfere.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This will return a single entry for each name/value pair, and for now
all the "image specific" metadata (labels, config, size) should be
either "default platform" or "first platform we have locally" (which
then matches the logic for commands like `docker image inspect`, etc)
with everything else (just ID, maybe?) coming from the manifest
list/index.
That leaves room for the longer-term implementation to add new fields to
describe the _other_ images that are part of the manifest list/index.
Co-authored-by: Tianon Gravi <admwiggin@gmail.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
v1.33.0 is also available, but it would also cause
`github.com/aws/aws-sdk-go-v2` change from v1.24.1 to v1.25.0
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
DNS names were only set up for user-defined networks. On Linux, none
of the built-in networks (bridge/host/none) have built-in DNS, so they
don't need DNS names.
But, on Windows, the default network is "nat" and it does need the DNS
names.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This matches the prior behavior before 2a6ff3c24f.
This also updates the Swagger documentation for the current version to note that the field might be the empty string and what that means.
Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
Archives being unpacked by Dockerfiles may have been created on other
OSes with different conventions and semantics for xattrs, making them
impossible to apply when extracting. Restore the old best-effort xattr
behaviour users have come to depend on in the classic builder.
The (archive.Archiver).UntarPath function does not allow the options
passed to Untar to be customized. It also happens to be a trivial
wrapper around the Untar function. Inline the function body and add the
option.
Signed-off-by: Cory Snider <csnider@mirantis.com>
