Adding tests

Signed-off-by: alanprot <[email protected]>

Author: alanprot

docs/blocks-storage/querier.md

@@ -215,7 +215,7 @@ querier:
       # The approximate amount of time between health checks of an individual
       # target.
       # CLI flag: -querier.store-gateway-client.interval
-      [interval: <duration> | default = 1s]
+      [interval: <duration> | default = 5s]
 
       # The amount of time during which no response from a target means a failed
       # health check.

docs/configuration/config-file-reference.md

@@ -3110,7 +3110,7 @@ grpc_client_config:
     # The approximate amount of time between health checks of an individual
     # target.
     # CLI flag: -ingester.client.interval
-    [interval: <duration> | default = 1s]
+    [interval: <duration> | default = 5s]
 
     # The amount of time during which no response from a target means a failed
     # health check.
@@ -3844,7 +3844,7 @@ store_gateway_client:
     # The approximate amount of time between health checks of an individual
     # target.
     # CLI flag: -querier.store-gateway-client.interval
-    [interval: <duration> | default = 1s]
+    [interval: <duration> | default = 5s]
 
     # The amount of time during which no response from a target means a failed
     # health check.

pkg/util/grpcclient/health_check.go

@@ -32,7 +32,7 @@ type HealthCheckConfig struct {
 
 // RegisterFlagsWithPrefix for Config.
 func (cfg *HealthCheckConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
-	f.IntVar(&cfg.UnhealthyThreshold, prefix+".unhealthy-threshold", 3, "The number of consecutive failed health checks required before considering a target unhealthy. 0 means disabled.")
+	f.IntVar(&cfg.UnhealthyThreshold, prefix+".unhealthy-threshold", 0, "The number of consecutive failed health checks required before considering a target unhealthy. 0 means disabled.")
 	f.DurationVar(&cfg.Timeout, prefix+".timeout", 1*time.Second, "The amount of time during which no response from a target means a failed health check.")
 	f.DurationVar(&cfg.Interval, prefix+".interval", 5*time.Second, "The approximate amount of time between health checks of an individual target.")
 }
@@ -103,7 +103,7 @@ func (h *HealthCheckInterceptors) registeredInstances() []*healthCheckEntry {
 }
 
 func (h *HealthCheckInterceptors) iteration(ctx context.Context) error {
-	level.Warn(h.logger).Log("msg", "Performing health check", "registeredInstances", len(h.registeredInstances()))
+	level.Debug(h.logger).Log("msg", "Performing health check", "registeredInstances", len(h.registeredInstances()))
 	for _, instance := range h.registeredInstances() {
 		dialOpts, err := instance.clientConfig.Config.DialOption(nil, nil)
 		if err != nil {
@@ -125,6 +125,7 @@ func (h *HealthCheckInterceptors) iteration(ctx context.Context) error {
 		if time.Since(instance.lastCheckTime.Load()) < instance.clientConfig.HealthCheckConfig.Interval {
 			continue
 		}
+
 		instance.lastCheckTime.Store(time.Now())
 
 		go func(i *healthCheckEntry) {

pkg/util/grpcclient/health_check_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+
+	"go.uber.org/atomic"
 	"testing"
 	"time"
 
@@ -13,25 +15,69 @@ import (
 	"google.golang.org/grpc/health/grpc_health_v1"
 
 	utillog "github.com/cortexproject/cortex/pkg/util/log"
+	"github.com/cortexproject/cortex/pkg/util/services"
 	cortex_testutil "github.com/cortexproject/cortex/pkg/util/test"
 )
 
 type healthClientMock struct {
 	grpc_health_v1.HealthClient
-	err error
+	err atomic.Error
 }
 
 func (h *healthClientMock) Check(ctx context.Context, in *grpc_health_v1.HealthCheckRequest, opts ...grpc.CallOption) (*grpc_health_v1.HealthCheckResponse, error) {
 	return &grpc_health_v1.HealthCheckResponse{
 		Status: grpc_health_v1.HealthCheckResponse_SERVING,
-	}, h.err
+	}, h.err.Load()
 }
 
-func TestNewHealthCheckInterceptors(t *testing.T) {
+func TestNewHealthCheckService(t *testing.T) {
 	i := NewHealthCheckInterceptors(utillog.Logger)
-	hMock := &healthClientMock{
-		err: fmt.Errorf("some error"),
+	hMock := &healthClientMock{}
+	i.healthClientFactory = func(cc grpc.ClientConnInterface) grpc_health_v1.HealthClient {
+		return hMock
+	}
+
+	require.NoError(t, services.StartAndAwaitRunning(context.Background(), i))
+	defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck
+
+	cfg := ConfigWithHealthCheck{
+		HealthCheckConfig: HealthCheckConfig{
+			UnhealthyThreshold: 2,
+			Interval:           0,
+			Timeout:            time.Second,
+		},
 	}
+
+	client, err := grpc.NewClient("localhost:999", grpc.WithTransportCredentials(insecure.NewCredentials()))
+	require.NoError(t, err)
+
+	ui := i.UnaryHealthCheckInterceptor(&cfg)
+	require.NoError(t, ui(context.Background(), "", struct{}{}, struct{}{}, client,
+		func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, opts ...grpc.CallOption) error {
+			return nil
+		}))
+
+	instances := i.registeredInstances()
+	require.Len(t, instances, 1)
+
+	// Generate healthcheck error and wait instance to become unhealthy
+	hMock.err.Store(errors.New("some error"))
+
+	cortex_testutil.Poll(t, 5*time.Second, false, func() interface{} {
+		return instances[0].isHealthy()
+	})
+
+	// Mark instance back to a healthy state
+	hMock.err.Store(nil)
+	cortex_testutil.Poll(t, 5*time.Second, true, func() interface{} {
+		return instances[0].isHealthy()
+	})
+}
+
+func TestNewHealthCheckInterceptors(t *testing.T) {
+	i := NewHealthCheckInterceptors(utillog.Logger)
+	hMock := &healthClientMock{}
+	hMock.err.Store(fmt.Errorf("some error"))
 	cfg := ConfigWithHealthCheck{
 		HealthCheckConfig: HealthCheckConfig{
 			UnhealthyThreshold: 2,
@@ -61,7 +107,7 @@ func TestNewHealthCheckInterceptors(t *testing.T) {
 	// first health check
 	require.NoError(t, i.iteration(context.Background()))
 
-	//Should second first call
+	//Should second call even with error
 	require.NoError(t, ui(context.Background(), "", struct{}{}, struct{}{}, ccUnhealthy, invoker))
 
 	require.Equal(t, invokedMap["localhost:999"], 2)
@@ -77,10 +123,9 @@ func TestNewHealthCheckInterceptors(t *testing.T) {
 	require.NoError(t, ui(context.Background(), "", struct{}{}, struct{}{}, ccHealthy, invoker))
 
 	// Should mark the instance back to healthy
-	hMock.err = nil
+	hMock.err.Store(nil)
 	require.NoError(t, i.iteration(context.Background()))
 	cortex_testutil.Poll(t, time.Second, true, func() interface{} {
 		return ui(context.Background(), "", struct{}{}, struct{}{}, ccUnhealthy, invoker) == nil
 	})
-
 }