From b0b9a25e7e60abbe143e149ccaaf4dfb62044016 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 8 Feb 2018 17:16:20 -0500 Subject: [PATCH] Move log validator logic after plugins are loaded This ensures that all log plugins are registered when the log validator is run. Signed-off-by: Brian Goff --- cmd/dockerd/daemon.go | 7 -- daemon/daemon.go | 11 ++- daemon/logs.go | 17 ++++- integration-cli/docker_cli_daemon_test.go | 4 +- integration/plugin/logging/cmd/cmd_test.go | 1 + integration/plugin/logging/cmd/dummy/main.go | 19 +++++ .../plugin/logging/cmd/dummy/main_test.go | 1 + integration/plugin/logging/helpers_test.go | 69 +++++++++++++++++++ integration/plugin/logging/validation_test.go | 33 +++++++++ 9 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 integration/plugin/logging/cmd/cmd_test.go create mode 100644 integration/plugin/logging/cmd/dummy/main.go create mode 100644 integration/plugin/logging/cmd/dummy/main_test.go create mode 100644 integration/plugin/logging/helpers_test.go create mode 100644 integration/plugin/logging/validation_test.go diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 6665cf032e..cba9a7bbfc 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -34,7 +34,6 @@ import ( "github.com/docker/docker/daemon/cluster" "github.com/docker/docker/daemon/config" "github.com/docker/docker/daemon/listeners" - "github.com/docker/docker/daemon/logger" "github.com/docker/docker/dockerversion" "github.com/docker/docker/libcontainerd" dopts "github.com/docker/docker/opts" @@ -106,12 +105,6 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { return fmt.Errorf("Failed to set umask: %v", err) } - if len(cli.LogConfig.Config) > 0 { - if err := logger.ValidateLogOpts(cli.LogConfig.Type, cli.LogConfig.Config); err != nil { - return fmt.Errorf("Failed to set log opts: %v", err) - } - } - // Create the daemon root before we create ANY other files (PID, or migrate keys) // to ensure the appropriate ACL is set (particularly relevant on Windows) if err := daemon.CreateDaemonRoot(cli.Config); err != nil { diff --git a/daemon/daemon.go b/daemon/daemon.go index ab28a56ebe..c93415cd7d 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -671,8 +671,6 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe return nil, fmt.Errorf("error setting default isolation mode: %v", err) } - logrus.Debugf("Using default logging driver %s", config.LogConfig.Type) - if err := configureMaxThreads(config); err != nil { logrus.Warnf("Failed to configure golang's threads limit: %v", err) } @@ -753,6 +751,10 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe return nil, errors.Wrap(err, "couldn't create plugin manager") } + if err := d.setupDefaultLogConfig(); err != nil { + return nil, err + } + for operatingSystem, gd := range d.graphDrivers { d.layerStores[operatingSystem], err = layer.NewStoreFromOptions(layer.StoreOptions{ Root: config.Root, @@ -873,10 +875,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe d.trustKey = trustKey d.idIndex = truncindex.NewTruncIndex([]string{}) d.statsCollector = d.newStatsCollector(1 * time.Second) - d.defaultLogConfig = containertypes.LogConfig{ - Type: config.LogConfig.Type, - Config: config.LogConfig.Config, - } + d.EventsService = eventsService d.volumes = volStore d.root = config.Root diff --git a/daemon/logs.go b/daemon/logs.go index 0701310326..f1ca5c9a06 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -1,7 +1,6 @@ package daemon // import "github.com/docker/docker/daemon" import ( - "errors" "strconv" "time" @@ -14,6 +13,7 @@ import ( "github.com/docker/docker/container" "github.com/docker/docker/daemon/logger" "github.com/docker/docker/errdefs" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -184,3 +184,18 @@ func (daemon *Daemon) mergeAndVerifyLogConfig(cfg *containertypes.LogConfig) err return logger.ValidateLogOpts(cfg.Type, cfg.Config) } + +func (daemon *Daemon) setupDefaultLogConfig() error { + config := daemon.configStore + if len(config.LogConfig.Config) > 0 { + if err := logger.ValidateLogOpts(config.LogConfig.Type, config.LogConfig.Config); err != nil { + return errors.Wrap(err, "failed to set log opts") + } + } + daemon.defaultLogConfig = containertypes.LogConfig{ + Type: config.LogConfig.Type, + Config: config.LogConfig.Config, + } + logrus.Debugf("Using default logging driver %s", daemon.defaultLogConfig.Type) + return nil +} diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index d6d2ef7ed7..2646de3fb7 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -1697,7 +1697,7 @@ func (s *DockerDaemonSuite) TestDaemonCorruptedLogDriverAddress(c *check.C) { Experimental: testEnv.DaemonInfo.ExperimentalBuild, }) c.Assert(d.StartWithError("--log-driver=syslog", "--log-opt", "syslog-address=corrupted:42"), check.NotNil) - expected := "Failed to set log opts: syslog-address should be in form proto://address" + expected := "syslog-address should be in form proto://address" icmd.RunCommand("grep", expected, d.LogFileName()).Assert(c, icmd.Success) } @@ -1707,7 +1707,7 @@ func (s *DockerDaemonSuite) TestDaemonCorruptedFluentdAddress(c *check.C) { Experimental: testEnv.DaemonInfo.ExperimentalBuild, }) c.Assert(d.StartWithError("--log-driver=fluentd", "--log-opt", "fluentd-address=corrupted:c"), check.NotNil) - expected := "Failed to set log opts: invalid fluentd-address corrupted:c: " + expected := "invalid fluentd-address corrupted:c: " icmd.RunCommand("grep", expected, d.LogFileName()).Assert(c, icmd.Success) } diff --git a/integration/plugin/logging/cmd/cmd_test.go b/integration/plugin/logging/cmd/cmd_test.go new file mode 100644 index 0000000000..1d619dd05e --- /dev/null +++ b/integration/plugin/logging/cmd/cmd_test.go @@ -0,0 +1 @@ +package cmd diff --git a/integration/plugin/logging/cmd/dummy/main.go b/integration/plugin/logging/cmd/dummy/main.go new file mode 100644 index 0000000000..f91b4f3b02 --- /dev/null +++ b/integration/plugin/logging/cmd/dummy/main.go @@ -0,0 +1,19 @@ +package main + +import ( + "net" + "net/http" +) + +func main() { + l, err := net.Listen("unix", "/run/docker/plugins/plugin.sock") + if err != nil { + panic(err) + } + + server := http.Server{ + Addr: l.Addr().String(), + Handler: http.NewServeMux(), + } + server.Serve(l) +} diff --git a/integration/plugin/logging/cmd/dummy/main_test.go b/integration/plugin/logging/cmd/dummy/main_test.go new file mode 100644 index 0000000000..06ab7d0f9a --- /dev/null +++ b/integration/plugin/logging/cmd/dummy/main_test.go @@ -0,0 +1 @@ +package main diff --git a/integration/plugin/logging/helpers_test.go b/integration/plugin/logging/helpers_test.go new file mode 100644 index 0000000000..61b2f35f43 --- /dev/null +++ b/integration/plugin/logging/helpers_test.go @@ -0,0 +1,69 @@ +package logging + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/integration-cli/fixtures/plugin" + "github.com/docker/docker/pkg/locker" + "github.com/pkg/errors" +) + +const dockerdBinary = "dockerd" + +var pluginBuildLock = locker.New() + +func ensurePlugin(t *testing.T, name string) string { + pluginBuildLock.Lock(name) + defer pluginBuildLock.Unlock(name) + + installPath := filepath.Join(os.Getenv("GOPATH"), "bin", name) + if _, err := os.Stat(installPath); err == nil { + return installPath + } + + goBin, err := exec.LookPath("go") + if err != nil { + t.Fatal(err) + } + + cmd := exec.Command(goBin, "build", "-o", installPath, "./"+filepath.Join("cmd", name)) + cmd.Env = append(cmd.Env, "CGO_ENABLED=0") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(errors.Wrapf(err, "error building basic plugin bin: %s", string(out))) + } + + return installPath +} + +func withSockPath(name string) func(*plugin.Config) { + return func(cfg *plugin.Config) { + cfg.Interface.Socket = name + } +} + +func createPlugin(t *testing.T, client plugin.CreateClient, alias, bin string, opts ...plugin.CreateOpt) { + pluginBin := ensurePlugin(t, bin) + + opts = append(opts, withSockPath("plugin.sock")) + opts = append(opts, plugin.WithBinary(pluginBin)) + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + err := plugin.Create(ctx, client, alias, opts...) + cancel() + + if err != nil { + t.Fatal(err) + } +} + +func asLogDriver(cfg *plugin.Config) { + cfg.Interface.Types = []types.PluginInterfaceType{ + {Capability: "logdriver", Prefix: "docker", Version: "1.0"}, + } +} diff --git a/integration/plugin/logging/validation_test.go b/integration/plugin/logging/validation_test.go new file mode 100644 index 0000000000..6607321613 --- /dev/null +++ b/integration/plugin/logging/validation_test.go @@ -0,0 +1,33 @@ +package logging + +import ( + "context" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/integration-cli/daemon" + "github.com/stretchr/testify/assert" +) + +// Regression test for #35553 +// Ensure that a daemon with a log plugin set as the default logger for containers +// does not keep the daemon from starting. +func TestDaemonStartWithLogOpt(t *testing.T) { + t.Parallel() + + d := daemon.New(t, "", dockerdBinary, daemon.Config{}) + d.Start(t, "--iptables=false") + defer d.Stop(t) + + client, err := d.NewClient() + assert.NoError(t, err) + ctx := context.Background() + + createPlugin(t, client, "test", "dummy", asLogDriver) + err = client.PluginEnable(ctx, "test", types.PluginEnableOptions{Timeout: 30}) + assert.NoError(t, err) + defer client.PluginRemove(ctx, "test", types.PluginRemoveOptions{Force: true}) + + d.Stop(t) + d.Start(t, "--iptables=false", "--log-driver=test", "--log-opt=foo=bar") +}