Extract indicator configuration and address PR comments

Peripli · Aug 20, 2019 · f008f59 · f008f59
1 parent f197ae0
commit f008f59
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 80 deletions.
diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go
@@ -33,11 +33,7 @@ type controller struct {
 }
 
 // NewController returns a new healthcheck controller with the given health and tresholds
-func NewController(health h.IHealth, indicators []health.Indicator) web.Controller {
-	tresholds := make(map[string]int64)
-	for _, v := range indicators {
-		tresholds[v.Name()] = v.FailuresTreshold()
-	}
+func NewController(health h.IHealth, tresholds map[string]int64) web.Controller {
 	return &controller{
 		health:    health,
 		tresholds: tresholds,

diff --git a/api/healthcheck/healthcheck_controller_test.go b/api/healthcheck/healthcheck_controller_test.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 	h "github.com/InVisionApp/go-health"
 	"github.com/Peripli/service-manager/pkg/health"
-	"github.com/Peripli/service-manager/pkg/health/healthfakes"
 	"github.com/Peripli/service-manager/pkg/web"
 	"net/http"
 	"testing"
@@ -138,34 +137,6 @@ var _ = Describe("Healthcheck controller", func() {
 			})
 		})
 	})
-
-	Describe("create controller", func() {
-		var c web.Controller
-		tresholds := map[string]int64{
-			"test1": 2,
-			"test2": 3,
-		}
-
-		BeforeEach(func() {
-			indicators := make([]health.Indicator, 0, len(tresholds))
-			for i, v := range tresholds {
-				indicator := &healthfakes.FakeIndicator{}
-				indicator.NameReturns(i)
-				indicator.FailuresTresholdReturns(v)
-
-				indicators = append(indicators, indicator)
-			}
-			c = NewController(HealthFake{}, indicators)
-		})
-
-		When("Controller created with given indicators", func() {
-			It("Should extract tresholds", func() {
-				controllerStruct := c.(*controller)
-
-				Expect(controllerStruct.tresholds).To(Equal(tresholds))
-			})
-		})
-	})
 })
 
 func createController(status health.Status) *controller {

diff --git a/pkg/health/registry_test.go b/pkg/health/registry_test.go
@@ -50,35 +50,6 @@ var _ = Describe("Healthcheck Registry", func() {
 			Expect(postAddIndicators).To(ContainElement(newIndicator))
 		})
 	})
-
-	When("When configure indicators", func() {
-		It("Should configure with default settings if settings not provided", func() {
-			newIndicator := &testIndicator{}
-			registry.HealthIndicators = append(registry.HealthIndicators, newIndicator)
-
-			registry.ConfigureIndicators()
-
-			Expect(newIndicator.Interval()).To(Equal(DefaultIndicatorSettings().Interval))
-			Expect(newIndicator.FailuresTreshold()).To(Equal(DefaultIndicatorSettings().FailuresTreshold))
-		})
-
-		It("Should configure with provided settings", func() {
-			newIndicator := &testIndicator{}
-			registry.HealthIndicators = append(registry.HealthIndicators, newIndicator)
-
-			settings := &IndicatorSettings{
-				Interval:         50,
-				FailuresTreshold: 2,
-			}
-
-			registry.HealthSettings[newIndicator.Name()] = settings
-
-			registry.ConfigureIndicators()
-
-			Expect(newIndicator.Interval()).To(Equal(settings.Interval))
-			Expect(newIndicator.FailuresTreshold()).To(Equal(settings.FailuresTreshold))
-		})
-	})
 })
 
 type testIndicator struct {

diff --git a/pkg/health/types.go b/pkg/health/types.go
@@ -180,16 +180,3 @@ type Registry struct {
 	// Indicator Settings of the registry
 	HealthSettings map[string]*IndicatorSettings
 }
-
-// ConfigureIndicators configures registry's indicators with provided settings
-func (r *Registry) ConfigureIndicators() {
-	for _, hi := range r.HealthIndicators {
-		if indicator, ok := hi.(ConfigurableIndicator); ok {
-			if settings, ok := r.HealthSettings[indicator.Name()]; ok {
-				indicator.Configure(settings)
-			} else {
-				indicator.Configure(DefaultIndicatorSettings())
-			}
-		}
-	}
-}
diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go
@@ -183,7 +183,7 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) (
 // Build builds the Service Manager
 func (smb *ServiceManagerBuilder) Build() *ServiceManager {
 	if err := smb.installHealth(); err != nil {
-		panic(err)
+		log.C(smb.ctx).Panic()
 	}
 
 	// setup server and add relevant global middleware
@@ -204,15 +204,22 @@ func (smb *ServiceManagerBuilder) installHealth() error {
 		return nil
 	}
 
-	smb.ConfigureIndicators()
-
 	healthz := h.New()
 	logger := log.C(smb.ctx).Logger
 
 	healthz.Logger = l.New(logger)
 	healthz.StatusListener = &health.StatusListener{}
 
+	tresholds := make(map[string]int64)
+
 	for _, indicator := range smb.HealthIndicators {
+		if configurableIndicator, ok := indicator.(health.ConfigurableIndicator); ok {
+			if settings, ok := smb.HealthSettings[configurableIndicator.Name()]; ok {
+				configurableIndicator.Configure(settings)
+			} else {
+				configurableIndicator.Configure(health.DefaultIndicatorSettings())
+			}
+		}
 		if err := healthz.AddCheck(&h.Config{
 			Name:     indicator.Name(),
 			Checker:  indicator,
@@ -221,9 +228,10 @@ func (smb *ServiceManagerBuilder) installHealth() error {
 		}); err != nil {
 			return err
 		}
+		tresholds[indicator.Name()] = indicator.FailuresTreshold()
 	}
 
-	smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthIndicators))
+	smb.RegisterControllers(healthcheck.NewController(healthz, tresholds))
 
 	if err := healthz.Start(); err != nil {
 		return err