This helps ensure that users are not surprised by unexpected tokens in
the JSON parser, or fallout later in the daemon.
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
(cherry picked from commit 8dbc5df952)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is a pragmatic but impure choice, in order to better support the
default tools available on Windows Server, and reduce user confusion due
to otherwise inscrutable-to-the-uninitiated errors like the following:
> invalid character 'þ' looking for beginning of value
> invalid character 'ÿ' looking for beginning of value
While meaningful to those who are familiar with and are equipped to
diagnose encoding issues, these characters will be hidden when the file
is edited with a BOM-aware text editor, and further confuse the user.
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
(cherry picked from commit d42495033e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This centralizes more defaults, to be part of the config struct that's
created, instead of interweaving the defaults with other code in various
places.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b28e66cf4f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[RFC 8259] allows for JSON implementations to optionally ignore a BOM
when it helps with interoperability; do so in Moby as Notepad (the only
text editor available out of the box in many versions of Windows Server)
insists on writing UTF-8 with a BOM.
[RFC 8259]: https://tools.ietf.org/html/rfc8259#section-8.1
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
(cherry picked from commit bb19265ba8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also cleaning up some errors
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 56e64270f3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since runtimes can now just be containerd shims, we need to check if the
reference is possibly a containerd shim.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit e6ee27a541)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The `-g` / `--graph` options were soft deprecated in favor of `--data-root` in
261ef1fa27 (v17.05.0) and at the time considered
to not be removed. However, with the move towards containerd snapshotters, having
these options around adds additional complexity to handle fallbacks for deprecated
(and hidden) flags, so completing the deprecation.
With this patch:
dockerd --graph=/var/lib/docker --validate
Flag --graph has been deprecated, Use --data-root instead
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: the "graph" config file option is deprecated; use "data-root" instead
mkdir -p /etc/docker
echo '{"graph":"/var/lib/docker"}' > /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: the "graph" config file option is deprecated; use "data-root" instead
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b58de39ca7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Removes some custom handling, some of which were giving the wrong
error on failure ("expected no error" when we were checking for an
error).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Set the defaults when constructing the config, instead of setting them
indirectly through the command-line flags.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was validating that the config file would not overwrite the
log-opt, but the test did not set up the flags correctly; as the flags
were not marked as "changed", it would not detect a conflict between
the config-file and daemon-flags.
This patch:
- removes the incorrect fields from the JSON file
- initializes the Config using config.New(), so that any defaults are also set
- sets flag values by actually setting them through the flags
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- TestReloadWithDuplicateLabels() also check value
- TestReloadDefaultConfigNotExist, TestReloadBadDefaultConfig,
TestReloadWithConflictingLabels: verify that config is not
reloaded.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use the default (0) value to indicate "not set", which simplifies
working with these configuration options, preventing the need to
use intermediate variables etc.
While changing this code, also making some small cleanups, such
as replacing "fmt.Sprintf()" for "strconv" variants.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
reloadMaxDownloadAttempts() is used to reload the configuration,
but validation happened before merging the config with the defaults.
This removes the validation from this function, instead centralizing
validation in config.Validate().
NOTE:
Currently this validation is "ok", as it checks for "nil" values;
I am working on changes to reduce the use of pointers in the config,
and instead provide a mechanism to fill in defaults. This change is
in preparation of that.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Reload logic is problematic and needs a rewrite.
Currently, config.Reload() is validating newConfig before the reload callback
is executed. At that point, newConfig may be a partial configuration, yet to be
merged with the existing configuration (in the "reload()" callback). Validating
this config before it's merged can result in incorrect validation errors.
However, the current "reload()" callback we use is DaemonCli.reloadConfig(),
which includes a call to Daemon.Reload(), which both performs "merging" and
validation, as well as actually updating the daemon configuration. Calling
DaemonCli.reloadConfig() *before* validation, could thus lead to a failure in
that function (making the reload non-atomic).
While *some* errors could always occur when applying/updating the config, we
should make it more atomic, and;
1. get (a copy of) the active configuration
2. get the new configuration
3. apply the (reloadable) options from the new configuration
4. validate the merged results
5. apply the new configuration.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
MergeDaemonConfigurations was validating the configs before and after
merging. However, the "fileConfig" configuration may contain only a
"partial" configuration (options to apply to / override the existing
config). This means that some options may not be set and contain default
or empty values.
Validating such partial configurations can produce validation failures,
so to prevent those, we should validate the configuration _after_
merging, to validate the "final" state.
There's more cleaning up / improvements to be made in this area; for
example, we currently use our "self crafted" `getConflictFreeConfiguration()`
function, which is used to detect options that are not allowed to
be overridden, and which could potentially be handled by mergo.Merge(),
but leaving those changes for a future exercise.
This patch removes the first validation step, changing the function
to only validate the resulting configuration after merging.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Log-level validation was previously performed when configuring the daemon-logs;
this moves the validation to config.Validate() so that we can catch invalid
settings when running dockerd --validate.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is a follow-up to 427c7cc5f8, which added
proxy-configuration options ("http-proxy", "https-proxy", "no-proxy") to the
dockerd cli and in `daemon.json`.
While working on documentation changes for this feature, I realised that those
options won't be "next" to each-other when formatting the daemon.json JSON, for
example using `jq` (which sorts the fields alphabetically). As it's possible that
additional proxy configuration options are added in future, I considered that
grouping these options in a struct within the JSON may help setting these options,
as well as discovering related options.
This patch introduces a "proxies" field in the JSON, which includes the
"http-proxy", "https-proxy", "no-proxy" options.
Conflict detection continues to work as before; with this patch applied:
mkdir -p /etc/docker/
echo '{"proxies":{"http-proxy":"http-config", "https-proxy":"https-config", "no-proxy": "no-proxy-config"}}' > /etc/docker/daemon.json
dockerd --http-proxy=http-flag --https-proxy=https-flag --no-proxy=no-proxy-flag --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:
http-proxy: (from flag: http-flag, from file: http-config),
https-proxy: (from flag: https-flag, from file: https-config),
no-proxy: (from flag: no-proxy-flag, from file: no-proxy-config)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The config.Validate() function did not validate hosts that were configured in
the daemon.json configuration file, resulting in `--validate` to pass, but the
daemon failing to start.
before this patch:
echo '{"hosts":["127.0.0.1:2375/path"]}' > /etc/docker/daemon.json
dockerd --validate
configuration OK
dockerd
INFO[2022-04-03T11:42:22.162366200Z] Starting up
failed to load listeners: error parsing -H 127.0.0.1:2375/path: invalid bind address (127.0.0.1:2375/path): should not contain a path element
with this patch:
echo '{"hosts":["127.0.0.1:2375/path"]}' > /etc/docker/daemon.json
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: configuration validation from file failed: invalid bind address (127.0.0.1:2375/path): should not contain a path element
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The TestReloadDefaultConfigNotExist() test assumed it was running in a clean
environment, in which the `/etc/docker/daemon.json` file doesn't exist, and
would fail if that was not the case.
This patch updates the test to override the default location to a a non-existing
path, to allow running the test in an environment where `/etc/docker/daemon.json`
is present.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This removes the plugin section from the containerd configuration file
(`/var/run/docker/containerd/containerd.toml`) that is generated when
starting containerd as child process;
```toml
[plugins]
[plugins.linux]
shim = "containerd-shim"
runtime = "runc"
runtime_root = "/var/lib/docker/runc"
no_shim = false
shim_debug = true
```
This configuration doesn't appear to be used since commit:
0b14c2b67a, which switched the default runtime
to to io.containerd.runc.v2.
Note that containerd itself uses `containerd-shim` and `runc` as default
for `shim` and `runtime` v1, so omitting that configuration doesn't seem
to make a difference.
I'm slightly confused if any of the other options in this configuration were
actually used: for example, even though `runtime_root` was configured to be
`/var/lib/docker/runc`, when starting a container with that coniguration set
on docker 19.03, `/var/lib/docker/runc` doesn't appear to exist:
```console
$ docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
098baa4cb0e7 nginx:alpine "/docker-entrypoint.…" 59 minutes ago Up 59 minutes 80/tcp foo
$ ls /var/lib/docker/runc
ls: /var/lib/docker/runc: No such file or directory
$ ps auxf
PID USER TIME COMMAND
1 root 0:00 sh
16 root 0:11 dockerd --debug
26 root 0:09 containerd --config /var/run/docker/containerd/containerd.toml --log-level debug
234 root 0:00 containerd-shim -namespace moby -workdir /var/lib/docker/containerd/daemon/io.containerd.runtime.v1.linux/moby/09
251 root 0:00 nginx: master process nginx -g daemon off;
304 101 0:00 nginx: worker process
...
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
I had to check what the actual size was, so added it to the const's documentation.
While at it, also made use of it in a test, so that we're testing against the expected
value, and changed one alias to be consistent with other places where we alias this
import.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The daemon can print the proxy configuration as part of error-messages,
and when reloading the daemon configuration (SIGHUP). Make sure that
the configuration is sanitized before printing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This allows configuring the daemon's proxy server through the daemon.json con-
figuration file or command-line flags configuration file, in addition to the
existing option (through environment variables).
Configuring environment variables on Windows to configure a service is more
complicated than on Linux, and adding alternatives for this to the daemon con-
figuration makes the configuration more transparent and easier to use.
The configuration as set through command-line flags or through the daemon.json
configuration file takes precedence over env-vars in the daemon's environment,
which allows the daemon to use a different proxy. If both command-line flags
and a daemon.json configuration option is set, an error is produced when starting
the daemon.
Note that this configuration is not "live reloadable" due to Golang's use of
`sync.Once()` for proxy configuration, which means that changing the proxy
configuration requires a restart of the daemon (reload / SIGHUP will not update
the configuration.
With this patch:
cat /etc/docker/daemon.json
{
"http-proxy": "http://proxytest.example.com:80",
"https-proxy": "https://proxytest.example.com:443"
}
docker pull busybox
Using default tag: latest
Error response from daemon: Get "https://registry-1.docker.io/v2/": proxyconnect tcp: dial tcp: lookup proxytest.example.com on 127.0.0.11:53: no such host
docker build .
Sending build context to Docker daemon 89.28MB
Step 1/3 : FROM golang:1.16-alpine AS base
Get "https://registry-1.docker.io/v2/": proxyconnect tcp: dial tcp: lookup proxytest.example.com on 127.0.0.11:53: no such host
Integration tests were added to test the behavior:
- verify that the configuration through all means are used (env-var,
command-line flags, damon.json), and used in the expected order of
preference.
- verify that conflicting options produce an error.
Signed-off-by: Anca Iordache <anca.iordache@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This adds support for 2 runtimes on Windows, one that uses the built-in
HCSv1 integration and another which uses containerd with the runhcs
shim.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
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>
Using "default" as a name is a bit ambiguous, because the _daemon_ default
can be changed using the '--seccomp-profile' daemon flag.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
