fix: checks are properly configured on start

Signed-off-by: Bruno Bressi <[email protected]>

cmd/run.go

@@ -69,7 +69,7 @@ func run() func(cmd *cobra.Command, args []string) error {
 
 		var cfg config.Config
 
-		err := viper.Unmarshal(cfg)
+		err := viper.Unmarshal(&cfg)
 		if err != nil {
 			return fmt.Errorf("failed to parse config: %w", err)
 		}

pkg/checks/factory/factory.go

@@ -29,10 +29,14 @@ import (
 
 // New creates a new check instance from the given name
 func New(cfg checks.Runtime) (checks.Check, error) {
-	if f, ok := RegisteredChecks[cfg.For()]; ok {
+	if cfg == nil {
+		return nil, errors.New("config is nil")
+	}
+
+	if f, ok := registry[cfg.For()]; ok {
 		c := f()
 		err := c.SetConfig(cfg)
-		return f(), err
+		return c, err
 	}
 	return nil, errors.New("unknown check type")
 }
@@ -50,10 +54,8 @@ func NewChecksFromConfig(cfg runtime.Config) (map[string]checks.Check, error) {
 	return result, nil
 }
 
-// RegisteredChecks will be registered in this map
-// The key is the name of the Check
-// The name needs to map the configuration item key
-var RegisteredChecks = map[string]func() checks.Check{
+// registry is a convenience map to create new checks
+var registry = map[string]func() checks.Check{
 	"health":  health.NewCheck,
 	"latency": latency.NewCheck,
 }

pkg/checks/factory/factory_test.go

@@ -0,0 +1,155 @@
+package factory
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/caas-team/sparrow/pkg/checks"
+	"github.com/caas-team/sparrow/pkg/checks/health"
+	"github.com/caas-team/sparrow/pkg/checks/latency"
+	"github.com/caas-team/sparrow/pkg/checks/runtime"
+)
+
+var (
+	latencyCfg = &latency.Config{
+		Targets:  []string{"http://localhost:8080/health"},
+		Interval: 1 * time.Second,
+		Timeout:  1 * time.Second,
+	}
+	healthCfg = &health.Config{
+		Targets:  []string{"http://localhost:8080/health"},
+		Interval: 1 * time.Second,
+		Timeout:  1 * time.Second,
+	}
+)
+
+func TestNewChecksFromConfig(t *testing.T) {
+	tests := []struct {
+		name    string
+		cfg     runtime.Config
+		want    map[string]checks.Check
+		wantErr bool
+	}{
+		{
+			name: "empty config",
+			cfg:  runtime.Config{},
+			want: map[string]checks.Check{},
+		},
+		{
+			name: "healthcheck",
+			cfg: runtime.Config{
+				Checks: runtime.Checks{
+					Health: healthCfg,
+				},
+			},
+			want: map[string]checks.Check{
+				"health": newHealthCheck(),
+			},
+		},
+		{
+			name: "latency",
+			cfg: runtime.Config{
+				Checks: runtime.Checks{
+					Latency: latencyCfg,
+				},
+			},
+			want: map[string]checks.Check{
+				"latency": newLatencyCheck(),
+			},
+		},
+		{
+			name: "multiple checks",
+			cfg: runtime.Config{
+				Checks: runtime.Checks{
+					Health:  healthCfg,
+					Latency: latencyCfg,
+				},
+			},
+			want: map[string]checks.Check{
+				"health":  newHealthCheck(),
+				"latency": newLatencyCheck(),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := NewChecksFromConfig(tt.cfg)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NewChecksFromConfig() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			// assert the configurations of the checks are equal -
+			// the checks themselves cannot be compared because of the done channels
+			for name, check := range got {
+				if !reflect.DeepEqual(check.GetConfig(), tt.want[name].GetConfig()) {
+					t.Errorf("NewChecksFromConfig() got = %v, want %v", check.GetConfig(), tt.want[name].GetConfig())
+				}
+			}
+		})
+	}
+}
+
+// newHealthCheck creates a new health check with a testing configuration
+func newHealthCheck() checks.Check {
+	res := health.NewCheck()
+	_ = res.SetConfig(healthCfg)
+	return res
+}
+
+// newLatencyCheck creates a new latency check with a testing configuration
+func newLatencyCheck() checks.Check {
+	res := latency.NewCheck()
+	_ = res.SetConfig(latencyCfg)
+	return res
+}
+
+func TestNew(t *testing.T) {
+	tests := []struct {
+		name    string
+		cfg     checks.Runtime
+		want    checks.Check
+		wantErr bool
+	}{
+		{
+			name:    "empty config",
+			cfg:     nil,
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name:    "healthcheck",
+			cfg:     healthCfg,
+			want:    newHealthCheck(),
+			wantErr: false,
+		},
+		{
+			name:    "latency",
+			cfg:     latencyCfg,
+			want:    newLatencyCheck(),
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := New(tt.cfg)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if got == nil && tt.want == nil {
+				return
+			}
+
+			if !reflect.DeepEqual(got.GetConfig(), tt.want.GetConfig()) {
+				t.Errorf("Check config got = %v, want %v", got.GetConfig(), tt.want.GetConfig())
+			}
+
+			if got.Name() != tt.want.Name() {
+				t.Errorf("Check name got = %v, want %v", got.Name(), tt.want.Name())
+			}
+		})
+	}
+}

pkg/sparrow/run.go

@@ -26,11 +26,11 @@ import (
 	"sync"
 	"time"
 
+	"github.com/caas-team/sparrow/pkg/checks"
+
 	"github.com/caas-team/sparrow/pkg/checks/factory"
 	"github.com/caas-team/sparrow/pkg/checks/runtime"
 
-	checks2 "github.com/caas-team/sparrow/pkg/checks"
-
 	"github.com/caas-team/sparrow/pkg/sparrow/targets"
 
 	"github.com/caas-team/sparrow/internal/logger"
@@ -45,19 +45,19 @@ const shutdownTimeout = time.Second * 90
 type Sparrow struct {
 	db db.DB
 	// the existing checks
-	checks map[string]checks2.Check
+	checks map[string]checks.Check
 	server *http.Server
 
 	metrics Metrics
 
-	resultFanIn map[string]chan checks2.Result
+	resultFanIn map[string]chan checks.Result
 
 	cfg *config.Config
 
 	// cCfgChecks is the channel where the loader sends the checkCfg configuration of the checks
 	cCfgChecks chan runtime.Config
 	// cResult is the channel where the checks send their results to
-	cResult chan checks2.ResultDTO
+	cResult chan checks.ResultDTO
 	// cErr is used to handle non-recoverable errors of the sparrow components
 	cErr chan error
 	// cDone is used to signal that the sparrow was shut down because of an error
@@ -76,10 +76,10 @@ type Sparrow struct {
 func New(cfg *config.Config) *Sparrow {
 	sparrow := &Sparrow{
 		db:          db.NewInMemory(),
-		checks:      make(map[string]checks2.Check),
+		checks:      make(map[string]checks.Check),
 		metrics:     NewMetrics(),
-		resultFanIn: make(map[string]chan checks2.Result),
-		cResult:     make(chan checks2.ResultDTO, 1),
+		resultFanIn: make(map[string]chan checks.Result),
+		cResult:     make(chan checks.ResultDTO, 1),
 		cfg:         cfg,
 		cCfgChecks:  make(chan runtime.Config, 1),
 		routingTree: api.NewRoutingTree(),
@@ -211,13 +211,13 @@ func (s *Sparrow) enrichTargets(cfg runtime.Config) runtime.Config {
 }
 
 // registerCheck registers and executes a new check
-func (s *Sparrow) registerCheck(ctx context.Context, check checks2.Check) error {
+func (s *Sparrow) registerCheck(ctx context.Context, check checks.Check) error {
 	log := logger.FromContext(ctx).With("name", check.Name())
 
 	s.checks[check.Name()] = check
 
 	// Create a fan in a channel for the check
-	checkChan := make(chan checks2.Result, 1)
+	checkChan := make(chan checks.Result, 1)
 	s.resultFanIn[check.Name()] = checkChan
 
 	go fanInResults(checkChan, s.cResult, check.Name())
@@ -246,7 +246,7 @@ func (s *Sparrow) registerCheck(ctx context.Context, check checks2.Check) error
 }
 
 // UnregisterCheck removes the check from sparrow and performs a soft shutdown for the check
-func (s *Sparrow) unregisterCheck(ctx context.Context, check checks2.Check) {
+func (s *Sparrow) unregisterCheck(ctx context.Context, check checks.Check) {
 	log := logger.FromContext(ctx).With("name", check.Name())
 	// Check has been removed from config; shutdown and remove
 	check.DeregisterHandler(ctx, s.routingTree)
@@ -275,9 +275,9 @@ func (s *Sparrow) unregisterCheck(ctx context.Context, check checks2.Check) {
 //
 // It allows augmenting the results with the check name which is needed by the db
 // without putting the responsibility of providing the name on every iteration on the check
-func fanInResults(checkChan chan checks2.Result, cResult chan checks2.ResultDTO, name string) {
+func fanInResults(checkChan chan checks.Result, cResult chan checks.ResultDTO, name string) {
 	for i := range checkChan {
-		cResult <- checks2.ResultDTO{
+		cResult <- checks.ResultDTO{
 			Name:   name,
 			Result: &i,
 		}