Allow to set daemon and server configurations in a file. #18587

calavera · 2015-12-10T23:39:44Z

Still a work in progress but I thought it could be a good idea to show this now that mostly works.

Read configuration after flags making this the priority:

1- Apply configuration from file.
2- Apply configuration from flags.

Reload configuration when it a signal is received, HUP in Linux:

Reload router if the debug configuration changes.
Reload daemon labels.
Reconfigure and restart cluster discovery.

Closes #17559
Signed-off-by: David Calavera david.calavera@gmail.com

dhiltgen · 2015-12-11T00:43:16Z

docker/daemon.go

@@ -47,6 +47,9 @@ func NewDaemonCli() *DaemonCli {
 	daemonConfig := new(daemon.Config)
 	daemonConfig.LogConfig.Config = make(map[string]string)
 	daemonConfig.ClusterOpts = make(map[string]string)
+
+	daemonFlags.StringVar(&daemonConfig.ConfigurationFile, []string{"-config"}, "", "Daemon configuration file")


What do you think about having a default path, and then doing the fsnotify watch even if it doesn't exist so we can catch creation of the file after the daemon starts up?

I think it makes sense. I'll play with it to see how it feels.

dhiltgen · 2015-12-11T00:54:32Z

Very cool! Thanks for picking this up! :-)

duglin · 2015-12-11T12:35:35Z

@calavera I might be misreading your comment, but does the config file flags override the cmd line flags? If so, I think it should be the other way around. I believe, typically, its: process config file, env vars and then cmd line flags - where last one in that list wins.

calavera · 2015-12-11T17:35:54Z

@duglin I think I agree with you. I started implementing it that way but I had some issues with duplicated flags. Definitely worth considering.

cpuguy83 · 2015-12-12T13:22:52Z

daemon/config.go

+		}
+	}()
+
+	watcher.Add(configFile)


Note that this can return an error due to things like inotify exhaustion.
This is really where fallback to polling should happen (if you want it to fallback).

cpuguy83 · 2015-12-12T13:24:49Z

While I like auto-reloading on changes, it seems like it could be dangerous due to partial changes.
I'd go with the safer route of having an explicit way to reload (such as a signal handler and/or an API endpoint).

thaJeztah · 2015-12-12T14:10:41Z

We may need a "configcheck" option, so that the configuration can be validated before its applied

runcom · 2015-12-18T15:24:28Z

I agree with @duglin the order of configs should be first config file, then flags (elasticsearch for instance works this way IIRC)
The config reload honestly seems like a big plus to me (for an initial config file PR), I might live w/o it and have to restart the daemon :)

calavera · 2016-01-04T21:23:34Z

Thank you all for your feedback! 💪

Given what I'm reading in these comments, and that the release freeze is close, I think this is the best plan to approach this feature for 1.10:

Config reading order: file first, flags second
Remove file watcher, use a signal (HUP) to trigger a config reload.
Check configuration before applying reload.

I'd also like to restrict the configuration elements we reload in this first version to only these:

Debug flag: reload logger and server routes to enable/disable debugging without restarting the daemon.
Damon labels: reset these labels with new values. We won't do any diff-ing, it's all or nothing.
Discovery: reset backend, stop advertising in the old backend and advertise in the new backend.

@dhiltgen please, take a look at this, I'd like to make sure you're okay with this initial set of features.

dhiltgen · 2016-01-04T22:27:09Z

Yes, I think this should work. Thanks!

nathanleclaire · 2016-01-05T19:51:43Z

Cool @calavera . Like @cpuguy83 mentioned, what's the reasoning for watching the file and reloading automatically when something changes? Some things to consider in this regard:

