From 55e00be36e62b6fe798db1954abc2eea329f4b1b Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Tue, 8 Aug 2023 15:40:05 +0100 Subject: [PATCH] Allow short healthcheck interval with long timeout --- docs/content/routing/services/index.md | 1 - pkg/healthcheck/healthcheck.go | 5 --- pkg/healthcheck/healthcheck_test.go | 51 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/docs/content/routing/services/index.md b/docs/content/routing/services/index.md index c809392f4..fb6af7563 100644 --- a/docs/content/routing/services/index.md +++ b/docs/content/routing/services/index.md @@ -338,7 +338,6 @@ Below are the available options for the health check mechanism: !!! info "Interval & Timeout Format" Interval and timeout are to be given in a format understood by [time.ParseDuration](https://golang.org/pkg/time/#ParseDuration). - The interval must be greater than the timeout. If configuration doesn't reflect this, the interval will be set to timeout + 1 second. !!! info "Recovering Servers" diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 1452bfa66..747999889 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -69,11 +69,6 @@ func NewServiceHealthChecker(ctx context.Context, metrics metricsHealthCheck, co timeout = time.Duration(dynamic.DefaultHealthCheckTimeout) } - if timeout >= interval { - logger.Warn().Msgf("Health check timeout should be lower than the health check interval. Interval set to timeout + 1 second (%s).", interval) - interval = timeout + time.Second - } - client := &http.Client{ Transport: transport, } diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index c7813fae0..52b70448c 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -18,6 +18,57 @@ import ( healthpb "google.golang.org/grpc/health/grpc_health_v1" ) +func TestNewServiceHealthChecker_durations(t *testing.T) { + testCases := []struct { + desc string + config *dynamic.ServerHealthCheck + expInterval time.Duration + expTimeout time.Duration + }{ + { + desc: "default values", + config: &dynamic.ServerHealthCheck{}, + expInterval: time.Duration(dynamic.DefaultHealthCheckInterval), + expTimeout: time.Duration(dynamic.DefaultHealthCheckTimeout), + }, + { + desc: "out of range values", + config: &dynamic.ServerHealthCheck{ + Interval: ptypes.Duration(-time.Second), + Timeout: ptypes.Duration(-time.Second), + }, + expInterval: time.Duration(dynamic.DefaultHealthCheckInterval), + expTimeout: time.Duration(dynamic.DefaultHealthCheckTimeout), + }, + { + desc: "custom durations", + config: &dynamic.ServerHealthCheck{ + Interval: ptypes.Duration(time.Second * 10), + Timeout: ptypes.Duration(time.Second * 5), + }, + expInterval: time.Second * 10, + expTimeout: time.Second * 5, + }, + { + desc: "interval shorter than timeout", + config: &dynamic.ServerHealthCheck{ + Interval: ptypes.Duration(time.Second), + Timeout: ptypes.Duration(time.Second * 5), + }, + expInterval: time.Second, + expTimeout: time.Second * 5, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + healthChecker := NewServiceHealthChecker(context.Background(), nil, test.config, nil, nil, http.DefaultTransport, nil) + assert.Equal(t, test.expInterval, healthChecker.interval) + assert.Equal(t, test.expTimeout, healthChecker.timeout) + }) + } +} + func TestServiceHealthChecker_newRequest(t *testing.T) { testCases := []struct { desc string