Merge pull request #61 from steadybit/windows-support

feat: handle signals on windows

Author: joshiste

extcmd/extcmd_test.go

@@ -6,32 +6,39 @@ package extcmd
 import (
 	"github.com/stretchr/testify/assert"
 	"os/exec"
+	"runtime"
 	"testing"
 )
 
 func TestNewCmdState(t *testing.T) {
 	_, err := GetCmdState("I am unknown")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
+
+	var cmd *exec.Cmd
+	if runtime.GOOS == "windows" {
+		cmd = exec.Command("cmd", "/C", "echo hello world")
+	} else {
+		cmd = exec.Command("echo", "hello", "world")
+	}
 
-	cmd := exec.Command("echo", "hello", "world")
 	cs := NewCmdState(cmd)
 
 	persistedState, err := GetCmdState(cs.Id)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.NotNil(t, persistedState)
 
 	err = cmd.Start()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	err = cmd.Wait()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	messages := cs.GetMessages(true)
 	assert.Len(t, messages, 1)
 	assert.Equal(t, "info", *messages[0].Level)
-	assert.Equal(t, "hello world\n", messages[0].Message)
+	assert.Contains(t, messages[0].Message, "hello world")
 
 	RemoveCmdState(cs.Id)
 	persistedState, err = GetCmdState(cs.Id)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 	assert.Nil(t, persistedState)
 }

exthttp/listener_test.go

