Skip to content

Commit b0b9a25

Browse files
cpuguy83cpuguy83
committed
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 <[email protected]>
1 parent 7d8fa84 commit b0b9a25

9 files changed

Lines changed: 146 additions & 16 deletions

File tree

‎cmd/dockerd/daemon.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import (
3434
"github.com/docker/docker/daemon/cluster"
3535
"github.com/docker/docker/daemon/config"
3636
"github.com/docker/docker/daemon/listeners"
37-
"github.com/docker/docker/daemon/logger"
3837
"github.com/docker/docker/dockerversion"
3938
"github.com/docker/docker/libcontainerd"
4039
dopts "github.com/docker/docker/opts"
@@ -106,12 +105,6 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
106105
return fmt.Errorf("Failed to set umask: %v", err)
107106
}
108107

109-
if len(cli.LogConfig.Config) > 0 {
110-
if err := logger.ValidateLogOpts(cli.LogConfig.Type, cli.LogConfig.Config); err != nil {
111-
return fmt.Errorf("Failed to set log opts: %v", err)
112-
}
113-
}
114-
115108
// Create the daemon root before we create ANY other files (PID, or migrate keys)
116109
// to ensure the appropriate ACL is set (particularly relevant on Windows)
117110
if err := daemon.CreateDaemonRoot(cli.Config); err != nil {

‎daemon/daemon.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,6 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
671671
return nil, fmt.Errorf("error setting default isolation mode: %v", err)
672672
}
673673

674-
logrus.Debugf("Using default logging driver %s", config.LogConfig.Type)
675-
676674
if err := configureMaxThreads(config); err != nil {
677675
logrus.Warnf("Failed to configure golang's threads limit: %v", err)
678676
}
@@ -753,6 +751,10 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
753751
return nil, errors.Wrap(err, "couldn't create plugin manager")
754752
}
755753

754+
if err := d.setupDefaultLogConfig(); err != nil {
755+
return nil, err
756+
}
757+
756758
for operatingSystem, gd := range d.graphDrivers {
757759
d.layerStores[operatingSystem], err = layer.NewStoreFromOptions(layer.StoreOptions{
758760
Root: config.Root,
@@ -873,10 +875,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
873875
d.trustKey = trustKey
874876
d.idIndex = truncindex.NewTruncIndex([]string{})
875877
d.statsCollector = d.newStatsCollector(1 * time.Second)
876-
d.defaultLogConfig = containertypes.LogConfig{
877-
Type: config.LogConfig.Type,
878-
Config: config.LogConfig.Config,
879-
}
878+
880879
d.EventsService = eventsService
881880
d.volumes = volStore
882881
d.root = config.Root

‎daemon/logs.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package daemon // import "github.com/docker/docker/daemon"
22

33
import (
4-
"errors"
54
"strconv"
65
"time"
76

@@ -14,6 +13,7 @@ import (
1413
"github.com/docker/docker/container"
1514
"github.com/docker/docker/daemon/logger"
1615
"github.com/docker/docker/errdefs"
16+
"github.com/pkg/errors"
1717
"github.com/sirupsen/logrus"
1818
)
1919

@@ -184,3 +184,18 @@ func (daemon *Daemon) mergeAndVerifyLogConfig(cfg *containertypes.LogConfig) err
184184

185185
return logger.ValidateLogOpts(cfg.Type, cfg.Config)
186186
}
187+
188+
func (daemon *Daemon) setupDefaultLogConfig() error {
189+
config := daemon.configStore
190+
if len(config.LogConfig.Config) > 0 {
191+
if err := logger.ValidateLogOpts(config.LogConfig.Type, config.LogConfig.Config); err != nil {
192+
return errors.Wrap(err, "failed to set log opts")
193+
}
194+
}
195+
daemon.defaultLogConfig = containertypes.LogConfig{
196+
Type: config.LogConfig.Type,
197+
Config: config.LogConfig.Config,
198+
}
199+
logrus.Debugf("Using default logging driver %s", daemon.defaultLogConfig.Type)
200+
return nil
201+
}

‎integration-cli/docker_cli_daemon_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,7 +1697,7 @@ func (s *DockerDaemonSuite) TestDaemonCorruptedLogDriverAddress(c *check.C) {
16971697
Experimental: testEnv.DaemonInfo.ExperimentalBuild,
16981698
})
16991699
c.Assert(d.StartWithError("--log-driver=syslog", "--log-opt", "syslog-address=corrupted:42"), check.NotNil)
1700-
expected := "Failed to set log opts: syslog-address should be in form proto://address"
1700+
expected := "syslog-address should be in form proto://address"
17011701
icmd.RunCommand("grep", expected, d.LogFileName()).Assert(c, icmd.Success)
17021702
}
17031703

@@ -1707,7 +1707,7 @@ func (s *DockerDaemonSuite) TestDaemonCorruptedFluentdAddress(c *check.C) {
17071707
Experimental: testEnv.DaemonInfo.ExperimentalBuild,
17081708
})
17091709
c.Assert(d.StartWithError("--log-driver=fluentd", "--log-opt", "fluentd-address=corrupted:c"), check.NotNil)
1710-
expected := "Failed to set log opts: invalid fluentd-address corrupted:c: "
1710+
expected := "invalid fluentd-address corrupted:c: "
17111711
icmd.RunCommand("grep", expected, d.LogFileName()).Assert(c, icmd.Success)
17121712
}
17131713