Why violate existing convention of tools such as HAproxy and nginx doing "hot reload" of configuration using UNIX signals? (e.g. SIGHUP instructs nginx to reload configuration http://nginx.org/en/docs/beginners_guide.html#control)
What happens if an operator saves the file in an incomplete or invalid state?
What happens if the editor or method of modifying the file (e.g. a tool like Ansible) happens to end up sending a lot of write events? Yes there is a mutex, but the daemon could still potentially trigger an order of magnitude more reloads than required.

calavera · 2016-01-05T20:42:38Z

@nathanleclaire read my latest comment (before this one)

nathanleclaire · 2016-01-05T21:58:03Z

@calavera Ah OK I didn't see. That seems reasonable -- thanks!

thaJeztah · 2016-01-14T19:03:48Z

ping @vdemeester @MHBauer @jamtur01 for review <3

calavera · 2016-01-14T19:07:37Z

docs/reference/commandline/daemon.md

+## Daemon configuration file
+
+The `--config-file` option allows you to set any configuration option for the daemon
+in a JSON format. This file uses the flag names as keys to set documentation. By


This file uses the flag names as keys to set documentation

no idea what this means, looking for alternatives.

vdemeester · 2016-01-14T19:11:40Z

@calavera you need to update man/docker-daemon.8.md too 😝

calavera · 2016-01-14T19:12:14Z

docs/reference/commandline/daemon.md

+to restart the process. We use SIGHUP in Linux to reload and a global event
+in Windows with the key `Global\docker-daemon-config-$PID`. The options can
+be modified in the configuration file but still will check for conflicts with
+the provided flags. The daemon fails to reconfigure itself if there are conflicts


conflicts but

conflicts, but

calavera · 2016-01-14T19:21:04Z

@vdemeester done, thanks!

vdemeester · 2016-01-14T19:47:47Z

@calavera another question about the output when there is conflicts. With 2 conflicts, I ended up with that :

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: debug
: (from flag: true, from file: true), labels: (from flag: [bar=foo], from file: [foo=bar])

I think it would be a clearer if we put one conflict by line (carriage return, etc..), something like :

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: 
- debug: (from flag: true, from file: true)
- labels: (from flag: [bar=foo], from file: [foo=bar])

wdyt ? 😝 👼

calavera · 2016-01-14T19:48:37Z

I think it would be a clearer if we put one conflict by line (carriage return, etc..), something like

that option was discarded after @cpuguy83's comment

vdemeester · 2016-01-14T21:35:09Z

docs/reference/commandline/daemon.md

+### Configuration reloading
+
+Some options can be reconfigured when the daemon is running without requiring
+to restart the process. We use SIGHUP in Linux to reload, and a global event


Maybe SIGHUP 😝

or: "We use the SIGHUP signal"

fixed with "We use the SIGHUP signal"`

vdemeester · 2016-01-14T21:36:31Z

small nit but docs LGTM 🐮

Read configuration after flags making this the priority: 1- Apply configuration from file. 2- Apply configuration from flags. Reload configuration when a signal is received, USR2 in Linux: - Reload router if the debug configuration changes. - Reload daemon labels. - Reload cluster discovery. Signed-off-by: David Calavera <david.calavera@gmail.com>

thaJeztah · 2016-01-14T22:35:26Z

docs LGTM \o/

tiborvass · 2016-01-14T23:47:09Z

https://jenkins.dockerproject.org/job/Docker-PRs-experimental/13695/console hang and timed out...

thaJeztah · 2016-01-15T00:44:53Z

It's greeeeeeen!

Allow to set daemon and server configurations in a file.

neg3ntropy · 2016-02-24T06:16:03Z

This looks cool, but its potential is far from delivered. It's still totally undocumented at https://docs.docker.com/engine/admin/configuring/ and a user has no idea of what settings are supported and what is the syntax.

I will say that config file is the way to go by default for packaged docker: it's way easier than overriding the systemd unit.
Possibly yaml (with plenty of comments) is a better file format than json. A lot of unix services come with a well documented config file and most of the time it's enough to read through it and uncomment/change lines. It saves many trips to the web.

Other than SIGHUP, I would also consider a docker daemon --reload-config or similar.

thaJeztah · 2016-02-24T10:26:17Z

@soulrebel agreed that the documentation for configuring the daemon needs to be touched up (in general as well, because the docs is now spread over several locations)

For using this file by default, see this issue #20151, which I opened to discuss that.

GordonTheTurtle added the status/0-triage label Dec 10, 2015

calavera added status/1-design-review and removed status/0-triage labels Dec 10, 2015

calavera changed the title ~~[WIP] Add daemon configuration flag.~~ [WIP] Add daemon configuration file. Dec 10, 2015

calavera changed the title ~~[WIP] Add daemon configuration file.~~ [WIP] Allow to set daemon and server configurations in a file. Dec 10, 2015

calavera force-pushed the daemon_configuration_file branch 2 times, most recently from 20d7352 to 7255c49 Compare December 11, 2015 00:03

dhiltgen reviewed Dec 11, 2015
View reviewed changes

calavera force-pushed the daemon_configuration_file branch from 7255c49 to 4bf22ca Compare December 11, 2015 17:25

calavera force-pushed the daemon_configuration_file branch 4 times, most recently from 20f3bbc to d7b5f20 Compare December 11, 2015 23:14

cpuguy83 reviewed Dec 12, 2015
View reviewed changes

calavera force-pushed the daemon_configuration_file branch from d7b5f20 to ebe418c Compare December 16, 2015 23:50

thaJeztah added the status/needs-attention Calls for a collective discussion during a review session label Dec 29, 2015

calavera added this to the 1.10.0 milestone Jan 5, 2016

calavera force-pushed the daemon_configuration_file branch from ebe418c to 6a9aaad Compare January 5, 2016 23:35

calavera reviewed Jan 14, 2016
View reviewed changes

calavera force-pushed the daemon_configuration_file branch 2 times, most recently from 5993f86 to 961de3c Compare January 14, 2016 19:20

vdemeester reviewed Jan 14, 2016
View reviewed changes

calavera force-pushed the daemon_configuration_file branch from 961de3c to 677a6b3 Compare January 14, 2016 21:44

thaJeztah added the docs/revisit label Jan 14, 2016

thaJeztah added status/4-merge and removed status/3-docs-review labels Jan 14, 2016

icecrime removed the docs/revisit label Jan 14, 2016

thaJeztah added a commit that referenced this pull request Jan 15, 2016

Merge pull request #18587 from calavera/daemon_configuration_file

e44364e

Allow to set daemon and server configurations in a file.

thaJeztah merged commit e44364e into moby:master Jan 15, 2016

calavera deleted the daemon_configuration_file branch January 15, 2016 02:54

thaJeztah mentioned this pull request Jan 19, 2016

Daemon configuration not preserved on reboots in boot2docker #19454

Closed

allencloud mentioned this pull request Mar 28, 2016

cluster store and advertise info does not show in docker info when reload configuration #21564

Closed

thaJeztah mentioned this pull request Dec 15, 2022

daemon/config: rename CommonTLSOptions, and remove RootDeprecated field #44649

Merged

thaJeztah mentioned this pull request Mar 1, 2023

api/server: delete Wait method #45088

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to set daemon and server configurations in a file. #18587

Allow to set daemon and server configurations in a file. #18587

calavera commented Dec 10, 2015

dhiltgen Dec 11, 2015

calavera Dec 11, 2015

dhiltgen commented Dec 11, 2015

duglin commented Dec 11, 2015

calavera commented Dec 11, 2015

cpuguy83 Dec 12, 2015

cpuguy83 commented Dec 12, 2015

thaJeztah commented Dec 12, 2015

runcom commented Dec 18, 2015

calavera commented Jan 4, 2016

dhiltgen commented Jan 4, 2016

nathanleclaire commented Jan 5, 2016

calavera commented Jan 5, 2016

nathanleclaire commented Jan 5, 2016

thaJeztah commented Jan 14, 2016

calavera Jan 14, 2016

vdemeester commented Jan 14, 2016

calavera Jan 14, 2016

calavera commented Jan 14, 2016

vdemeester commented Jan 14, 2016

calavera commented Jan 14, 2016

vdemeester Jan 14, 2016

tiborvass Jan 14, 2016

calavera Jan 14, 2016

vdemeester commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

tiborvass commented Jan 14, 2016

thaJeztah commented Jan 15, 2016

neg3ntropy commented Feb 24, 2016

thaJeztah commented Feb 24, 2016

Allow to set daemon and server configurations in a file. #18587

Allow to set daemon and server configurations in a file. #18587

Conversation

calavera commented Dec 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhiltgen commented Dec 11, 2015

duglin commented Dec 11, 2015

calavera commented Dec 11, 2015

Choose a reason for hiding this comment

cpuguy83 commented Dec 12, 2015

thaJeztah commented Dec 12, 2015

runcom commented Dec 18, 2015

calavera commented Jan 4, 2016

dhiltgen commented Jan 4, 2016

nathanleclaire commented Jan 5, 2016

calavera commented Jan 5, 2016

nathanleclaire commented Jan 5, 2016

thaJeztah commented Jan 14, 2016

Choose a reason for hiding this comment

vdemeester commented Jan 14, 2016

Choose a reason for hiding this comment

calavera commented Jan 14, 2016

vdemeester commented Jan 14, 2016

calavera commented Jan 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdemeester commented Jan 14, 2016

thaJeztah commented Jan 14, 2016

tiborvass commented Jan 14, 2016

thaJeztah commented Jan 15, 2016

neg3ntropy commented Feb 24, 2016

thaJeztah commented Feb 24, 2016