@@ -12,16 +12,18 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
-	"github.com/madflojo/testcerts"
-	"github.com/phayes/freeport"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
+
+	"github.com/madflojo/testcerts"
+	"github.com/phayes/freeport"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestValidateSpecificationSuccessHttp(t *testing.T) {
@@ -126,7 +128,16 @@ func TestStartHttpsServerMustFailWhenCertificateCannotBeFound(t *testing.T) {
 	t.Setenv("STEADYBIT_EXTENSION_TLS_SERVER_CERT", filepath.Join(t.TempDir(), "unknown.pem"))
 
 	err = listen(ListenOpts{Port: port})
-	assert.ErrorContains(t, err, "no such file or directory")
+
+	var expected string
+
+	if runtime.GOOS == "windows" {
+		expected = "cannot find the file specified"
+	} else {
+		expected = "no such file or directory"
+	}
+
+	assert.ErrorContains(t, err, expected)
 }
 
 func TestStartHttpsServerMustFailWhenKeyCannotBeFound(t *testing.T) {
@@ -142,7 +153,15 @@ func TestStartHttpsServerMustFailWhenKeyCannotBeFound(t *testing.T) {
 	t.Setenv("STEADYBIT_EXTENSION_TLS_SERVER_CERT", cert)
 	err = listen(ListenOpts{Port: port})
 
-	assert.ErrorContains(t, err, "no such file or directory")
+	var expected string
+
+	if runtime.GOOS == "windows" {
+		expected = "cannot find the file specified"
+	} else {
+		expected = "no such file or directory"
+	}
+
+	assert.ErrorContains(t, err, expected)
 }
 
 func TestStartHttpsServerWithMutualTlsMustRefuseConnectionsWithoutMutualTls(t *testing.T) {

extlogging/extlogging.go

@@ -6,10 +6,12 @@
 package extlogging
 
 import (
-	"github.com/rs/zerolog"
-	"github.com/rs/zerolog/log"
 	"os"
 	"strings"
+	"time"
+
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
 )
 
 const RFC3339Micro = "2006-01-02T15:04:05.999Z07:00"
@@ -21,7 +23,10 @@ func InitZeroLog() {
 
 	var logger zerolog.Logger
 	if strings.ToLower(os.Getenv("STEADYBIT_LOG_FORMAT")) != "json" {
-		logger = zerolog.New(zerolog.ConsoleWriter{Out: os.Stderr, NoColor: getNoColor(), TimeFormat: RFC3339Micro})
+		logger = zerolog.New(zerolog.ConsoleWriter{Out: os.Stderr, NoColor: getNoColor(), TimeFormat: RFC3339Micro, FormatTimestamp: func(i interface{}) string {
+			timestamp, _ := time.Parse(time.RFC3339, i.(string))
+			return timestamp.Format(RFC3339Micro)
+		}})
 	} else {
 		logger = zerolog.New(os.Stderr)
 	}

extlogging/extlogging_test.go

@@ -1,14 +1,15 @@
 package extlogging
 
 import (
-	"github.com/rs/zerolog"
-	"github.com/rs/zerolog/log"
-	"github.com/stretchr/testify/assert"
 	"io"
 	"os"
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
+	"github.com/stretchr/testify/assert"
 )
 
 var captureLock sync.Mutex
@@ -30,11 +31,12 @@ func TestInitZeroLog_Format(t *testing.T) {
 		t.Run(tt.format, func(t *testing.T) {
 			t.Setenv("STEADYBIT_LOG_FORMAT", tt.format)
 
-			msg := captureStdErr(func() {
+			msg, err := captureStdErr(func() {
 				InitZeroLog()
 				log.Info().Msg("test")
 			})
 
+			assert.Nil(t, err)
 			assert.Equal(t, tt.wantedOutput, msg)
 		})
 	}
@@ -60,21 +62,32 @@ func TestInitZeroLog_Level(t *testing.T) {
 	}
 }
 
-func captureStdErr(f func()) string {
+func captureStdErr(f func()) (string, error) {
 	captureLock.Lock()
 	defer captureLock.Unlock()
 
 	rescueStderr := os.Stderr
-	r, w, _ := os.Pipe()
+	rescueStdout := os.Stdout
+	r, w, err := os.Pipe()
+
+	if err != nil {
+		log.Error().Msgf("unable to create os pipe: %s", err)
+		return "", err
+	}
+
 	os.Stderr = w
 	os.Stdout = w
 
 	f()
-	_ = w.Sync()
 
-	defer func() { os.Stderr = rescueStderr }()
+	defer func() {
+		os.Stderr = rescueStderr
+		os.Stdout = rescueStdout
+	}()
 
 	_ = r.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
+
+	w.Close()
 	captured, _ := io.ReadAll(r)
-	return string(captured)
+	return string(captured), nil
 }

extsignals/signalhandler.go

@@ -1,14 +1,14 @@
 package extsignals
 
 import (
-	"fmt"
-	"github.com/rs/zerolog/log"
-	"golang.org/x/sys/unix"
+	"context"
 	"os"
 	"os/signal"
 	"sort"
 	"sync"
 	"syscall"
+
+	"github.com/rs/zerolog/log"
 )
 
 var (
@@ -40,44 +40,64 @@ func AddSignalHandler(signalHandler SignalHandler) {
 	handlers.Store(signalHandler.Name, signalHandler)
 }
 
+func ClearSignalHandlers() {
+	handlers.Range(func(key, value interface{}) bool {
+		handlers.Delete(key)
+		return true
+	})
+}
+
 // RemoveSignalHandlersByName removes signal handlers by name. This is mainly used for testing.
 func RemoveSignalHandlersByName(names ...string) {
 	for _, name := range names {
 		handlers.Delete(name)
 	}
 }
 
+func createSignalChannel(context context.Context) {
+	signalChannel := make(chan os.Signal, 1)
+	Notify(signalChannel)
+	go func(signals <-chan os.Signal) {
+		for {
+			select {
+			case <-context.Done():
+				signal.Stop(signalChannel)
+				return
+			case s := <-signals:
+				handlerList := make([]SignalHandler, 0)
+				handlers.Range(func(key, value interface{}) bool {
+					handlerList = append(handlerList, value.(SignalHandler))
+					return true
+				})
+				sort.Sort(ByOrder(handlerList))
+				signalName := GetSignalName(s.(syscall.Signal))
+				for _, handler := range handlerList {
+					log.Debug().Str("signal", signalName).Str("handler", handler.Name).Int("order", handler.Order).Msg("received signal - call handler")
+					handler.Handler(s)
+				}
+			}
+		}
+	}(signalChannel)
+}
+
 func ActivateSignalHandlers() {
+	ActivateSignalHandlerWithContext(context.Background())
+}
+
+func ActivateSignalHandlerWithContext(context context.Context) {
 	AddSignalHandler(SignalHandler{
 		Handler: func(signal os.Signal) {
 			switch signal {
 			case syscall.SIGINT:
 				os.Exit(128 + int(signal.(syscall.Signal)))
 
 			case syscall.SIGTERM:
-				fmt.Printf("Terminated: %d\n", int(signal.(syscall.Signal)))
 				os.Exit(128 + int(signal.(syscall.Signal)))
 			}
 		},
 		Order: OrderTermination,
 		Name:  "Termination",
 	})
 
-	signalChannel := make(chan os.Signal, 1)
-	signal.Notify(signalChannel, syscall.SIGINT, syscall.SIGTERM, syscall.SIGUSR1)
-	go func(signals <-chan os.Signal) {
-		for s := range signals {
-			handlerList := make([]SignalHandler, 0)
-			handlers.Range(func(key, value interface{}) bool {
-				handlerList = append(handlerList, value.(SignalHandler))
-				return true
-			})
-			sort.Sort(ByOrder(handlerList))
-			signalName := unix.SignalName(s.(syscall.Signal))
-			for _, handler := range handlerList {
-				log.Debug().Str("signal", signalName).Str("handler", handler.Name).Int("order", handler.Order).Msg("received signal - call handler")
-				handler.Handler(s)
-			}
-		}
-	}(signalChannel)
+	createSignalChannel(context)
 }

extsignals/signalhandler_test.go

@@ -1,23 +1,25 @@
 package extsignals
 
+// These tests don't consistently work in IntelliJ,
+// use the native go test runner instead.
+
 import (
-	"github.com/rs/zerolog/log"
-	"github.com/stretchr/testify/require"
 	"os"
 	"sync/atomic"
-	"syscall"
 	"testing"
 	"time"
+
+	"github.com/rs/zerolog/log"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSignalHandlers(t *testing.T) {
-	//cleanup previous test
-	RemoveSignalHandlersByName("Termination", "Handler1", "Handler2")
-
 	handler1Run := atomic.Bool{}
 	handler2Run := atomic.Bool{}
 	handlerList := atomic.Value{}
 
+	ClearSignalHandlers()
+	defer ClearSignalHandlers()
 	ActivateSignalHandlers()
 	RemoveSignalHandlersByName("Termination")
 	AddSignalHandler(SignalHandler{
@@ -39,7 +41,7 @@ func TestSignalHandlers(t *testing.T) {
 		Name:  "Handler2",
 	})
 
-	err := syscall.Kill(os.Getpid(), syscall.SIGUSR1)
+	err := Kill(os.Getpid())
 	require.NoError(t, err)
 
 	// Wait for the signal to be processed
@@ -51,12 +53,11 @@ func TestSignalHandlers(t *testing.T) {
 }
 
 func TestRemoveSignalHandlersByName(t *testing.T) {
-	//cleanup previous test
-	RemoveSignalHandlersByName("Termination", "Handler1", "Handler2")
-
 	handler1Run := atomic.Bool{}
 	handler2Run := atomic.Bool{}
 
+	ClearSignalHandlers()
+	defer ClearSignalHandlers()
 	ActivateSignalHandlers()
 	AddSignalHandler(SignalHandler{
 		Handler: func(signal os.Signal) {
@@ -76,7 +77,7 @@ func TestRemoveSignalHandlersByName(t *testing.T) {
 	})
 
 	RemoveSignalHandlersByName("Termination", "Handler1")
-	err := syscall.Kill(os.Getpid(), syscall.SIGUSR1)
+	err := Kill(os.Getpid())
 	require.NoError(t, err)
 
 	// Wait for the signal to be processed

extsignals/signalhandler_utils_unix.go

@@ -0,0 +1,23 @@
+//go:build !windows
+
+package extsignals
+
+import (
+	"os"
+	"os/signal"
+	"syscall"
+
+	"golang.org/x/sys/unix"
+)
+
+func Notify(c chan<- os.Signal, sig ...os.Signal) {
+	signal.Notify(c, syscall.SIGINT, syscall.SIGTERM, syscall.SIGUSR1)
+}
+
+func GetSignalName(s syscall.Signal) string {
+	return unix.SignalName(s)
+}
+
+func Kill(pid int) (e error) {
+	return syscall.Kill(os.Getpid(), syscall.SIGUSR1)
+}