Remove LogOutput from Server

agent/agent.go

@@ -1517,9 +1517,6 @@ func (a *Agent) consulConfig() (*consul.Config, error) {
 		}
 	}
 
-	// Setup the loggers
-	base.LogOutput = a.LogOutput
-
 	// This will set up the LAN keyring, as well as the WAN and any segments
 	// for servers.
 	if err := a.setupKeyrings(base); err != nil {

agent/consul/client_test.go

@@ -438,7 +438,7 @@ func TestClient_RPC_TLS(t *testing.T) {
 	conf1.VerifyIncoming = true
 	conf1.VerifyOutgoing = true
 	configureTLS(conf1)
-	s1, err := NewServer(conf1)
+	s1, err := newServer(t, conf1)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -494,7 +494,7 @@ func newClient(t *testing.T, config *Config) (*Client, error) {
 func TestClient_RPC_RateLimit(t *testing.T) {
 	t.Parallel()
 	dir1, conf1 := testServerConfig(t)
-	s1, err := NewServer(conf1)
+	s1, err := newServer(t, conf1)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -606,7 +606,7 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) {
 	conf1.VerifyIncoming = true
 	conf1.VerifyOutgoing = true
 	configureTLS(conf1)
-	s1, err := NewServer(conf1)
+	s1, err := newServer(t, conf1)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}

agent/consul/config.go

@@ -2,7 +2,6 @@ package consul
 
 import (
 	"fmt"
-	"io"
 	"net"
 	"os"
 	"time"
@@ -161,10 +160,6 @@ type Config struct {
 	// leader election.
 	ReconcileInterval time.Duration
 
-	// LogOutput is the location to write logs to. If this is not set,
-	// logs will go to stderr.
-	LogOutput io.Writer
-
 	// ProtocolVersion is the protocol version to speak. This must be between
 	// ProtocolVersionMin and ProtocolVersionMax.
 	ProtocolVersion uint8

agent/consul/leader_test.go

@@ -10,10 +10,13 @@ import (
 	"time"
 
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/consul/agent/token"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
+	"github.com/hashicorp/consul/tlsutil"
+	"github.com/hashicorp/go-hclog"
 	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
 	"github.com/hashicorp/serf/serf"
 	"github.com/stretchr/testify/require"
@@ -1283,10 +1286,10 @@ func TestLeader_ConfigEntryBootstrap_Fail(t *testing.T) {
 		}
 	}()
 
-	dir1, s1 := testServerWithConfig(t, func(c *Config) {
-		c.LogOutput = io.MultiWriter(pw, testutil.NewLogBuffer(t))
-		c.Build = "1.6.0"
-		c.ConfigEntryBootstrap = []structs.ConfigEntry{
+	dir, config := testServerConfig(t)
+	defer os.RemoveAll(dir)
+	config.Build = "1.6.0"
+	config.ConfigEntryBootstrap = []structs.ConfigEntry{
 		&structs.ServiceSplitterConfigEntry{
 			Kind: structs.ServiceSplitter,
 			Name: "web",
@@ -1295,12 +1298,27 @@ func TestLeader_ConfigEntryBootstrap_Fail(t *testing.T) {
 			},
 		},
 	}
+
+	logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{
+		Name:   config.NodeName,
+		Level:  hclog.Debug,
+		Output: io.MultiWriter(pw, testutil.NewLogBuffer(t)),
 	})
-	defer os.RemoveAll(dir1)
-	defer s1.Shutdown()
+	tlsConf, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), logger)
+	require.NoError(t, err)
+	srv, err := NewServerWithOptions(config,
+		WithLogger(logger),
+		WithTokenStore(new(token.Store)),
+		WithTLSConfigurator(tlsConf))
+	require.NoError(t, err)
+	defer srv.Shutdown()
 
-	result := <-ch
+	select {
+	case result := <-ch:
 		require.Empty(t, result)
+	case <-time.After(time.Second):
+		t.Fatal("timeout waiting for a result from tailing logs")
+	}
 }
 
 func TestLeader_ACLLegacyReplication(t *testing.T) {

agent/consul/server.go

@@ -313,25 +313,6 @@ type Server struct {
 	EnterpriseServer
 }
 
-// NewServer is only used to help setting up a server for testing. Normal code
-// exercises NewServerLogger.
-func NewServer(config *Config) (*Server, error) {
-	c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil)
-	if err != nil {
-		return nil, err
-	}
-	return NewServerLogger(config, nil, new(token.Store), c)
-}
-
-// NewServerLogger is used to construct a new Consul server from the
-// configuration, potentially returning an error
-func NewServerLogger(config *Config, logger hclog.InterceptLogger, tokens *token.Store, tlsConfigurator *tlsutil.Configurator) (*Server, error) {
-	return NewServerWithOptions(config,
-		WithLogger(logger),
-		WithTokenStore(tokens),
-		WithTLSConfigurator(tlsConfigurator))
-}
-
 // NewServerWithOptions is used to construct a new Consul server from the configuration
 // and extra options, potentially returning an error
 func NewServerWithOptions(config *Config, options ...ConsulOption) (*Server, error) {
@@ -342,31 +323,17 @@ func NewServerWithOptions(config *Config, options ...ConsulOption) (*Server, err
 	tlsConfigurator := flat.tlsConfigurator
 	connPool := flat.connPool
 
-	// Check the protocol version.
 	if err := config.CheckProtocolVersion(); err != nil {
 		return nil, err
 	}
-
-	// Check for a data directory.
 	if config.DataDir == "" && !config.DevMode {
 		return nil, fmt.Errorf("Config must provide a DataDir")
 	}
-
-	// Sanity check the ACLs.
 	if err := config.CheckACL(); err != nil {
 		return nil, err
 	}
-
-	// Ensure we have a log output and create a logger.
-	if config.LogOutput == nil {
-		config.LogOutput = os.Stderr
-	}
-
 	if logger == nil {
-		logger = hclog.NewInterceptLogger(&hclog.LoggerOptions{
-			Level:  hclog.Debug,
-			Output: config.LogOutput,
-		})
+		return nil, fmt.Errorf("logger is required")
 	}
 
 	// Check if TLS is enabled
@@ -735,7 +702,7 @@ func (s *Server) setupRaft() error {
 		log = cacheStore
 
 		// Create the snapshot store.
-		snapshots, err := raft.NewFileSnapshotStore(path, snapshotsRetained, s.config.LogOutput)
+		snapshots, err := raft.NewFileSnapshotStoreWithLogger(path, snapshotsRetained, s.logger.Named("snapshot"))
 		if err != nil {
 			return err
 		}

agent/consul/server_test.go

@@ -146,7 +146,6 @@ func testServerConfig(t *testing.T) (string, *Config) {
 	config.Bootstrap = true
 	config.Datacenter = "dc1"
 	config.DataDir = dir
-	config.LogOutput = testutil.NewLogBuffer(t)
 
 	// bind the rpc server to a random port. config.RPCAdvertise will be
 	// set to the listen address unless it was set in the configuration.
@@ -258,18 +257,18 @@ func testServerDCExpectNonVoter(t *testing.T, dc string, expect int) (string, *S
 
 func testServerWithConfig(t *testing.T, cb func(*Config)) (string, *Server) {
 	var dir string
-	var config *Config
 	var srv *Server
-	var err error
 
 	// Retry added to avoid cases where bind addr is already in use
 	retry.RunWith(retry.ThreeTimes(), t, func(r *retry.R) {
+		var config *Config
 		dir, config = testServerConfig(t)
 		if cb != nil {
 			cb(config)
 		}
 
-		srv, err = newServer(config)
+		var err error
+		srv, err = newServer(t, config)
 		if err != nil {
 			config.NotifyShutdown()
 			os.RemoveAll(dir)
@@ -295,7 +294,7 @@ func testACLServerWithConfig(t *testing.T, cb func(*Config), initReplicationToke
 	return dir, srv, codec
 }
 
-func newServer(c *Config) (*Server, error) {
+func newServer(t *testing.T, c *Config) (*Server, error) {
 	// chain server up notification
 	oldNotify := c.NotifyListen
 	up := make(chan struct{})
@@ -306,21 +305,19 @@ func newServer(c *Config) (*Server, error) {
 		}
 	}
 
-	// start server
-	w := c.LogOutput
-	if w == nil {
-		w = os.Stderr
-	}
 	logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{
 		Name:   c.NodeName,
 		Level:  hclog.Debug,
-		Output: w,
+		Output: testutil.NewLogBuffer(t),
 	})
 	tlsConf, err := tlsutil.NewConfigurator(c.ToTLSUtilConfig(), logger)
 	if err != nil {
 		return nil, err
 	}
-	srv, err := NewServerLogger(c, logger, new(token.Store), tlsConf)
+	srv, err := NewServer(c,
+		WithLogger(logger),
+		WithTokenStore(new(token.Store)),
+		WithTLSConfigurator(tlsConf))
 	if err != nil {
 		return nil, err
 	}
@@ -1088,7 +1085,7 @@ func TestServer_JoinLAN_TLS(t *testing.T) {
 	conf1.VerifyIncoming = true
 	conf1.VerifyOutgoing = true
 	configureTLS(conf1)
-	s1, err := newServer(conf1)
+	s1, err := newServer(t, conf1)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -1101,7 +1098,7 @@ func TestServer_JoinLAN_TLS(t *testing.T) {
 	conf2.VerifyIncoming = true
 	conf2.VerifyOutgoing = true
 	configureTLS(conf2)
-	s2, err := newServer(conf2)
+	s2, err := newServer(t, conf2)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -1486,7 +1483,7 @@ func TestServer_RPC_RateLimit(t *testing.T) {
 	dir1, conf1 := testServerConfig(t)
 	conf1.RPCRate = 2
 	conf1.RPCMaxBurst = 2
-	s1, err := NewServer(conf1)
+	s1, err := newServer(t, conf1)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -1512,7 +1509,11 @@ func TestServer_CALogging(t *testing.T) {
 
 	c, err := tlsutil.NewConfigurator(conf1.ToTLSUtilConfig(), logger)
 	require.NoError(t, err)
-	s1, err := NewServerLogger(conf1, logger, new(token.Store), c)
+
+	s1, err := NewServer(conf1,
+		WithLogger(logger),
+		WithTokenStore(new(token.Store)),
+		WithTLSConfigurator(c))
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}