Add configuration validation option. by justrich · Pull Request #38138 · moby/moby

justrich · 2018-11-05T00:56:24Z

Add a command line option.
Add option to exit before daemon startup if validating config.

If config file is invalid we'll exit anyhow, so this just prevents
the daemon from starting if the configuration is fine.

Mainly useful for making config changes and restarting the daemon
iff the config is valid.

Signed-off-by: Rich Horwood [email protected]

- What I did

- How I did it

- How to verify it

- Description for the changelog

Add option to validate daemon configuration file and exit.

- A picture of a cute animal (not mandatory but encouraged)

cmd/dockerd/daemon.go

codecov · 2018-12-19T17:29:24Z

Codecov Report

Merging #38138 into master will increase coverage by 0.52%.
The diff coverage is 29.35%.

@@            Coverage Diff             @@
##           master   #38138      +/-   ##
==========================================
+ Coverage   36.18%   36.70%   +0.52%     
==========================================
  Files         610      608       -2     
  Lines       45363    45082     -281     
==========================================
+ Hits        16413    16547     +134     
+ Misses      26707    26252     -455     
- Partials     2243     2283      +40

olljanat · 2019-01-02T04:51:49Z

@justrich this one need to be rebased

olljanat · 2019-01-08T18:51:16Z

(reserved for my derek comments)

olljanat · 2019-01-10T20:12:00Z

@thaJeztah FYI. this looks to be updated based on your review and passing CI (except codecov/patch).

thaJeztah

oh boy, looks like I started a review, and never submitted

thaJeztah · 2018-11-23T11:20:54Z

cmd/dockerd/options.go

We're generally conservative when adding single-letter flags, and want to reserve new ones for "frequently used options"; could you remove the -V shorthand? (It's easy to add in future if we see a need, but it's much harder to remove an option, once it's been released 😅)

Suggested change

flags.BoolVarP(&o.Validate, "validate", "V", false, "Validate configuration file and exit")

flags.BoolVar(&o.Validate, "validate", false, "Validate configuration file and exit")

thaJeztah

