From 4139ea3a9b56210cdc806f6609b7efaffad51624 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 11 Jun 2019 13:07:54 +0300 Subject: [PATCH 01/19] Async health check --- Gopkg.lock | 21 +++++ Gopkg.toml | 6 +- api/healthcheck/composite_indicator.go | 68 ---------------- api/healthcheck/composite_indicator_test.go | 87 --------------------- api/healthcheck/healthcheck_controller.go | 16 ++-- pkg/health/aggregation_policy.go | 19 ++++- pkg/health/ping.go | 4 +- pkg/health/types.go | 9 ++- pkg/sm/sm.go | 42 +++++++++- storage/healthcheck.go | 26 ++++-- storage/interfaces.go | 14 +--- storage/postgres/storage.go | 2 +- 12 files changed, 120 insertions(+), 194 deletions(-) delete mode 100644 api/healthcheck/composite_indicator.go delete mode 100644 api/healthcheck/composite_indicator_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 3d8c4cd45..95199d98a 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -8,6 +8,25 @@ pruneopts = "UT" revision = "852fc940e4b9b895dc144b88ee8f6e39228127b0" +[[projects]] + digest = "1:12156f9d8523585d9d875a03c4b5c6c6374f7370716cd814aa6781732ac942c5" + name = "github.com/InVisionApp/go-health" + packages = [ + ".", + "checkers", + ] + pruneopts = "UT" + revision = "06e1878ab5a8acf6e841861c6cbfcc4d3036add0" + version = "v2.1.0" + +[[projects]] + digest = "1:137a441b9903a0179439f1c497406765c3d3758a0f1e2922174dc61ae3c21fe6" + name = "github.com/InVisionApp/go-logger" + packages = ["."] + pruneopts = "UT" + revision = "c377c6c3f6a48a6366517e86f77bafea18454ff1" + version = "v1.0.1" + [[projects]] digest = "1:f780d408067189c4c42b53f7bb24ebf8fd2a1e4510b813ed6e79dd6563e38cc5" name = "github.com/ajg/form" @@ -680,6 +699,8 @@ analyzer-version = 1 input-imports = [ "github.com/DATA-DOG/go-sqlmock", + "github.com/InVisionApp/go-health", + "github.com/InVisionApp/go-health/checkers", "github.com/benjamintf1/unmarshalledmatchers", "github.com/cloudfoundry-community/go-cfenv", "github.com/coreos/go-oidc", diff --git a/Gopkg.toml b/Gopkg.toml index 9a361e541..4b182bedb 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -108,4 +108,8 @@ # Refer to issue https://github.com/golang/dep/issues/1799 [[override]] name = "gopkg.in/fsnotify.v1" -source = "https://github.com/fsnotify/fsnotify.git" \ No newline at end of file +source = "https://github.com/fsnotify/fsnotify.git" + +[[constraint]] + name = "github.com/InVisionApp/go-health" + version = "v2.1.0" \ No newline at end of file diff --git a/api/healthcheck/composite_indicator.go b/api/healthcheck/composite_indicator.go deleted file mode 100644 index 1e2a9962c..000000000 --- a/api/healthcheck/composite_indicator.go +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright 2018 The Service Manager Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package healthcheck - -import ( - "strings" - - "github.com/Peripli/service-manager/pkg/health" -) - -const defaultCompositeName = "composite" - -// compositeIndicator aggregates multiple health indicators and provides one detailed health -type compositeIndicator struct { - aggregationPolicy health.AggregationPolicy - indicators []health.Indicator -} - -// newCompositeIndicator returns a new compositeIndicator for the provided health indicators -func newCompositeIndicator(indicators []health.Indicator, aggregator health.AggregationPolicy) health.Indicator { - return &compositeIndicator{ - aggregationPolicy: aggregator, - indicators: indicators, - } -} - -// Name returns the name of the compositeIndicator -func (i *compositeIndicator) Name() string { - if len(i.indicators) == 0 { - return defaultCompositeName - } - return aggregateIndicatorNames(i.indicators) -} - -// Health returns the aggregated health of all health indicators -func (i *compositeIndicator) Health() *health.Health { - healths := make(map[string]*health.Health) - for _, indicator := range i.indicators { - healths[indicator.Name()] = indicator.Health() - } - return i.aggregationPolicy.Apply(healths) -} - -func aggregateIndicatorNames(indicators []health.Indicator) string { - indicatorsCnt := len(indicators) - builder := strings.Builder{} - for index, indicator := range indicators { - builder.WriteString(indicator.Name()) - if index < indicatorsCnt-1 { - builder.WriteString(", ") - } - } - return builder.String() -} diff --git a/api/healthcheck/composite_indicator_test.go b/api/healthcheck/composite_indicator_test.go deleted file mode 100644 index d27df5259..000000000 --- a/api/healthcheck/composite_indicator_test.go +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Copyright 2018 The Service Manager Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package healthcheck - -import ( - "testing" - - "github.com/Peripli/service-manager/pkg/health/healthfakes" - - "github.com/Peripli/service-manager/pkg/health" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -func TestNewCompositeIndicator(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Healthcheck Suite") -} - -var _ = Describe("Healthcheck Composite indicator", func() { - - Context("New Composite indicator", func() { - When("No indicators are provided ", func() { - It("Has default name", func() { - indicator := newCompositeIndicator(nil, nil) - Expect(indicator.Name()).To(Equal(defaultCompositeName)) - }) - }) - - When("Has indicators", func() { - It("Has the name of the indicator", func() { - fakeIndicator1 := &healthfakes.FakeIndicator{} - fakeIndicator1.NameReturns("fake1") - - fakeIndicator2 := &healthfakes.FakeIndicator{} - fakeIndicator2.NameReturns("fake2") - indicators := []health.Indicator{fakeIndicator1, fakeIndicator2} - indicator := newCompositeIndicator(indicators, nil) - - Expect(indicator.Name()).To(Equal(aggregateIndicatorNames(indicators))) - }) - }) - }) - - When("Checking health", func() { - Context("With empty indicators", func() { - It("Returns unknown status", func() { - indicator := newCompositeIndicator(nil, &health.DefaultAggregationPolicy{}) - h := indicator.Health() - Expect(h.Status).To(Equal(health.StatusUnknown)) - Expect(h.Details["error"]).ToNot(BeNil()) - }) - }) - - Context("With provided indicators", func() { - It("Aggregates the healths", func() { - testIndicator := &healthfakes.FakeIndicator{} - testIndicator.HealthReturns(health.New().Up()) - testIndicator.NameReturns("fake") - - aggregationPolicy := &healthfakes.FakeAggregationPolicy{} - defaultAggregationPolicy := &health.DefaultAggregationPolicy{} - aggregationPolicy.ApplyStub = defaultAggregationPolicy.Apply - - indicator := newCompositeIndicator([]health.Indicator{testIndicator}, aggregationPolicy) - invocationsCnt := aggregationPolicy.ApplyCallCount() - health := indicator.Health() - Expect(aggregationPolicy.ApplyCallCount()).To(Equal(invocationsCnt + 1)) - Expect(health.Details[testIndicator.Name()]).ToNot(BeNil()) - }) - }) - }) -}) diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index 90f7ddcbf..de811333a 100644 --- a/api/healthcheck/healthcheck_controller.go +++ b/api/healthcheck/healthcheck_controller.go @@ -17,23 +17,26 @@ package healthcheck import ( + h "github.com/InVisionApp/go-health" + "github.com/Peripli/service-manager/pkg/health" "github.com/Peripli/service-manager/pkg/util" "net/http" - "github.com/Peripli/service-manager/pkg/health" "github.com/Peripli/service-manager/pkg/log" "github.com/Peripli/service-manager/pkg/web" ) // controller platform controller type controller struct { - indicator health.Indicator + health h.IHealth + aggPolicy health.AggregationPolicy } // NewController returns a new healthcheck controller with the given indicators and aggregation policy -func NewController(indicators []health.Indicator, aggregator health.AggregationPolicy) web.Controller { +func NewController(health h.IHealth, aggPolict health.AggregationPolicy) web.Controller { return &controller{ - indicator: newCompositeIndicator(indicators, aggregator), + health: health, + aggPolicy: aggPolict, } } @@ -41,8 +44,9 @@ func NewController(indicators []health.Indicator, aggregator health.AggregationP func (c *controller) healthCheck(r *web.Request) (*web.Response, error) { ctx := r.Context() logger := log.C(ctx) - logger.Debugf("Performing health check with %s...", c.indicator.Name()) - healthResult := c.indicator.Health() + logger.Debugf("Performing health check...") + healthState, _, _ := c.health.State() + healthResult := c.aggPolicy.Apply(healthState) var status int if healthResult.Status == health.StatusUp { status = http.StatusOK diff --git a/pkg/health/aggregation_policy.go b/pkg/health/aggregation_policy.go index 3fba18ec1..15b4aed22 100644 --- a/pkg/health/aggregation_policy.go +++ b/pkg/health/aggregation_policy.go @@ -16,26 +16,39 @@ package health +import "github.com/InVisionApp/go-health" + // DefaultAggregationPolicy aggregates the healths by constructing a new Health based on the given // where the overall health status is negative if one of the healths is negative and positive if all are positive type DefaultAggregationPolicy struct { } // Apply aggregates the given healths -func (*DefaultAggregationPolicy) Apply(healths map[string]*Health) *Health { +func (*DefaultAggregationPolicy) Apply(healths map[string]health.State) *Health { if len(healths) == 0 { return New().WithDetail("error", "no health indicators registered").Unknown() } overallStatus := StatusUp for _, health := range healths { - if health.Status == StatusDown { + if health.Status == "failed" && health.ContiguousFailures > 3 { overallStatus = StatusDown break } } details := make(map[string]interface{}) for k, v := range healths { - details[k] = v + details[k] = convertStatus(v.Status) } return New().WithStatus(overallStatus).WithDetails(details) } + +func convertStatus(status string) Status { + switch status { + case "ok": + return StatusUp + case "failed": + return StatusDown + default: + return StatusUnknown + } +} diff --git a/pkg/health/ping.go b/pkg/health/ping.go index c145376ea..6331a89fb 100644 --- a/pkg/health/ping.go +++ b/pkg/health/ping.go @@ -24,6 +24,6 @@ func (*pingIndicator) Name() string { return "ping" } -func (*pingIndicator) Health() *Health { - return New().Up() +func (*pingIndicator) Status() (interface{}, error) { + return nil, nil } diff --git a/pkg/health/types.go b/pkg/health/types.go index 6ed339b17..f9cff7243 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -16,6 +16,8 @@ package health +import "github.com/InVisionApp/go-health" + // Status represents the overall health status of a component type Status string @@ -91,15 +93,16 @@ func (h *Health) WithDetails(details map[string]interface{}) *Health { type Indicator interface { // Name returns the name of the component Name() string - // Health returns the health of the component - Health() *Health + + // Status returns the health information of the component + Status() (interface{}, error) } // AggregationPolicy is an interface to provide aggregated health information //go:generate counterfeiter . AggregationPolicy type AggregationPolicy interface { // Apply processes the given healths to build a single health - Apply(healths map[string]*Health) *Health + Apply(healths map[string]health.State) *Health } // NewDefaultRegistry returns a default health registry with a single ping indicator and a default aggregation policy diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 4cf1e70ed..3059f48ea 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -21,9 +21,11 @@ import ( "crypto/tls" "database/sql" "fmt" + health "github.com/InVisionApp/go-health" "net" "net/http" "sync" + "time" "github.com/Peripli/service-manager/api/osb" @@ -132,7 +134,12 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( return nil, fmt.Errorf("error creating core api: %s", err) } - API.HealthIndicators = append(API.HealthIndicators, &storage.HealthIndicator{Pinger: storage.PingFunc(smStorage.Ping)}) + storageHealthIndicator, err := storage.NewStorageHealthIndicator(storage.PingFunc(smStorage.Ping)) + if err != nil { + return nil, fmt.Errorf("error creating storage health indicator: %s", err) + } + + API.HealthIndicators = append(API.HealthIndicators, storageHealthIndicator) notificationCleaner := &storage.NotificationCleaner{ Storage: interceptableRepository, @@ -149,6 +156,11 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( cfg: cfg, } + err = smb.installHealth() + if err != nil { + return nil, fmt.Errorf("error adding health chech to sm: %s", err) + } + // Register default interceptors that represent the core SM business logic smb. WithCreateInterceptorProvider(types.ServiceBrokerType, &interceptors.BrokerCreateCatalogInterceptorProvider{ @@ -189,10 +201,32 @@ func (smb *ServiceManagerBuilder) Build() *ServiceManager { } } -func (smb *ServiceManagerBuilder) installHealth() { - if len(smb.HealthIndicators) > 0 { - smb.RegisterControllers(healthcheck.NewController(smb.HealthIndicators, smb.HealthAggregationPolicy)) +func (smb *ServiceManagerBuilder) installHealth() error { + if len(smb.HealthIndicators) == 0 { + return nil + } + + h := health.New() + + for _, indicator := range smb.HealthIndicators { + err := h.AddCheck(&health.Config{ + Name: indicator.Name(), + Checker: indicator, + Interval: time.Duration(1) * time.Minute, + }) + + if err != nil { + return err + } } + smb.RegisterControllers(healthcheck.NewController(h, smb.HealthAggregationPolicy)) + + err := h.Start() + if err != nil { + return err + } + + return nil } // Run starts the Service Manager diff --git a/storage/healthcheck.go b/storage/healthcheck.go index 3d70320ca..3d6988e45 100644 --- a/storage/healthcheck.go +++ b/storage/healthcheck.go @@ -16,11 +16,14 @@ package storage -import "github.com/Peripli/service-manager/pkg/health" +import ( + "github.com/InVisionApp/go-health/checkers" + "github.com/Peripli/service-manager/pkg/health" +) // HealthIndicator returns a new indicator for the storage type HealthIndicator struct { - Pinger Pinger + checkers.SQL } // Name returns the name of the storage component @@ -28,12 +31,19 @@ func (i *HealthIndicator) Name() string { return "storage" } -// Health returns the health of the storage component -func (i *HealthIndicator) Health() *health.Health { - err := i.Pinger.Ping() - healthz := health.New() +func NewStorageHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { + sqlConfig := &checkers.SQLConfig{ + Pinger: pingFunc, + } + sqlChecker, err := checkers.NewSQL(sqlConfig) if err != nil { - return healthz.WithError(err).WithDetail("message", "TransactionalRepository ping failed") + return nil, err + } + + indicator := &HealthIndicator{ + SQL: *sqlChecker, } - return healthz.Up() + + return indicator, nil + } diff --git a/storage/interfaces.go b/storage/interfaces.go index 508324e42..9d047e55a 100644 --- a/storage/interfaces.go +++ b/storage/interfaces.go @@ -135,19 +135,12 @@ type OpenCloser interface { Close() error } -// Pinger allows pinging the storage to check liveliness -//go:generate counterfeiter . Pinger -type Pinger interface { - // Ping verifies a connection to the database is still alive, establishing a connection if necessary. - Ping() error -} - // PingFunc is an adapter that allows to use regular functions as Pinger -type PingFunc func() error +type PingFunc func(context.Context) error // Ping allows PingFunc to act as a Pinger -func (mf PingFunc) Ping() error { - return mf() +func (mf PingFunc) PingContext(ctx context.Context) error { + return mf(ctx) } type Repository interface { @@ -182,7 +175,6 @@ type TransactionalRepositoryDecorator func(TransactionalRepository) (Transaction //go:generate counterfeiter . Storage type Storage interface { OpenCloser - Pinger TransactionalRepository Introduce(entity Entity) diff --git a/storage/postgres/storage.go b/storage/postgres/storage.go index 9072977f0..4fe2ab9f5 100644 --- a/storage/postgres/storage.go +++ b/storage/postgres/storage.go @@ -138,7 +138,7 @@ func (ps *Storage) updateSchema(migrationsURL, pgDriverName string) error { return err } -func (ps *Storage) Ping() error { +func (ps *Storage) Ping(ctx context.Context) error { ps.checkOpen() return ps.state.Get() } From 144ea916b3527bd177c6a7ed090688fdc35fb8e9 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 11 Jun 2019 16:01:25 +0300 Subject: [PATCH 02/19] Tests adaptation --- api/healthcheck/healthcheck_controller.go | 4 +- .../healthcheck_controller_test.go | 43 ++++- pkg/health/aggregation_policy_test.go | 21 ++- .../healthfakes/fake_aggregation_policy.go | 13 +- pkg/health/healthfakes/fake_indicator.go | 165 ------------------ pkg/health/registry_test.go | 7 +- pkg/health/types.go | 1 - storage/healthcheck_test.go | 33 ++-- storage/postgres/storage_test.go | 2 +- storage/storagefakes/fake_pinger.go | 101 ----------- storage/storagefakes/fake_storage.go | 64 ------- 11 files changed, 77 insertions(+), 377 deletions(-) delete mode 100644 pkg/health/healthfakes/fake_indicator.go delete mode 100644 storage/storagefakes/fake_pinger.go diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index de811333a..fa1db8076 100644 --- a/api/healthcheck/healthcheck_controller.go +++ b/api/healthcheck/healthcheck_controller.go @@ -33,10 +33,10 @@ type controller struct { } // NewController returns a new healthcheck controller with the given indicators and aggregation policy -func NewController(health h.IHealth, aggPolict health.AggregationPolicy) web.Controller { +func NewController(health h.IHealth, aggPolicy health.AggregationPolicy) web.Controller { return &controller{ health: health, - aggPolicy: aggPolict, + aggPolicy: aggPolicy, } } diff --git a/api/healthcheck/healthcheck_controller_test.go b/api/healthcheck/healthcheck_controller_test.go index 6d39f3a3c..f7e049ae1 100644 --- a/api/healthcheck/healthcheck_controller_test.go +++ b/api/healthcheck/healthcheck_controller_test.go @@ -17,13 +17,12 @@ package healthcheck import ( "fmt" + h "github.com/InVisionApp/go-health" + "github.com/Peripli/service-manager/pkg/health" + "github.com/Peripli/service-manager/pkg/health/healthfakes" "net/http" "testing" - "github.com/Peripli/service-manager/pkg/health/healthfakes" - - "github.com/Peripli/service-manager/pkg/health" - "github.com/Peripli/service-manager/pkg/web" . "github.com/onsi/ginkgo" @@ -70,9 +69,39 @@ var _ = Describe("Healthcheck controller", func() { }) func createController(status health.Status) *controller { - indicator := &healthfakes.FakeIndicator{} - indicator.HealthReturns(health.New().WithStatus(status)) + aggPolicy := &healthfakes.FakeAggregationPolicy{} + aggPolicy.ApplyReturns(&health.Health{Status: status}) return &controller{ - indicator: indicator, + health: HealthFake{}, + aggPolicy: aggPolicy, } } + +type HealthFake struct { + state map[string]h.State + failed bool + err error +} + +func (hf HealthFake) AddChecks(cfgs []*h.Config) error { + return nil +} + +func (hf HealthFake) AddCheck(cfg *h.Config) error { + return nil +} + +func (hf HealthFake) Start() error { + return nil +} + +func (hf HealthFake) Stop() error { + return nil +} + +func (hf HealthFake) State() (map[string]h.State, bool, error) { + return hf.state, hf.failed, hf.err +} +func (hf HealthFake) Failed() bool { + return hf.failed +} diff --git a/pkg/health/aggregation_policy_test.go b/pkg/health/aggregation_policy_test.go index da9c0d365..048129d0f 100644 --- a/pkg/health/aggregation_policy_test.go +++ b/pkg/health/aggregation_policy_test.go @@ -17,6 +17,7 @@ package health import ( + "github.com/InVisionApp/go-health" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -24,12 +25,12 @@ import ( var _ = Describe("Healthcheck AggregationPolicy", func() { aggregationPolicy := &DefaultAggregationPolicy{} - var healths map[string]*Health + var healths map[string]health.State BeforeEach(func() { - healths = map[string]*Health{ - "test1": New().Up(), - "test2": New().Up(), + healths = map[string]health.State{ + "test1": {Status: "ok"}, + "test2": {Status: "ok"}, } }) @@ -43,12 +44,20 @@ var _ = Describe("Healthcheck AggregationPolicy", func() { When("At least one health is DOWN", func() { It("Returns DOWN", func() { - healths["test3"] = New().Down() + healths["test3"] = health.State{Status: "failed", ContiguousFailures: 4} aggregatedHealth := aggregationPolicy.Apply(healths) Expect(aggregatedHealth.Status).To(Equal(StatusDown)) }) }) + When("There is DOWN healths but not more than 3 times in a row", func() { + It("Returns UP", func() { + healths["test3"] = health.State{Status: "failed"} + aggregatedHealth := aggregationPolicy.Apply(healths) + Expect(aggregatedHealth.Status).To(Equal(StatusUp)) + }) + }) + When("All healths are UP", func() { It("Returns UP", func() { aggregatedHealth := aggregationPolicy.Apply(healths) @@ -60,7 +69,7 @@ var _ = Describe("Healthcheck AggregationPolicy", func() { It("Includes them as overall details", func() { aggregatedHealth := aggregationPolicy.Apply(healths) for name, h := range healths { - Expect(aggregatedHealth.Details[name]).To(Equal(h)) + Expect(aggregatedHealth.Details[name]).To(Equal(convertStatus(h.Status))) } }) }) diff --git a/pkg/health/healthfakes/fake_aggregation_policy.go b/pkg/health/healthfakes/fake_aggregation_policy.go index 45a1f9504..f11b40a81 100644 --- a/pkg/health/healthfakes/fake_aggregation_policy.go +++ b/pkg/health/healthfakes/fake_aggregation_policy.go @@ -4,14 +4,15 @@ package healthfakes import ( "sync" + healtha "github.com/InVisionApp/go-health" "github.com/Peripli/service-manager/pkg/health" ) type FakeAggregationPolicy struct { - ApplyStub func(map[string]*health.Health) *health.Health + ApplyStub func(map[string]healtha.State) *health.Health applyMutex sync.RWMutex applyArgsForCall []struct { - arg1 map[string]*health.Health + arg1 map[string]healtha.State } applyReturns struct { result1 *health.Health @@ -23,11 +24,11 @@ type FakeAggregationPolicy struct { invocationsMutex sync.RWMutex } -func (fake *FakeAggregationPolicy) Apply(arg1 map[string]*health.Health) *health.Health { +func (fake *FakeAggregationPolicy) Apply(arg1 map[string]healtha.State) *health.Health { fake.applyMutex.Lock() ret, specificReturn := fake.applyReturnsOnCall[len(fake.applyArgsForCall)] fake.applyArgsForCall = append(fake.applyArgsForCall, struct { - arg1 map[string]*health.Health + arg1 map[string]healtha.State }{arg1}) fake.recordInvocation("Apply", []interface{}{arg1}) fake.applyMutex.Unlock() @@ -47,13 +48,13 @@ func (fake *FakeAggregationPolicy) ApplyCallCount() int { return len(fake.applyArgsForCall) } -func (fake *FakeAggregationPolicy) ApplyCalls(stub func(map[string]*health.Health) *health.Health) { +func (fake *FakeAggregationPolicy) ApplyCalls(stub func(map[string]healtha.State) *health.Health) { fake.applyMutex.Lock() defer fake.applyMutex.Unlock() fake.ApplyStub = stub } -func (fake *FakeAggregationPolicy) ApplyArgsForCall(i int) map[string]*health.Health { +func (fake *FakeAggregationPolicy) ApplyArgsForCall(i int) map[string]healtha.State { fake.applyMutex.RLock() defer fake.applyMutex.RUnlock() argsForCall := fake.applyArgsForCall[i] diff --git a/pkg/health/healthfakes/fake_indicator.go b/pkg/health/healthfakes/fake_indicator.go deleted file mode 100644 index 5959dc549..000000000 --- a/pkg/health/healthfakes/fake_indicator.go +++ /dev/null @@ -1,165 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package healthfakes - -import ( - "sync" - - "github.com/Peripli/service-manager/pkg/health" -) - -type FakeIndicator struct { - HealthStub func() *health.Health - healthMutex sync.RWMutex - healthArgsForCall []struct { - } - healthReturns struct { - result1 *health.Health - } - healthReturnsOnCall map[int]struct { - result1 *health.Health - } - NameStub func() string - nameMutex sync.RWMutex - nameArgsForCall []struct { - } - nameReturns struct { - result1 string - } - nameReturnsOnCall map[int]struct { - result1 string - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeIndicator) Health() *health.Health { - fake.healthMutex.Lock() - ret, specificReturn := fake.healthReturnsOnCall[len(fake.healthArgsForCall)] - fake.healthArgsForCall = append(fake.healthArgsForCall, struct { - }{}) - fake.recordInvocation("Health", []interface{}{}) - fake.healthMutex.Unlock() - if fake.HealthStub != nil { - return fake.HealthStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.healthReturns - return fakeReturns.result1 -} - -func (fake *FakeIndicator) HealthCallCount() int { - fake.healthMutex.RLock() - defer fake.healthMutex.RUnlock() - return len(fake.healthArgsForCall) -} - -func (fake *FakeIndicator) HealthCalls(stub func() *health.Health) { - fake.healthMutex.Lock() - defer fake.healthMutex.Unlock() - fake.HealthStub = stub -} - -func (fake *FakeIndicator) HealthReturns(result1 *health.Health) { - fake.healthMutex.Lock() - defer fake.healthMutex.Unlock() - fake.HealthStub = nil - fake.healthReturns = struct { - result1 *health.Health - }{result1} -} - -func (fake *FakeIndicator) HealthReturnsOnCall(i int, result1 *health.Health) { - fake.healthMutex.Lock() - defer fake.healthMutex.Unlock() - fake.HealthStub = nil - if fake.healthReturnsOnCall == nil { - fake.healthReturnsOnCall = make(map[int]struct { - result1 *health.Health - }) - } - fake.healthReturnsOnCall[i] = struct { - result1 *health.Health - }{result1} -} - -func (fake *FakeIndicator) Name() string { - fake.nameMutex.Lock() - ret, specificReturn := fake.nameReturnsOnCall[len(fake.nameArgsForCall)] - fake.nameArgsForCall = append(fake.nameArgsForCall, struct { - }{}) - fake.recordInvocation("Name", []interface{}{}) - fake.nameMutex.Unlock() - if fake.NameStub != nil { - return fake.NameStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.nameReturns - return fakeReturns.result1 -} - -func (fake *FakeIndicator) NameCallCount() int { - fake.nameMutex.RLock() - defer fake.nameMutex.RUnlock() - return len(fake.nameArgsForCall) -} - -func (fake *FakeIndicator) NameCalls(stub func() string) { - fake.nameMutex.Lock() - defer fake.nameMutex.Unlock() - fake.NameStub = stub -} - -func (fake *FakeIndicator) NameReturns(result1 string) { - fake.nameMutex.Lock() - defer fake.nameMutex.Unlock() - fake.NameStub = nil - fake.nameReturns = struct { - result1 string - }{result1} -} - -func (fake *FakeIndicator) NameReturnsOnCall(i int, result1 string) { - fake.nameMutex.Lock() - defer fake.nameMutex.Unlock() - fake.NameStub = nil - if fake.nameReturnsOnCall == nil { - fake.nameReturnsOnCall = make(map[int]struct { - result1 string - }) - } - fake.nameReturnsOnCall[i] = struct { - result1 string - }{result1} -} - -func (fake *FakeIndicator) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.healthMutex.RLock() - defer fake.healthMutex.RUnlock() - fake.nameMutex.RLock() - defer fake.nameMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeIndicator) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ health.Indicator = new(FakeIndicator) diff --git a/pkg/health/registry_test.go b/pkg/health/registry_test.go index faf4d8525..d73d65e25 100644 --- a/pkg/health/registry_test.go +++ b/pkg/health/registry_test.go @@ -17,6 +17,7 @@ package health import ( + "github.com/InVisionApp/go-health" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -68,7 +69,7 @@ var _ = Describe("Healthcheck Registry", func() { type testAggregationPolicy struct { } -func (*testAggregationPolicy) Apply(healths map[string]*Health) *Health { +func (*testAggregationPolicy) Apply(healths map[string]health.State) *Health { return New().Up() } @@ -79,6 +80,6 @@ func (*testIndicator) Name() string { return "test" } -func (*testIndicator) Health() *Health { - return New().Up() +func (*testIndicator) Status() (interface{}, error) { + return nil, nil } diff --git a/pkg/health/types.go b/pkg/health/types.go index f9cff7243..fb1d09869 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -89,7 +89,6 @@ func (h *Health) WithDetails(details map[string]interface{}) *Health { } // Indicator is an interface to provide the health of a component -//go:generate counterfeiter . Indicator type Indicator interface { // Name returns the name of the component Name() string diff --git a/storage/healthcheck_test.go b/storage/healthcheck_test.go index 4d6eb8490..4b7d15a56 100644 --- a/storage/healthcheck_test.go +++ b/storage/healthcheck_test.go @@ -17,27 +17,22 @@ package storage_test import ( + "context" "fmt" - "github.com/Peripli/service-manager/pkg/health" "github.com/Peripli/service-manager/storage" - "github.com/Peripli/service-manager/storage/storagefakes" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) var _ = Describe("Healthcheck", func() { - var healthIndicator *storage.HealthIndicator - var pinger *storagefakes.FakePinger + var healthIndicator health.Indicator BeforeEach(func() { - pinger = &storagefakes.FakePinger{} - pinger.PingStub = func() error { + ping := func(ctx context.Context) error { return nil } - healthIndicator = &storage.HealthIndicator{ - Pinger: pinger, - } + healthIndicator, _ = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) }) Context("Name", func() { @@ -47,27 +42,23 @@ var _ = Describe("Healthcheck", func() { }) Context("Ping does not return error", func() { - It("Returns health status UP", func() { - healthz := healthIndicator.Health() - Expect(healthz.Status).To(Equal(health.StatusUp)) + It("Status doest not contains error", func() { + _, err := healthIndicator.Status() + Expect(err).ShouldNot(HaveOccurred()) }) }) Context("Ping returns error", func() { expectedError := fmt.Errorf("could not connect to database") BeforeEach(func() { - pinger.PingStub = func() error { + ping := func(ctx context.Context) error { return expectedError } - }) - It("Returns status DOWN", func() { - healthz := healthIndicator.Health() - Expect(healthz.Status).To(Equal(health.StatusDown)) + healthIndicator, _ = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) }) It("Contains error", func() { - healthz := healthIndicator.Health() - errorDetails := healthz.Details["error"] - Expect(errorDetails).To(Equal(expectedError)) + _, err := healthIndicator.Status() + Expect(err).Should(HaveOccurred()) + Expect(err).To(Equal(expectedError)) }) - }) }) diff --git a/storage/postgres/storage_test.go b/storage/postgres/storage_test.go index 5b0a8a351..1078313da 100644 --- a/storage/postgres/storage_test.go +++ b/storage/postgres/storage_test.go @@ -70,7 +70,7 @@ var _ = Describe("Postgres Storage", func() { Describe("Ping", func() { Context("Called with uninitialized db", func() { It("Should panic", func() { - Expect(func() { pgStorage.Ping() }).To(Panic()) + Expect(func() { pgStorage.Ping(context.Background()) }).To(Panic()) }) }) }) diff --git a/storage/storagefakes/fake_pinger.go b/storage/storagefakes/fake_pinger.go deleted file mode 100644 index a8549274c..000000000 --- a/storage/storagefakes/fake_pinger.go +++ /dev/null @@ -1,101 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package storagefakes - -import ( - "sync" - - "github.com/Peripli/service-manager/storage" -) - -type FakePinger struct { - PingStub func() error - pingMutex sync.RWMutex - pingArgsForCall []struct { - } - pingReturns struct { - result1 error - } - pingReturnsOnCall map[int]struct { - result1 error - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakePinger) Ping() error { - fake.pingMutex.Lock() - ret, specificReturn := fake.pingReturnsOnCall[len(fake.pingArgsForCall)] - fake.pingArgsForCall = append(fake.pingArgsForCall, struct { - }{}) - fake.recordInvocation("Ping", []interface{}{}) - fake.pingMutex.Unlock() - if fake.PingStub != nil { - return fake.PingStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.pingReturns - return fakeReturns.result1 -} - -func (fake *FakePinger) PingCallCount() int { - fake.pingMutex.RLock() - defer fake.pingMutex.RUnlock() - return len(fake.pingArgsForCall) -} - -func (fake *FakePinger) PingCalls(stub func() error) { - fake.pingMutex.Lock() - defer fake.pingMutex.Unlock() - fake.PingStub = stub -} - -func (fake *FakePinger) PingReturns(result1 error) { - fake.pingMutex.Lock() - defer fake.pingMutex.Unlock() - fake.PingStub = nil - fake.pingReturns = struct { - result1 error - }{result1} -} - -func (fake *FakePinger) PingReturnsOnCall(i int, result1 error) { - fake.pingMutex.Lock() - defer fake.pingMutex.Unlock() - fake.PingStub = nil - if fake.pingReturnsOnCall == nil { - fake.pingReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.pingReturnsOnCall[i] = struct { - result1 error - }{result1} -} - -func (fake *FakePinger) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.pingMutex.RLock() - defer fake.pingMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakePinger) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ storage.Pinger = new(FakePinger) diff --git a/storage/storagefakes/fake_storage.go b/storage/storagefakes/fake_storage.go index dc16ce5de..4ec5ff95d 100644 --- a/storage/storagefakes/fake_storage.go +++ b/storage/storagefakes/fake_storage.go @@ -108,16 +108,6 @@ type FakeStorage struct { openReturnsOnCall map[int]struct { result1 error } - PingStub func() error - pingMutex sync.RWMutex - pingArgsForCall []struct { - } - pingReturns struct { - result1 error - } - pingReturnsOnCall map[int]struct { - result1 error - } UpdateStub func(context.Context, types.Object, query.LabelChanges, ...query.Criterion) (types.Object, error) updateMutex sync.RWMutex updateArgsForCall []struct { @@ -601,58 +591,6 @@ func (fake *FakeStorage) OpenReturnsOnCall(i int, result1 error) { }{result1} } -func (fake *FakeStorage) Ping() error { - fake.pingMutex.Lock() - ret, specificReturn := fake.pingReturnsOnCall[len(fake.pingArgsForCall)] - fake.pingArgsForCall = append(fake.pingArgsForCall, struct { - }{}) - fake.recordInvocation("Ping", []interface{}{}) - fake.pingMutex.Unlock() - if fake.PingStub != nil { - return fake.PingStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.pingReturns - return fakeReturns.result1 -} - -func (fake *FakeStorage) PingCallCount() int { - fake.pingMutex.RLock() - defer fake.pingMutex.RUnlock() - return len(fake.pingArgsForCall) -} - -func (fake *FakeStorage) PingCalls(stub func() error) { - fake.pingMutex.Lock() - defer fake.pingMutex.Unlock() - fake.PingStub = stub -} - -func (fake *FakeStorage) PingReturns(result1 error) { - fake.pingMutex.Lock() - defer fake.pingMutex.Unlock() - fake.PingStub = nil - fake.pingReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeStorage) PingReturnsOnCall(i int, result1 error) { - fake.pingMutex.Lock() - defer fake.pingMutex.Unlock() - fake.PingStub = nil - if fake.pingReturnsOnCall == nil { - fake.pingReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.pingReturnsOnCall[i] = struct { - result1 error - }{result1} -} - func (fake *FakeStorage) Update(arg1 context.Context, arg2 types.Object, arg3 query.LabelChanges, arg4 ...query.Criterion) (types.Object, error) { fake.updateMutex.Lock() ret, specificReturn := fake.updateReturnsOnCall[len(fake.updateArgsForCall)] @@ -738,8 +676,6 @@ func (fake *FakeStorage) Invocations() map[string][][]interface{} { defer fake.listMutex.RUnlock() fake.openMutex.RLock() defer fake.openMutex.RUnlock() - fake.pingMutex.RLock() - defer fake.pingMutex.RUnlock() fake.updateMutex.RLock() defer fake.updateMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} From 61157b439c73a224fe2d40b117cca06a8c0e404b Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 17 Jun 2019 15:24:28 +0300 Subject: [PATCH 03/19] Make health configurable externally --- api/healthcheck/healthcheck_controller.go | 14 ++++---- .../healthcheck_controller_test.go | 5 +-- config/config.go | 3 ++ pkg/health/aggregation_policy.go | 4 +-- pkg/health/aggregation_policy_test.go | 13 ++++---- .../healthfakes/fake_aggregation_policy.go | 18 ++++++----- pkg/health/registry_test.go | 2 +- pkg/health/types.go | 32 +++++++++++++++++-- pkg/sm/sm.go | 17 +++++----- 9 files changed, 73 insertions(+), 35 deletions(-) diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index fa1db8076..c13d90ffb 100644 --- a/api/healthcheck/healthcheck_controller.go +++ b/api/healthcheck/healthcheck_controller.go @@ -28,15 +28,17 @@ import ( // controller platform controller type controller struct { - health h.IHealth - aggPolicy health.AggregationPolicy + health h.IHealth + aggPolicy health.AggregationPolicy + failuresTreshold int64 } // NewController returns a new healthcheck controller with the given indicators and aggregation policy -func NewController(health h.IHealth, aggPolicy health.AggregationPolicy) web.Controller { +func NewController(health h.IHealth, aggPolicy health.AggregationPolicy, failuresTreshold int64) web.Controller { return &controller{ - health: health, - aggPolicy: aggPolicy, + health: health, + aggPolicy: aggPolicy, + failuresTreshold: failuresTreshold, } } @@ -46,7 +48,7 @@ func (c *controller) healthCheck(r *web.Request) (*web.Response, error) { logger := log.C(ctx) logger.Debugf("Performing health check...") healthState, _, _ := c.health.State() - healthResult := c.aggPolicy.Apply(healthState) + healthResult := c.aggPolicy.Apply(healthState, c.failuresTreshold) var status int if healthResult.Status == health.StatusUp { status = http.StatusOK diff --git a/api/healthcheck/healthcheck_controller_test.go b/api/healthcheck/healthcheck_controller_test.go index f7e049ae1..e4a1b7916 100644 --- a/api/healthcheck/healthcheck_controller_test.go +++ b/api/healthcheck/healthcheck_controller_test.go @@ -72,8 +72,9 @@ func createController(status health.Status) *controller { aggPolicy := &healthfakes.FakeAggregationPolicy{} aggPolicy.ApplyReturns(&health.Health{Status: status}) return &controller{ - health: HealthFake{}, - aggPolicy: aggPolicy, + health: HealthFake{}, + aggPolicy: aggPolicy, + failuresTreshold: 3, } } diff --git a/config/config.go b/config/config.go index f9e28f561..9a39e9025 100644 --- a/config/config.go +++ b/config/config.go @@ -23,6 +23,7 @@ import ( "github.com/Peripli/service-manager/api" "github.com/Peripli/service-manager/pkg/env" + "github.com/Peripli/service-manager/pkg/health" "github.com/Peripli/service-manager/pkg/log" "github.com/Peripli/service-manager/pkg/server" "github.com/Peripli/service-manager/pkg/ws" @@ -38,6 +39,7 @@ type Settings struct { API *api.Settings WebSocket *ws.Settings HTTPClient *httpclient.Settings + Health *health.Settings } // AddPFlags adds the SM config flags to the provided flag set @@ -55,6 +57,7 @@ func DefaultSettings() *Settings { API: api.DefaultSettings(), WebSocket: ws.DefaultSettings(), HTTPClient: httpclient.DefaultSettings(), + Health: health.DefaultSettings(), } } diff --git a/pkg/health/aggregation_policy.go b/pkg/health/aggregation_policy.go index 15b4aed22..553db911d 100644 --- a/pkg/health/aggregation_policy.go +++ b/pkg/health/aggregation_policy.go @@ -24,13 +24,13 @@ type DefaultAggregationPolicy struct { } // Apply aggregates the given healths -func (*DefaultAggregationPolicy) Apply(healths map[string]health.State) *Health { +func (*DefaultAggregationPolicy) Apply(healths map[string]health.State, failureTreshold int64) *Health { if len(healths) == 0 { return New().WithDetail("error", "no health indicators registered").Unknown() } overallStatus := StatusUp for _, health := range healths { - if health.Status == "failed" && health.ContiguousFailures > 3 { + if health.Status == "failed" && health.ContiguousFailures > failureTreshold { overallStatus = StatusDown break } diff --git a/pkg/health/aggregation_policy_test.go b/pkg/health/aggregation_policy_test.go index 048129d0f..f288dba9f 100644 --- a/pkg/health/aggregation_policy_test.go +++ b/pkg/health/aggregation_policy_test.go @@ -26,6 +26,7 @@ var _ = Describe("Healthcheck AggregationPolicy", func() { aggregationPolicy := &DefaultAggregationPolicy{} var healths map[string]health.State + var failuresTreshold int64 = 3 BeforeEach(func() { healths = map[string]health.State{ @@ -36,7 +37,7 @@ var _ = Describe("Healthcheck AggregationPolicy", func() { When("No healths are provided", func() { It("Returns UNKNOWN and an error detail", func() { - aggregatedHealth := aggregationPolicy.Apply(nil) + aggregatedHealth := aggregationPolicy.Apply(nil, failuresTreshold) Expect(aggregatedHealth.Status).To(Equal(StatusUnknown)) Expect(aggregatedHealth.Details["error"]).ToNot(BeNil()) }) @@ -45,29 +46,29 @@ var _ = Describe("Healthcheck AggregationPolicy", func() { When("At least one health is DOWN", func() { It("Returns DOWN", func() { healths["test3"] = health.State{Status: "failed", ContiguousFailures: 4} - aggregatedHealth := aggregationPolicy.Apply(healths) + aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) Expect(aggregatedHealth.Status).To(Equal(StatusDown)) }) }) - When("There is DOWN healths but not more than 3 times in a row", func() { + When("There is DOWN healths but not more than treshold times in a row", func() { It("Returns UP", func() { healths["test3"] = health.State{Status: "failed"} - aggregatedHealth := aggregationPolicy.Apply(healths) + aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) Expect(aggregatedHealth.Status).To(Equal(StatusUp)) }) }) When("All healths are UP", func() { It("Returns UP", func() { - aggregatedHealth := aggregationPolicy.Apply(healths) + aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) Expect(aggregatedHealth.Status).To(Equal(StatusUp)) }) }) When("Aggregating healths", func() { It("Includes them as overall details", func() { - aggregatedHealth := aggregationPolicy.Apply(healths) + aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) for name, h := range healths { Expect(aggregatedHealth.Details[name]).To(Equal(convertStatus(h.Status))) } diff --git a/pkg/health/healthfakes/fake_aggregation_policy.go b/pkg/health/healthfakes/fake_aggregation_policy.go index f11b40a81..69a1220cc 100644 --- a/pkg/health/healthfakes/fake_aggregation_policy.go +++ b/pkg/health/healthfakes/fake_aggregation_policy.go @@ -9,10 +9,11 @@ import ( ) type FakeAggregationPolicy struct { - ApplyStub func(map[string]healtha.State) *health.Health + ApplyStub func(map[string]healtha.State, int64) *health.Health applyMutex sync.RWMutex applyArgsForCall []struct { arg1 map[string]healtha.State + arg2 int64 } applyReturns struct { result1 *health.Health @@ -24,16 +25,17 @@ type FakeAggregationPolicy struct { invocationsMutex sync.RWMutex } -func (fake *FakeAggregationPolicy) Apply(arg1 map[string]healtha.State) *health.Health { +func (fake *FakeAggregationPolicy) Apply(arg1 map[string]healtha.State, arg2 int64) *health.Health { fake.applyMutex.Lock() ret, specificReturn := fake.applyReturnsOnCall[len(fake.applyArgsForCall)] fake.applyArgsForCall = append(fake.applyArgsForCall, struct { arg1 map[string]healtha.State - }{arg1}) - fake.recordInvocation("Apply", []interface{}{arg1}) + arg2 int64 + }{arg1, arg2}) + fake.recordInvocation("Apply", []interface{}{arg1, arg2}) fake.applyMutex.Unlock() if fake.ApplyStub != nil { - return fake.ApplyStub(arg1) + return fake.ApplyStub(arg1, arg2) } if specificReturn { return ret.result1 @@ -48,17 +50,17 @@ func (fake *FakeAggregationPolicy) ApplyCallCount() int { return len(fake.applyArgsForCall) } -func (fake *FakeAggregationPolicy) ApplyCalls(stub func(map[string]healtha.State) *health.Health) { +func (fake *FakeAggregationPolicy) ApplyCalls(stub func(map[string]healtha.State, int64) *health.Health) { fake.applyMutex.Lock() defer fake.applyMutex.Unlock() fake.ApplyStub = stub } -func (fake *FakeAggregationPolicy) ApplyArgsForCall(i int) map[string]healtha.State { +func (fake *FakeAggregationPolicy) ApplyArgsForCall(i int) (map[string]healtha.State, int64) { fake.applyMutex.RLock() defer fake.applyMutex.RUnlock() argsForCall := fake.applyArgsForCall[i] - return argsForCall.arg1 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeAggregationPolicy) ApplyReturns(result1 *health.Health) { diff --git a/pkg/health/registry_test.go b/pkg/health/registry_test.go index d73d65e25..a0d543446 100644 --- a/pkg/health/registry_test.go +++ b/pkg/health/registry_test.go @@ -69,7 +69,7 @@ var _ = Describe("Healthcheck Registry", func() { type testAggregationPolicy struct { } -func (*testAggregationPolicy) Apply(healths map[string]health.State) *Health { +func (*testAggregationPolicy) Apply(healths map[string]health.State, failuresTreshold int64) *Health { return New().Up() } diff --git a/pkg/health/types.go b/pkg/health/types.go index fb1d09869..5d5806f8f 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -16,7 +16,35 @@ package health -import "github.com/InVisionApp/go-health" +import ( + "fmt" + "github.com/InVisionApp/go-health" +) + +// Settings type to be loaded from the environment +type Settings struct { + FailuresTreshold int64 `mapstructure:"failures_treshold" description:"maximum failures in a row until component is considered down"` + Interval int64 `description:"seconds between health checks of components"` +} + +// DefaultSettings returns default values for health settings +func DefaultSettings() *Settings { + return &Settings{ + FailuresTreshold: 3, + Interval: 60, + } +} + +// Validate validates the health settings +func (s *Settings) Validate() error { + if s.FailuresTreshold <= 0 { + return fmt.Errorf("validate Settings: FailuresTreshold must be > 0") + } + if s.Interval <= 0 { + return fmt.Errorf("valudate Settings: Interval must be > 0") + } + return nil +} // Status represents the overall health status of a component type Status string @@ -101,7 +129,7 @@ type Indicator interface { //go:generate counterfeiter . AggregationPolicy type AggregationPolicy interface { // Apply processes the given healths to build a single health - Apply(healths map[string]health.State) *Health + Apply(healths map[string]health.State, failureTreshold int64) *Health } // NewDefaultRegistry returns a default health registry with a single ping indicator and a default aggregation policy diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 3059f48ea..0462953c8 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -21,7 +21,8 @@ import ( "crypto/tls" "database/sql" "fmt" - health "github.com/InVisionApp/go-health" + h "github.com/InVisionApp/go-health" + "github.com/Peripli/service-manager/pkg/health" "net" "net/http" "sync" @@ -156,7 +157,7 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( cfg: cfg, } - err = smb.installHealth() + err = smb.installHealth(cfg.Health) if err != nil { return nil, fmt.Errorf("error adding health chech to sm: %s", err) } @@ -201,27 +202,27 @@ func (smb *ServiceManagerBuilder) Build() *ServiceManager { } } -func (smb *ServiceManagerBuilder) installHealth() error { +func (smb *ServiceManagerBuilder) installHealth(cfg *health.Settings) error { if len(smb.HealthIndicators) == 0 { return nil } - h := health.New() + healthz := h.New() for _, indicator := range smb.HealthIndicators { - err := h.AddCheck(&health.Config{ + err := healthz.AddCheck(&h.Config{ Name: indicator.Name(), Checker: indicator, - Interval: time.Duration(1) * time.Minute, + Interval: time.Duration(cfg.Interval) * time.Second, }) if err != nil { return err } } - smb.RegisterControllers(healthcheck.NewController(h, smb.HealthAggregationPolicy)) + smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthAggregationPolicy, cfg.FailuresTreshold)) - err := h.Start() + err := healthz.Start() if err != nil { return err } From 9f62a8120135b84ec9bbb8d8e014b51914452be9 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 17 Jun 2019 16:40:11 +0300 Subject: [PATCH 04/19] Install health later --- pkg/sm/sm.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 0462953c8..8dc6bb99b 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -187,6 +187,11 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( // Build builds the Service Manager func (smb *ServiceManagerBuilder) Build() *ServiceManager { + err := smb.installHealth() + if err != nil { + panic(err) + } + // setup server and add relevant global middleware smb.installHealth() @@ -202,7 +207,7 @@ func (smb *ServiceManagerBuilder) Build() *ServiceManager { } } -func (smb *ServiceManagerBuilder) installHealth(cfg *health.Settings) error { +func (smb *ServiceManagerBuilder) installHealth() error { if len(smb.HealthIndicators) == 0 { return nil } @@ -213,14 +218,14 @@ func (smb *ServiceManagerBuilder) installHealth(cfg *health.Settings) error { err := healthz.AddCheck(&h.Config{ Name: indicator.Name(), Checker: indicator, - Interval: time.Duration(cfg.Interval) * time.Second, + Interval: time.Duration(smb.health.Interval) * time.Second, }) if err != nil { return err } } - smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthAggregationPolicy, cfg.FailuresTreshold)) + smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthAggregationPolicy, smb.health.FailuresTreshold)) err := healthz.Start() if err != nil { From 81ff6653695fe906156d883badbb945a665e5cd0 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 17 Jun 2019 16:49:46 +0300 Subject: [PATCH 05/19] Export ConvertStatus function --- pkg/health/aggregation_policy.go | 5 +++-- pkg/health/aggregation_policy_test.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/health/aggregation_policy.go b/pkg/health/aggregation_policy.go index 553db911d..8c73ca395 100644 --- a/pkg/health/aggregation_policy.go +++ b/pkg/health/aggregation_policy.go @@ -37,12 +37,13 @@ func (*DefaultAggregationPolicy) Apply(healths map[string]health.State, failureT } details := make(map[string]interface{}) for k, v := range healths { - details[k] = convertStatus(v.Status) + details[k] = ConvertStatus(v.Status) } return New().WithStatus(overallStatus).WithDetails(details) } -func convertStatus(status string) Status { +// ConvertStatus converts go-health status to Status +func ConvertStatus(status string) Status { switch status { case "ok": return StatusUp diff --git a/pkg/health/aggregation_policy_test.go b/pkg/health/aggregation_policy_test.go index f288dba9f..c16a24d10 100644 --- a/pkg/health/aggregation_policy_test.go +++ b/pkg/health/aggregation_policy_test.go @@ -70,7 +70,7 @@ var _ = Describe("Healthcheck AggregationPolicy", func() { It("Includes them as overall details", func() { aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) for name, h := range healths { - Expect(aggregatedHealth.Details[name]).To(Equal(convertStatus(h.Status))) + Expect(aggregatedHealth.Details[name]).To(Equal(ConvertStatus(h.Status))) } }) }) From 115e2a429f2efb737f27aee0cbd9fda463249a4c Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 17 Jun 2019 17:07:25 +0300 Subject: [PATCH 06/19] Fix health settings validation --- pkg/health/types.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/health/types.go b/pkg/health/types.go index 5d5806f8f..d0408cb73 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -37,11 +37,11 @@ func DefaultSettings() *Settings { // Validate validates the health settings func (s *Settings) Validate() error { - if s.FailuresTreshold <= 0 { - return fmt.Errorf("validate Settings: FailuresTreshold must be > 0") + if s.FailuresTreshold < 0 { + return fmt.Errorf("validate Settings: FailuresTreshold must be >= 0") } - if s.Interval <= 0 { - return fmt.Errorf("valudate Settings: Interval must be > 0") + if s.Interval < 0 { + return fmt.Errorf("valudate Settings: Interval must be >= 0") } return nil } From 37f9bc426947a2598c255541888cadec520235ef Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 18 Jun 2019 12:37:09 +0300 Subject: [PATCH 07/19] Attach logger to health --- Gopkg.lock | 8 ++++++-- pkg/sm/sm.go | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 95199d98a..9d7656807 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -20,9 +20,12 @@ version = "v2.1.0" [[projects]] - digest = "1:137a441b9903a0179439f1c497406765c3d3758a0f1e2922174dc61ae3c21fe6" + digest = "1:be2deb3cba3d2526b3692d80483f59df6462aabc875aace2db263f2664f30670" name = "github.com/InVisionApp/go-logger" - packages = ["."] + packages = [ + ".", + "shims/logrus", + ] pruneopts = "UT" revision = "c377c6c3f6a48a6366517e86f77bafea18454ff1" version = "v1.0.1" @@ -701,6 +704,7 @@ "github.com/DATA-DOG/go-sqlmock", "github.com/InVisionApp/go-health", "github.com/InVisionApp/go-health/checkers", + "github.com/InVisionApp/go-logger/shims/logrus", "github.com/benjamintf1/unmarshalledmatchers", "github.com/cloudfoundry-community/go-cfenv", "github.com/coreos/go-oidc", diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 8dc6bb99b..0bd7af54a 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -22,6 +22,7 @@ import ( "database/sql" "fmt" h "github.com/InVisionApp/go-health" + l "github.com/InVisionApp/go-logger/shims/logrus" "github.com/Peripli/service-manager/pkg/health" "net" "net/http" @@ -213,6 +214,8 @@ func (smb *ServiceManagerBuilder) installHealth() error { } healthz := h.New() + logger := log.C(smb.ctx).Logger + healthz.Logger = l.New(logger) for _, indicator := range smb.HealthIndicators { err := healthz.AddCheck(&h.Config{ From 4f21ed987ff6accbf464b0a91e4af4a40d0984e9 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 18 Jun 2019 18:09:20 +0300 Subject: [PATCH 08/19] Address PR comments --- config/config.go | 2 +- pkg/health/types.go | 9 ++- pkg/sm/sm.go | 17 ++-- storage/interfaces.go | 10 ++- storage/postgres/storage.go | 2 +- storage/postgres/storage_test.go | 2 +- storage/storagefakes/fake_pinger.go | 111 +++++++++++++++++++++++++++ storage/storagefakes/fake_storage.go | 73 ++++++++++++++++++ 8 files changed, 213 insertions(+), 13 deletions(-) create mode 100644 storage/storagefakes/fake_pinger.go diff --git a/config/config.go b/config/config.go index 9a39e9025..2d156f401 100644 --- a/config/config.go +++ b/config/config.go @@ -85,7 +85,7 @@ func NewForEnv(env env.Environment) (*Settings, error) { func (c *Settings) Validate() error { validatable := []interface { Validate() error - }{c.Server, c.Storage, c.Log, c.API, c.WebSocket} + }{c.Server, c.Storage, c.Log, c.Health, c.API, c.WebSocket} for _, item := range validatable { if err := item.Validate(); err != nil { diff --git a/pkg/health/types.go b/pkg/health/types.go index d0408cb73..240379427 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -19,12 +19,13 @@ package health import ( "fmt" "github.com/InVisionApp/go-health" + "time" ) // Settings type to be loaded from the environment type Settings struct { - FailuresTreshold int64 `mapstructure:"failures_treshold" description:"maximum failures in a row until component is considered down"` - Interval int64 `description:"seconds between health checks of components"` + FailuresTreshold int64 `mapstructure:"failures_treshold" description:"maximum failures in a row until component is considered down"` + Interval time.Duration `mapstructure:"interval" description:"seconds between health checks of components"` } // DefaultSettings returns default values for health settings @@ -40,8 +41,8 @@ func (s *Settings) Validate() error { if s.FailuresTreshold < 0 { return fmt.Errorf("validate Settings: FailuresTreshold must be >= 0") } - if s.Interval < 0 { - return fmt.Errorf("valudate Settings: Interval must be >= 0") + if s.Interval < 30 { + return fmt.Errorf("validate Settings: Minimum interval is 30 seconds") } return nil } diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 0bd7af54a..db07d7af8 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -23,7 +23,6 @@ import ( "fmt" h "github.com/InVisionApp/go-health" l "github.com/InVisionApp/go-logger/shims/logrus" - "github.com/Peripli/service-manager/pkg/health" "net" "net/http" "sync" @@ -136,7 +135,7 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( return nil, fmt.Errorf("error creating core api: %s", err) } - storageHealthIndicator, err := storage.NewStorageHealthIndicator(storage.PingFunc(smStorage.Ping)) + storageHealthIndicator, err := storage.NewStorageHealthIndicator(storage.PingFunc(smStorage.PingContext)) if err != nil { return nil, fmt.Errorf("error creating storage health indicator: %s", err) } @@ -221,7 +220,7 @@ func (smb *ServiceManagerBuilder) installHealth() error { err := healthz.AddCheck(&h.Config{ Name: indicator.Name(), Checker: indicator, - Interval: time.Duration(smb.health.Interval) * time.Second, + Interval: smb.health.Interval * time.Second, }) if err != nil { @@ -230,11 +229,19 @@ func (smb *ServiceManagerBuilder) installHealth() error { } smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthAggregationPolicy, smb.health.FailuresTreshold)) - err := healthz.Start() - if err != nil { + if err := healthz.Start(); err != nil { return err } + // Handles safe termination of sm + util.StartInWaitGroupWithContext(smb.ctx, func(c context.Context) { + <-c.Done() + log.C(c).Debug("Context cancelled. Stopping health checks...") + if err := healthz.Stop(); err != nil { + log.D().Error(err) + } + }, smb.wg) + return nil } diff --git a/storage/interfaces.go b/storage/interfaces.go index 9d047e55a..7eb24b0dd 100644 --- a/storage/interfaces.go +++ b/storage/interfaces.go @@ -135,10 +135,17 @@ type OpenCloser interface { Close() error } +// Pinger allows pinging the storage to check liveliness +//go:generate counterfeiter . Pinger +type Pinger interface { + // PingContext verifies a connection to the database is still alive, establishing a connection if necessary. + PingContext(context.Context) error +} + // PingFunc is an adapter that allows to use regular functions as Pinger type PingFunc func(context.Context) error -// Ping allows PingFunc to act as a Pinger +// PingContext allows PingFunc to act as a Pinger func (mf PingFunc) PingContext(ctx context.Context) error { return mf(ctx) } @@ -176,6 +183,7 @@ type TransactionalRepositoryDecorator func(TransactionalRepository) (Transaction type Storage interface { OpenCloser TransactionalRepository + Pinger Introduce(entity Entity) } diff --git a/storage/postgres/storage.go b/storage/postgres/storage.go index 4fe2ab9f5..3c7681d9b 100644 --- a/storage/postgres/storage.go +++ b/storage/postgres/storage.go @@ -138,7 +138,7 @@ func (ps *Storage) updateSchema(migrationsURL, pgDriverName string) error { return err } -func (ps *Storage) Ping(ctx context.Context) error { +func (ps *Storage) PingContext(ctx context.Context) error { ps.checkOpen() return ps.state.Get() } diff --git a/storage/postgres/storage_test.go b/storage/postgres/storage_test.go index 1078313da..ac1cecd98 100644 --- a/storage/postgres/storage_test.go +++ b/storage/postgres/storage_test.go @@ -70,7 +70,7 @@ var _ = Describe("Postgres Storage", func() { Describe("Ping", func() { Context("Called with uninitialized db", func() { It("Should panic", func() { - Expect(func() { pgStorage.Ping(context.Background()) }).To(Panic()) + Expect(func() { pgStorage.PingContext(context.Background()) }).To(Panic()) }) }) }) diff --git a/storage/storagefakes/fake_pinger.go b/storage/storagefakes/fake_pinger.go new file mode 100644 index 000000000..ea4f34a7e --- /dev/null +++ b/storage/storagefakes/fake_pinger.go @@ -0,0 +1,111 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package storagefakes + +import ( + "context" + "sync" + + "github.com/Peripli/service-manager/storage" +) + +type FakePinger struct { + PingContextStub func(context.Context) error + pingContextMutex sync.RWMutex + pingContextArgsForCall []struct { + arg1 context.Context + } + pingContextReturns struct { + result1 error + } + pingContextReturnsOnCall map[int]struct { + result1 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakePinger) PingContext(arg1 context.Context) error { + fake.pingContextMutex.Lock() + ret, specificReturn := fake.pingContextReturnsOnCall[len(fake.pingContextArgsForCall)] + fake.pingContextArgsForCall = append(fake.pingContextArgsForCall, struct { + arg1 context.Context + }{arg1}) + fake.recordInvocation("PingContext", []interface{}{arg1}) + fake.pingContextMutex.Unlock() + if fake.PingContextStub != nil { + return fake.PingContextStub(arg1) + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.pingContextReturns + return fakeReturns.result1 +} + +func (fake *FakePinger) PingContextCallCount() int { + fake.pingContextMutex.RLock() + defer fake.pingContextMutex.RUnlock() + return len(fake.pingContextArgsForCall) +} + +func (fake *FakePinger) PingContextCalls(stub func(context.Context) error) { + fake.pingContextMutex.Lock() + defer fake.pingContextMutex.Unlock() + fake.PingContextStub = stub +} + +func (fake *FakePinger) PingContextArgsForCall(i int) context.Context { + fake.pingContextMutex.RLock() + defer fake.pingContextMutex.RUnlock() + argsForCall := fake.pingContextArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakePinger) PingContextReturns(result1 error) { + fake.pingContextMutex.Lock() + defer fake.pingContextMutex.Unlock() + fake.PingContextStub = nil + fake.pingContextReturns = struct { + result1 error + }{result1} +} + +func (fake *FakePinger) PingContextReturnsOnCall(i int, result1 error) { + fake.pingContextMutex.Lock() + defer fake.pingContextMutex.Unlock() + fake.PingContextStub = nil + if fake.pingContextReturnsOnCall == nil { + fake.pingContextReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.pingContextReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakePinger) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.pingContextMutex.RLock() + defer fake.pingContextMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakePinger) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ storage.Pinger = new(FakePinger) diff --git a/storage/storagefakes/fake_storage.go b/storage/storagefakes/fake_storage.go index 4ec5ff95d..1cb6fa4f7 100644 --- a/storage/storagefakes/fake_storage.go +++ b/storage/storagefakes/fake_storage.go @@ -108,6 +108,17 @@ type FakeStorage struct { openReturnsOnCall map[int]struct { result1 error } + PingContextStub func(context.Context) error + pingContextMutex sync.RWMutex + pingContextArgsForCall []struct { + arg1 context.Context + } + pingContextReturns struct { + result1 error + } + pingContextReturnsOnCall map[int]struct { + result1 error + } UpdateStub func(context.Context, types.Object, query.LabelChanges, ...query.Criterion) (types.Object, error) updateMutex sync.RWMutex updateArgsForCall []struct { @@ -591,6 +602,66 @@ func (fake *FakeStorage) OpenReturnsOnCall(i int, result1 error) { }{result1} } +func (fake *FakeStorage) PingContext(arg1 context.Context) error { + fake.pingContextMutex.Lock() + ret, specificReturn := fake.pingContextReturnsOnCall[len(fake.pingContextArgsForCall)] + fake.pingContextArgsForCall = append(fake.pingContextArgsForCall, struct { + arg1 context.Context + }{arg1}) + fake.recordInvocation("PingContext", []interface{}{arg1}) + fake.pingContextMutex.Unlock() + if fake.PingContextStub != nil { + return fake.PingContextStub(arg1) + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.pingContextReturns + return fakeReturns.result1 +} + +func (fake *FakeStorage) PingContextCallCount() int { + fake.pingContextMutex.RLock() + defer fake.pingContextMutex.RUnlock() + return len(fake.pingContextArgsForCall) +} + +func (fake *FakeStorage) PingContextCalls(stub func(context.Context) error) { + fake.pingContextMutex.Lock() + defer fake.pingContextMutex.Unlock() + fake.PingContextStub = stub +} + +func (fake *FakeStorage) PingContextArgsForCall(i int) context.Context { + fake.pingContextMutex.RLock() + defer fake.pingContextMutex.RUnlock() + argsForCall := fake.pingContextArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeStorage) PingContextReturns(result1 error) { + fake.pingContextMutex.Lock() + defer fake.pingContextMutex.Unlock() + fake.PingContextStub = nil + fake.pingContextReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeStorage) PingContextReturnsOnCall(i int, result1 error) { + fake.pingContextMutex.Lock() + defer fake.pingContextMutex.Unlock() + fake.PingContextStub = nil + if fake.pingContextReturnsOnCall == nil { + fake.pingContextReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.pingContextReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeStorage) Update(arg1 context.Context, arg2 types.Object, arg3 query.LabelChanges, arg4 ...query.Criterion) (types.Object, error) { fake.updateMutex.Lock() ret, specificReturn := fake.updateReturnsOnCall[len(fake.updateArgsForCall)] @@ -676,6 +747,8 @@ func (fake *FakeStorage) Invocations() map[string][][]interface{} { defer fake.listMutex.RUnlock() fake.openMutex.RLock() defer fake.openMutex.RUnlock() + fake.pingContextMutex.RLock() + defer fake.pingContextMutex.RUnlock() fake.updateMutex.RLock() defer fake.updateMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} From 421d88fe7345bbd59c983da33e0a795e7fdfd073 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 24 Jun 2019 18:06:40 +0300 Subject: [PATCH 09/19] Configuration per indicator and refactoring --- api/healthcheck/healthcheck_controller.go | 50 ++- .../healthcheck_controller_test.go | 130 +++++- pkg/health/aggregation_policy.go | 55 --- pkg/health/aggregation_policy_test.go | 77 ---- .../healthfakes/fake_aggregation_policy.go | 113 ----- .../fake_configurable_indicator.go | 401 ++++++++++++++++++ pkg/health/healthfakes/fake_indicator.go | 363 ++++++++++++++++ pkg/health/ping.go | 14 + pkg/health/registry_test.go | 75 ++-- pkg/health/types.go | 86 +++- pkg/sm/sm.go | 20 +- storage/healthcheck.go | 23 +- storage/healthcheck_test.go | 8 +- 13 files changed, 1099 insertions(+), 316 deletions(-) delete mode 100644 pkg/health/aggregation_policy.go delete mode 100644 pkg/health/aggregation_policy_test.go delete mode 100644 pkg/health/healthfakes/fake_aggregation_policy.go create mode 100644 pkg/health/healthfakes/fake_configurable_indicator.go create mode 100644 pkg/health/healthfakes/fake_indicator.go diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index c13d90ffb..0d10c134f 100644 --- a/api/healthcheck/healthcheck_controller.go +++ b/api/healthcheck/healthcheck_controller.go @@ -28,17 +28,19 @@ import ( // controller platform controller type controller struct { - health h.IHealth - aggPolicy health.AggregationPolicy - failuresTreshold int64 + health h.IHealth + tresholds map[string]int64 } -// NewController returns a new healthcheck controller with the given indicators and aggregation policy -func NewController(health h.IHealth, aggPolicy health.AggregationPolicy, failuresTreshold int64) web.Controller { +// 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() + } return &controller{ - health: health, - aggPolicy: aggPolicy, - failuresTreshold: failuresTreshold, + health: health, + tresholds: tresholds, } } @@ -48,7 +50,7 @@ func (c *controller) healthCheck(r *web.Request) (*web.Response, error) { logger := log.C(ctx) logger.Debugf("Performing health check...") healthState, _, _ := c.health.State() - healthResult := c.aggPolicy.Apply(healthState, c.failuresTreshold) + healthResult := c.aggregate(healthState) var status int if healthResult.Status == health.StatusUp { status = http.StatusOK @@ -57,3 +59,33 @@ func (c *controller) healthCheck(r *web.Request) (*web.Response, error) { } return util.NewJSONResponse(status, healthResult) } + +func (c *controller) aggregate(state map[string]h.State) *health.Health { + if len(state) == 0 { + return health.New().WithDetail("error", "no health indicators registered").Unknown() + } + overallStatus := health.StatusUp + for i, v := range state { + if v.Fatal && v.ContiguousFailures >= c.tresholds[i] { + overallStatus = health.StatusDown + break + } + } + details := make(map[string]interface{}) + for k, v := range state { + v.Status = convertStatus(v.Status) + details[k] = v + } + return health.New().WithStatus(overallStatus).WithDetails(details) +} + +func convertStatus(status string) string { + switch status { + case "ok": + return string(health.StatusUp) + case "failed": + return string(health.StatusDown) + default: + return string(health.StatusUnknown) + } +} diff --git a/api/healthcheck/healthcheck_controller_test.go b/api/healthcheck/healthcheck_controller_test.go index e4a1b7916..a91325e25 100644 --- a/api/healthcheck/healthcheck_controller_test.go +++ b/api/healthcheck/healthcheck_controller_test.go @@ -20,10 +20,11 @@ import ( 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" - "github.com/Peripli/service-manager/pkg/web" + //"github.com/Peripli/service-manager/pkg/web" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -66,15 +67,132 @@ var _ = Describe("Healthcheck controller", func() { }) }) }) + + Describe("aggregation", func() { + var c *controller + var healths map[string]h.State + var tresholds map[string]int64 + + BeforeEach(func() { + healths = map[string]h.State{ + "test1": {Status: "ok"}, + "test2": {Status: "ok"}, + } + tresholds = map[string]int64{ + "test1": 3, + "test2": 3, + } + c = &controller{ + health: HealthFake{}, + tresholds: tresholds, + } + }) + + When("No healths are provided", func() { + It("Returns UNKNOWN and an error detail", func() { + aggregatedHealth := c.aggregate(nil) + Expect(aggregatedHealth.Status).To(Equal(health.StatusUnknown)) + Expect(aggregatedHealth.Details["error"]).ToNot(BeNil()) + }) + }) + + When("At least one health is DOWN more than treshold times and is Fatal", func() { + It("Returns DOWN", func() { + healths["test3"] = h.State{Status: "failed", Fatal: true, ContiguousFailures: 4} + c.tresholds["test3"] = 3 + aggregatedHealth := c.aggregate(healths) + Expect(aggregatedHealth.Status).To(Equal(health.StatusDown)) + }) + }) + + When("At least one health is DOWN more than treshold times and is not Fatal", func() { + It("Returns UP", func() { + healths["test3"] = h.State{Status: "failed", Fatal: false, ContiguousFailures: 4} + c.tresholds["test3"] = 3 + aggregatedHealth := c.aggregate(healths) + Expect(aggregatedHealth.Status).To(Equal(health.StatusUp)) + }) + }) + + When("There is DOWN healths but not more than treshold times in a row", func() { + It("Returns UP", func() { + healths["test3"] = h.State{Status: "failed"} + c.tresholds["test3"] = 3 + aggregatedHealth := c.aggregate(healths) + Expect(aggregatedHealth.Status).To(Equal(health.StatusUp)) + }) + }) + + When("All healths are UP", func() { + It("Returns UP", func() { + aggregatedHealth := c.aggregate(healths) + Expect(aggregatedHealth.Status).To(Equal(health.StatusUp)) + }) + }) + + When("Aggregating healths", func() { + It("Includes them as overall details", func() { + aggregatedHealth := c.aggregate(healths) + for name, h := range healths { + h.Status = convertStatus(h.Status) + Expect(aggregatedHealth.Details[name]).To(Equal(h)) + } + }) + }) + }) + + 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 { - aggPolicy := &healthfakes.FakeAggregationPolicy{} - aggPolicy.ApplyReturns(&health.Health{Status: status}) + if status == health.StatusUnknown { + return &controller{ + health: HealthFake{}, + } + } + + stringStatus := "ok" + var contiguousFailures int64 = 0 + if status == health.StatusDown { + stringStatus = "failed" + contiguousFailures = 1 + } + return &controller{ - health: HealthFake{}, - aggPolicy: aggPolicy, - failuresTreshold: 3, + health: HealthFake{ + state: map[string]h.State{ + "test1": {Status: stringStatus, Fatal: true, ContiguousFailures: contiguousFailures}, + }, + }, + tresholds: map[string]int64{ + "test1": 1, + }, } } diff --git a/pkg/health/aggregation_policy.go b/pkg/health/aggregation_policy.go deleted file mode 100644 index 8c73ca395..000000000 --- a/pkg/health/aggregation_policy.go +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright 2018 The Service Manager Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package health - -import "github.com/InVisionApp/go-health" - -// DefaultAggregationPolicy aggregates the healths by constructing a new Health based on the given -// where the overall health status is negative if one of the healths is negative and positive if all are positive -type DefaultAggregationPolicy struct { -} - -// Apply aggregates the given healths -func (*DefaultAggregationPolicy) Apply(healths map[string]health.State, failureTreshold int64) *Health { - if len(healths) == 0 { - return New().WithDetail("error", "no health indicators registered").Unknown() - } - overallStatus := StatusUp - for _, health := range healths { - if health.Status == "failed" && health.ContiguousFailures > failureTreshold { - overallStatus = StatusDown - break - } - } - details := make(map[string]interface{}) - for k, v := range healths { - details[k] = ConvertStatus(v.Status) - } - return New().WithStatus(overallStatus).WithDetails(details) -} - -// ConvertStatus converts go-health status to Status -func ConvertStatus(status string) Status { - switch status { - case "ok": - return StatusUp - case "failed": - return StatusDown - default: - return StatusUnknown - } -} diff --git a/pkg/health/aggregation_policy_test.go b/pkg/health/aggregation_policy_test.go deleted file mode 100644 index c16a24d10..000000000 --- a/pkg/health/aggregation_policy_test.go +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright 2018 The Service Manager Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package health - -import ( - "github.com/InVisionApp/go-health" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("Healthcheck AggregationPolicy", func() { - - aggregationPolicy := &DefaultAggregationPolicy{} - var healths map[string]health.State - var failuresTreshold int64 = 3 - - BeforeEach(func() { - healths = map[string]health.State{ - "test1": {Status: "ok"}, - "test2": {Status: "ok"}, - } - }) - - When("No healths are provided", func() { - It("Returns UNKNOWN and an error detail", func() { - aggregatedHealth := aggregationPolicy.Apply(nil, failuresTreshold) - Expect(aggregatedHealth.Status).To(Equal(StatusUnknown)) - Expect(aggregatedHealth.Details["error"]).ToNot(BeNil()) - }) - }) - - When("At least one health is DOWN", func() { - It("Returns DOWN", func() { - healths["test3"] = health.State{Status: "failed", ContiguousFailures: 4} - aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) - Expect(aggregatedHealth.Status).To(Equal(StatusDown)) - }) - }) - - When("There is DOWN healths but not more than treshold times in a row", func() { - It("Returns UP", func() { - healths["test3"] = health.State{Status: "failed"} - aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) - Expect(aggregatedHealth.Status).To(Equal(StatusUp)) - }) - }) - - When("All healths are UP", func() { - It("Returns UP", func() { - aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) - Expect(aggregatedHealth.Status).To(Equal(StatusUp)) - }) - }) - - When("Aggregating healths", func() { - It("Includes them as overall details", func() { - aggregatedHealth := aggregationPolicy.Apply(healths, failuresTreshold) - for name, h := range healths { - Expect(aggregatedHealth.Details[name]).To(Equal(ConvertStatus(h.Status))) - } - }) - }) -}) diff --git a/pkg/health/healthfakes/fake_aggregation_policy.go b/pkg/health/healthfakes/fake_aggregation_policy.go deleted file mode 100644 index 69a1220cc..000000000 --- a/pkg/health/healthfakes/fake_aggregation_policy.go +++ /dev/null @@ -1,113 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package healthfakes - -import ( - "sync" - - healtha "github.com/InVisionApp/go-health" - "github.com/Peripli/service-manager/pkg/health" -) - -type FakeAggregationPolicy struct { - ApplyStub func(map[string]healtha.State, int64) *health.Health - applyMutex sync.RWMutex - applyArgsForCall []struct { - arg1 map[string]healtha.State - arg2 int64 - } - applyReturns struct { - result1 *health.Health - } - applyReturnsOnCall map[int]struct { - result1 *health.Health - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeAggregationPolicy) Apply(arg1 map[string]healtha.State, arg2 int64) *health.Health { - fake.applyMutex.Lock() - ret, specificReturn := fake.applyReturnsOnCall[len(fake.applyArgsForCall)] - fake.applyArgsForCall = append(fake.applyArgsForCall, struct { - arg1 map[string]healtha.State - arg2 int64 - }{arg1, arg2}) - fake.recordInvocation("Apply", []interface{}{arg1, arg2}) - fake.applyMutex.Unlock() - if fake.ApplyStub != nil { - return fake.ApplyStub(arg1, arg2) - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.applyReturns - return fakeReturns.result1 -} - -func (fake *FakeAggregationPolicy) ApplyCallCount() int { - fake.applyMutex.RLock() - defer fake.applyMutex.RUnlock() - return len(fake.applyArgsForCall) -} - -func (fake *FakeAggregationPolicy) ApplyCalls(stub func(map[string]healtha.State, int64) *health.Health) { - fake.applyMutex.Lock() - defer fake.applyMutex.Unlock() - fake.ApplyStub = stub -} - -func (fake *FakeAggregationPolicy) ApplyArgsForCall(i int) (map[string]healtha.State, int64) { - fake.applyMutex.RLock() - defer fake.applyMutex.RUnlock() - argsForCall := fake.applyArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 -} - -func (fake *FakeAggregationPolicy) ApplyReturns(result1 *health.Health) { - fake.applyMutex.Lock() - defer fake.applyMutex.Unlock() - fake.ApplyStub = nil - fake.applyReturns = struct { - result1 *health.Health - }{result1} -} - -func (fake *FakeAggregationPolicy) ApplyReturnsOnCall(i int, result1 *health.Health) { - fake.applyMutex.Lock() - defer fake.applyMutex.Unlock() - fake.ApplyStub = nil - if fake.applyReturnsOnCall == nil { - fake.applyReturnsOnCall = make(map[int]struct { - result1 *health.Health - }) - } - fake.applyReturnsOnCall[i] = struct { - result1 *health.Health - }{result1} -} - -func (fake *FakeAggregationPolicy) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.applyMutex.RLock() - defer fake.applyMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeAggregationPolicy) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ health.AggregationPolicy = new(FakeAggregationPolicy) diff --git a/pkg/health/healthfakes/fake_configurable_indicator.go b/pkg/health/healthfakes/fake_configurable_indicator.go new file mode 100644 index 000000000..05876bd5a --- /dev/null +++ b/pkg/health/healthfakes/fake_configurable_indicator.go @@ -0,0 +1,401 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package healthfakes + +import ( + "sync" + "time" + + "github.com/Peripli/service-manager/pkg/health" +) + +type FakeConfigurableIndicator struct { + ConfigureStub func(*health.IndicatorSettings) + configureMutex sync.RWMutex + configureArgsForCall []struct { + arg1 *health.IndicatorSettings + } + FailuresTresholdStub func() int64 + failuresTresholdMutex sync.RWMutex + failuresTresholdArgsForCall []struct { + } + failuresTresholdReturns struct { + result1 int64 + } + failuresTresholdReturnsOnCall map[int]struct { + result1 int64 + } + FatalStub func() bool + fatalMutex sync.RWMutex + fatalArgsForCall []struct { + } + fatalReturns struct { + result1 bool + } + fatalReturnsOnCall map[int]struct { + result1 bool + } + IntervalStub func() time.Duration + intervalMutex sync.RWMutex + intervalArgsForCall []struct { + } + intervalReturns struct { + result1 time.Duration + } + intervalReturnsOnCall map[int]struct { + result1 time.Duration + } + NameStub func() string + nameMutex sync.RWMutex + nameArgsForCall []struct { + } + nameReturns struct { + result1 string + } + nameReturnsOnCall map[int]struct { + result1 string + } + StatusStub func() (interface{}, error) + statusMutex sync.RWMutex + statusArgsForCall []struct { + } + statusReturns struct { + result1 interface{} + result2 error + } + statusReturnsOnCall map[int]struct { + result1 interface{} + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeConfigurableIndicator) Configure(arg1 *health.IndicatorSettings) { + fake.configureMutex.Lock() + fake.configureArgsForCall = append(fake.configureArgsForCall, struct { + arg1 *health.IndicatorSettings + }{arg1}) + fake.recordInvocation("Configure", []interface{}{arg1}) + fake.configureMutex.Unlock() + if fake.ConfigureStub != nil { + fake.ConfigureStub(arg1) + } +} + +func (fake *FakeConfigurableIndicator) ConfigureCallCount() int { + fake.configureMutex.RLock() + defer fake.configureMutex.RUnlock() + return len(fake.configureArgsForCall) +} + +func (fake *FakeConfigurableIndicator) ConfigureCalls(stub func(*health.IndicatorSettings)) { + fake.configureMutex.Lock() + defer fake.configureMutex.Unlock() + fake.ConfigureStub = stub +} + +func (fake *FakeConfigurableIndicator) ConfigureArgsForCall(i int) *health.IndicatorSettings { + fake.configureMutex.RLock() + defer fake.configureMutex.RUnlock() + argsForCall := fake.configureArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeConfigurableIndicator) FailuresTreshold() int64 { + fake.failuresTresholdMutex.Lock() + ret, specificReturn := fake.failuresTresholdReturnsOnCall[len(fake.failuresTresholdArgsForCall)] + fake.failuresTresholdArgsForCall = append(fake.failuresTresholdArgsForCall, struct { + }{}) + fake.recordInvocation("FailuresTreshold", []interface{}{}) + fake.failuresTresholdMutex.Unlock() + if fake.FailuresTresholdStub != nil { + return fake.FailuresTresholdStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.failuresTresholdReturns + return fakeReturns.result1 +} + +func (fake *FakeConfigurableIndicator) FailuresTresholdCallCount() int { + fake.failuresTresholdMutex.RLock() + defer fake.failuresTresholdMutex.RUnlock() + return len(fake.failuresTresholdArgsForCall) +} + +func (fake *FakeConfigurableIndicator) FailuresTresholdCalls(stub func() int64) { + fake.failuresTresholdMutex.Lock() + defer fake.failuresTresholdMutex.Unlock() + fake.FailuresTresholdStub = stub +} + +func (fake *FakeConfigurableIndicator) FailuresTresholdReturns(result1 int64) { + fake.failuresTresholdMutex.Lock() + defer fake.failuresTresholdMutex.Unlock() + fake.FailuresTresholdStub = nil + fake.failuresTresholdReturns = struct { + result1 int64 + }{result1} +} + +func (fake *FakeConfigurableIndicator) FailuresTresholdReturnsOnCall(i int, result1 int64) { + fake.failuresTresholdMutex.Lock() + defer fake.failuresTresholdMutex.Unlock() + fake.FailuresTresholdStub = nil + if fake.failuresTresholdReturnsOnCall == nil { + fake.failuresTresholdReturnsOnCall = make(map[int]struct { + result1 int64 + }) + } + fake.failuresTresholdReturnsOnCall[i] = struct { + result1 int64 + }{result1} +} + +func (fake *FakeConfigurableIndicator) Fatal() bool { + fake.fatalMutex.Lock() + ret, specificReturn := fake.fatalReturnsOnCall[len(fake.fatalArgsForCall)] + fake.fatalArgsForCall = append(fake.fatalArgsForCall, struct { + }{}) + fake.recordInvocation("Fatal", []interface{}{}) + fake.fatalMutex.Unlock() + if fake.FatalStub != nil { + return fake.FatalStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.fatalReturns + return fakeReturns.result1 +} + +func (fake *FakeConfigurableIndicator) FatalCallCount() int { + fake.fatalMutex.RLock() + defer fake.fatalMutex.RUnlock() + return len(fake.fatalArgsForCall) +} + +func (fake *FakeConfigurableIndicator) FatalCalls(stub func() bool) { + fake.fatalMutex.Lock() + defer fake.fatalMutex.Unlock() + fake.FatalStub = stub +} + +func (fake *FakeConfigurableIndicator) FatalReturns(result1 bool) { + fake.fatalMutex.Lock() + defer fake.fatalMutex.Unlock() + fake.FatalStub = nil + fake.fatalReturns = struct { + result1 bool + }{result1} +} + +func (fake *FakeConfigurableIndicator) FatalReturnsOnCall(i int, result1 bool) { + fake.fatalMutex.Lock() + defer fake.fatalMutex.Unlock() + fake.FatalStub = nil + if fake.fatalReturnsOnCall == nil { + fake.fatalReturnsOnCall = make(map[int]struct { + result1 bool + }) + } + fake.fatalReturnsOnCall[i] = struct { + result1 bool + }{result1} +} + +func (fake *FakeConfigurableIndicator) Interval() time.Duration { + fake.intervalMutex.Lock() + ret, specificReturn := fake.intervalReturnsOnCall[len(fake.intervalArgsForCall)] + fake.intervalArgsForCall = append(fake.intervalArgsForCall, struct { + }{}) + fake.recordInvocation("Interval", []interface{}{}) + fake.intervalMutex.Unlock() + if fake.IntervalStub != nil { + return fake.IntervalStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.intervalReturns + return fakeReturns.result1 +} + +func (fake *FakeConfigurableIndicator) IntervalCallCount() int { + fake.intervalMutex.RLock() + defer fake.intervalMutex.RUnlock() + return len(fake.intervalArgsForCall) +} + +func (fake *FakeConfigurableIndicator) IntervalCalls(stub func() time.Duration) { + fake.intervalMutex.Lock() + defer fake.intervalMutex.Unlock() + fake.IntervalStub = stub +} + +func (fake *FakeConfigurableIndicator) IntervalReturns(result1 time.Duration) { + fake.intervalMutex.Lock() + defer fake.intervalMutex.Unlock() + fake.IntervalStub = nil + fake.intervalReturns = struct { + result1 time.Duration + }{result1} +} + +func (fake *FakeConfigurableIndicator) IntervalReturnsOnCall(i int, result1 time.Duration) { + fake.intervalMutex.Lock() + defer fake.intervalMutex.Unlock() + fake.IntervalStub = nil + if fake.intervalReturnsOnCall == nil { + fake.intervalReturnsOnCall = make(map[int]struct { + result1 time.Duration + }) + } + fake.intervalReturnsOnCall[i] = struct { + result1 time.Duration + }{result1} +} + +func (fake *FakeConfigurableIndicator) Name() string { + fake.nameMutex.Lock() + ret, specificReturn := fake.nameReturnsOnCall[len(fake.nameArgsForCall)] + fake.nameArgsForCall = append(fake.nameArgsForCall, struct { + }{}) + fake.recordInvocation("Name", []interface{}{}) + fake.nameMutex.Unlock() + if fake.NameStub != nil { + return fake.NameStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.nameReturns + return fakeReturns.result1 +} + +func (fake *FakeConfigurableIndicator) NameCallCount() int { + fake.nameMutex.RLock() + defer fake.nameMutex.RUnlock() + return len(fake.nameArgsForCall) +} + +func (fake *FakeConfigurableIndicator) NameCalls(stub func() string) { + fake.nameMutex.Lock() + defer fake.nameMutex.Unlock() + fake.NameStub = stub +} + +func (fake *FakeConfigurableIndicator) NameReturns(result1 string) { + fake.nameMutex.Lock() + defer fake.nameMutex.Unlock() + fake.NameStub = nil + fake.nameReturns = struct { + result1 string + }{result1} +} + +func (fake *FakeConfigurableIndicator) NameReturnsOnCall(i int, result1 string) { + fake.nameMutex.Lock() + defer fake.nameMutex.Unlock() + fake.NameStub = nil + if fake.nameReturnsOnCall == nil { + fake.nameReturnsOnCall = make(map[int]struct { + result1 string + }) + } + fake.nameReturnsOnCall[i] = struct { + result1 string + }{result1} +} + +func (fake *FakeConfigurableIndicator) Status() (interface{}, error) { + fake.statusMutex.Lock() + ret, specificReturn := fake.statusReturnsOnCall[len(fake.statusArgsForCall)] + fake.statusArgsForCall = append(fake.statusArgsForCall, struct { + }{}) + fake.recordInvocation("Status", []interface{}{}) + fake.statusMutex.Unlock() + if fake.StatusStub != nil { + return fake.StatusStub() + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.statusReturns + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeConfigurableIndicator) StatusCallCount() int { + fake.statusMutex.RLock() + defer fake.statusMutex.RUnlock() + return len(fake.statusArgsForCall) +} + +func (fake *FakeConfigurableIndicator) StatusCalls(stub func() (interface{}, error)) { + fake.statusMutex.Lock() + defer fake.statusMutex.Unlock() + fake.StatusStub = stub +} + +func (fake *FakeConfigurableIndicator) StatusReturns(result1 interface{}, result2 error) { + fake.statusMutex.Lock() + defer fake.statusMutex.Unlock() + fake.StatusStub = nil + fake.statusReturns = struct { + result1 interface{} + result2 error + }{result1, result2} +} + +func (fake *FakeConfigurableIndicator) StatusReturnsOnCall(i int, result1 interface{}, result2 error) { + fake.statusMutex.Lock() + defer fake.statusMutex.Unlock() + fake.StatusStub = nil + if fake.statusReturnsOnCall == nil { + fake.statusReturnsOnCall = make(map[int]struct { + result1 interface{} + result2 error + }) + } + fake.statusReturnsOnCall[i] = struct { + result1 interface{} + result2 error + }{result1, result2} +} + +func (fake *FakeConfigurableIndicator) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.configureMutex.RLock() + defer fake.configureMutex.RUnlock() + fake.failuresTresholdMutex.RLock() + defer fake.failuresTresholdMutex.RUnlock() + fake.fatalMutex.RLock() + defer fake.fatalMutex.RUnlock() + fake.intervalMutex.RLock() + defer fake.intervalMutex.RUnlock() + fake.nameMutex.RLock() + defer fake.nameMutex.RUnlock() + fake.statusMutex.RLock() + defer fake.statusMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeConfigurableIndicator) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ health.ConfigurableIndicator = new(FakeConfigurableIndicator) diff --git a/pkg/health/healthfakes/fake_indicator.go b/pkg/health/healthfakes/fake_indicator.go new file mode 100644 index 000000000..f3b08c113 --- /dev/null +++ b/pkg/health/healthfakes/fake_indicator.go @@ -0,0 +1,363 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package healthfakes + +import ( + "sync" + "time" + + "github.com/Peripli/service-manager/pkg/health" +) + +type FakeIndicator struct { + FailuresTresholdStub func() int64 + failuresTresholdMutex sync.RWMutex + failuresTresholdArgsForCall []struct { + } + failuresTresholdReturns struct { + result1 int64 + } + failuresTresholdReturnsOnCall map[int]struct { + result1 int64 + } + FatalStub func() bool + fatalMutex sync.RWMutex + fatalArgsForCall []struct { + } + fatalReturns struct { + result1 bool + } + fatalReturnsOnCall map[int]struct { + result1 bool + } + IntervalStub func() time.Duration + intervalMutex sync.RWMutex + intervalArgsForCall []struct { + } + intervalReturns struct { + result1 time.Duration + } + intervalReturnsOnCall map[int]struct { + result1 time.Duration + } + NameStub func() string + nameMutex sync.RWMutex + nameArgsForCall []struct { + } + nameReturns struct { + result1 string + } + nameReturnsOnCall map[int]struct { + result1 string + } + StatusStub func() (interface{}, error) + statusMutex sync.RWMutex + statusArgsForCall []struct { + } + statusReturns struct { + result1 interface{} + result2 error + } + statusReturnsOnCall map[int]struct { + result1 interface{} + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeIndicator) FailuresTreshold() int64 { + fake.failuresTresholdMutex.Lock() + ret, specificReturn := fake.failuresTresholdReturnsOnCall[len(fake.failuresTresholdArgsForCall)] + fake.failuresTresholdArgsForCall = append(fake.failuresTresholdArgsForCall, struct { + }{}) + fake.recordInvocation("FailuresTreshold", []interface{}{}) + fake.failuresTresholdMutex.Unlock() + if fake.FailuresTresholdStub != nil { + return fake.FailuresTresholdStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.failuresTresholdReturns + return fakeReturns.result1 +} + +func (fake *FakeIndicator) FailuresTresholdCallCount() int { + fake.failuresTresholdMutex.RLock() + defer fake.failuresTresholdMutex.RUnlock() + return len(fake.failuresTresholdArgsForCall) +} + +func (fake *FakeIndicator) FailuresTresholdCalls(stub func() int64) { + fake.failuresTresholdMutex.Lock() + defer fake.failuresTresholdMutex.Unlock() + fake.FailuresTresholdStub = stub +} + +func (fake *FakeIndicator) FailuresTresholdReturns(result1 int64) { + fake.failuresTresholdMutex.Lock() + defer fake.failuresTresholdMutex.Unlock() + fake.FailuresTresholdStub = nil + fake.failuresTresholdReturns = struct { + result1 int64 + }{result1} +} + +func (fake *FakeIndicator) FailuresTresholdReturnsOnCall(i int, result1 int64) { + fake.failuresTresholdMutex.Lock() + defer fake.failuresTresholdMutex.Unlock() + fake.FailuresTresholdStub = nil + if fake.failuresTresholdReturnsOnCall == nil { + fake.failuresTresholdReturnsOnCall = make(map[int]struct { + result1 int64 + }) + } + fake.failuresTresholdReturnsOnCall[i] = struct { + result1 int64 + }{result1} +} + +func (fake *FakeIndicator) Fatal() bool { + fake.fatalMutex.Lock() + ret, specificReturn := fake.fatalReturnsOnCall[len(fake.fatalArgsForCall)] + fake.fatalArgsForCall = append(fake.fatalArgsForCall, struct { + }{}) + fake.recordInvocation("Fatal", []interface{}{}) + fake.fatalMutex.Unlock() + if fake.FatalStub != nil { + return fake.FatalStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.fatalReturns + return fakeReturns.result1 +} + +func (fake *FakeIndicator) FatalCallCount() int { + fake.fatalMutex.RLock() + defer fake.fatalMutex.RUnlock() + return len(fake.fatalArgsForCall) +} + +func (fake *FakeIndicator) FatalCalls(stub func() bool) { + fake.fatalMutex.Lock() + defer fake.fatalMutex.Unlock() + fake.FatalStub = stub +} + +func (fake *FakeIndicator) FatalReturns(result1 bool) { + fake.fatalMutex.Lock() + defer fake.fatalMutex.Unlock() + fake.FatalStub = nil + fake.fatalReturns = struct { + result1 bool + }{result1} +} + +func (fake *FakeIndicator) FatalReturnsOnCall(i int, result1 bool) { + fake.fatalMutex.Lock() + defer fake.fatalMutex.Unlock() + fake.FatalStub = nil + if fake.fatalReturnsOnCall == nil { + fake.fatalReturnsOnCall = make(map[int]struct { + result1 bool + }) + } + fake.fatalReturnsOnCall[i] = struct { + result1 bool + }{result1} +} + +func (fake *FakeIndicator) Interval() time.Duration { + fake.intervalMutex.Lock() + ret, specificReturn := fake.intervalReturnsOnCall[len(fake.intervalArgsForCall)] + fake.intervalArgsForCall = append(fake.intervalArgsForCall, struct { + }{}) + fake.recordInvocation("Interval", []interface{}{}) + fake.intervalMutex.Unlock() + if fake.IntervalStub != nil { + return fake.IntervalStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.intervalReturns + return fakeReturns.result1 +} + +func (fake *FakeIndicator) IntervalCallCount() int { + fake.intervalMutex.RLock() + defer fake.intervalMutex.RUnlock() + return len(fake.intervalArgsForCall) +} + +func (fake *FakeIndicator) IntervalCalls(stub func() time.Duration) { + fake.intervalMutex.Lock() + defer fake.intervalMutex.Unlock() + fake.IntervalStub = stub +} + +func (fake *FakeIndicator) IntervalReturns(result1 time.Duration) { + fake.intervalMutex.Lock() + defer fake.intervalMutex.Unlock() + fake.IntervalStub = nil + fake.intervalReturns = struct { + result1 time.Duration + }{result1} +} + +func (fake *FakeIndicator) IntervalReturnsOnCall(i int, result1 time.Duration) { + fake.intervalMutex.Lock() + defer fake.intervalMutex.Unlock() + fake.IntervalStub = nil + if fake.intervalReturnsOnCall == nil { + fake.intervalReturnsOnCall = make(map[int]struct { + result1 time.Duration + }) + } + fake.intervalReturnsOnCall[i] = struct { + result1 time.Duration + }{result1} +} + +func (fake *FakeIndicator) Name() string { + fake.nameMutex.Lock() + ret, specificReturn := fake.nameReturnsOnCall[len(fake.nameArgsForCall)] + fake.nameArgsForCall = append(fake.nameArgsForCall, struct { + }{}) + fake.recordInvocation("Name", []interface{}{}) + fake.nameMutex.Unlock() + if fake.NameStub != nil { + return fake.NameStub() + } + if specificReturn { + return ret.result1 + } + fakeReturns := fake.nameReturns + return fakeReturns.result1 +} + +func (fake *FakeIndicator) NameCallCount() int { + fake.nameMutex.RLock() + defer fake.nameMutex.RUnlock() + return len(fake.nameArgsForCall) +} + +func (fake *FakeIndicator) NameCalls(stub func() string) { + fake.nameMutex.Lock() + defer fake.nameMutex.Unlock() + fake.NameStub = stub +} + +func (fake *FakeIndicator) NameReturns(result1 string) { + fake.nameMutex.Lock() + defer fake.nameMutex.Unlock() + fake.NameStub = nil + fake.nameReturns = struct { + result1 string + }{result1} +} + +func (fake *FakeIndicator) NameReturnsOnCall(i int, result1 string) { + fake.nameMutex.Lock() + defer fake.nameMutex.Unlock() + fake.NameStub = nil + if fake.nameReturnsOnCall == nil { + fake.nameReturnsOnCall = make(map[int]struct { + result1 string + }) + } + fake.nameReturnsOnCall[i] = struct { + result1 string + }{result1} +} + +func (fake *FakeIndicator) Status() (interface{}, error) { + fake.statusMutex.Lock() + ret, specificReturn := fake.statusReturnsOnCall[len(fake.statusArgsForCall)] + fake.statusArgsForCall = append(fake.statusArgsForCall, struct { + }{}) + fake.recordInvocation("Status", []interface{}{}) + fake.statusMutex.Unlock() + if fake.StatusStub != nil { + return fake.StatusStub() + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.statusReturns + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeIndicator) StatusCallCount() int { + fake.statusMutex.RLock() + defer fake.statusMutex.RUnlock() + return len(fake.statusArgsForCall) +} + +func (fake *FakeIndicator) StatusCalls(stub func() (interface{}, error)) { + fake.statusMutex.Lock() + defer fake.statusMutex.Unlock() + fake.StatusStub = stub +} + +func (fake *FakeIndicator) StatusReturns(result1 interface{}, result2 error) { + fake.statusMutex.Lock() + defer fake.statusMutex.Unlock() + fake.StatusStub = nil + fake.statusReturns = struct { + result1 interface{} + result2 error + }{result1, result2} +} + +func (fake *FakeIndicator) StatusReturnsOnCall(i int, result1 interface{}, result2 error) { + fake.statusMutex.Lock() + defer fake.statusMutex.Unlock() + fake.StatusStub = nil + if fake.statusReturnsOnCall == nil { + fake.statusReturnsOnCall = make(map[int]struct { + result1 interface{} + result2 error + }) + } + fake.statusReturnsOnCall[i] = struct { + result1 interface{} + result2 error + }{result1, result2} +} + +func (fake *FakeIndicator) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.failuresTresholdMutex.RLock() + defer fake.failuresTresholdMutex.RUnlock() + fake.fatalMutex.RLock() + defer fake.fatalMutex.RUnlock() + fake.intervalMutex.RLock() + defer fake.intervalMutex.RUnlock() + fake.nameMutex.RLock() + defer fake.nameMutex.RUnlock() + fake.statusMutex.RLock() + defer fake.statusMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeIndicator) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ health.Indicator = new(FakeIndicator) diff --git a/pkg/health/ping.go b/pkg/health/ping.go index 6331a89fb..ce55fbcaf 100644 --- a/pkg/health/ping.go +++ b/pkg/health/ping.go @@ -16,6 +16,8 @@ package health +import "time" + // pingIndicator is a default indicator that always returns up type pingIndicator struct { } @@ -24,6 +26,18 @@ func (*pingIndicator) Name() string { return "ping" } +func (*pingIndicator) Interval() time.Duration { + return 30 +} + +func (*pingIndicator) FailuresTreshold() int64 { + return 1 +} + +func (*pingIndicator) Fatal() bool { + return true +} + func (*pingIndicator) Status() (interface{}, error) { return nil, nil } diff --git a/pkg/health/registry_test.go b/pkg/health/registry_test.go index a0d543446..cd38ad351 100644 --- a/pkg/health/registry_test.go +++ b/pkg/health/registry_test.go @@ -17,9 +17,9 @@ package health import ( - "github.com/InVisionApp/go-health" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "time" ) var _ = Describe("Healthcheck Registry", func() { @@ -31,27 +31,13 @@ var _ = Describe("Healthcheck Registry", func() { }) When("Constructing default registry", func() { - It("Has ping indicator and default aggregation policy", func() { + It("Has ping indicator", func() { indicators := registry.HealthIndicators Expect(indicators).To(ConsistOf(&pingIndicator{})) - - policy := registry.HealthAggregationPolicy - Expect(policy).To(BeAssignableToTypeOf(&DefaultAggregationPolicy{})) - }) - }) - - Context("Register aggregation policy", func() { - It("Overrides the previous", func() { - policy := registry.HealthAggregationPolicy - Expect(policy).To(BeAssignableToTypeOf(&DefaultAggregationPolicy{})) - - registry.HealthAggregationPolicy = &testAggregationPolicy{} - policy = registry.HealthAggregationPolicy - Expect(policy).To(BeAssignableToTypeOf(&testAggregationPolicy{})) }) }) - Context("Register health indicator", func() { + When("Register health indicator", func() { It("Adds a new indicator", func() { preAddIndicators := registry.HealthIndicators @@ -64,22 +50,63 @@ 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)) + }) + }) + + When("When configure indicators", func() { + 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 testAggregationPolicy struct { +type testIndicator struct { + settings *IndicatorSettings } -func (*testAggregationPolicy) Apply(healths map[string]health.State, failuresTreshold int64) *Health { - return New().Up() +func (i *testIndicator) Name() string { + return "test" } -type testIndicator struct { +func (i *testIndicator) Interval() time.Duration { + return i.settings.Interval } -func (*testIndicator) Name() string { - return "test" +func (i *testIndicator) FailuresTreshold() int64 { + return i.settings.FailuresTreshold } -func (*testIndicator) Status() (interface{}, error) { +func (i *testIndicator) Fatal() bool { + return i.settings.Fatal +} + +func (i *testIndicator) Status() (interface{}, error) { return nil, nil } + +func (i *testIndicator) Configure(settings *IndicatorSettings) { + i.settings = settings +} diff --git a/pkg/health/types.go b/pkg/health/types.go index 240379427..440f54b5f 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -18,30 +18,54 @@ package health import ( "fmt" - "github.com/InVisionApp/go-health" "time" ) // Settings type to be loaded from the environment type Settings struct { - FailuresTreshold int64 `mapstructure:"failures_treshold" description:"maximum failures in a row until component is considered down"` - Interval time.Duration `mapstructure:"interval" description:"seconds between health checks of components"` + IndicatorsSettings map[string]*IndicatorSettings `mapstructure:"indicators,omitempty"` } // DefaultSettings returns default values for health settings func DefaultSettings() *Settings { + emptySettings := make(map[string]*IndicatorSettings) return &Settings{ + IndicatorsSettings: emptySettings, + } +} + +// Validate validates health settings +func (s *Settings) Validate() error { + for _, v := range s.IndicatorsSettings { + if err := v.Validate(); err != nil { + return err + } + } + return nil +} + +// IndicatorSettings type to be loaded from the environment +type IndicatorSettings struct { + Fatal bool `mapstructure:"fatal" description:"if the indicator affects the overall status "` + FailuresTreshold int64 `mapstructure:"failures_treshold" description:"maximum failures in a row until component is considered down"` + Interval time.Duration `mapstructure:"interval" description:"time between health checks of components"` +} + +// DefaultIndicatorSettings returns default values for indicator settings +func DefaultIndicatorSettings() *IndicatorSettings { + return &IndicatorSettings{ + Fatal: true, FailuresTreshold: 3, Interval: 60, } } -// Validate validates the health settings -func (s *Settings) Validate() error { - if s.FailuresTreshold < 0 { - return fmt.Errorf("validate Settings: FailuresTreshold must be >= 0") +// Validate validates indicator settings +func (is *IndicatorSettings) Validate() error { + if is.FailuresTreshold <= 0 { + return fmt.Errorf("validate Settings: FailuresTreshold must be > 0") } - if s.Interval < 30 { + if is.Interval < 30 { return fmt.Errorf("validate Settings: Minimum interval is 30 seconds") } return nil @@ -118,26 +142,39 @@ func (h *Health) WithDetails(details map[string]interface{}) *Health { } // Indicator is an interface to provide the health of a component +//go:generate counterfeiter . Indicator type Indicator interface { // Name returns the name of the component Name() string + // Interval returns settings of the indicator + Interval() time.Duration + + // FailuresTreshold returns the maximum failures in a row until component is considered down + FailuresTreshold() int64 + + // Fatal returns if the health indicator is fatal for the overall status + Fatal() bool + // Status returns the health information of the component Status() (interface{}, error) } -// AggregationPolicy is an interface to provide aggregated health information -//go:generate counterfeiter . AggregationPolicy -type AggregationPolicy interface { - // Apply processes the given healths to build a single health - Apply(healths map[string]health.State, failureTreshold int64) *Health +// ConfigurableIndicator is an interface to provide configurable health of a component +//go:generate counterfeiter . ConfigurableIndicator +type ConfigurableIndicator interface { + Indicator + + // Configure configures the indicator with given settings + Configure(*IndicatorSettings) } -// NewDefaultRegistry returns a default health registry with a single ping indicator and a default aggregation policy +// NewDefaultRegistry returns a default health registry with a single ping indicator func NewDefaultRegistry() *Registry { + emptySettings := make(map[string]*IndicatorSettings) return &Registry{ - HealthIndicators: []Indicator{&pingIndicator{}}, - HealthAggregationPolicy: &DefaultAggregationPolicy{}, + HealthIndicators: []Indicator{&pingIndicator{}}, + HealthSettings: emptySettings, } } @@ -146,6 +183,19 @@ type Registry struct { // HealthIndicators are the currently registered health indicators HealthIndicators []Indicator - // HealthAggregationPolicy is the registered health aggregationPolicy - HealthAggregationPolicy AggregationPolicy + // 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 index db07d7af8..d2394f753 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -141,6 +141,7 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( } API.HealthIndicators = append(API.HealthIndicators, storageHealthIndicator) + API.HealthSettings = cfg.Health.IndicatorsSettings notificationCleaner := &storage.NotificationCleaner{ Storage: interceptableRepository, @@ -187,8 +188,7 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( // Build builds the Service Manager func (smb *ServiceManagerBuilder) Build() *ServiceManager { - err := smb.installHealth() - if err != nil { + if err := smb.installHealth(); err != nil { panic(err) } @@ -212,33 +212,33 @@ func (smb *ServiceManagerBuilder) installHealth() error { return nil } + smb.ConfigureIndicators() + healthz := h.New() logger := log.C(smb.ctx).Logger healthz.Logger = l.New(logger) for _, indicator := range smb.HealthIndicators { - err := healthz.AddCheck(&h.Config{ + if err := healthz.AddCheck(&h.Config{ Name: indicator.Name(), Checker: indicator, - Interval: smb.health.Interval * time.Second, - }) - - if err != nil { + Interval: indicator.Interval() * time.Second, + Fatal: indicator.Fatal(), + }); err != nil { return err } } - smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthAggregationPolicy, smb.health.FailuresTreshold)) + smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthIndicators)) if err := healthz.Start(); err != nil { return err } - // Handles safe termination of sm util.StartInWaitGroupWithContext(smb.ctx, func(c context.Context) { <-c.Done() log.C(c).Debug("Context cancelled. Stopping health checks...") if err := healthz.Stop(); err != nil { - log.D().Error(err) + log.C(c).Error(err) } }, smb.wg) diff --git a/storage/healthcheck.go b/storage/healthcheck.go index 3d6988e45..f2847ae42 100644 --- a/storage/healthcheck.go +++ b/storage/healthcheck.go @@ -19,11 +19,14 @@ package storage import ( "github.com/InVisionApp/go-health/checkers" "github.com/Peripli/service-manager/pkg/health" + "time" ) // HealthIndicator returns a new indicator for the storage type HealthIndicator struct { - checkers.SQL + *checkers.SQL + + settings *health.IndicatorSettings } // Name returns the name of the storage component @@ -31,6 +34,22 @@ func (i *HealthIndicator) Name() string { return "storage" } +func (i *HealthIndicator) Configure(settings *health.IndicatorSettings) { + i.settings = settings +} + +func (i *HealthIndicator) Interval() time.Duration { + return i.settings.Interval +} + +func (i *HealthIndicator) FailuresTreshold() int64 { + return i.settings.FailuresTreshold +} + +func (i *HealthIndicator) Fatal() bool { + return i.settings.Fatal +} + func NewStorageHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { sqlConfig := &checkers.SQLConfig{ Pinger: pingFunc, @@ -41,7 +60,7 @@ func NewStorageHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { } indicator := &HealthIndicator{ - SQL: *sqlChecker, + SQL: sqlChecker, } return indicator, nil diff --git a/storage/healthcheck_test.go b/storage/healthcheck_test.go index 4b7d15a56..0a178d0c7 100644 --- a/storage/healthcheck_test.go +++ b/storage/healthcheck_test.go @@ -32,7 +32,9 @@ var _ = Describe("Healthcheck", func() { ping := func(ctx context.Context) error { return nil } - healthIndicator, _ = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) + var err error + healthIndicator, err = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) + Expect(err).ShouldNot(HaveOccurred()) }) Context("Name", func() { @@ -53,7 +55,9 @@ var _ = Describe("Healthcheck", func() { ping := func(ctx context.Context) error { return expectedError } - healthIndicator, _ = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) + var err error + healthIndicator, err = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) + Expect(err).ShouldNot(HaveOccurred()) }) It("Contains error", func() { _, err := healthIndicator.Status() From 30ab52e0f53878c66a0472a5fb1df2bd82b10d03 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 25 Jun 2019 11:43:37 +0300 Subject: [PATCH 10/19] Add health status listener --- pkg/health/settings_test.go | 87 +++++++++++++++++++++++++++++++++++++ pkg/health/types.go | 12 +++++ pkg/sm/sm.go | 4 ++ 3 files changed, 103 insertions(+) create mode 100644 pkg/health/settings_test.go diff --git a/pkg/health/settings_test.go b/pkg/health/settings_test.go new file mode 100644 index 000000000..f40b3f930 --- /dev/null +++ b/pkg/health/settings_test.go @@ -0,0 +1,87 @@ +/* + * Copyright 2018 The Service Manager Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package health + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "time" +) + +var _ = Describe("Healthcheck Settings", func() { + + var settings *Settings + var fatal bool + var failuresTreshold int64 + var interval time.Duration + + BeforeEach(func() { + settings = DefaultSettings() + + fatal = false + failuresTreshold = 1 + interval = 30 + }) + + var registerIndicatorSettings = func() { + indicatorSettings := &IndicatorSettings{ + Fatal: fatal, + FailuresTreshold: failuresTreshold, + Interval: interval, + } + + settings.IndicatorsSettings["test"] = indicatorSettings + } + + var assertValidationErrorOccured = func(err error) { + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("validate Settings")) + } + + When("Indicator with negative treshold", func() { + It("Should be invalid", func() { + failuresTreshold = -1 + registerIndicatorSettings() + + err := settings.Validate() + + assertValidationErrorOccured(err) + }) + }) + + When("Indicator with 0 treshold", func() { + It("Should be invalid", func() { + failuresTreshold = 0 + registerIndicatorSettings() + + err := settings.Validate() + + assertValidationErrorOccured(err) + }) + }) + + When("Indicator with interval less than 30", func() { + It("Should be invalid", func() { + interval = 15 + registerIndicatorSettings() + + err := settings.Validate() + + assertValidationErrorOccured(err) + }) + }) +}) diff --git a/pkg/health/types.go b/pkg/health/types.go index 440f54b5f..4f8c451de 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -18,6 +18,8 @@ package health import ( "fmt" + "github.com/InVisionApp/go-health" + "github.com/Peripli/service-manager/pkg/log" "time" ) @@ -83,6 +85,16 @@ const ( StatusUnknown Status = "UNKNOWN" ) +type StatusListener struct{} + +func (sl *StatusListener) HealthCheckFailed(state *health.State) { + log.D().Errorf("Health check for %v failed with: %v", state.Name, state.Err) +} + +func (sl *StatusListener) HealthCheckRecovered(state *health.State, numberOfFailures int64, unavailableDuration float64) { + log.D().Infof("Health check for %v recovered after %v failures and was unavailable for %v seconds roughly", state.Name, numberOfFailures, unavailableDuration) +} + // Health contains information about the health of a component. type Health struct { Status Status `json:"status"` diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index d2394f753..e318081ea 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -23,6 +23,7 @@ import ( "fmt" h "github.com/InVisionApp/go-health" l "github.com/InVisionApp/go-logger/shims/logrus" + "github.com/Peripli/service-manager/pkg/health" "net" "net/http" "sync" @@ -216,7 +217,9 @@ func (smb *ServiceManagerBuilder) installHealth() error { healthz := h.New() logger := log.C(smb.ctx).Logger + healthz.Logger = l.New(logger) + healthz.StatusListener = &health.StatusListener{} for _, indicator := range smb.HealthIndicators { if err := healthz.AddCheck(&h.Config{ @@ -228,6 +231,7 @@ func (smb *ServiceManagerBuilder) installHealth() error { return err } } + smb.RegisterControllers(healthcheck.NewController(healthz, smb.HealthIndicators)) if err := healthz.Start(); err != nil { From d25a87dc9917926891b0c3a84e9d738a6fe78f19 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 25 Jun 2019 12:41:08 +0300 Subject: [PATCH 11/19] Minor tweaks --- api/healthcheck/healthcheck_controller.go | 2 +- pkg/health/registry_test.go | 2 -- pkg/health/settings_test.go | 12 ++++++++++++ pkg/health/types.go | 18 ------------------ 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index 0d10c134f..95f4975a7 100644 --- a/api/healthcheck/healthcheck_controller.go +++ b/api/healthcheck/healthcheck_controller.go @@ -62,7 +62,7 @@ func (c *controller) healthCheck(r *web.Request) (*web.Response, error) { func (c *controller) aggregate(state map[string]h.State) *health.Health { if len(state) == 0 { - return health.New().WithDetail("error", "no health indicators registered").Unknown() + return health.New().WithDetail("error", "no health indicators registered") } overallStatus := health.StatusUp for i, v := range state { diff --git a/pkg/health/registry_test.go b/pkg/health/registry_test.go index cd38ad351..f8c98da6a 100644 --- a/pkg/health/registry_test.go +++ b/pkg/health/registry_test.go @@ -61,9 +61,7 @@ var _ = Describe("Healthcheck Registry", func() { Expect(newIndicator.Interval()).To(Equal(DefaultIndicatorSettings().Interval)) Expect(newIndicator.FailuresTreshold()).To(Equal(DefaultIndicatorSettings().FailuresTreshold)) }) - }) - When("When configure indicators", func() { It("Should configure with provided settings", func() { newIndicator := &testIndicator{} registry.HealthIndicators = append(registry.HealthIndicators, newIndicator) diff --git a/pkg/health/settings_test.go b/pkg/health/settings_test.go index f40b3f930..4436f3d90 100644 --- a/pkg/health/settings_test.go +++ b/pkg/health/settings_test.go @@ -84,4 +84,16 @@ var _ = Describe("Healthcheck Settings", func() { assertValidationErrorOccured(err) }) }) + + When("Indicator with positive treshold and interval > 30", func() { + It("Should be considered valid", func() { + interval = 30 + failuresTreshold = 3 + registerIndicatorSettings() + + err := settings.Validate() + + Expect(err).ShouldNot(HaveOccurred()) + }) + }) }) diff --git a/pkg/health/types.go b/pkg/health/types.go index 4f8c451de..54c105fe4 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -127,24 +127,6 @@ func (h *Health) WithDetail(key string, val interface{}) *Health { return h } -// Up sets the health status to up -func (h *Health) Up() *Health { - h.Status = StatusUp - return h -} - -// Down sets the health status to down -func (h *Health) Down() *Health { - h.Status = StatusDown - return h -} - -// Unknown sets the health status to unknown -func (h *Health) Unknown() *Health { - h.Status = StatusUnknown - return h -} - // WithDetails adds the given details to the health func (h *Health) WithDetails(details map[string]interface{}) *Health { for k, v := range details { From 794c3b08a0c53bb2612f2599d7589ed475f6c3bc Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 8 Jul 2019 09:44:54 +0300 Subject: [PATCH 12/19] Fix indicator interval type --- pkg/health/types.go | 2 +- pkg/sm/sm.go | 13 ++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/pkg/health/types.go b/pkg/health/types.go index 54c105fe4..0d127cc6b 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -67,7 +67,7 @@ func (is *IndicatorSettings) Validate() error { if is.FailuresTreshold <= 0 { return fmt.Errorf("validate Settings: FailuresTreshold must be > 0") } - if is.Interval < 30 { + if is.Interval < 30*time.Second { return fmt.Errorf("validate Settings: Minimum interval is 30 seconds") } return nil diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index e318081ea..d433860c1 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -23,13 +23,11 @@ import ( "fmt" h "github.com/InVisionApp/go-health" l "github.com/InVisionApp/go-logger/shims/logrus" + "github.com/Peripli/service-manager/api/osb" "github.com/Peripli/service-manager/pkg/health" "net" "net/http" "sync" - "time" - - "github.com/Peripli/service-manager/api/osb" "github.com/Peripli/service-manager/storage/catalog" @@ -159,11 +157,6 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( cfg: cfg, } - err = smb.installHealth(cfg.Health) - if err != nil { - return nil, fmt.Errorf("error adding health chech to sm: %s", err) - } - // Register default interceptors that represent the core SM business logic smb. WithCreateInterceptorProvider(types.ServiceBrokerType, &interceptors.BrokerCreateCatalogInterceptorProvider{ @@ -194,8 +187,6 @@ func (smb *ServiceManagerBuilder) Build() *ServiceManager { } // setup server and add relevant global middleware - smb.installHealth() - srv := server.New(smb.cfg.Server, smb.API) srv.Use(filters.NewRecoveryMiddleware()) @@ -225,7 +216,7 @@ func (smb *ServiceManagerBuilder) installHealth() error { if err := healthz.AddCheck(&h.Config{ Name: indicator.Name(), Checker: indicator, - Interval: indicator.Interval() * time.Second, + Interval: indicator.Interval(), Fatal: indicator.Fatal(), }); err != nil { return err From 40fb02b9a6d757e1c7659bd7f5daf41faac327f6 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Mon, 8 Jul 2019 10:15:20 +0300 Subject: [PATCH 13/19] Fix tests --- config/config_test.go | 55 +++++++++++++++++++++ pkg/health/ping.go | 2 +- pkg/health/settings_test.go | 99 ------------------------------------- 3 files changed, 56 insertions(+), 100 deletions(-) delete mode 100644 pkg/health/settings_test.go diff --git a/config/config_test.go b/config/config_test.go index ac4a24a7e..1ebe67b18 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -18,6 +18,7 @@ package config_test import ( "fmt" + "github.com/Peripli/service-manager/pkg/health" "testing" "time" @@ -44,11 +45,25 @@ var _ = Describe("config", func() { ) Describe("Validate", func() { + var fatal bool + var failuresTreshold int64 + var interval time.Duration + assertErrorDuringValidate := func() { err = config.Validate() Expect(err).To(HaveOccurred()) } + registerIndicatorSettings := func() { + indicatorSettings := &health.IndicatorSettings{ + Fatal: fatal, + FailuresTreshold: failuresTreshold, + Interval: interval, + } + + config.Health.IndicatorsSettings["test"] = indicatorSettings + } + BeforeEach(func() { config = cfg.DefaultSettings() config.Storage.URI = "postgres://postgres:postgres@localhost:5555/postgres?sslmode=disable" @@ -56,6 +71,46 @@ var _ = Describe("config", func() { config.API.ClientID = "sm" config.API.SkipSSLValidation = true config.Storage.EncryptionKey = "ejHjRNHbS0NaqARSRvnweVV9zcmhQEa8" + + fatal = false + failuresTreshold = 1 + interval = 30 * time.Second + }) + + Context("health indicator with negative treshold", func() { + It("should be considered invalid", func() { + failuresTreshold = -1 + registerIndicatorSettings() + assertErrorDuringValidate() + }) + }) + + Context("health indicator with 0 treshold", func() { + It("should be considered invalid", func() { + failuresTreshold = 0 + registerIndicatorSettings() + assertErrorDuringValidate() + }) + }) + + Context("health indicator with interval less than 30", func() { + It("should be considered invalid", func() { + interval = 15 * time.Second + registerIndicatorSettings() + assertErrorDuringValidate() + }) + }) + + Context("health indicator with positive treshold and interval >= 30", func() { + It("should be considered valid", func() { + interval = 30 * time.Second + failuresTreshold = 3 + registerIndicatorSettings() + + err := config.Validate() + + Expect(err).ShouldNot(HaveOccurred()) + }) }) Context("when config is valid", func() { diff --git a/pkg/health/ping.go b/pkg/health/ping.go index ce55fbcaf..f1e169b22 100644 --- a/pkg/health/ping.go +++ b/pkg/health/ping.go @@ -27,7 +27,7 @@ func (*pingIndicator) Name() string { } func (*pingIndicator) Interval() time.Duration { - return 30 + return 30 * time.Second } func (*pingIndicator) FailuresTreshold() int64 { diff --git a/pkg/health/settings_test.go b/pkg/health/settings_test.go deleted file mode 100644 index 4436f3d90..000000000 --- a/pkg/health/settings_test.go +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright 2018 The Service Manager Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package health - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "time" -) - -var _ = Describe("Healthcheck Settings", func() { - - var settings *Settings - var fatal bool - var failuresTreshold int64 - var interval time.Duration - - BeforeEach(func() { - settings = DefaultSettings() - - fatal = false - failuresTreshold = 1 - interval = 30 - }) - - var registerIndicatorSettings = func() { - indicatorSettings := &IndicatorSettings{ - Fatal: fatal, - FailuresTreshold: failuresTreshold, - Interval: interval, - } - - settings.IndicatorsSettings["test"] = indicatorSettings - } - - var assertValidationErrorOccured = func(err error) { - Expect(err).Should(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("validate Settings")) - } - - When("Indicator with negative treshold", func() { - It("Should be invalid", func() { - failuresTreshold = -1 - registerIndicatorSettings() - - err := settings.Validate() - - assertValidationErrorOccured(err) - }) - }) - - When("Indicator with 0 treshold", func() { - It("Should be invalid", func() { - failuresTreshold = 0 - registerIndicatorSettings() - - err := settings.Validate() - - assertValidationErrorOccured(err) - }) - }) - - When("Indicator with interval less than 30", func() { - It("Should be invalid", func() { - interval = 15 - registerIndicatorSettings() - - err := settings.Validate() - - assertValidationErrorOccured(err) - }) - }) - - When("Indicator with positive treshold and interval > 30", func() { - It("Should be considered valid", func() { - interval = 30 - failuresTreshold = 3 - registerIndicatorSettings() - - err := settings.Validate() - - Expect(err).ShouldNot(HaveOccurred()) - }) - }) -}) From 37c03ddb81a31880e51bb94f0a7ecfd514613168 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 20 Aug 2019 11:22:49 +0300 Subject: [PATCH 14/19] Remove unused import --- api/healthcheck/healthcheck_controller_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/healthcheck/healthcheck_controller_test.go b/api/healthcheck/healthcheck_controller_test.go index a91325e25..e12546c0f 100644 --- a/api/healthcheck/healthcheck_controller_test.go +++ b/api/healthcheck/healthcheck_controller_test.go @@ -24,8 +24,6 @@ import ( "net/http" "testing" - //"github.com/Peripli/service-manager/pkg/web" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) From f1388078c8b5b747a2a8020f69bc0b9e5eca67be Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 20 Aug 2019 14:39:32 +0300 Subject: [PATCH 15/19] Extract indicator configuration and address PR comments --- api/healthcheck/healthcheck_controller.go | 6 +--- .../healthcheck_controller_test.go | 29 ------------------- pkg/health/registry_test.go | 29 ------------------- pkg/health/types.go | 13 --------- pkg/sm/sm.go | 16 +++++++--- 5 files changed, 13 insertions(+), 80 deletions(-) diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index 95f4975a7..b560aec9d 100644 --- 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 index e12546c0f..5de22ea69 100644 --- 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 index f8c98da6a..7b1f8044b 100644 --- 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 index 0d127cc6b..985b36f6d 100644 --- 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 index d433860c1..56182b91e 100644 --- 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 From ce4d4bbe2c076cd2b261788fb27a6a917681af63 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Tue, 20 Aug 2019 14:55:36 +0300 Subject: [PATCH 16/19] Add error to panic --- pkg/sm/sm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 56182b91e..a3d0cabe5 100644 --- 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 { - log.C(smb.ctx).Panic() + log.C(smb.ctx).Panic(err) } // setup server and add relevant global middleware From 3a3438123593a014ff57eb01b2ab04354327762d Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Wed, 21 Aug 2019 12:12:37 +0300 Subject: [PATCH 17/19] Address PR comments --- api/healthcheck/healthcheck_controller.go | 28 +- .../healthcheck_controller_test.go | 38 +- config/config_test.go | 47 +- .../fake_configurable_indicator.go | 401 ------------------ pkg/health/healthfakes/fake_indicator.go | 193 --------- pkg/health/ping.go | 43 -- pkg/health/registry_test.go | 22 +- pkg/health/types.go | 50 +-- pkg/sm/sm.go | 26 +- storage/healthcheck.go | 40 +- storage/healthcheck_test.go | 4 +- 11 files changed, 97 insertions(+), 795 deletions(-) delete mode 100644 pkg/health/healthfakes/fake_configurable_indicator.go delete mode 100644 pkg/health/ping.go diff --git a/api/healthcheck/healthcheck_controller.go b/api/healthcheck/healthcheck_controller.go index b560aec9d..9e55f611e 100644 --- a/api/healthcheck/healthcheck_controller.go +++ b/api/healthcheck/healthcheck_controller.go @@ -28,15 +28,15 @@ import ( // controller platform controller type controller struct { - health h.IHealth - tresholds map[string]int64 + health h.IHealth + thresholds map[string]int64 } -// NewController returns a new healthcheck controller with the given health and tresholds -func NewController(health h.IHealth, tresholds map[string]int64) web.Controller { +// NewController returns a new healthcheck controller with the given health and thresholds +func NewController(health h.IHealth, thresholds map[string]int64) web.Controller { return &controller{ - health: health, - tresholds: tresholds, + health: health, + thresholds: thresholds, } } @@ -56,21 +56,21 @@ func (c *controller) healthCheck(r *web.Request) (*web.Response, error) { return util.NewJSONResponse(status, healthResult) } -func (c *controller) aggregate(state map[string]h.State) *health.Health { - if len(state) == 0 { - return health.New().WithDetail("error", "no health indicators registered") +func (c *controller) aggregate(overallState map[string]h.State) *health.Health { + if len(overallState) == 0 { + return health.New().WithStatus(health.StatusUp) } overallStatus := health.StatusUp - for i, v := range state { - if v.Fatal && v.ContiguousFailures >= c.tresholds[i] { + for name, state := range overallState { + if state.Fatal && state.ContiguousFailures >= c.thresholds[name] { overallStatus = health.StatusDown break } } details := make(map[string]interface{}) - for k, v := range state { - v.Status = convertStatus(v.Status) - details[k] = v + for name, state := range overallState { + state.Status = convertStatus(state.Status) + details[name] = state } return health.New().WithStatus(overallStatus).WithDetails(details) } diff --git a/api/healthcheck/healthcheck_controller_test.go b/api/healthcheck/healthcheck_controller_test.go index 5de22ea69..fc353d341 100644 --- a/api/healthcheck/healthcheck_controller_test.go +++ b/api/healthcheck/healthcheck_controller_test.go @@ -52,12 +52,6 @@ var _ = Describe("Healthcheck controller", func() { }) }) - When("health returns unknown", func() { - It("should respond with 503", func() { - assertResponse(health.StatusUnknown, http.StatusServiceUnavailable) - }) - }) - When("health returns up", func() { It("should respond with 200", func() { assertResponse(health.StatusUp, http.StatusOK) @@ -68,53 +62,51 @@ var _ = Describe("Healthcheck controller", func() { Describe("aggregation", func() { var c *controller var healths map[string]h.State - var tresholds map[string]int64 + var thresholds map[string]int64 BeforeEach(func() { healths = map[string]h.State{ "test1": {Status: "ok"}, "test2": {Status: "ok"}, } - tresholds = map[string]int64{ + thresholds = map[string]int64{ "test1": 3, "test2": 3, } c = &controller{ - health: HealthFake{}, - tresholds: tresholds, + health: HealthFake{}, + thresholds: thresholds, } }) When("No healths are provided", func() { - It("Returns UNKNOWN and an error detail", func() { + It("Returns UP", func() { aggregatedHealth := c.aggregate(nil) - Expect(aggregatedHealth.Status).To(Equal(health.StatusUnknown)) - Expect(aggregatedHealth.Details["error"]).ToNot(BeNil()) + Expect(aggregatedHealth.Status).To(Equal(health.StatusUp)) }) }) - When("At least one health is DOWN more than treshold times and is Fatal", func() { + When("At least one health is DOWN more than threshold times and is Fatal", func() { It("Returns DOWN", func() { healths["test3"] = h.State{Status: "failed", Fatal: true, ContiguousFailures: 4} - c.tresholds["test3"] = 3 + c.thresholds["test3"] = 3 aggregatedHealth := c.aggregate(healths) Expect(aggregatedHealth.Status).To(Equal(health.StatusDown)) }) }) - When("At least one health is DOWN more than treshold times and is not Fatal", func() { + When("At least one health is DOWN and is not Fatal", func() { It("Returns UP", func() { healths["test3"] = h.State{Status: "failed", Fatal: false, ContiguousFailures: 4} - c.tresholds["test3"] = 3 aggregatedHealth := c.aggregate(healths) Expect(aggregatedHealth.Status).To(Equal(health.StatusUp)) }) }) - When("There is DOWN healths but not more than treshold times in a row", func() { + When("There is DOWN healths but not more than threshold times in a row", func() { It("Returns UP", func() { healths["test3"] = h.State{Status: "failed"} - c.tresholds["test3"] = 3 + c.thresholds["test3"] = 3 aggregatedHealth := c.aggregate(healths) Expect(aggregatedHealth.Status).To(Equal(health.StatusUp)) }) @@ -140,12 +132,6 @@ var _ = Describe("Healthcheck controller", func() { }) func createController(status health.Status) *controller { - if status == health.StatusUnknown { - return &controller{ - health: HealthFake{}, - } - } - stringStatus := "ok" var contiguousFailures int64 = 0 if status == health.StatusDown { @@ -159,7 +145,7 @@ func createController(status health.Status) *controller { "test1": {Status: stringStatus, Fatal: true, ContiguousFailures: contiguousFailures}, }, }, - tresholds: map[string]int64{ + thresholds: map[string]int64{ "test1": 1, }, } diff --git a/config/config_test.go b/config/config_test.go index 1ebe67b18..a4f32fd3f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -46,7 +46,7 @@ var _ = Describe("config", func() { Describe("Validate", func() { var fatal bool - var failuresTreshold int64 + var failuresThreshold int64 var interval time.Duration assertErrorDuringValidate := func() { @@ -56,12 +56,12 @@ var _ = Describe("config", func() { registerIndicatorSettings := func() { indicatorSettings := &health.IndicatorSettings{ - Fatal: fatal, - FailuresTreshold: failuresTreshold, - Interval: interval, + Fatal: fatal, + FailuresThreshold: failuresThreshold, + Interval: interval, } - config.Health.IndicatorsSettings["test"] = indicatorSettings + config.Health.Indicators["test"] = indicatorSettings } BeforeEach(func() { @@ -72,22 +72,41 @@ var _ = Describe("config", func() { config.API.SkipSSLValidation = true config.Storage.EncryptionKey = "ejHjRNHbS0NaqARSRvnweVV9zcmhQEa8" - fatal = false - failuresTreshold = 1 + fatal = true + failuresThreshold = 1 interval = 30 * time.Second }) - Context("health indicator with negative treshold", func() { + Context("health indicator with negative threshold", func() { It("should be considered invalid", func() { - failuresTreshold = -1 + failuresThreshold = -1 registerIndicatorSettings() assertErrorDuringValidate() }) }) - Context("health indicator with 0 treshold", func() { - It("should be considered invalid", func() { - failuresTreshold = 0 + Context("health indicator with 0 threshold", func() { + It("should be considered invalid if it is fatal", func() { + failuresThreshold = 0 + registerIndicatorSettings() + assertErrorDuringValidate() + }) + }) + + Context("health indicator with 0 threshold", func() { + It("should be considered valid if it is not fatal", func() { + fatal = false + failuresThreshold = 0 + registerIndicatorSettings() + err := config.Validate() + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + Context("health indicator with positive threshold", func() { + It("should be considered invalid if it is not fatal", func() { + fatal = false + failuresThreshold = 3 registerIndicatorSettings() assertErrorDuringValidate() }) @@ -101,10 +120,10 @@ var _ = Describe("config", func() { }) }) - Context("health indicator with positive treshold and interval >= 30", func() { + Context("health indicator with positive threshold and interval >= 30", func() { It("should be considered valid", func() { interval = 30 * time.Second - failuresTreshold = 3 + failuresThreshold = 3 registerIndicatorSettings() err := config.Validate() diff --git a/pkg/health/healthfakes/fake_configurable_indicator.go b/pkg/health/healthfakes/fake_configurable_indicator.go deleted file mode 100644 index 05876bd5a..000000000 --- a/pkg/health/healthfakes/fake_configurable_indicator.go +++ /dev/null @@ -1,401 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package healthfakes - -import ( - "sync" - "time" - - "github.com/Peripli/service-manager/pkg/health" -) - -type FakeConfigurableIndicator struct { - ConfigureStub func(*health.IndicatorSettings) - configureMutex sync.RWMutex - configureArgsForCall []struct { - arg1 *health.IndicatorSettings - } - FailuresTresholdStub func() int64 - failuresTresholdMutex sync.RWMutex - failuresTresholdArgsForCall []struct { - } - failuresTresholdReturns struct { - result1 int64 - } - failuresTresholdReturnsOnCall map[int]struct { - result1 int64 - } - FatalStub func() bool - fatalMutex sync.RWMutex - fatalArgsForCall []struct { - } - fatalReturns struct { - result1 bool - } - fatalReturnsOnCall map[int]struct { - result1 bool - } - IntervalStub func() time.Duration - intervalMutex sync.RWMutex - intervalArgsForCall []struct { - } - intervalReturns struct { - result1 time.Duration - } - intervalReturnsOnCall map[int]struct { - result1 time.Duration - } - NameStub func() string - nameMutex sync.RWMutex - nameArgsForCall []struct { - } - nameReturns struct { - result1 string - } - nameReturnsOnCall map[int]struct { - result1 string - } - StatusStub func() (interface{}, error) - statusMutex sync.RWMutex - statusArgsForCall []struct { - } - statusReturns struct { - result1 interface{} - result2 error - } - statusReturnsOnCall map[int]struct { - result1 interface{} - result2 error - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeConfigurableIndicator) Configure(arg1 *health.IndicatorSettings) { - fake.configureMutex.Lock() - fake.configureArgsForCall = append(fake.configureArgsForCall, struct { - arg1 *health.IndicatorSettings - }{arg1}) - fake.recordInvocation("Configure", []interface{}{arg1}) - fake.configureMutex.Unlock() - if fake.ConfigureStub != nil { - fake.ConfigureStub(arg1) - } -} - -func (fake *FakeConfigurableIndicator) ConfigureCallCount() int { - fake.configureMutex.RLock() - defer fake.configureMutex.RUnlock() - return len(fake.configureArgsForCall) -} - -func (fake *FakeConfigurableIndicator) ConfigureCalls(stub func(*health.IndicatorSettings)) { - fake.configureMutex.Lock() - defer fake.configureMutex.Unlock() - fake.ConfigureStub = stub -} - -func (fake *FakeConfigurableIndicator) ConfigureArgsForCall(i int) *health.IndicatorSettings { - fake.configureMutex.RLock() - defer fake.configureMutex.RUnlock() - argsForCall := fake.configureArgsForCall[i] - return argsForCall.arg1 -} - -func (fake *FakeConfigurableIndicator) FailuresTreshold() int64 { - fake.failuresTresholdMutex.Lock() - ret, specificReturn := fake.failuresTresholdReturnsOnCall[len(fake.failuresTresholdArgsForCall)] - fake.failuresTresholdArgsForCall = append(fake.failuresTresholdArgsForCall, struct { - }{}) - fake.recordInvocation("FailuresTreshold", []interface{}{}) - fake.failuresTresholdMutex.Unlock() - if fake.FailuresTresholdStub != nil { - return fake.FailuresTresholdStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.failuresTresholdReturns - return fakeReturns.result1 -} - -func (fake *FakeConfigurableIndicator) FailuresTresholdCallCount() int { - fake.failuresTresholdMutex.RLock() - defer fake.failuresTresholdMutex.RUnlock() - return len(fake.failuresTresholdArgsForCall) -} - -func (fake *FakeConfigurableIndicator) FailuresTresholdCalls(stub func() int64) { - fake.failuresTresholdMutex.Lock() - defer fake.failuresTresholdMutex.Unlock() - fake.FailuresTresholdStub = stub -} - -func (fake *FakeConfigurableIndicator) FailuresTresholdReturns(result1 int64) { - fake.failuresTresholdMutex.Lock() - defer fake.failuresTresholdMutex.Unlock() - fake.FailuresTresholdStub = nil - fake.failuresTresholdReturns = struct { - result1 int64 - }{result1} -} - -func (fake *FakeConfigurableIndicator) FailuresTresholdReturnsOnCall(i int, result1 int64) { - fake.failuresTresholdMutex.Lock() - defer fake.failuresTresholdMutex.Unlock() - fake.FailuresTresholdStub = nil - if fake.failuresTresholdReturnsOnCall == nil { - fake.failuresTresholdReturnsOnCall = make(map[int]struct { - result1 int64 - }) - } - fake.failuresTresholdReturnsOnCall[i] = struct { - result1 int64 - }{result1} -} - -func (fake *FakeConfigurableIndicator) Fatal() bool { - fake.fatalMutex.Lock() - ret, specificReturn := fake.fatalReturnsOnCall[len(fake.fatalArgsForCall)] - fake.fatalArgsForCall = append(fake.fatalArgsForCall, struct { - }{}) - fake.recordInvocation("Fatal", []interface{}{}) - fake.fatalMutex.Unlock() - if fake.FatalStub != nil { - return fake.FatalStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.fatalReturns - return fakeReturns.result1 -} - -func (fake *FakeConfigurableIndicator) FatalCallCount() int { - fake.fatalMutex.RLock() - defer fake.fatalMutex.RUnlock() - return len(fake.fatalArgsForCall) -} - -func (fake *FakeConfigurableIndicator) FatalCalls(stub func() bool) { - fake.fatalMutex.Lock() - defer fake.fatalMutex.Unlock() - fake.FatalStub = stub -} - -func (fake *FakeConfigurableIndicator) FatalReturns(result1 bool) { - fake.fatalMutex.Lock() - defer fake.fatalMutex.Unlock() - fake.FatalStub = nil - fake.fatalReturns = struct { - result1 bool - }{result1} -} - -func (fake *FakeConfigurableIndicator) FatalReturnsOnCall(i int, result1 bool) { - fake.fatalMutex.Lock() - defer fake.fatalMutex.Unlock() - fake.FatalStub = nil - if fake.fatalReturnsOnCall == nil { - fake.fatalReturnsOnCall = make(map[int]struct { - result1 bool - }) - } - fake.fatalReturnsOnCall[i] = struct { - result1 bool - }{result1} -} - -func (fake *FakeConfigurableIndicator) Interval() time.Duration { - fake.intervalMutex.Lock() - ret, specificReturn := fake.intervalReturnsOnCall[len(fake.intervalArgsForCall)] - fake.intervalArgsForCall = append(fake.intervalArgsForCall, struct { - }{}) - fake.recordInvocation("Interval", []interface{}{}) - fake.intervalMutex.Unlock() - if fake.IntervalStub != nil { - return fake.IntervalStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.intervalReturns - return fakeReturns.result1 -} - -func (fake *FakeConfigurableIndicator) IntervalCallCount() int { - fake.intervalMutex.RLock() - defer fake.intervalMutex.RUnlock() - return len(fake.intervalArgsForCall) -} - -func (fake *FakeConfigurableIndicator) IntervalCalls(stub func() time.Duration) { - fake.intervalMutex.Lock() - defer fake.intervalMutex.Unlock() - fake.IntervalStub = stub -} - -func (fake *FakeConfigurableIndicator) IntervalReturns(result1 time.Duration) { - fake.intervalMutex.Lock() - defer fake.intervalMutex.Unlock() - fake.IntervalStub = nil - fake.intervalReturns = struct { - result1 time.Duration - }{result1} -} - -func (fake *FakeConfigurableIndicator) IntervalReturnsOnCall(i int, result1 time.Duration) { - fake.intervalMutex.Lock() - defer fake.intervalMutex.Unlock() - fake.IntervalStub = nil - if fake.intervalReturnsOnCall == nil { - fake.intervalReturnsOnCall = make(map[int]struct { - result1 time.Duration - }) - } - fake.intervalReturnsOnCall[i] = struct { - result1 time.Duration - }{result1} -} - -func (fake *FakeConfigurableIndicator) Name() string { - fake.nameMutex.Lock() - ret, specificReturn := fake.nameReturnsOnCall[len(fake.nameArgsForCall)] - fake.nameArgsForCall = append(fake.nameArgsForCall, struct { - }{}) - fake.recordInvocation("Name", []interface{}{}) - fake.nameMutex.Unlock() - if fake.NameStub != nil { - return fake.NameStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.nameReturns - return fakeReturns.result1 -} - -func (fake *FakeConfigurableIndicator) NameCallCount() int { - fake.nameMutex.RLock() - defer fake.nameMutex.RUnlock() - return len(fake.nameArgsForCall) -} - -func (fake *FakeConfigurableIndicator) NameCalls(stub func() string) { - fake.nameMutex.Lock() - defer fake.nameMutex.Unlock() - fake.NameStub = stub -} - -func (fake *FakeConfigurableIndicator) NameReturns(result1 string) { - fake.nameMutex.Lock() - defer fake.nameMutex.Unlock() - fake.NameStub = nil - fake.nameReturns = struct { - result1 string - }{result1} -} - -func (fake *FakeConfigurableIndicator) NameReturnsOnCall(i int, result1 string) { - fake.nameMutex.Lock() - defer fake.nameMutex.Unlock() - fake.NameStub = nil - if fake.nameReturnsOnCall == nil { - fake.nameReturnsOnCall = make(map[int]struct { - result1 string - }) - } - fake.nameReturnsOnCall[i] = struct { - result1 string - }{result1} -} - -func (fake *FakeConfigurableIndicator) Status() (interface{}, error) { - fake.statusMutex.Lock() - ret, specificReturn := fake.statusReturnsOnCall[len(fake.statusArgsForCall)] - fake.statusArgsForCall = append(fake.statusArgsForCall, struct { - }{}) - fake.recordInvocation("Status", []interface{}{}) - fake.statusMutex.Unlock() - if fake.StatusStub != nil { - return fake.StatusStub() - } - if specificReturn { - return ret.result1, ret.result2 - } - fakeReturns := fake.statusReturns - return fakeReturns.result1, fakeReturns.result2 -} - -func (fake *FakeConfigurableIndicator) StatusCallCount() int { - fake.statusMutex.RLock() - defer fake.statusMutex.RUnlock() - return len(fake.statusArgsForCall) -} - -func (fake *FakeConfigurableIndicator) StatusCalls(stub func() (interface{}, error)) { - fake.statusMutex.Lock() - defer fake.statusMutex.Unlock() - fake.StatusStub = stub -} - -func (fake *FakeConfigurableIndicator) StatusReturns(result1 interface{}, result2 error) { - fake.statusMutex.Lock() - defer fake.statusMutex.Unlock() - fake.StatusStub = nil - fake.statusReturns = struct { - result1 interface{} - result2 error - }{result1, result2} -} - -func (fake *FakeConfigurableIndicator) StatusReturnsOnCall(i int, result1 interface{}, result2 error) { - fake.statusMutex.Lock() - defer fake.statusMutex.Unlock() - fake.StatusStub = nil - if fake.statusReturnsOnCall == nil { - fake.statusReturnsOnCall = make(map[int]struct { - result1 interface{} - result2 error - }) - } - fake.statusReturnsOnCall[i] = struct { - result1 interface{} - result2 error - }{result1, result2} -} - -func (fake *FakeConfigurableIndicator) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.configureMutex.RLock() - defer fake.configureMutex.RUnlock() - fake.failuresTresholdMutex.RLock() - defer fake.failuresTresholdMutex.RUnlock() - fake.fatalMutex.RLock() - defer fake.fatalMutex.RUnlock() - fake.intervalMutex.RLock() - defer fake.intervalMutex.RUnlock() - fake.nameMutex.RLock() - defer fake.nameMutex.RUnlock() - fake.statusMutex.RLock() - defer fake.statusMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeConfigurableIndicator) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ health.ConfigurableIndicator = new(FakeConfigurableIndicator) diff --git a/pkg/health/healthfakes/fake_indicator.go b/pkg/health/healthfakes/fake_indicator.go index f3b08c113..5636134e2 100644 --- a/pkg/health/healthfakes/fake_indicator.go +++ b/pkg/health/healthfakes/fake_indicator.go @@ -3,42 +3,11 @@ package healthfakes import ( "sync" - "time" "github.com/Peripli/service-manager/pkg/health" ) type FakeIndicator struct { - FailuresTresholdStub func() int64 - failuresTresholdMutex sync.RWMutex - failuresTresholdArgsForCall []struct { - } - failuresTresholdReturns struct { - result1 int64 - } - failuresTresholdReturnsOnCall map[int]struct { - result1 int64 - } - FatalStub func() bool - fatalMutex sync.RWMutex - fatalArgsForCall []struct { - } - fatalReturns struct { - result1 bool - } - fatalReturnsOnCall map[int]struct { - result1 bool - } - IntervalStub func() time.Duration - intervalMutex sync.RWMutex - intervalArgsForCall []struct { - } - intervalReturns struct { - result1 time.Duration - } - intervalReturnsOnCall map[int]struct { - result1 time.Duration - } NameStub func() string nameMutex sync.RWMutex nameArgsForCall []struct { @@ -65,162 +34,6 @@ type FakeIndicator struct { invocationsMutex sync.RWMutex } -func (fake *FakeIndicator) FailuresTreshold() int64 { - fake.failuresTresholdMutex.Lock() - ret, specificReturn := fake.failuresTresholdReturnsOnCall[len(fake.failuresTresholdArgsForCall)] - fake.failuresTresholdArgsForCall = append(fake.failuresTresholdArgsForCall, struct { - }{}) - fake.recordInvocation("FailuresTreshold", []interface{}{}) - fake.failuresTresholdMutex.Unlock() - if fake.FailuresTresholdStub != nil { - return fake.FailuresTresholdStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.failuresTresholdReturns - return fakeReturns.result1 -} - -func (fake *FakeIndicator) FailuresTresholdCallCount() int { - fake.failuresTresholdMutex.RLock() - defer fake.failuresTresholdMutex.RUnlock() - return len(fake.failuresTresholdArgsForCall) -} - -func (fake *FakeIndicator) FailuresTresholdCalls(stub func() int64) { - fake.failuresTresholdMutex.Lock() - defer fake.failuresTresholdMutex.Unlock() - fake.FailuresTresholdStub = stub -} - -func (fake *FakeIndicator) FailuresTresholdReturns(result1 int64) { - fake.failuresTresholdMutex.Lock() - defer fake.failuresTresholdMutex.Unlock() - fake.FailuresTresholdStub = nil - fake.failuresTresholdReturns = struct { - result1 int64 - }{result1} -} - -func (fake *FakeIndicator) FailuresTresholdReturnsOnCall(i int, result1 int64) { - fake.failuresTresholdMutex.Lock() - defer fake.failuresTresholdMutex.Unlock() - fake.FailuresTresholdStub = nil - if fake.failuresTresholdReturnsOnCall == nil { - fake.failuresTresholdReturnsOnCall = make(map[int]struct { - result1 int64 - }) - } - fake.failuresTresholdReturnsOnCall[i] = struct { - result1 int64 - }{result1} -} - -func (fake *FakeIndicator) Fatal() bool { - fake.fatalMutex.Lock() - ret, specificReturn := fake.fatalReturnsOnCall[len(fake.fatalArgsForCall)] - fake.fatalArgsForCall = append(fake.fatalArgsForCall, struct { - }{}) - fake.recordInvocation("Fatal", []interface{}{}) - fake.fatalMutex.Unlock() - if fake.FatalStub != nil { - return fake.FatalStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.fatalReturns - return fakeReturns.result1 -} - -func (fake *FakeIndicator) FatalCallCount() int { - fake.fatalMutex.RLock() - defer fake.fatalMutex.RUnlock() - return len(fake.fatalArgsForCall) -} - -func (fake *FakeIndicator) FatalCalls(stub func() bool) { - fake.fatalMutex.Lock() - defer fake.fatalMutex.Unlock() - fake.FatalStub = stub -} - -func (fake *FakeIndicator) FatalReturns(result1 bool) { - fake.fatalMutex.Lock() - defer fake.fatalMutex.Unlock() - fake.FatalStub = nil - fake.fatalReturns = struct { - result1 bool - }{result1} -} - -func (fake *FakeIndicator) FatalReturnsOnCall(i int, result1 bool) { - fake.fatalMutex.Lock() - defer fake.fatalMutex.Unlock() - fake.FatalStub = nil - if fake.fatalReturnsOnCall == nil { - fake.fatalReturnsOnCall = make(map[int]struct { - result1 bool - }) - } - fake.fatalReturnsOnCall[i] = struct { - result1 bool - }{result1} -} - -func (fake *FakeIndicator) Interval() time.Duration { - fake.intervalMutex.Lock() - ret, specificReturn := fake.intervalReturnsOnCall[len(fake.intervalArgsForCall)] - fake.intervalArgsForCall = append(fake.intervalArgsForCall, struct { - }{}) - fake.recordInvocation("Interval", []interface{}{}) - fake.intervalMutex.Unlock() - if fake.IntervalStub != nil { - return fake.IntervalStub() - } - if specificReturn { - return ret.result1 - } - fakeReturns := fake.intervalReturns - return fakeReturns.result1 -} - -func (fake *FakeIndicator) IntervalCallCount() int { - fake.intervalMutex.RLock() - defer fake.intervalMutex.RUnlock() - return len(fake.intervalArgsForCall) -} - -func (fake *FakeIndicator) IntervalCalls(stub func() time.Duration) { - fake.intervalMutex.Lock() - defer fake.intervalMutex.Unlock() - fake.IntervalStub = stub -} - -func (fake *FakeIndicator) IntervalReturns(result1 time.Duration) { - fake.intervalMutex.Lock() - defer fake.intervalMutex.Unlock() - fake.IntervalStub = nil - fake.intervalReturns = struct { - result1 time.Duration - }{result1} -} - -func (fake *FakeIndicator) IntervalReturnsOnCall(i int, result1 time.Duration) { - fake.intervalMutex.Lock() - defer fake.intervalMutex.Unlock() - fake.IntervalStub = nil - if fake.intervalReturnsOnCall == nil { - fake.intervalReturnsOnCall = make(map[int]struct { - result1 time.Duration - }) - } - fake.intervalReturnsOnCall[i] = struct { - result1 time.Duration - }{result1} -} - func (fake *FakeIndicator) Name() string { fake.nameMutex.Lock() ret, specificReturn := fake.nameReturnsOnCall[len(fake.nameArgsForCall)] @@ -331,12 +144,6 @@ func (fake *FakeIndicator) StatusReturnsOnCall(i int, result1 interface{}, resul func (fake *FakeIndicator) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.failuresTresholdMutex.RLock() - defer fake.failuresTresholdMutex.RUnlock() - fake.fatalMutex.RLock() - defer fake.fatalMutex.RUnlock() - fake.intervalMutex.RLock() - defer fake.intervalMutex.RUnlock() fake.nameMutex.RLock() defer fake.nameMutex.RUnlock() fake.statusMutex.RLock() diff --git a/pkg/health/ping.go b/pkg/health/ping.go deleted file mode 100644 index f1e169b22..000000000 --- a/pkg/health/ping.go +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2018 The Service Manager Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package health - -import "time" - -// pingIndicator is a default indicator that always returns up -type pingIndicator struct { -} - -func (*pingIndicator) Name() string { - return "ping" -} - -func (*pingIndicator) Interval() time.Duration { - return 30 * time.Second -} - -func (*pingIndicator) FailuresTreshold() int64 { - return 1 -} - -func (*pingIndicator) Fatal() bool { - return true -} - -func (*pingIndicator) Status() (interface{}, error) { - return nil, nil -} diff --git a/pkg/health/registry_test.go b/pkg/health/registry_test.go index 7b1f8044b..1bdbbc203 100644 --- a/pkg/health/registry_test.go +++ b/pkg/health/registry_test.go @@ -19,7 +19,6 @@ package health import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "time" ) var _ = Describe("Healthcheck Registry", func() { @@ -31,9 +30,9 @@ var _ = Describe("Healthcheck Registry", func() { }) When("Constructing default registry", func() { - It("Has ping indicator", func() { + It("Has empty indicators", func() { indicators := registry.HealthIndicators - Expect(indicators).To(ConsistOf(&pingIndicator{})) + Expect(len(indicators)).To(Equal(0)) }) }) @@ -53,29 +52,12 @@ var _ = Describe("Healthcheck Registry", func() { }) type testIndicator struct { - settings *IndicatorSettings } func (i *testIndicator) Name() string { return "test" } -func (i *testIndicator) Interval() time.Duration { - return i.settings.Interval -} - -func (i *testIndicator) FailuresTreshold() int64 { - return i.settings.FailuresTreshold -} - -func (i *testIndicator) Fatal() bool { - return i.settings.Fatal -} - func (i *testIndicator) Status() (interface{}, error) { return nil, nil } - -func (i *testIndicator) Configure(settings *IndicatorSettings) { - i.settings = settings -} diff --git a/pkg/health/types.go b/pkg/health/types.go index 985b36f6d..27fd497f1 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -25,20 +25,20 @@ import ( // Settings type to be loaded from the environment type Settings struct { - IndicatorsSettings map[string]*IndicatorSettings `mapstructure:"indicators,omitempty"` + Indicators map[string]*IndicatorSettings `mapstructure:"indicators,omitempty"` } // DefaultSettings returns default values for health settings func DefaultSettings() *Settings { emptySettings := make(map[string]*IndicatorSettings) return &Settings{ - IndicatorsSettings: emptySettings, + Indicators: emptySettings, } } // Validate validates health settings func (s *Settings) Validate() error { - for _, v := range s.IndicatorsSettings { + for _, v := range s.Indicators { if err := v.Validate(); err != nil { return err } @@ -48,24 +48,27 @@ func (s *Settings) Validate() error { // IndicatorSettings type to be loaded from the environment type IndicatorSettings struct { - Fatal bool `mapstructure:"fatal" description:"if the indicator affects the overall status "` - FailuresTreshold int64 `mapstructure:"failures_treshold" description:"maximum failures in a row until component is considered down"` - Interval time.Duration `mapstructure:"interval" description:"time between health checks of components"` + Fatal bool `mapstructure:"fatal" description:"if the indicator affects the overall status, if false not failures_threshold expected"` + FailuresThreshold int64 `mapstructure:"failures_threshold" description:"number of failures in a row that will affect overall status"` + Interval time.Duration `mapstructure:"interval" description:"time between health checks of components"` } // DefaultIndicatorSettings returns default values for indicator settings func DefaultIndicatorSettings() *IndicatorSettings { return &IndicatorSettings{ - Fatal: true, - FailuresTreshold: 3, - Interval: 60, + Fatal: true, + FailuresThreshold: 3, + Interval: 60 * time.Second, } } // Validate validates indicator settings func (is *IndicatorSettings) Validate() error { - if is.FailuresTreshold <= 0 { - return fmt.Errorf("validate Settings: FailuresTreshold must be > 0") + if !is.Fatal && is.FailuresThreshold != 0 { + return fmt.Errorf("validate Settings: FailuresThreshold not applicable for non-fatal indicators") + } + if is.Fatal && is.FailuresThreshold <= 0 { + return fmt.Errorf("validate Settings: FailuresThreshold must be > 0 for fatal indicators") } if is.Interval < 30*time.Second { return fmt.Errorf("validate Settings: Minimum interval is 30 seconds") @@ -141,34 +144,14 @@ type Indicator interface { // Name returns the name of the component Name() string - // Interval returns settings of the indicator - Interval() time.Duration - - // FailuresTreshold returns the maximum failures in a row until component is considered down - FailuresTreshold() int64 - - // Fatal returns if the health indicator is fatal for the overall status - Fatal() bool - // Status returns the health information of the component Status() (interface{}, error) } -// ConfigurableIndicator is an interface to provide configurable health of a component -//go:generate counterfeiter . ConfigurableIndicator -type ConfigurableIndicator interface { - Indicator - - // Configure configures the indicator with given settings - Configure(*IndicatorSettings) -} - // NewDefaultRegistry returns a default health registry with a single ping indicator func NewDefaultRegistry() *Registry { - emptySettings := make(map[string]*IndicatorSettings) return &Registry{ - HealthIndicators: []Indicator{&pingIndicator{}}, - HealthSettings: emptySettings, + HealthIndicators: []Indicator{}, } } @@ -176,7 +159,4 @@ func NewDefaultRegistry() *Registry { type Registry struct { // HealthIndicators are the currently registered health indicators HealthIndicators []Indicator - - // Indicator Settings of the registry - HealthSettings map[string]*IndicatorSettings } diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index a3d0cabe5..5b92b68a6 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -134,13 +134,12 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( return nil, fmt.Errorf("error creating core api: %s", err) } - storageHealthIndicator, err := storage.NewStorageHealthIndicator(storage.PingFunc(smStorage.PingContext)) + storageHealthIndicator, err := storage.NewHealthIndicator(storage.PingFunc(smStorage.PingContext)) if err != nil { return nil, fmt.Errorf("error creating storage health indicator: %s", err) } API.HealthIndicators = append(API.HealthIndicators, storageHealthIndicator) - API.HealthSettings = cfg.Health.IndicatorsSettings notificationCleaner := &storage.NotificationCleaner{ Storage: interceptableRepository, @@ -200,38 +199,31 @@ func (smb *ServiceManagerBuilder) Build() *ServiceManager { } func (smb *ServiceManagerBuilder) installHealth() error { - if len(smb.HealthIndicators) == 0 { - return nil - } - healthz := h.New() logger := log.C(smb.ctx).Logger healthz.Logger = l.New(logger) healthz.StatusListener = &health.StatusListener{} - tresholds := make(map[string]int64) + thresholds := 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()) - } + settings, ok := smb.cfg.Health.Indicators[indicator.Name()] + if !ok { + settings = health.DefaultIndicatorSettings() } if err := healthz.AddCheck(&h.Config{ Name: indicator.Name(), Checker: indicator, - Interval: indicator.Interval(), - Fatal: indicator.Fatal(), + Interval: settings.Interval, + Fatal: settings.Fatal, }); err != nil { return err } - tresholds[indicator.Name()] = indicator.FailuresTreshold() + thresholds[indicator.Name()] = settings.FailuresThreshold } - smb.RegisterControllers(healthcheck.NewController(healthz, tresholds)) + smb.RegisterControllers(healthcheck.NewController(healthz, thresholds)) if err := healthz.Start(); err != nil { return err diff --git a/storage/healthcheck.go b/storage/healthcheck.go index f2847ae42..d44b73a87 100644 --- a/storage/healthcheck.go +++ b/storage/healthcheck.go @@ -19,38 +19,9 @@ package storage import ( "github.com/InVisionApp/go-health/checkers" "github.com/Peripli/service-manager/pkg/health" - "time" ) -// HealthIndicator returns a new indicator for the storage -type HealthIndicator struct { - *checkers.SQL - - settings *health.IndicatorSettings -} - -// Name returns the name of the storage component -func (i *HealthIndicator) Name() string { - return "storage" -} - -func (i *HealthIndicator) Configure(settings *health.IndicatorSettings) { - i.settings = settings -} - -func (i *HealthIndicator) Interval() time.Duration { - return i.settings.Interval -} - -func (i *HealthIndicator) FailuresTreshold() int64 { - return i.settings.FailuresTreshold -} - -func (i *HealthIndicator) Fatal() bool { - return i.settings.Fatal -} - -func NewStorageHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { +func NewHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { sqlConfig := &checkers.SQLConfig{ Pinger: pingFunc, } @@ -64,5 +35,14 @@ func NewStorageHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { } return indicator, nil +} + +// HealthIndicator returns a new indicator for the storage +type HealthIndicator struct { + *checkers.SQL +} +// Name returns the name of the storage component +func (i *HealthIndicator) Name() string { + return "storage" } diff --git a/storage/healthcheck_test.go b/storage/healthcheck_test.go index 0a178d0c7..1ea4d0ac2 100644 --- a/storage/healthcheck_test.go +++ b/storage/healthcheck_test.go @@ -33,7 +33,7 @@ var _ = Describe("Healthcheck", func() { return nil } var err error - healthIndicator, err = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) + healthIndicator, err = storage.NewHealthIndicator(storage.PingFunc(ping)) Expect(err).ShouldNot(HaveOccurred()) }) @@ -56,7 +56,7 @@ var _ = Describe("Healthcheck", func() { return expectedError } var err error - healthIndicator, err = storage.NewStorageHealthIndicator(storage.PingFunc(ping)) + healthIndicator, err = storage.NewHealthIndicator(storage.PingFunc(ping)) Expect(err).ShouldNot(HaveOccurred()) }) It("Contains error", func() { From 91eb2a9efe22dabe0a2571d521935af6d7f6e5f7 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Wed, 21 Aug 2019 12:25:58 +0300 Subject: [PATCH 18/19] minor fix --- pkg/health/types.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/health/types.go b/pkg/health/types.go index 27fd497f1..7c1dfba13 100644 --- a/pkg/health/types.go +++ b/pkg/health/types.go @@ -148,14 +148,14 @@ type Indicator interface { Status() (interface{}, error) } -// NewDefaultRegistry returns a default health registry with a single ping indicator +// NewDefaultRegistry returns a default empty health registry func NewDefaultRegistry() *Registry { return &Registry{ - HealthIndicators: []Indicator{}, + HealthIndicators: make([]Indicator, 0), } } -// Registry is an interface to store and fetch health indicators +// Registry is a struct to store health indicators type Registry struct { // HealthIndicators are the currently registered health indicators HealthIndicators []Indicator From 06295e68d4b828e15622389374d6fdaa1f8283f1 Mon Sep 17 00:00:00 2001 From: Dimitar Petrov Date: Thu, 22 Aug 2019 15:25:37 +0300 Subject: [PATCH 19/19] rename storage indicator --- pkg/sm/sm.go | 2 +- storage/healthcheck.go | 11 ++++++----- storage/healthcheck_test.go | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/sm/sm.go b/pkg/sm/sm.go index 5b92b68a6..c9b946af3 100644 --- a/pkg/sm/sm.go +++ b/pkg/sm/sm.go @@ -134,7 +134,7 @@ func New(ctx context.Context, cancel context.CancelFunc, cfg *config.Settings) ( return nil, fmt.Errorf("error creating core api: %s", err) } - storageHealthIndicator, err := storage.NewHealthIndicator(storage.PingFunc(smStorage.PingContext)) + storageHealthIndicator, err := storage.NewSQLHealthIndicator(storage.PingFunc(smStorage.PingContext)) if err != nil { return nil, fmt.Errorf("error creating storage health indicator: %s", err) } diff --git a/storage/healthcheck.go b/storage/healthcheck.go index d44b73a87..d74d07396 100644 --- a/storage/healthcheck.go +++ b/storage/healthcheck.go @@ -21,7 +21,8 @@ import ( "github.com/Peripli/service-manager/pkg/health" ) -func NewHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { +// NewSQLHealthIndicator returns new health indicator for sql storage given a ping function +func NewSQLHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { sqlConfig := &checkers.SQLConfig{ Pinger: pingFunc, } @@ -30,19 +31,19 @@ func NewHealthIndicator(pingFunc PingFunc) (health.Indicator, error) { return nil, err } - indicator := &HealthIndicator{ + indicator := &SQLHealthIndicator{ SQL: sqlChecker, } return indicator, nil } -// HealthIndicator returns a new indicator for the storage -type HealthIndicator struct { +// SQLHealthIndicator returns a new indicator for SQL storage +type SQLHealthIndicator struct { *checkers.SQL } // Name returns the name of the storage component -func (i *HealthIndicator) Name() string { +func (i *SQLHealthIndicator) Name() string { return "storage" } diff --git a/storage/healthcheck_test.go b/storage/healthcheck_test.go index 1ea4d0ac2..fcc4da2ba 100644 --- a/storage/healthcheck_test.go +++ b/storage/healthcheck_test.go @@ -33,7 +33,7 @@ var _ = Describe("Healthcheck", func() { return nil } var err error - healthIndicator, err = storage.NewHealthIndicator(storage.PingFunc(ping)) + healthIndicator, err = storage.NewSQLHealthIndicator(storage.PingFunc(ping)) Expect(err).ShouldNot(HaveOccurred()) }) @@ -56,7 +56,7 @@ var _ = Describe("Healthcheck", func() { return expectedError } var err error - healthIndicator, err = storage.NewHealthIndicator(storage.PingFunc(ping)) + healthIndicator, err = storage.NewSQLHealthIndicator(storage.PingFunc(ping)) Expect(err).ShouldNot(HaveOccurred()) }) It("Contains error", func() {