Support Windows

The syslog logrus hook does not support Windows:
https://github.com/sirupsen/logrus/blob/master/hooks/syslog/syslog.go
So we have to work around that.

While at it, ensure logger initialization errors are really
bubbling up, and prevent healthcheck tests from depending on
our logger implementation.

Author: bpineau

.goreleaser.yml

@@ -6,6 +6,7 @@ builds:
     goos:
       - darwin
       - linux
+      - windows
     goarch:
       - amd64
       - arm

cmd/execute.go

@@ -35,7 +35,10 @@ var (
 )
 
 func runE(cmd *cobra.Command, args []string) (err error) {
-	logger := log.New(logLevel, logServer, logOutput)
+	logger, err := log.New(logLevel, logServer, logOutput)
+	if err != nil {
+		return fmt.Errorf("failed to create a logger: %v", err)
+	}
 
 	if restcfg == nil {
 		restcfg, err = client.New(apiServer, kubeConf)

pkg/health/health_test.go

@@ -4,14 +4,16 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+)
 
-	"github.com/sirupsen/logrus"
-	"github.com/sirupsen/logrus/hooks/test"
+type mockLog struct {
+	count int
+}
 
-	"github.com/bpineau/katafygio/pkg/log"
-)
+func (m *mockLog) Infof(format string, args ...interface{})  {}
+func (m *mockLog) Errorf(format string, args ...interface{}) { m.count++ }
 
