If the daemon is configured to use a mirror for the default (Docker Hub)
registry, the endpoint did not fall back to querying the upstream if the mirror
did not contain the given reference.
For pull-through registry-mirrors, this was not a problem, as in that case the
registry would forward the request, but for other mirrors, no fallback would
happen. This was inconsistent with how "pulling" images handled this situation;
when pulling images, both the mirror and upstream would be tried.
This patch brings the daemon-side lookup of image-manifests on-par with the
client-side lookup (the GET /distribution endpoint) as used in API 1.30 and
higher.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If the daemon is configured to use a mirror for the default (Docker Hub)
registry, the endpoint did not fall back to querying the upstream if the mirror
did not contain the given reference.
If the daemon is configured to use a mirror for the default (Docker Hub)
registry, did not fall back to querying the upstream if the mirror did not
contain the given reference.
For pull-through registry-mirrors, this was not a problem, as in that case the
registry would forward the request, but for other mirrors, no fallback would
happen. This was inconsistent with how "pulling" images handled this situation;
when pulling images, both the mirror and upstream would be tried.
This problem was caused by the logic used in GetRepository, which had an
optimization to only return the first registry it was successfully able to
configure (and connect to), with the assumption that the mirror either contained
all images used, or to be configured as a pull-through mirror.
This patch:
- Introduces a GetRepositories method, which returns all candidates (both
mirror(s) and upstream).
- Updates the endpoint to try all
Before this patch:
# the daemon is configured to use a mirror for Docker Hub
cat /etc/docker/daemon.json
{ "registry-mirrors": ["http://localhost:5000"]}
# start the mirror (empty registry, not configured as pull-through mirror)
docker run -d --name registry -p 127.0.0.1:5000:5000 registry:2
# querying the endpoint fails, because the image-manifest is not found in the mirror:
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.43/distribution/docker.io/library/hello-world:latest/json
{
"message": "manifest unknown: manifest unknown"
}
With this patch applied:
# the daemon is configured to use a mirror for Docker Hub
cat /etc/docker/daemon.json
{ "registry-mirrors": ["http://localhost:5000"]}
# start the mirror (empty registry, not configured as pull-through mirror)
docker run -d --name registry -p 127.0.0.1:5000:5000 registry:2
# querying the endpoint succeeds (manifest is fetched from the upstream Docker Hub registry):
curl -s --unix-socket /var/run/docker.sock http://localhost/v1.43/distribution/docker.io/library/hello-world:latest/json | jq .
{
"Descriptor": {
"mediaType": "application/vnd.oci.image.index.v1+json",
"digest": "sha256:1b9844d846ce3a6a6af7013e999a373112c3c0450aca49e155ae444526a2c45e",
"size": 3849
},
"Platforms": [
{
"architecture": "amd64",
"os": "linux"
}
]
}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Go 1.21.5 compiler has a bug: per-file language version override
directives do not take effect when instantiating generic functions which
have certain nontrivial type constraints. Consequently, a module-mode
project with Moby as a dependency may fail to compile when the compiler
incorrectly applies go1.16 semantics to the generic function call.
As the offending function is trivial and is only used in one place, work
around the issue by converting it to a concretely-typed function.
Signed-off-by: Cory Snider <csnider@mirantis.com>
When mapping a port with the userland-proxy enabled, the daemon would
perform an "exec.LookPath" for every mapped port (which, in case of
a range of ports, would be for every port in the range).
This was both inefficient (looking up the binary for each port), inconsistent
(when running in rootless-mode, the binary was looked-up once), as well as
inconvenient, because a missing binary, or a mis-configureed userland-proxy-path
would not be detected daeemon startup, and not produce an error until starting
the container;
docker run -d -P nginx:alpine
4f7b6589a1680f883d98d03db12203973387f9061e7a963331776170e4414194
docker: Error response from daemon: driver failed programming external connectivity on endpoint romantic_wiles (7cfdc361821f75cbc665564cf49856cf216a5b09046d3c22d5b9988836ee088d): fork/exec docker-proxy: no such file or directory.
However, the container would still be created (but invalid);
docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
869f41d7e94f nginx:alpine "/docker-entrypoint.…" 10 seconds ago Created romantic_wiles
This patch changes how the userland-proxy is configured;
- The path of the userland-proxy is now looked up / configured at daemon
startup; this is similar to how the proxy is configured in rootless-mode.
- A warning is logged when failing to lookup the binary.
- If the daemon is configured with "userland-proxy" enabled, an error is
produced, and the daemon will refuse to start.
- The "proxyPath" argument for newProxyCommand() (in libnetwork/portmapper)
is now required to be set. It no longer looks up the executable, and
produces an error if no path was provided. While this change was not
required, it makes the daemon config the canonical source of truth, instead
of logic spread accross multiplee locations.
Some of this logic is a change of behavior, but these changes were made with
the assumption that we don't want to support;
- installing the userland proxy _after_ the daemon was started
- moving the userland proxy (or installing a proxy with a higher
preference in PATH)
With this patch:
Validating the config produces an error if the binary is not found:
dockerd --validate
WARN[2023-12-29T11:36:39.748699591Z] failed to lookup default userland-proxy binary error="exec: \"docker-proxy\": executable file not found in $PATH"
userland-proxy is enabled, but userland-proxy-path is not set
Disabling userland-proxy prints a warning, but validates as "OK":
dockerd --userland-proxy=false --validate
WARN[2023-12-29T11:38:30.752523879Z] ffailed to lookup default userland-proxy binary error="exec: \"docker-proxy\": executable file not found in $PATH"
configuration OK
Speficying a non-absolute path produces an error:
dockerd --userland-proxy-path=docker-proxy --validate
invalid userland-proxy-path: must be an absolute path: docker-proxy
Befor this patch, we would not validate this path, which would allow the daemon
to start, but fail to map a port;
docker run -d -P nginx:alpine
4f7b6589a1680f883d98d03db12203973387f9061e7a963331776170e4414194
docker: Error response from daemon: driver failed programming external connectivity on endpoint romantic_wiles (7cfdc361821f75cbc665564cf49856cf216a5b09046d3c22d5b9988836ee088d): fork/exec docker-proxy: no such file or directory.
Specifying an invalid userland-proxy-path produces an error as well:
dockerd --userland-proxy-path=/usr/local/bin/no-such-binary --validate
userland-proxy-path is invalid: stat /usr/local/bin/no-such-binary: no such file or directory
mkdir -p /usr/local/bin/not-a-file
dockerd --userland-proxy-path=/usr/local/bin/not-a-file --validate
userland-proxy-path is invalid: exec: "/usr/local/bin/not-a-file": is a directory
touch /usr/local/bin/not-an-executable
dockerd --userland-proxy-path=/usr/local/bin/not-an-executable --validate
userland-proxy-path is invalid: exec: "/usr/local/bin/not-an-executable": permission denied
Same when using the daemon.json config-file;
echo '{"userland-proxy-path":"no-such-binary"}' > /etc/docker/daemon.json
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: invalid userland-proxy-path: must be an absolute path: no-such-binary
dockerd --userland-proxy-path=hello --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: userland-proxy-path: (from flag: hello, from file: /usr/local/bin/docker-proxy)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
No more concept of "anonymous endpoints". The equivalent is now an
endpoint with no DNSNames set.
Some of the code removed by this commit was mutating user-supplied
endpoint's Aliases to add container's short ID to that list. In order to
preserve backward compatibility for the ContainerInspect endpoint, this
commit also takes care of adding that short ID (and the container
hostname) to `EndpointSettings.Aliases` before returning the response.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The remapping in the commit code was in the wrong place, we would create
a diff and then remap the snapshot, but the descriptor created in
"CreateDiff" was still pointing to the old snapshot, we now remap the
snapshot before creating a diff. Also make sure we don't lose any
capabilities, they used to be lost after the chown.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
The semantics of an "anonymous" endpoint has always been weird: it was
set on endpoints which name shouldn't be taken into account when
inserting DNS records into libnetwork's `Controller.svcRecords` (and
into the NetworkDB). However, in that case the endpoint's aliases would
still be used to create DNS records; thus, making those "anonymous
endpoints" not so anonymous.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The `(*Endpoint).rename()` method is changed to only mutate `ep.name`
and let a new method `(*Endpoint).UpdateDNSNames()` handle DNS updates.
As a consequence, the rollback code that was part of
`(*Endpoint).rename()` is now removed, and DNS updates are now
rolled back by `ContainerRename`.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Instead of special-casing anonymous endpoints in libnetwork, let the
daemon specify what (non fully qualified) DNS names should be associated
to container's endpoints.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Ensure that when removing an image, an image is checked consistently
against the images with the same target digest. Add unit testing around
delete.
Signed-off-by: Derek McGowan <derek@mcg.dev>
They just happen to exist on a network that doesn't support DNS-based
service discovery (ie. no embedded DNS servers are started for them).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Calculate the IPv6 addreesses needed on a bridge, then reconcile them
with the addresses on an existing bridge by deleting then adding as
required.
(Previously, required addresses were added one-by-one, then unwanted
addresses were removed. This meant the daemon failed to start if, for
example, an existing bridge had address '2000:db8::/64' and the config
was changed to '2000:db8::/80'.)
IPv6 addresses are now calculated and applied in one go, so there's no
need for setupVerifyAndReconcile() to check the set of IPv6 addresses on
the bridge. And, it was guarded by !config.InhibitIPv4, which can't have
been right. So, removed its IPv6 parts, and added IPv4 to its name.
Link local addresses, the example given in the original ticket, are now
released when containers are stopped. Not releasing them meant that
when using an LL subnet on the default bridge, no container could be
started after a container was stopped (because the calculated address
could not be re-allocated). In non-default bridge networks using an
LL subnet, addresses leaked.
Linux always uses the standard 'fe80::/64' LL network. So, if a bridge
is configured with an LL subnet prefix that overlaps with it, a config
error is reported. Non-overlapping LL subnet prefixes are allowed.
Signed-off-by: Rob Murray <rob.murray@docker.com>
Before this change `ParentId` was filled for images when calling the
`/images/json` (image list) endpoint but was not for the
`/images/<image>/json` (image inspect).
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This repository is not yet a module (i.e., does not have a `go.mod`). This
is not problematic when building the code in GOPATH or "vendor" mode, but
when using the code as a module-dependency (in module-mode), different semantics
are applied since Go1.21, which switches Go _language versions_ on a per-module,
per-package, or even per-file base.
A condensed summary of that logic [is as follows][1]:
- For modules that have a go.mod containing a go version directive; that
version is considered a minimum _required_ version (starting with the
go1.19.13 and go1.20.8 patch releases: before those, it was only a
recommendation).
- For dependencies that don't have a go.mod (not a module), go language
version go1.16 is assumed.
- Likewise, for modules that have a go.mod, but the file does not have a
go version directive, go language version go1.16 is assumed.
- If a go.work file is present, but does not have a go version directive,
language version go1.17 is assumed.
When switching language versions, Go _downgrades_ the language version,
which means that language features (such as generics, and `any`) are not
available, and compilation fails. For example:
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
Note that these fallbacks are per-module, per-package, and can even be
per-file, so _(indirect) dependencies_ can still use modern language
features, as long as their respective go.mod has a version specified.
Unfortunately, these failures do not occur when building locally (using
vendor / GOPATH mode), but will affect consumers of the module.
Obviously, this situation is not ideal, and the ultimate solution is to
move to go modules (add a go.mod), but this comes with a non-insignificant
risk in other areas (due to our complex dependency tree).
We can revert to using go1.16 language features only, but this may be
limiting, and may still be problematic when (e.g.) matching signatures
of dependencies.
There is an escape hatch: adding a `//go:build` directive to files that
make use of go language features. From the [go toolchain docs][2]:
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a `go.mod` file with a go line such as `go 1.21` or `go 1.21.3`.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding `//go:build go1.22` to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This patch adds `//go:build` directives to those files using recent additions
to the language. It's currently using go1.19 as version to match the version
in our "vendor.mod", but we can consider being more permissive ("any" requires
go1.18 or up), or more "optimistic" (force go1.21, which is the version we
currently use to build).
For completeness sake, note that any file _without_ a `//go:build` directive
will continue to use go1.16 language version when used as a module.
[1]: 58c28ba286/src/cmd/go/internal/gover/version.go (L9-L56)
[2]: https://go.dev/doc/toolchain
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
No need to copy the parent label from the source dangling image, because
it will already be copied from the source image.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
A validation step was added to prevent the daemon from considering "logentries"
as a dynamically loaded plugin, causing it to continue trying to load the plugin;
WARN[2023-12-12T21:53:16.866857127Z] Unable to locate plugin: logentries, retrying in 1s
WARN[2023-12-12T21:53:17.868296836Z] Unable to locate plugin: logentries, retrying in 2s
WARN[2023-12-12T21:53:19.874259254Z] Unable to locate plugin: logentries, retrying in 4s
WARN[2023-12-12T21:53:23.879869881Z] Unable to locate plugin: logentries, retrying in 8s
But would ultimately be returned as an error to the user:
docker container create --name foo --log-driver=logentries nginx:alpine
Error response from daemon: error looking up logging plugin logentries: plugin "logentries" not found
With the additional validation step, an error is returned immediately:
docker container create --log-driver=logentries busybox
Error response from daemon: the logentries logging driver has been deprecated and removed
A migration step was added on container restore. Containers using the
"logentries" logging driver are migrated to use the "local" logging driver:
WARN[2023-12-12T22:38:53.108349297Z] migrated deprecated logentries logging driver container=4c9309fedce75d807340ea1820cc78dc5c774d7bfcae09f3744a91b84ce6e4f7 error="<nil>"
As an alternative to the validation step, I also considered using a "stub"
deprecation driver, however this would not result in an error when creating
the container, and only produce an error when starting:
docker container create --name foo --log-driver=logentries nginx:alpine
4c9309fedce75d807340ea1820cc78dc5c774d7bfcae09f3744a91b84ce6e4f7
docker start foo
Error response from daemon: failed to create task for container: failed to initialize logging driver: the logentries logging driver has been deprecated and removed
Error: failed to start containers: foo
For containers, this validation is added in the backend (daemon). For services,
this was not sufficient, as SwarmKit would try to schedule the task, which
caused a close loop;
docker service create --log-driver=logentries --name foo nginx:alpine
zo0lputagpzaua7cwga4lfmhp
overall progress: 0 out of 1 tasks
1/1: no suitable node (missing plugin on 1 node)
Operation continuing in background.
DEBU[2023-12-12T22:50:28.132732757Z] Calling GET /v1.43/tasks?filters=%7B%22_up-to-date%22%3A%7B%22true%22%3Atrue%7D%2C%22service%22%3A%7B%22zo0lputagpzaua7cwga4lfmhp%22%3Atrue%7D%7D
DEBU[2023-12-12T22:50:28.137961549Z] Calling GET /v1.43/nodes
DEBU[2023-12-12T22:50:28.340665007Z] Calling GET /v1.43/services/zo0lputagpzaua7cwga4lfmhp?insertDefaults=false
DEBU[2023-12-12T22:50:28.343437632Z] Calling GET /v1.43/tasks?filters=%7B%22_up-to-date%22%3A%7B%22true%22%3Atrue%7D%2C%22service%22%3A%7B%22zo0lputagpzaua7cwga4lfmhp%22%3Atrue%7D%7D
DEBU[2023-12-12T22:50:28.345201257Z] Calling GET /v1.43/nodes
So a validation was added in the service create and update endpoints;
docker service create --log-driver=logentries --name foo nginx:alpine
Error response from daemon: the logentries logging driver has been deprecated and removed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Logentries service will be discontinued next week:
> Dear Logentries user,
>
> We have identified you as the owner of, or collaborator of, a Logentries account.
>
> The Logentries service will be discontinued on November 15th, 2022. This means that your Logentries account access will be removed and all your log data will be permanently deleted on this date.
>
> Next Steps
> If you are interested in an alternative Rapid7 log management solution, InsightOps will be available for purchase through December 16th, 2022. Please note, there is no support to migrate your existing Logentries account to InsightOps.
>
> Thank you for being a valued user of Logentries.
>
> Thank you,
> Rapid7 Customer Success
There is no reason to preserve this code in Moby as a result.
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When reading logs, timestamps should always be presented in UTC. Unlike
the "json-file" and other logging drivers, the "local" logging driver
was using local time.
Thanks to Roman Valov for reporting this issue, and locating the bug.
Before this change:
echo $TZ
Europe/Amsterdam
docker run -d --log-driver=local nginx:alpine
fc166c6b2c35c871a13247dddd95de94f5796459e2130553eee91cac82766af3
docker logs --timestamps fc166c6b2c35c871a13247dddd95de94f5796459e2130553eee91cac82766af3
2023-12-08T18:16:56.291023422+01:00 /docker-entrypoint.sh: /docker-entrypoint.d/ is not empty, will attempt to perform configuration
2023-12-08T18:16:56.291056463+01:00 /docker-entrypoint.sh: Looking for shell scripts in /docker-entrypoint.d/
2023-12-08T18:16:56.291890130+01:00 /docker-entrypoint.sh: Launching /docker-entrypoint.d/10-listen-on-ipv6-by-default.sh
...
With this patch:
echo $TZ
Europe/Amsterdam
docker run -d --log-driver=local nginx:alpine
14e780cce4c827ce7861d7bc3ccf28b21f6e460b9bfde5cd39effaa73a42b4d5
docker logs --timestamps 14e780cce4c827ce7861d7bc3ccf28b21f6e460b9bfde5cd39effaa73a42b4d5
2023-12-08T17:18:46.635967625Z /docker-entrypoint.sh: /docker-entrypoint.d/ is not empty, will attempt to perform configuration
2023-12-08T17:18:46.635989792Z /docker-entrypoint.sh: Looking for shell scripts in /docker-entrypoint.d/
2023-12-08T17:18:46.636897417Z /docker-entrypoint.sh: Launching /docker-entrypoint.d/10-listen-on-ipv6-by-default.sh
...
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
If no `dangling` filter is specified, prune should only delete dangling
images.
This wasn't visible by doing `docker image prune` because the CLI
explicitly sets this filter to true.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This struct is intended for internal use only for the backend, and is
not intended to be used externally.
This moves the plugin-related `NetworkListConfig` types to the backend
package to prevent it being imported in the client, and to make it more
clear that this is part of internal APIs, and not public-facing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These structs are intended for internal use only for the backend, and are
not intended to be used externally.
This moves the plugin-related `PluginRmConfig`, `PluginEnableConfig`, and
`PluginDisableConfig` types to the backend package to prevent them being
imported in the client, and to make it more clear that this is part of
internal APIs, and not public-facing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The daemon currently provides support for API versions all the way back
to v1.12, which is the version of the API that shipped with docker 1.0. On
Windows, the minimum supported version is v1.24.
Such old versions of the client are rare, and supporting older API versions
has accumulated significant amounts of code to remain backward-compatible
(which is largely untested, and a "best-effort" at most).
This patch updates the minimum API version to v1.24, which is the fallback
API version used when API-version negotiation fails. The intent is to start
deprecating older API versions, but no code is removed yet as part of this
patch, and a DOCKER_MIN_API_VERSION environment variable is added, which
allows overriding the minimum version (to allow restoring the behavior from
before this patch).
With this patch the daemon defaults to API v1.24 as minimum:
docker version
Client:
Version: 24.0.2
API version: 1.43
Go version: go1.20.4
Git commit: cb74dfc
Built: Thu May 25 21:50:49 2023
OS/Arch: linux/arm64
Context: default
Server:
Engine:
Version: dev
API version: 1.44 (minimum version 1.24)
Go version: go1.21.3
Git commit: 0322a29b9ef8806aaa4b45dc9d9a2ebcf0244bf4
Built: Mon Dec 4 15:22:17 2023
OS/Arch: linux/arm64
Experimental: false
containerd:
Version: v1.7.9
GitCommit: 4f03e100cb967922bec7459a78d16ccbac9bb81d
runc:
Version: 1.1.10
GitCommit: v1.1.10-0-g18a0cb0
docker-init:
Version: 0.19.0
GitCommit: de40ad0
Trying to use an older version of the API produces an error:
DOCKER_API_VERSION=1.23 docker version
Client:
Version: 24.0.2
API version: 1.23 (downgraded from 1.43)
Go version: go1.20.4
Git commit: cb74dfc
Built: Thu May 25 21:50:49 2023
OS/Arch: linux/arm64
Context: default
Error response from daemon: client version 1.23 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version
To restore the previous minimum, users can start the daemon with the
DOCKER_MIN_API_VERSION environment variable set:
DOCKER_MIN_API_VERSION=1.12 dockerd
API 1.12 is the oldest supported API version on Linux;
docker version
Client:
Version: 24.0.2
API version: 1.43
Go version: go1.20.4
Git commit: cb74dfc
Built: Thu May 25 21:50:49 2023
OS/Arch: linux/arm64
Context: default
Server:
Engine:
Version: dev
API version: 1.44 (minimum version 1.12)
Go version: go1.21.3
Git commit: 0322a29b9ef8806aaa4b45dc9d9a2ebcf0244bf4
Built: Mon Dec 4 15:22:17 2023
OS/Arch: linux/arm64
Experimental: false
containerd:
Version: v1.7.9
GitCommit: 4f03e100cb967922bec7459a78d16ccbac9bb81d
runc:
Version: 1.1.10
GitCommit: v1.1.10-0-g18a0cb0
docker-init:
Version: 0.19.0
GitCommit: de40ad0
When using the `DOCKER_MIN_API_VERSION` with a version of the API that
is not supported, an error is produced when starting the daemon;
DOCKER_MIN_API_VERSION=1.11 dockerd --validate
invalid DOCKER_MIN_API_VERSION: minimum supported API version is 1.12: 1.11
DOCKER_MIN_API_VERSION=1.45 dockerd --validate
invalid DOCKER_MIN_API_VERSION: maximum supported API version is 1.44: 1.45
Specifying a malformed API version also produces the same error;
DOCKER_MIN_API_VERSION=hello dockerd --validate
invalid DOCKER_MIN_API_VERSION: minimum supported API version is 1.12: hello
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `ContainerCreateConfig` and `ContainerRmConfig` structs are used for
options to be passed to the backend, and are not used in client code.
Thess struct currently is intended for internal use only (for example, the
`AdjustCPUShares` is an internal implementation details to adjust the container's
config when older API versions are used).
Somewhat ironically, the signature of the Backend has a nicer UX than that
of the client's `ContainerCreate` signature (which expects all options to
be passed as separate arguments), so we may want to update that signature
to be closer to what the backend is using, but that can be left as a future
exercise.
This patch moves the `ContainerCreateConfig` and `ContainerRmConfig` structs
to the backend package to prevent it being imported in the client, and to make
it more clear that this is part of internal APIs, and not public-facing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the initialization logic to the attachContext itself, so that
the container doesn't have to be aware about mutexes and other logic.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
I noticed this log being logged as an error, but the kill logic actually
proceeds after this (doing a "direct" kill instead). While usually containers
are expected to be exiting within the given timeout, I don't think this
needs to be logged as an error (an error is returned after we fail to
kill the container).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When live-restore is enabled, containers with autoremove enabled
shouldn't be forcibly killed when engine restarts.
They still should be removed if they exited while the engine was down
though.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
BaseFS is not serialized and is lost after an unclean shutdown. Unmount
method in the containerd image service implementation will not work
correctly in that case.
This patch will allow Unmount to restore the BaseFS if the target is
still mounted.
The reason it works with graphdrivers is that it doesn't directly
operate on BaseFS. It uses RWLayer, which is explicitly restored
immediately as soon as container is loaded.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Also, err `e` is renamed into the more standard `err` as the defer
already uses `retErr` to avoid clashes (changed in f5a611a74).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
DNS config is a property of each adapter on Windows, thus we've a
dedicated `EndpointOption` for that.
The list of `EndpointOption` that should be applied to a given endpoint
is built by `buildCreateEndpointOptions`. This function contains a
seemingly flawed condition that adds the DNS config _iff_:
1. the network isn't internal ;
2. no ports are published / exposed through another sandbox endpoint ;
While 1. does make sense, there's actually no justification for 2.,
hence this commit remove this part of the condition.
This logic flaw has been made obvious by 0fd0e82, but it was originally
introduced by d1e0a78. Commit and PR comments don't mention why this is
done like so. Most probably, this was overlooked both by the original
author and the PR reviewers.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The `buildCreateEndpointOptions` does a lot of things to build the list
of `libnetwork.EndpointOption` from the `EndpointSettings` spec. To skip
ports-related options, an early return was put in the middle of that
function body.
Early returns are generally great, but put in the middle of a 150-loc
long function that does a lot, they're just a potential footgun. And I'm
the one who pulled the trigger in 052562f. Since this commit, generic
options won't be applied to endpoints if there's already one with
exposed/published ports. As a consequence, only the first endpoint can
have a user-defined MAC address right now.
Instead of moving up the code line that adds generic options, a better
change IMO is to move ports-related options, and the early-return gating
those options, to a dedicated func to make `buildCreateEndpointOptions`
slightly easier to read and reason about.
There was actually one oddity in the original
`buildCreateEndpointOptions`: the early-return also gates the addition
of `CreateOptionDNS`. These options are Windows-specific; a comment is
added to explain that. But the oddity is really: why are we checking if
an endpoint with exposed / published ports joined this sandbox to decide
whether we want to configure DNS server on the endpoint's adapter? Well,
this early-return was most probably overlooked by the original author
and by reviewers at the time these options were added (in commit d1e0a78)
Let's fix that in a follow-up commit.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The DirCopy() function in "graphdriver/copy/copy.go" has a special case for
skip file-attribute copying when making a hard link to an already-copied
file, if "copyMode == Hardlink". Do the same for copies of hard-links in
the source filesystem.
Significantly speeds up vfs's copy of a BusyBox filesystem (which
consists mainly of hard links to a single binary), making moby's
integration tests run more quickly and more reliably in a dev container.
Fixes#46810
Signed-off-by: Rob Murray <rob.murray@docker.com>
Use a strong type for the DNS IP-addresses so that we can use flags.IPSliceVar,
instead of implementing our own option-type and validation.
Behavior should be the same, although error-messages have slightly changed:
Before this patch:
dockerd --dns 1.1.1.1oooo --validate
Status: invalid argument "1.1.1.1oooo" for "--dns" flag: 1.1.1.1oooo is not an ip address
See 'dockerd --help'., Code: 125
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1"]}
dockerd --dns 2.2.2.2 --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1])
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1oooo"]}
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: 1.1.1.1ooooo is not an ip address
With this patch:
dockerd --dns 1.1.1.1oooo --validate
Status: invalid argument "1.1.1.1oooo" for "--dns" flag: invalid string being converted to IP address: 1.1.1.1oooo
See 'dockerd --help'., Code: 125
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1"]}
dockerd --dns 2.2.2.2 --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1])
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1oooo"]}
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: invalid IP address: 1.1.1.1oooo
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- document accepted values
- add test-coverage for the function's behavior (including whitespace handling),
and use sub-tests.
- improve error-message to use uppercase for "IP", and to use a common prefix.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
I was trying to find out why `docker info` was sometimes slow so
plumbing a context through to propagate trace data through.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
It's used in various defers, but was using `err` as name, which can be
confusing, and increases the risk of accidentally shadowing the error.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
We remap the snapshot when we create a container, we have to to the
inverse when we commit the container into an image
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
When server address is not provided with the auth configuration,
use the domain from the image provided with the auth.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Change the non-refcounted implementation to perform the mount using the
same identity and access right. They should be the same regardless if
we're refcounting or not.
This also allows to refactor refCountMounter into a mounter decorator.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When the start interval is 0 we should treat that as unset.
This is especially important for older API versions where we reset the
value to 0.
Instead of using the default probe value we should be using the
configured `interval` value (which may be a default as well) which gives
us back the old behavior before support for start interval was added.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
When starting a daemon in debug mode (such as used in CI), many log-messages
are printed during startup. As a result, the log message indicating whether
graph-drivers or snapshotters are used may appear far separate from the
informational log about the daemon (and selected storage-driver).
The existing log-driver also unconditionally uses the legacy "graph-driver"
terminology, instead of the more generic "storage-driver".
This patch changes the log message shown during startup to use the generic
"graph-driver" as field, and adds a new field that indicates wheter we're
using snapshotters or graph-drivers.
Given that snapshotters will be the default at some point, an alternative
could be to include the _type_ of driver used, for example;
`io.containerd.snapshotter.v1`, which may continue to be relevant after
snapshotters become the default, and at which point (potentially) the
type of snapshotter becomes more relevant.
Before this change:
TEST_INTEGRATION_USE_SNAPSHOTTER=1 DOCKER_GRAPHDRIVER=overlayfs dockerd
...
INFO[2023-10-31T09:12:33.586269801Z] Starting daemon with containerd snapshotter integration enabled
INFO[2023-10-31T09:12:33.586322176Z] Loading containers: start.
INFO[2023-10-31T09:12:33.640514759Z] Loading containers: done.
INFO[2023-10-31T09:12:33.646498134Z] Docker daemon commit=dcf7287d647bcb515015e389df46ccf1e09855b7 graphdriver=overlayfs version=dev
INFO[2023-10-31T09:12:33.646706551Z] Daemon has completed initialization
INFO[2023-10-31T09:12:33.658840592Z] API listen on /var/run/docker.sock
With this change;
TEST_INTEGRATION_USE_SNAPSHOTTER=1 DOCKER_GRAPHDRIVER=overlayfs dockerd
...
INFO[2023-10-31T08:41:38.841155928Z] Starting daemon with containerd snapshotter integration enabled
INFO[2023-10-31T08:41:38.841207512Z] Loading containers: start.
INFO[2023-10-31T08:41:38.902461053Z] Loading containers: done.
INFO[2023-10-31T08:41:38.910535137Z] Docker daemon commit=dcf7287d647bcb515015e389df46ccf1e09855b7 containerd-snapshotter=true storage-driver=overlayfs version=dev
INFO[2023-10-31T08:41:38.910936803Z] Daemon has completed initialization
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In case of `docker push -a`, we need to return an error if there is no
image for the given repository.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Don't wrap the `no basic auth credentials` error from containerd and
return it as-is.
The error will look like:
```
failed to resolve reference "docker.io/library/aodkoakds:latest": pull access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed
```
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Kept `coci` import alias since we use it elsewhere,
maybe to prevent confusion with our own `oci` package.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Having a sandbox/container-wide MacAddress field makes little sense
since a container can be connected to multiple networks at the same
time. This field is an artefact of old times where a container could be
connected to a single network only.
As we now have a way to specify per-endpoint mac address, this field is
now deprecated.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prior to this commit, only container.Config had a MacAddress field and
it's used only for the first network the container connects to. It's a
relic of old times where custom networks were not supported.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The same error is already returned by `(*Daemon).containerCreate()` but
since this function is also called by the cluster executor, the error
has to be duplicated.
Doing that allows to remove a nil check on container config in
`postContainersCreate`.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
containerd's `WithUser` function now resets this property, starting with
[3eda46af12b1deedab3d0802adb2e81cb3521950][1] (v1.7.0-beta.4), so we no
longer need this function.
[1]: 3eda46af12
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The github.com/opencontainers/runc/libcontainer/user package was moved
to a separate module. While there's still uses of the old module in
our code-base, runc itself is migrating to the new module, and deprecated
the old package (for runc 1.2).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit def549c8f6 passed through the context
to the daemon.ContainerStart function. As a result, restarting containers
no longer is an atomic operation, because a context cancellation could
interrupt the restart (between "stopping" and "(re)starting"), resulting
in the container being stopped, but not restarted.
Restarting a container, or more factually; making a successful request on
the `/containers/{id]/restart` endpoint, should be an atomic operation.
This patch uses a context.WithoutCancel for restart requests.
It's worth noting that daemon.containerStop already uses context.WithoutCancel,
so in that function, we'll be wrapping the context twice, but this should
likely not cause issues (just redundant for this code-path).
Before this patch, starting a container that bind-mounts the docker socket,
then restarting itself from within the container would cancel the restart
operation. The container would be stopped, but not started after that:
docker run -dit --name myself -v /var/run/docker.sock:/var/run/docker.sock docker:cli sh
docker exec myself sh -c 'docker restart myself'
docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
3a2a741c65ff docker:cli "docker-entrypoint.s…" 26 seconds ago Exited (128) 7 seconds ago myself
With this patch: the stop still cancels the exec, but does not cancel the
restart operation, and the container is started again:
docker run -dit --name myself -v /var/run/docker.sock:/var/run/docker.sock docker:cli sh
docker exec myself sh -c 'docker restart myself'
docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
4393a01f7c75 docker:cli "docker-entrypoint.s…" About a minute ago Up 4 seconds myself
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use context.WithoutCancel so that both the containerStop and
container.Wait can share the same parent context. This context is still
a "TODO", but can be wired up in future.
It's worth noting that daemon.containerStop already uses context.WithoutCancel,
so in that function, we'll be wrapping the context twice, but this should
likely not cause issues (just redundant for this code-path).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Follow-up to fc94ed0a86. Now that
f6e44bc0e8 added the compatcontext
package, we can start using context.WithoutCancel.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In the tagged case the error message when the image/tag is not found
should be "tag does not exist: ref"
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
When choosing the next image, don't reject images without the classic
builder parent label. The intention was to *prefer* images them instead
of making that a condition.
This fixes the ID not being filled for parent images that weren't built
with the classic builder.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The `Tags` slice of each history entry was filled with tags of parent
image. Change it to correctly assign the current image tags.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Check for accurate values that may contain content sizes unknown to the
usage test in the calculation. Avoid asserting using deep equals when
only the expected value range is known to the test.
Signed-off-by: Derek McGowan <derek@mcg.dev>
After a successful push, all pushed blobs should have a
distribution.source label pointing to the new registry.
Before this commit, the label was only appended to the top-level blob
(manifest or manifest list). Adjust this to also do that recursively to
its children.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Use the distribution code to query the remote repository for tags and
pull them sequentially just like the non-c8d pull.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When the default bridge is disabled by setting dockerd's `--bridge=none`
option, the daemon still creates a sandbox for containers with no
network attachment specified. In that case `NetworkDisabled` will be set
to true.
However, currently the `releaseNetwork` call will early return if
NetworkDisabled is true. Thus, these sandboxes won't be deleted until
the daemon is restarted. If a high number of such containers are
created, the daemon would then take few minutes to start.
See https://github.com/moby/moby/issues/42461.
Signed-off-by: payall4u <payall4u@qq.com>
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This is a follow-up to 2216d3ca8d, which
implemented the StartInterval for health-checks, but did not add validation
for the minimum accepted interval;
> The time to wait between checks in nanoseconds during the start period.
> It should be 0 or at least 1000000 (1 ms). 0 means inherit.
This patch adds validation for the minimum accepted interval (1ms).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit 8777592397, which
turns out to break other test cases/the registry flow.
The correct place to handle missing credentials is instead
15bf23df09/remotes/docker/authorizer.go (L200).
Co-authored-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Use a unique parent view snapshot key for each diff request.
I considered using singleflight at first, but I realized it wouldn't
really be correct.
The diff can take some time, so there's a window of time between the
diff start and finish, where the file system can change.
These changes not always will be reflected in the running diff.
With singleflight, the second diff request which happened before the
previous diff was finished, would not include changes made to the
container filesystem after the first diff request has started.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Implement a behavior from the graphdriver's export where `docker save
something` (untagged reference) would export all images matching the
specified repository.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The github.com/containerd/containerd/log package was moved to a separate
module, which will also be used by upcoming (patch) releases of containerd.
This patch moves our own uses of the package to use the new module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`docker.io` is present in the `IndexConfigs` so the `Mirrors` property
would get lost because a fresh `RegistryConfig` object was created.
Instead of creating a new object, reuse the existing one and just
mutate its fields.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Extract the distribution source label append into its own function and
make it not fail on any error, we do still log the error.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
This isn't something that user should do, but technically the dangling
images exist in the image store and user can pass its name (`moby-dangling@digest`).
Change it so rmi now recognizes that it's actually a dangling image and
doesn't handle it like a regular tagged image.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Instead of passing a completely fresh context without any values, just
discard the cancellation.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This reverts commit a9fa147a92.
The commit is unfortunately broken as it is still using `providerHandle`
to write events but that handle is never actually set, so it is always
invalid. All logging fails.
Note: This is note a straight revert due to the change to
containerd/log.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
To match the graphdriver's push behavior which only shows the progress
for layers.
Exclude indexes, manifests and image configs from the push progress.
Don't explicitly check for `IsLayerType` to also handle other
potentially big blobs (like buildkit attestations).
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Fix issue 46563 "Rootful-in-Rootless dind doesn't work since systemd v250 (due to oom score adj)"
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
This commit moves one-shot stats processing out of the publishing
channels, i.e. collect stats directly.
Also changes the method of getSystemCPUUsage() on Linux to return
number of online CPUs also.
Signed-off-by: Xinfeng Liu <XinfengLiu@icloud.com>
With BuildKit 0.12, some existing types are now required to be wrapped
by new types:
* containerd's LeaseManager and ContentStore have to be a
(namespace-aware) BuildKit type since f044e0a946
* BuildKit's solver.CacheManager is used instead of
bboltstorage.CacheKeyStorage since 2b30693409
* The MaxAge config field is a bkconfig.Duration since e06c96274f
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
The DeepEqual ignore required in the daemon tests is a bit ugly, but it
works given the new protoc output.
We also have to ignore lints related to schema1 deprecations; these do
not apply as we must continue to support this schema version.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
This type was introduced in
0a79e67e4f
Make use of it throughout our log-format handling code, and convert back
to a string before we pass it to the containerd client.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Diffing a container yielded some extra changes that come from the
files/directories that we mount inside the container (/etc/resolv.conf
for example). To avoid that we create an intermediate snapshot that has
these files, with this we can now diff the container fs with its parent
and only get the differences that were made inside the container.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Final progress messages were sent after the progress updater finished
which made the "Downloading" progress not being updated into "Download
complete".
Fix by sending the final messages after the progress has finished.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
We weren't checking for the asked platform in the case the image was a
manifest, only if it was a manifest list.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Makes it possible to pull `application/vnd.docker.distribution.manifest.v1+prettyjws`
legacy manifests.
They are not stored in their original form but are converted to the OCI
manifests.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Return the number of containers that use an image if it was asked,
during a `docker system df` call for example.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
This issue wasn't caught on ContainerCreate or NetworkConnect (when
container wasn't started yet).
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Thus far, validation code would stop as soon as a bad value was found.
Now, we try to validate as much as we can, to return all errors to the
API client.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
So far, only a subset of NetworkingConfig was validated when calling
ContainerCreate. Other parameters would be validated when the container
was started. And the same goes for EndpointSettings on NetworkConnect.
This commit adds two validation steps:
1. Check if the IP addresses set in endpoint's IPAMConfig are valid,
when ContainerCreate and ConnectToNetwork is called ;
2. Check if the network allows static IP addresses, only on
ConnectToNetwork as we need the libnetwork's Network for that and it
might not exist until NetworkAttachment requests are sent to the
Swarm leader (which happens only when starting the container) ;
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Make `PullImage` accept `reference.Named` directly instead of
duplicating the parsing code for both graphdriver and containerd image
service implementations.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
We were sending the "Pulling from ..." message too early, if the pull
progress wasn't able to resolve the image we wouldn't sent the error
back. Sending that first message would have flushed the output stream
and image_routes.go would return a nil error.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
full diff: https://github.com/containerd/containerd/compare/v1.6.22...v1.6.24
v1.6.24 release notes:
full diff: https://github.com/containerd/containerd/compare/v1.6.23...v1.6.24
The twenty-fourth patch release for containerd 1.6 contains various fixes
and updates.
Notable Updates
- CRI: fix leaked shim caused by high IO pressure
- Update to go1.20.8
- Update runc to v1.1.9
- Backport: add configurable mount options to overlay snapshotter
- log: cleanups and improvements to decouple more from logrus
v1.6.23 release notes:
full diff: https://github.com/containerd/containerd/compare/v1.6.22...v1.6.23
The twenty-third patch release for containerd 1.6 contains various fixes
and updates.
Notable Updates
- Add stable ABI support in windows platform matcher + update hcsshim tag
- cri: Don't use rel path for image volumes
- Upgrade GitHub actions packages in release workflow
- update to go1.19.12
- backport: ro option for userxattr mount check + cherry-pick: Fix ro mount option being passed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add "The push referers to repository X" message which is present in the
push output when using the graphdrivers.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
