Use the same error message as the graphdrivers image store backend.
It's more informative as it also includes the requested platform and
won't break clients checking doing error check with string-matching.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Also fix some unhandled errors.
integration-cli/docker_cli_swarm_test.go:697:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"Error":"failed to add veth pair: `+err.Error()+`"}`)
^
integration-cli/docker_cli_swarm_test.go:731:18: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"LocalDefaultAddressSpace":"`+lAS+`", "GlobalDefaultAddressSpace": "`+gAS+`"}`)
^
integration-cli/docker_cli_swarm_test.go:742:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"Error":"Unknown address space in pool request: `+poolRequest.AddressSpace+`"}`)
^
integration-cli/docker_cli_swarm_test.go:746:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"PoolID":"`+poolID+`", "Pool":"`+pool+`"}`)
^
integration-cli/docker_cli_swarm_test.go:763:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"Address":"`+gw+`"}`)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also rename some variables that shadowed imports, and fix some
unhandled errors.
integration-cli/docker_cli_network_unix_test.go:102:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"Error":"failed to add veth pair: `+err.Error()+`"}`)
^
integration-cli/docker_cli_network_unix_test.go:136:18: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"LocalDefaultAddressSpace":"`+lAS+`", "GlobalDefaultAddressSpace": "`+gAS+`"}`)
^
integration-cli/docker_cli_network_unix_test.go:147:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"Error":"Unknown address space in pool request: `+poolRequest.AddressSpace+`"}`)
^
integration-cli/docker_cli_network_unix_test.go:151:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"PoolID":"`+poolID+`", "Pool":"`+pool+`"}`)
^
integration-cli/docker_cli_network_unix_test.go:168:19: printf: non-constant format string in call to fmt.Fprintf (govet)
fmt.Fprintf(w, `{"Address":"`+gw+`"}`)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
integration-cli/benchmark_test.go:49:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
integration-cli/benchmark_test.go:62:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
integration-cli/benchmark_test.go:68:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
integration-cli/benchmark_test.go:73:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
integration-cli/benchmark_test.go:78:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
integration-cli/benchmark_test.go:84:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
integration-cli/benchmark_test.go:94:27: printf: non-constant format string in call to fmt.Errorf (govet)
chErr <- fmt.Errorf(out)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This condition was added in 0215a62d5b, which
removed pkg/homedir as abstraction, but didn't consider that this test
is currently only ran on Unix.
integration-cli/docker_cli_run_unix_test.go:254:5: SA4032: due to the file's build constraints, runtime.GOOS will never equal "windows" (staticcheck)
if runtime.GOOS == "windows" {
^
integration-cli/docker_cli_run_unix_test.go:338:5: SA4032: due to the file's build constraints, runtime.GOOS will never equal "windows" (staticcheck)
if runtime.GOOS == "windows" {
^
Added a TODO, because this functionality should also be tested on Windows,
probably as part of tests in docker/cli instead.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before commit 4f09af6, when allocating host ports for a new
port mapping, iptables rules were set up then docker-proxy was
started. If the host port was already in-use, docker-proxy exited
with an error, and the iptables rules were removed. That could
potentially interfere with a non-docker service that was already
using the host port for something unrelated.
Commit 4f09af6 swapped that problem for a different one... in
order to check that a port was available before creating iptables
rules, it attempted to start docker-proxy first. If it failed, it
could then try a different host port, without interfering with
any other service. The problem with that is docker-proxy would
start listening before the iptables rules were in place, so it
could accept connections then become unusable because new NAT
rules diverted packets directly to the container. This would leave
the client with a broken connection, causing at-least a delay
while it figured that out and reconnected.
This change creates and binds the socket in the daemon, before
creating iptables rules. If the bind fails, it may try a different
port. When or if the bind succeeds, iptables rules are created,
then the daemon calls listen on the socket. If docker-proxy is
needed, the socket is handed over to it at that point.
In rootless mode, the ports have to be bound to an address in the
rootless network namespace (where dockerd is running). DNAT rules
now use the same address.
If docker-proxy is not needed ("--userland-proxy=false"), the daemon
still listens on TCP sockets as the old dummyProxy would have done.
This makes the socket show up in "netstat" output.
The dummyProxy is no longer needed on Linux. Its job was to bind the
host ports if docker-proxy was disabled, but that's now already
handled by binding the sockets early.
This change doesn't affect SCTP, because it's not currently possible
for docker-proxy to convert the file descriptor into an SCTPListener.
So, docker-proxy is still started early, and the window for lost
connections remains.
If the user has an old docker-proxy in their path and it's given a
listener docker with '-use-listen-fd', it'll fail because of the
unknown option. In this case, the daemon's error message suggests
checking $PATH.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This moves the `ContainerJSONBase`, `ContainerJSON` and `ContainerNode`
types to the api/types/container package and deprecates the old location.
- `ContainerJSONBase` was renamed to `InspectBase`
- `ContainerJSON` was rnamed to `InspectResponse`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This moves the `ImageInspect` and `RootFS` types to the image package,
and deprecates the old location.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This moves the `Health` and `HealthcheckResult` types to the container package,
as well as the related `NoHealthcheck`, `Starting`, `Healthy`, and `Unhealthy`
consts.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit 17c3269a37 moved the ContainerStats
type to the container package, and renamed it to StatsResponse. However,
this name is chosen poorly, as it documents it to be the response of
the API endpoint, but is more accurately a wrapper around a reader,
used to read a (stream of) StatsJSON. We want to change StatsJSON
to StatsResponse, as it's more consistent with other response types.
As 17c3269a37 did not make it into a
non-pre-release, we can still change this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This type is included in various types used in the API, but comes from
a separate module. The go-units module may be moving to the moby org,
and it is yet to be decided if the Ulimit type is a good fit for that
module (which deals with more generic units, such as "size" and "duration"
otherwise).
This patch introduces an alias to help during the transition of this type
to it's new location. The alias makes sure that existing code continues
to work (at least for now), but we need to start updating such code after
this PR is merged.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit 18f4f775ed.
Because buildkit doesn't run an internal resolver, and it bases its
/etc/resolv.conf on the host's ... when buildkit is run in a container
that has 'nameserver 127.0.0.11', its build containers will use Google's
DNS servers as a fallback (unless the build container uses host
networking).
Before, when the 127.0.0.11 resolver was not used for the default network,
the buildkit container would have inherited a site-local nameserver. So,
the build containers it created would also have inherited that DNS
server - and they'd be able to resolve site-local hostnames.
By replacing the site-local nameserver with Google's, we broke access
to local DNS and its hostnames.
Signed-off-by: Rob Murray <rob.murray@docker.com>
This test was depending on top-level fields that were deprecated since
API v1.18. These fields are no longer sent by current clients.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`Daemon.Kill()` was sending a SIGKILL to the daemon process but wasn't
waiting until the process was really killed. While the race window is
really small, better safe than sorry.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This is the response type; other types related to stats are left
for now, but should be moved (as well as utilities ported from
the CLI repository).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was added in 428328908dc529b1678fb3d8b033fb0591a294e3;
> Deprecate /containers/(id or name)/copy endpoint
> This endpoint has been deprecated since 1.8. Return an error starting
> from this API version (1.24) in order to make sure it's not used for the
> next API version and so that we can remove it sometimes later.
We deprecated and removed those older API versions, and the test was
effectively only verifying that a non-existing endpoint returns a 404,
so let's remove it.
This also removes api/types.CopyConfig, which was only used in this
test.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Until now, containers on the default bridge network have been configured
to talk directly to external DNS servers - their resolv.conf files have
either been populated with nameservers from the host's resolv.conf, or
with servers from '--dns' (or with Google's nameservers as a fallback).
This change makes the internal bridge more like other networks by using
the internal resolver. But, the internal resolver is not populated with
container names or aliases - it's only for external DNS lookups.
Containers on the default network, on a host that has a loopback
resolver (like systemd's on 127.0.0.53) will now use that resolver
via the internal resolver. So, the logic used to find systemd's current
set of resolvers is no longer needed by the daemon.
Legacy links work just as they did before, using '/etc/hosts' and magic.
(Buildkit does not use libnetwork, so it can't use the internal resolver.
But it does use libnetwork/resolvconf's logic to configure resolv.conf.
So, code to set up resolv.conf for a legacy networking without an internal
resolver can't be removed yet.)
Signed-off-by: Rob Murray <rob.murray@docker.com>
This moves the type to the api/types/network package, but also introduces
a "Summary" alias; the intent here is to allow diverging the types used
for "list" and "inspect" operations, as list operations may only be
producing a subset of the fields available.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Tests that start a daemon disable iptables, to avoid conflicts with
other tests running in parallel and also creating iptables chains.
Do the same for ip6tables, in prep for them being enabled by-default.
Signed-off-by: Rob Murray <rob.murray@docker.com>
It'd only return true on a host with no IPv6 in its kernel.
So, removed, having fixed the two tests that used it.
Signed-off-by: Rob Murray <rob.murray@docker.com>
The test hadn't been running, because it used testRequires(c, IPv6)
and predicate "IPv6" returns the opposite of the expected result.
TestDaemonIPv6Enabled tried to run with IPv6 on the default bridge,
but didn't set up a "fixed-cidr-v6" - so the daemon wouldn't start.
It then tried to check the bridge had address "fe80::1", which it
expected to work because it had just used setupV6() to add that
address.
Then it checked that "LinkLocalIPv6Address" was set in container
inspect output, but it wouldn't be (the field is deprecated).
There are working IPv6 tests in the suite (TestDaemonIPv6FixedCIDR,
TestDaemonIPv6FixedCIDRAndMac, TestDaemonIPv6HostMode) - and there's
more coverage in the network integration tests.
So, deleted the test as it didn't seem worth salvaging.
Also deleted now-unused helper functions setupV6(), teardownV6().
Signed-off-by: Rob Murray <rob.murray@docker.com>
The test hadn't been running, because it used testRequires(c, IPv6)
and predicate "IPv6" returns the opposite of the expected result.
If the test had run, it'd have failed because:
- it used "--listen-add", but the option is "--listen-addr"
- so, the daemon wouldn't have started
- it tried to use "--join ::1"
- address "::1" was interpreted as host:port so the Dial() failed,
it needed to be "[::1]".
- it didn't supply a join token
Signed-off-by: Rob Murray <rob.murray@docker.com>
The `normalizeWorkdir` function has two branches, one that returns a
result of `filepath.Join` which always returns a cleaned path, and
another one where the input string is returned unmodified.
To make these two outputs consistent, also clean the path in the second
branch.
This also makes the cleaning of the container workdir explicit in the
`normalizeWorkdir` function instead of relying on the
`SetupWorkingDirectory` to mutate it.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The field will still be present in the response, but will always be
`false`.
Searching for `is-automated=true` will yield no results, while
`is-automated=false` will effectively be a no-op.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
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>
Replace regex matching/replacement and re-reading of generated files
with a simple parser, and struct to remember and manipulate the file
content.
Annotate the generated file with a header comment saying the file is
generated, but can be modified, and a trailing comment describing how
the file was generated and listing external nameservers.
Always start with the host's resolv.conf file, whether generating config
for host networking, or with/without an internal resolver - rather than
editing a file previously generated for a different use-case.
Resolves an issue where rewrites of the generated file resulted in
default IPv6 nameservers being unnecessarily added to the config.
Signed-off-by: Rob Murray <rob.murray@docker.com>
API v1.20 (Docker Engine v1.11.0) and older allowed a HostConfig to be passed
when starting a container. This feature was deprecated in API v1.21 (Docker
Engine v1.10.0) in 3e7405aea8, and removed in
API v1.23 (Docker Engine v1.12.0) in commit 0a8386c8be.
API v1.23 and older are deprecated, and this patch removes the feature.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
