From 0c5428eea8e4abc7890637bc838aa7b9e33a47b2 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Jul 2020 15:26:15 -0400 Subject: [PATCH] Remove LogOutput from Client --- agent/consul/client.go | 41 ++++--------------------------------- agent/consul/client_test.go | 31 ++++++++++++++++------------ 2 files changed, 22 insertions(+), 50 deletions(-) diff --git a/agent/consul/client.go b/agent/consul/client.go index 5b4cd9e8c6..8b415f5500 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -3,7 +3,6 @@ package consul import ( "fmt" "io" - "os" "strconv" "sync" "sync/atomic" @@ -89,58 +88,30 @@ type Client struct { tlsConfigurator *tlsutil.Configurator } -// NewClient is used to construct a new Consul client from the configuration, -// potentially returning an error. -// NewClient only used to help setting up a client for testing. Normal code -// exercises NewClientLogger. -func NewClient(config *Config) (*Client, error) { - c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil) - if err != nil { - return nil, err - } - return NewClientLogger(config, nil, c) -} - func NewClientWithOptions(config *Config, options ...ConsulOption) (*Client, error) { flat := flattenConsulOptions(options) - logger := flat.logger 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 == "" { 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 - if config.LogOutput == nil { - config.LogOutput = os.Stderr - } - - // Create a logger - if logger == nil { - logger = hclog.NewInterceptLogger(&hclog.LoggerOptions{ - Level: hclog.Debug, - Output: config.LogOutput, - }) + if flat.logger == nil { + return nil, fmt.Errorf("logger is required") } if connPool == nil { connPool = &pool.ConnPool{ Server: false, SrcAddr: config.RPCSrcAddr, - Logger: logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}), + Logger: flat.logger.StandardLogger(&hclog.StandardLoggerOptions{InferLevels: true}), MaxTime: clientRPCConnMaxIdle, MaxStreams: clientMaxStreams, TLSConfigurator: tlsConfigurator, @@ -153,7 +124,7 @@ func NewClientWithOptions(config *Config, options ...ConsulOption) (*Client, err config: config, connPool: connPool, eventCh: make(chan serf.Event, serfEventBacklog), - logger: logger.NamedIntercept(logging.ConsulClient), + logger: flat.logger.NamedIntercept(logging.ConsulClient), shutdownCh: make(chan struct{}), tlsConfigurator: tlsConfigurator, } @@ -210,10 +181,6 @@ func NewClientWithOptions(config *Config, options ...ConsulOption) (*Client, err return c, nil } -func NewClientLogger(config *Config, logger hclog.InterceptLogger, tlsConfigurator *tlsutil.Configurator) (*Client, error) { - return NewClientWithOptions(config, WithLogger(logger), WithTLSConfigurator(tlsConfigurator)) -} - // Shutdown is used to shutdown the client func (c *Client) Shutdown() error { c.logger.Info("shutting down client") diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index 9adbb17fa4..3328c3404a 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -48,8 +48,6 @@ func testClientConfig(t *testing.T) (string, *Config) { config.SerfLANConfig.MemberlistConfig.ProbeTimeout = 200 * time.Millisecond config.SerfLANConfig.MemberlistConfig.ProbeInterval = time.Second config.SerfLANConfig.MemberlistConfig.GossipInterval = 100 * time.Millisecond - config.LogOutput = testutil.NewLogBuffer(t) - return dir, config } @@ -72,15 +70,10 @@ func testClientWithConfigWithErr(t *testing.T, cb func(c *Config)) (string, *Cli if cb != nil { cb(config) } - w := config.LogOutput - if w == nil { - w = os.Stderr - } - logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ Name: config.NodeName, Level: hclog.Debug, - Output: w, + Output: testutil.NewLogBuffer(t), }) tlsConf, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), logger) @@ -88,7 +81,7 @@ func testClientWithConfigWithErr(t *testing.T, cb func(c *Config)) (string, *Cli t.Fatalf("err: %v", err) } - client, err := NewClientLogger(config, logger, tlsConf) + client, err := NewClient(config, WithLogger(logger), WithTLSConfigurator(tlsConf)) if err != nil { config.NotifyShutdown() } @@ -456,7 +449,7 @@ func TestClient_RPC_TLS(t *testing.T) { defer conf2.NotifyShutdown() conf2.VerifyOutgoing = true configureTLS(conf2) - c1, err := NewClient(conf2) + c1, err := newClient(t, conf2) if err != nil { t.Fatalf("err: %v", err) } @@ -486,6 +479,18 @@ func TestClient_RPC_TLS(t *testing.T) { }) } +func newClient(t *testing.T, config *Config) (*Client, error) { + c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil) + if err != nil { + return nil, err + } + logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ + Level: hclog.Debug, + Output: testutil.NewLogBuffer(t), + }) + return NewClient(config, WithLogger(logger), WithTLSConfigurator(c)) +} + func TestClient_RPC_RateLimit(t *testing.T) { t.Parallel() dir1, conf1 := testServerConfig(t) @@ -501,7 +506,7 @@ func TestClient_RPC_RateLimit(t *testing.T) { defer conf2.NotifyShutdown() conf2.RPCRate = 2 conf2.RPCMaxBurst = 2 - c1, err := NewClient(conf2) + c1, err := newClient(t, conf2) if err != nil { t.Fatalf("err: %v", err) } @@ -569,7 +574,7 @@ func TestClient_SnapshotRPC_RateLimit(t *testing.T) { defer conf1.NotifyShutdown() conf1.RPCRate = 2 conf1.RPCMaxBurst = 2 - c1, err := NewClient(conf1) + c1, err := newClient(t, conf1) if err != nil { t.Fatalf("err: %v", err) } @@ -612,7 +617,7 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) { defer conf2.NotifyShutdown() conf2.VerifyOutgoing = true configureTLS(conf2) - c1, err := NewClient(conf2) + c1, err := newClient(t, conf2) if err != nil { t.Fatalf("err: %v", err) }