‎integration/plugin/logging/cmd/cmd_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package cmd

‎integration/plugin/logging/cmd/dummy/main.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package main
2+
3+
import (
4+
"net"
5+
"net/http"
6+
)
7+
8+
func main() {
9+
l, err := net.Listen("unix", "/run/docker/plugins/plugin.sock")
10+
if err != nil {
11+
panic(err)
12+
}
13+
14+
server := http.Server{
15+
Addr: l.Addr().String(),
16+
Handler: http.NewServeMux(),
17+
}
18+
server.Serve(l)
19+
}

‎integration/plugin/logging/cmd/dummy/main_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package main

‎integration/plugin/logging/helpers_test.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package logging
2+
3+
import (
4+
"context"
5+
"os"
6+
"os/exec"
7+
"path/filepath"
8+
"testing"
9+
"time"
10+
11+
"github.com/docker/docker/api/types"
12+
"github.com/docker/docker/integration-cli/fixtures/plugin"
13+
"github.com/docker/docker/pkg/locker"
14+
"github.com/pkg/errors"
15+
)
16+
17+
const dockerdBinary = "dockerd"
18+
19+
var pluginBuildLock = locker.New()
20+
21+
func ensurePlugin(t *testing.T, name string) string {
22+
pluginBuildLock.Lock(name)
23+
defer pluginBuildLock.Unlock(name)
24+
25+
installPath := filepath.Join(os.Getenv("GOPATH"), "bin", name)
26+
if _, err := os.Stat(installPath); err == nil {
27+
return installPath
28+
}
29+
30+
goBin, err := exec.LookPath("go")
31+
if err != nil {
32+
t.Fatal(err)
33+
}
34+
35+
cmd := exec.Command(goBin, "build", "-o", installPath, "./"+filepath.Join("cmd", name))
36+
cmd.Env = append(cmd.Env, "CGO_ENABLED=0")
37+
if out, err := cmd.CombinedOutput(); err != nil {
38+
t.Fatal(errors.Wrapf(err, "error building basic plugin bin: %s", string(out)))
39+
}
40+
41+
return installPath
42+
}
43+
44+
func withSockPath(name string) func(*plugin.Config) {
45+
return func(cfg *plugin.Config) {
46+
cfg.Interface.Socket = name
47+
}
48+
}
49+
50+
func createPlugin(t *testing.T, client plugin.CreateClient, alias, bin string, opts ...plugin.CreateOpt) {
51+
pluginBin := ensurePlugin(t, bin)
52+
53+
opts = append(opts, withSockPath("plugin.sock"))
54+
opts = append(opts, plugin.WithBinary(pluginBin))
55+
56+
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
57+
err := plugin.Create(ctx, client, alias, opts...)
58+
cancel()
59+
60+
if err != nil {
61+
t.Fatal(err)
62+
}
63+
}
64+
65+
func asLogDriver(cfg *plugin.Config) {
66+
cfg.Interface.Types = []types.PluginInterfaceType{
67+
{Capability: "logdriver", Prefix: "docker", Version: "1.0"},
68+
}
69+
}

‎integration/plugin/logging/validation_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package logging
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/docker/docker/api/types"
8+
"github.com/docker/docker/integration-cli/daemon"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
// Regression test for #35553
13+
// Ensure that a daemon with a log plugin set as the default logger for containers
14+
// does not keep the daemon from starting.
15+
func TestDaemonStartWithLogOpt(t *testing.T) {
16+
t.Parallel()
17+
18+
d := daemon.New(t, "", dockerdBinary, daemon.Config{})
19+
d.Start(t, "--iptables=false")
20+
defer d.Stop(t)
21+
22+
client, err := d.NewClient()
23+
assert.NoError(t, err)
24+
ctx := context.Background()
25+
26+
createPlugin(t, client, "test", "dummy", asLogDriver)
27+
err = client.PluginEnable(ctx, "test", types.PluginEnableOptions{Timeout: 30})
28+
assert.NoError(t, err)
29+
defer client.PluginRemove(ctx, "test", types.PluginRemoveOptions{Force: true})
30+
31+
d.Stop(t)
32+
d.Start(t, "--iptables=false", "--log-driver=test", "--log-opt=foo=bar")
33+
}

0 commit comments

Comments
 (0)