forgot about this one. I think this one is good to have (docker/cli#1922 would still be useful to have as well, but is orthogonal to this change)

LGTM

(I did a quick rebase to trigger CI again)

thaJeztah · 2020-07-08T08:56:18Z

@AkihiroSuda @tiborvass PTAL

chris-crone

LGTM

AkihiroSuda · 2021-02-03T09:23:20Z

LGTM, but https://ci-next.docker.com/public/job/moby/job/PR-38138/2/display/redirect is now 404, so I can't click the "restart CI" button.

Could you rebase to retrigger CI?

thaJeztah · 2021-02-03T09:34:25Z

Oh; I think editing the top-comment triggers ci (neat little trick); let me try

thaJeztah · 2021-02-03T09:35:29Z

Yup, that worked 😂. I think it's a bug in how jenkins is triggered, but comes in handy sometimes

thaJeztah · 2021-02-03T10:09:52Z

Failing CI; I guess it needs a rebase to get some CI fixes in; let me do so

thaJeztah · 2021-02-03T14:29:32Z

[2021-02-03T10:22:44.308Z] === Errors
[2021-02-03T10:22:44.308Z] cmd/dockerd/options.go:63: not enough arguments in call to flags.BoolVarP
[2021-02-03T10:22:44.308Z] 	have (*bool, string, bool, string)
[2021-02-03T10:22:44.308Z] 	want (*bool, string, string, bool, string)

Fixes moby#36911 Add a command line option. Add option to exit before daemon startup if validating config. If config file is invalid we'll exit anyhow, so this just prevents the daemon from starting if the configuration is fine. Mainly useful for making config changes and restarting the daemon iff the config is valid. Signed-off-by: Rich Horwood <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2021-02-03T14:41:35Z

Fixed the above; s/BoolVarP/BoolVar/ (as it no longer has a shorthand flag)

coolljt0725 · 2021-02-03T15:05:36Z

cmd/dockerd/daemon.go

+	if opts.Validate {
+		// If config wasn't OK we wouldn't have made it this far.
+		logrus.Infof("Config OK")
+		os.Exit(0)


IMO, we should be careful use os.Exit and let the program exit immediately in big program, this may cause some defer doesn't perform and then left with some uncleaned resoure. https://gobyexample.com/exit It's better to return a state to the caller.

Oh, good one, actually didn't know that.

Yes, looks like that would cause some things to not be run

moby/cmd/dockerd/daemon.go

Lines 77 to 83 in 94c0744

func (cli *DaemonCli) start(opts *daemonOptions) (err error) {

stopc := make(chan bool)

defer close(stopc)

opts.SetDefaultOptions(opts.flags)

if cli.Config, err = loadDaemonCliConfig(opts); err != nil {

Thinking how we can achieve this; we could return a "special" error, but that may need handling of that error is various places to make sure we still still the daemon, but with a zero exit code.

thaJeztah · 2021-02-03T15:21:34Z

Moved back to "code review" because of #38138 (comment); will have a look if there's a different approach possible

GordonTheTurtle added the status/0-triage label Nov 5, 2018

AkihiroSuda added the rebuild/experimental label Nov 5, 2018

GordonTheTurtle removed the rebuild/experimental label Nov 5, 2018

GordonTheTurtle assigned coolljt0725 Nov 20, 2018

thaJeztah added status/2-code-review and removed status/0-triage labels Nov 21, 2018

thaJeztah reviewed Nov 21, 2018

View reviewed changes

cmd/dockerd/daemon.go Outdated Show resolved Hide resolved

derek bot added the rebuild/* label Dec 19, 2018

GordonTheTurtle removed the rebuild/* label Dec 19, 2018

justrich force-pushed the 36911 branch from 1c41d70 to b09a3a7 Compare January 2, 2019 01:13

derek bot added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jan 2, 2019

justrich force-pushed the 36911 branch from b09a3a7 to a0e79b2 Compare January 4, 2019 07:57

derek bot added rebuild/experimental and removed status/failing-ci Indicates that the PR in its current state fails the test suite labels Jan 8, 2019

GordonTheTurtle removed the rebuild/experimental label Jan 8, 2019

olljanat mentioned this pull request Jan 8, 2019

Flaky test: TestServiceWithDefaultAddressPoolInit #38514

Closed

derek bot added the rebuild/experimental label Jan 10, 2019

GordonTheTurtle removed the rebuild/experimental label Jan 10, 2019

derek bot added the rebuild/experimental label Jan 10, 2019

GordonTheTurtle removed the rebuild/experimental label Jan 10, 2019

olljanat mentioned this pull request Jan 22, 2019

Print a warning if config has an unknown argument #38607

Closed

thaJeztah requested changes Jun 5, 2019

View reviewed changes

thaJeztah added the impact/changelog label Jun 5, 2019

thaJeztah mentioned this pull request Jun 5, 2019

Feature request: add json schema to validate the daemon settings docker/cli#1922

Open

justrich force-pushed the 36911 branch from a0e79b2 to eafc88d Compare June 5, 2019 17:17

thaJeztah force-pushed the 36911 branch from eafc88d to a31e7dc Compare July 8, 2020 08:50

thaJeztah approved these changes Jul 8, 2020

View reviewed changes

chris-crone approved these changes Feb 3, 2021

View reviewed changes

AkihiroSuda approved these changes Feb 3, 2021

View reviewed changes

AkihiroSuda added status/4-merge and removed status/2-code-review labels Feb 3, 2021

AkihiroSuda added the kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. label Feb 3, 2021

thaJeztah added the docs/revisit label Feb 3, 2021

thaJeztah added this to the 21.xx milestone Feb 3, 2021

thaJeztah force-pushed the 36911 branch from a31e7dc to 59852eb Compare February 3, 2021 10:11

thaJeztah added the area/daemon Core Engine label Feb 3, 2021

thaJeztah force-pushed the 36911 branch from 59852eb to dd3f164 Compare February 3, 2021 14:40

coolljt0725 reviewed Feb 3, 2021

View reviewed changes

thaJeztah added status/2-code-review and removed status/4-merge labels Feb 3, 2021

thaJeztah assigned thaJeztah and unassigned coolljt0725 Mar 31, 2021

aiordache mentioned this pull request May 19, 2021

Daemon config validation #42393

Merged

thaJeztah closed this in #42393 Jun 23, 2021

	flags.BoolVarP(&o.Validate, "validate", "V", false, "Validate configuration file and exit")
	flags.BoolVar(&o.Validate, "validate", false, "Validate configuration file and exit")

	func (cli DaemonCli) start(opts daemonOptions) (err error) {
	stopc := make(chan bool)
	defer close(stopc)

	opts.SetDefaultOptions(opts.flags)

	if cli.Config, err = loadDaemonCliConfig(opts); err != nil {

Conversation

justrich commented Nov 5, 2018 • edited by thaJeztah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

olljanat commented Jan 2, 2019

Uh oh!

olljanat commented Jan 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olljanat commented Jan 10, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Nov 23, 2018

Choose a reason for hiding this comment

Uh oh!

justrich Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 8, 2020

Uh oh!

chris-crone left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Feb 3, 2021

Uh oh!

thaJeztah commented Feb 3, 2021

Uh oh!

thaJeztah commented Feb 3, 2021

Uh oh!

thaJeztah commented Feb 3, 2021

Uh oh!

thaJeztah commented Feb 3, 2021

Uh oh!

thaJeztah commented Feb 3, 2021

Uh oh!

coolljt0725 Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah Feb 3, 2021

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Feb 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

justrich commented Nov 5, 2018 •

edited by thaJeztah

Loading

codecov bot commented Dec 19, 2018 •

edited

Loading

olljanat commented Jan 8, 2019 •

edited

Loading

coolljt0725 Feb 3, 2021 •

edited

Loading