-var logs = log.New("error", "", "test")
+var logs = new(mockLog)
 
 func TestNoopHealth(t *testing.T) {
 
@@ -23,8 +25,7 @@ func TestNoopHealth(t *testing.T) {
 	hc = New(logs, -42)
 	_ = hc.Start()
 	hc.Stop()
-	hook := logs.Hooks[logrus.InfoLevel][0].(*test.Hook)
-	if len(hook.Entries) != 1 {
+	if logs.count != 1 {
 		t.Error("Failed to log an issue with a bogus port")
 	}
 }

pkg/log/log.go

@@ -2,25 +2,33 @@
 package log
 
 import (
-	"io"
-	"io/ioutil"
-	"os"
-
-	"log/syslog"
+	"fmt"
 
 	"github.com/sirupsen/logrus"
-	ls "github.com/sirupsen/logrus/hooks/syslog"
-	"github.com/sirupsen/logrus/hooks/test"
+)
+
+const (
+	outputStdout = "stdout"
+	outputStderr = "stderr"
+	outputTest   = "test"
+	outputSyslog = "syslog"
 )
 
 // New initialize logrus and return a new logger.
-func New(logLevel string, logServer string, logOutput string) *logrus.Logger {
+func New(logLevel string, logServer string, logOutput string) (*logrus.Logger, error) {
+	if logLevel == "" {
+		logLevel = "info"
+	}
+
 	level, err := logrus.ParseLevel(logLevel)
 	if err != nil {
-		level = logrus.InfoLevel
+		return nil, err
 	}
 
-	output, hook := getOutput(logServer, logOutput)
+	output, hook, err := getOutput(logServer, logOutput)
+	if err != nil {
+		return nil, fmt.Errorf("failed to init logger: %v", err)
+	}
 
 	formatter := &logrus.TextFormatter{
 		FullTimestamp:   true,
@@ -34,38 +42,9 @@ func New(logLevel string, logServer string, logOutput string) *logrus.Logger {
 		Level:     level,
 	}
 
-	if logOutput == "syslog" || logOutput == "test" {
+	if logOutput == outputSyslog || logOutput == outputTest {
 		log.Hooks.Add(hook)
 	}
 
-	return log
-}
-
-func getOutput(logServer string, logOutput string) (io.Writer, logrus.Hook) {
-	var output io.Writer
-	var hook logrus.Hook
-	var err error
-
-	switch logOutput {
-	case "stdout":
-		output = os.Stdout
-	case "stderr":
-		output = os.Stderr
-	case "test":
-		output = ioutil.Discard
-		_, hook = test.NewNullLogger()
-	case "syslog":
-		output = os.Stderr // does not matter ?
-		if logServer == "" {
-			panic("syslog output needs a log server (ie. 127.0.0.1:514)")
-		}
-		hook, err = ls.NewSyslogHook("udp", logServer, syslog.LOG_INFO, "katafygio")
-		if err != nil {
-			panic(err)
-		}
-	default:
-		output = os.Stderr
-	}
-
-	return output, hook
+	return log, nil
 }

pkg/log/log_test.go

@@ -3,6 +3,7 @@ package log
 import (
 	"fmt"
 	"os"
+	"runtime"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -21,7 +22,10 @@ var (
 )
 
 func TestLog(t *testing.T) {
-	logger := New("warning", "", "test")
+	logger, err := New("warning", "", "test")
+	if err != nil {
+		t.Errorf("Creating a new test logger shouldn't fail: %v", err)
+	}
 
 	logger.Info("Changed: foo")
 	logger.Warn("Changed: bar")
@@ -37,55 +41,59 @@ func TestLog(t *testing.T) {
 		t.Errorf("Unexpected log entry: %s", hook.LastEntry().Message)
 	}
 
-	logger = New("", "", "test")
-	if logger.Level != logrus.InfoLevel {
-		t.Error("The default loglevel should be info")
+	for _, level := range levels {
+		lg, err2 := New(level, "", "test")
+		if err2 != nil || fmt.Sprintf("%T", lg) != "*logrus.Logger" {
+			t.Errorf("Failed to instantiate at %s level: %v", level, err2)
+		}
 	}
 
-	logger = New("", "", "")
-	if logger.Out != os.Stderr {
-		t.Error("The default output should be stderr")
+	logger, err = New("", "", "test")
+	if err != nil || logger.Level != logrus.InfoLevel {
+		t.Errorf("The default loglevel should be info %v", err)
 	}
 
-	logger = New("info", "127.0.0.1:514", "syslog")
-	if fmt.Sprintf("%T", logger) != "*logrus.Logger" {
-		t.Error("Failed to instantiate a syslog logger")
+	logger, err = New("", "", "")
+	if err != nil || logger.Out != os.Stderr {
+		t.Errorf("The default output should be stderr %v", err)
 	}
 
-	logger = New("info", "", "stdout")
-	if fmt.Sprintf("%T", logger) != "*logrus.Logger" {
-		t.Error("Failed to instantiate a stdout logger")
+	if runtime.GOOS != "windows" {
+		logger, err = New("info", "127.0.0.1:514", "syslog")
+		if err != nil || fmt.Sprintf("%T", logger) != "*logrus.Logger" {
+			t.Errorf("Failed to instantiate a syslog logger %v", err)
+		}
 	}
 
-	logger = New("info", "", "stderr")
-	if fmt.Sprintf("%T", logger) != "*logrus.Logger" {
-		t.Error("Failed to instantiate a stderr logger")
+	logger, err = New("info", "", "stdout")
+	if err != nil || fmt.Sprintf("%T", logger) != "*logrus.Logger" {
+		t.Errorf("Failed to instantiate a stdout logger %v", err)
 	}
 
-	for _, level := range levels {
-		lg := New(level, "", "test")
-		if fmt.Sprintf("%T", lg) != "*logrus.Logger" {
-			t.Errorf("Failed to instantiate at %s level", level)
-		}
+	logger, err = New("info", "", "stderr")
+	if err != nil || fmt.Sprintf("%T", logger) != "*logrus.Logger" {
+		t.Errorf("Failed to instantiate a stderr logger %v", err)
 	}
 }
 
 func TestSyslogMissingArg(t *testing.T) {
-	defer func() {
-		if r := recover(); r == nil {
-			t.Errorf("syslog logger should panic without a server")
-		}
-	}()
+	if runtime.GOOS == "windows" {
+		t.Skip("syslog is not supported on Windows")
+	}
 
-	_ = New("info", "", "syslog")
+	_, err := New("info", "", "syslog")
+	if err == nil {
+		t.Errorf("syslog logger should fail without a server")
+	}
 }
 
 func TestSyslogWrongArg(t *testing.T) {
-	defer func() {
-		if r := recover(); r == nil {
-			t.Errorf("syslog logger should panic on wrong server address")
-		}
-	}()
+	if runtime.GOOS == "windows" {
+		t.Skip("syslog is not supported on Windows")
+	}
 
-	_ = New("info", "wrong server", "syslog")
+	_, err := New("info", "wrong server", "syslog")
+	if err == nil {
+		t.Errorf("syslog logger should fail with a broken server address")
+	}
 }

pkg/log/output.go

@@ -0,0 +1,44 @@
+// +build !windows
+
+package log
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"log/syslog"
+	"os"
+
+	"github.com/sirupsen/logrus"
+	ls "github.com/sirupsen/logrus/hooks/syslog"
+	"github.com/sirupsen/logrus/hooks/test"
+)
+
+func getOutput(logServer string, logOutput string) (io.Writer, logrus.Hook, error) {
+	var output io.Writer
+	var hook logrus.Hook
+	var err error
+
+	switch logOutput {
+	case outputStdout:
+		output = os.Stdout
+	case outputStderr:
+		output = os.Stderr
+	case outputTest:
+		output = ioutil.Discard
+		_, hook = test.NewNullLogger()
+	case outputSyslog:
+		output = os.Stderr
+		if logServer == "" {
+			return nil, nil, fmt.Errorf("syslog output needs a log server (ie. 127.0.0.1:514)")
+		}
+		hook, err = ls.NewSyslogHook("udp", logServer, syslog.LOG_INFO, "katafygio")
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to hook syslog output")
+		}
+	default:
+		output = os.Stderr
+	}
+
+	return output, hook, nil
+}

pkg/log/output_windows.go

@@ -0,0 +1,34 @@
+// +build windows
+
+package log
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+
+	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
+)
+
+func getOutput(logServer string, logOutput string) (io.Writer, logrus.Hook, error) {
+	var output io.Writer
+	var hook logrus.Hook
+
+	switch logOutput {
+	case outputStdout:
+		output = os.Stdout
+	case outputStderr:
+		output = os.Stderr
+	case outputTest:
+		output = ioutil.Discard
+		_, hook = test.NewNullLogger()
+	case outputSyslog:
+		return nil, nil, fmt.Errorf("Syslog output isn't supported on Windows")
+	default:
+		output = os.Stderr
+	}
+
+	return output, hook, nil
+}