From f121d6eec2a8a5ce99ad7262e1b814cd6613c227 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Tue, 14 Apr 2020 09:50:59 +0200 Subject: [PATCH] Use interface to log in Consul package This will allow to replace log implementation by testing one to include tests into the Consul repository without adding a new dependency --- consul/logger.go | 13 +++++++++++++ consul/logger_testing.go | 32 ++++++++++++++++++++++++++++++ consul/watcher.go | 40 ++++++++++++++++++++------------------ haproxy/haproxy_cmd/run.go | 1 + main.go | 25 +++++++++++++++++++++++- utils_test.go | 2 +- 6 files changed, 92 insertions(+), 21 deletions(-) create mode 100644 consul/logger.go create mode 100644 consul/logger_testing.go diff --git a/consul/logger.go b/consul/logger.go new file mode 100644 index 0000000..ecc279b --- /dev/null +++ b/consul/logger.go @@ -0,0 +1,13 @@ +package consul + +// Logger Allows replacing easily the logger. +type Logger interface { + // Debugf Display debug message + Debugf(format string, args ...interface{}) + // Infof Display info message + Infof(format string, args ...interface{}) + // Warnf Display warning message + Warnf(format string, args ...interface{}) + // Errorf Display error message + Errorf(format string, args ...interface{}) +} \ No newline at end of file diff --git a/consul/logger_testing.go b/consul/logger_testing.go new file mode 100644 index 0000000..104aed6 --- /dev/null +++ b/consul/logger_testing.go @@ -0,0 +1,32 @@ +package consul + +import "testing" + +type testingLogger struct { + t *testing.T +} + +// Debugf Display debug message +func (l *testingLogger) Debugf(format string, args ...interface{}) { + l.t.Logf(format, args...) +} + +// Infof Display info message +func (l *testingLogger) Infof(format string, args ...interface{}) { + l.t.Logf(format, args...) +} + +// Warnf Display warning message +func (l *testingLogger) Warnf(format string, args ...interface{}) { + l.t.Logf(format, args...) +} + +// Errorf Display error message +func (l *testingLogger) Errorf(format string, args ...interface{}) { + l.t.Logf(format, args...) +} + +// NewTestingLogger creates a Logger for testing.T +func NewTestingLogger(t *testing.T) Logger { + return &testingLogger{t: t} +} diff --git a/consul/watcher.go b/consul/watcher.go index 9b0c105..893b7be 100644 --- a/consul/watcher.go +++ b/consul/watcher.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/connect/proxy" - log "github.com/sirupsen/logrus" ) const ( @@ -58,9 +57,11 @@ type Watcher struct { leaf *certLeaf update chan struct{} + log Logger } -func New(service string, consul *api.Client) *Watcher { +// New builds a new watcher +func New(service string, consul *api.Client, log Logger) *Watcher { return &Watcher{ service: service, consul: consul, @@ -68,6 +69,7 @@ func New(service string, consul *api.Client) *Watcher { C: make(chan Config), upstreams: make(map[string]*upstream), update: make(chan struct{}, 1), + log: log, } } @@ -144,7 +146,7 @@ func (w *Watcher) handleProxyChange(first bool, srv *api.AgentService) { } func (w *Watcher) startUpstream(up api.Upstream) { - log.Infof("consul: watching upstream for service %s", up.DestinationName) + w.log.Infof("consul: watching upstream for service %s", up.DestinationName) u := &upstream{ LocalBindAddress: up.LocalBindAddress, @@ -169,7 +171,7 @@ func (w *Watcher) startUpstream(up api.Upstream) { WaitIndex: index, }) if err != nil { - log.Errorf("consul: error fetching service definition for service %s: %s", up.DestinationName, err) + w.log.Errorf("consul: error fetching service definition for service %s: %s", up.DestinationName, err) time.Sleep(errorWaitTime) index = 0 continue @@ -188,7 +190,7 @@ func (w *Watcher) startUpstream(up api.Upstream) { } func (w *Watcher) removeUpstream(name string) { - log.Infof("consul: removing upstream for service %s", name) + w.log.Infof("consul: removing upstream for service %s", name) w.lock.Lock() w.upstreams[name].done = true @@ -197,7 +199,7 @@ func (w *Watcher) removeUpstream(name string) { } func (w *Watcher) watchLeaf() { - log.Debugf("consul: watching leaf cert for %s", w.serviceName) + w.log.Debugf("consul: watching leaf cert for %s", w.serviceName) var lastIndex uint64 first := true @@ -207,7 +209,7 @@ func (w *Watcher) watchLeaf() { WaitIndex: lastIndex, }) if err != nil { - log.Errorf("consul error fetching leaf cert for service %s: %s", w.serviceName, err) + w.log.Errorf("consul error fetching leaf cert for service %s: %s", w.serviceName, err) time.Sleep(errorWaitTime) lastIndex = 0 continue @@ -217,7 +219,7 @@ func (w *Watcher) watchLeaf() { lastIndex = meta.LastIndex if changed { - log.Infof("consul: leaf cert for service %s changed, serial: %s, valid before: %s, valid after: %s", w.serviceName, cert.SerialNumber, cert.ValidBefore, cert.ValidAfter) + w.log.Infof("consul: leaf cert for service %s changed, serial: %s, valid before: %s, valid after: %s", w.serviceName, cert.SerialNumber, cert.ValidBefore, cert.ValidAfter) w.lock.Lock() if w.leaf == nil { w.leaf = &certLeaf{} @@ -229,7 +231,7 @@ func (w *Watcher) watchLeaf() { } if first { - log.Infof("consul: leaf cert for %s ready", w.serviceName) + w.log.Infof("consul: leaf cert for %s ready", w.serviceName) w.ready.Done() first = false } @@ -237,7 +239,7 @@ func (w *Watcher) watchLeaf() { } func (w *Watcher) watchService(service string, handler func(first bool, srv *api.AgentService)) { - log.Infof("consul: watching service %s", service) + w.log.Infof("consul: watching service %s", service) hash := "" first := true @@ -247,7 +249,7 @@ func (w *Watcher) watchService(service string, handler func(first bool, srv *api WaitTime: 10 * time.Minute, }) if err != nil { - log.Errorf("consul: error fetching service %s definition: %s", service, err) + w.log.Errorf("consul: error fetching service %s definition: %s", service, err) time.Sleep(errorWaitTime) hash = "" continue @@ -257,7 +259,7 @@ func (w *Watcher) watchService(service string, handler func(first bool, srv *api hash = meta.LastContentHash if changed { - log.Debugf("consul: service %s changed", service) + w.log.Debugf("consul: service %s changed", service) handler(first, srv) w.notifyChanged() } @@ -267,7 +269,7 @@ func (w *Watcher) watchService(service string, handler func(first bool, srv *api } func (w *Watcher) watchCA() { - log.Debugf("consul: watching ca certs") + w.log.Debugf("consul: watching ca certs") first := true var lastIndex uint64 @@ -277,7 +279,7 @@ func (w *Watcher) watchCA() { WaitTime: 10 * time.Minute, }) if err != nil { - log.Errorf("consul: error fetching cas: %s", err) + w.log.Errorf("consul: error fetching cas: %s", err) time.Sleep(errorWaitTime) lastIndex = 0 continue @@ -287,7 +289,7 @@ func (w *Watcher) watchCA() { lastIndex = meta.LastIndex if changed { - log.Infof("consul: CA certs changed, active root id: %s", caList.ActiveRootID) + w.log.Infof("consul: CA certs changed, active root id: %s", caList.ActiveRootID) w.lock.Lock() w.certCAs = w.certCAs[:0] w.certCAPool = x509.NewCertPool() @@ -295,7 +297,7 @@ func (w *Watcher) watchCA() { w.certCAs = append(w.certCAs, []byte(ca.RootCertPEM)) ok := w.certCAPool.AppendCertsFromPEM([]byte(ca.RootCertPEM)) if !ok { - log.Warn("consul: unable to add CA certificate to pool") + w.log.Warnf("consul: unable to add CA certificate to pool for root id: %s", caList.ActiveRootID) } } w.lock.Unlock() @@ -303,7 +305,7 @@ func (w *Watcher) watchCA() { } if first { - log.Infof("consul: CA certs ready") + w.log.Infof("consul: CA certs ready") w.ready.Done() first = false } @@ -311,13 +313,13 @@ func (w *Watcher) watchCA() { } func (w *Watcher) genCfg() Config { - log.Debug("generating configuration...") + w.log.Debugf("generating configuration for service %s[%s]...", w.serviceName, w.service) w.lock.Lock() serviceInstancesAlive := 0 serviceInstancesTotal := 0 defer func() { w.lock.Unlock() - log.Debugf("done generating configuration, instances: %d/%d total", + w.log.Debugf("done generating configuration, instances: %d/%d total", serviceInstancesAlive, serviceInstancesTotal) }() diff --git a/haproxy/haproxy_cmd/run.go b/haproxy/haproxy_cmd/run.go index 02774ed..7a005d6 100644 --- a/haproxy/haproxy_cmd/run.go +++ b/haproxy/haproxy_cmd/run.go @@ -69,6 +69,7 @@ func Start(sd *lib.Shutdown, cfg Config) (*dataplane.Dataplane, error) { err = dataplaneClient.Ping() if err != nil { + fmt.Println("*****\n* SOUCHAY: wait for dataplane to be up\n*****") time.Sleep(100 * time.Millisecond) continue } diff --git a/main.go b/main.go index 3e445a6..e14a942 100644 --- a/main.go +++ b/main.go @@ -14,6 +14,28 @@ import ( "github.com/criteo/haproxy-consul-connect/consul" ) +type consulLogger struct{} + +// Debugf Display debug message +func (consulLogger) Debugf(format string, args ...interface{}) { + log.Debugf(format, args...) +} + +// Infof Display info message +func (consulLogger) Infof(format string, args ...interface{}) { + log.Infof(format, args...) +} + +// Warnf Display warning message +func (consulLogger) Warnf(format string, args ...interface{}) { + log.Infof(format, args...) +} + +// Errorf Display error message +func (consulLogger) Errorf(format string, args ...interface{}) { + log.Errorf(format, args...) +} + func main() { logLevel := flag.String("log-level", "INFO", "Log level") consulAddr := flag.String("http-addr", "127.0.0.1:8500", "Consul agent address") @@ -73,7 +95,8 @@ func main() { log.Fatalf("Please specify -sidecar-for or -sidecar-for-tag") } - watcher := consul.New(serviceID, consulClient) + consulLogger := &consulLogger{} + watcher := consul.New(serviceID, consulClient, consulLogger) go func() { if err := watcher.Run(); err != nil { log.Error(err) diff --git a/utils_test.go b/utils_test.go index 2d84715..b7454be 100644 --- a/utils_test.go +++ b/utils_test.go @@ -64,7 +64,7 @@ func startConnectService(t *testing.T, sd *lib.Shutdown, client *api.Client, reg errs := make(chan error, 2) - watcher := consul.New(reg.ID, client) + watcher := consul.New(reg.ID, client, consul.NewTestingLogger(t)) go func() { err := watcher.Run() if err != nil {