NR-170460: fix default NTP timeout to 5 seconds

newrelic · Oct 18, 2023 · 8512a94 · 8512a94
1 parent d402517
commit 8512a94
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 32 deletions.
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -1374,10 +1374,10 @@ func (lc *LogConfig) VerboseEnabled() int {
 
 // NtpConfig map all ntp configuration options.
 type NtpConfig struct {
-	Pool     []string `yaml:"pool" envconfig:"pool"`
-	Enabled  bool     `yaml:"enabled" envconfig:"enabled"`
-	Interval uint     `yaml:"interval" envconfig:"interval"`
-	Timeout  uint     `yaml:"timeout" envconfig:"timeout"`
+	Pool     []string      `yaml:"pool" envconfig:"pool"`
+	Enabled  bool          `yaml:"enabled" envconfig:"enabled"`
+	Interval time.Duration `yaml:"interval" envconfig:"interval"`
+	Timeout  time.Duration `yaml:"timeout" envconfig:"timeout"`
 }
 
 func NewNtpConfig() NtpConfig {

diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go
@@ -4,6 +4,7 @@ package config
 
 import (
 	"os/user"
+	"time"
 )
 
 const (
@@ -111,8 +112,8 @@ var (
 	defaultRegisterMaxRetryBoSecs        = 60
 	defaultNtpPool                       = []string{} // i.e: []string{"time.cloudflare.com"}
 	defaultNtpEnabled                    = false
-	defaultNtpInterval                   = uint(15)   // minutes
-	defaultNtpTimeout                    = uint(5000) // millisecods
+	defaultNtpInterval                   = 15 * time.Minute // 15 minutes
+	defaultNtpTimeout                    = 5 * time.Second  // 5 seconds
 )
 
 // Default internal values

diff --git a/pkg/metrics/ntp.go b/pkg/metrics/ntp.go
@@ -16,8 +16,8 @@ import (
 )
 
 const (
-	ntpIntervalMin    = 15 // minutes
-	ntpTimeoutDefault = 5  // seconds
+	ntpIntervalMin    = 15 * time.Minute // minutes
+	ntpTimeoutDefault = 5 * time.Second  // seconds
 )
 
 var (
@@ -38,7 +38,7 @@ type Ntp struct {
 // NewNtp creates a new Ntp instance
 // timeout is expressed in secods
 // interval is expressed in minutes.
-func NewNtp(pool []string, timeout uint, interval uint) *Ntp {
+func NewNtp(pool []string, timeout time.Duration, interval time.Duration) *Ntp {
 	validInterval := guardInterval(interval)
 	validTimeout := guardTimeout(timeout)
 	return &Ntp{
@@ -53,7 +53,7 @@ func NewNtp(pool []string, timeout uint, interval uint) *Ntp {
 // guardTimeout ensures that interval is not 0
 // when interval is 0 the used library defaults to 5s, so we check it here
 // to decouple from that decision in case that it changes.
-func guardTimeout(timeout uint) uint {
+func guardTimeout(timeout time.Duration) time.Duration {
 	if timeout == 0 {
 		return ntpTimeoutDefault
 	}
@@ -62,7 +62,7 @@ func guardTimeout(timeout uint) uint {
 
 // guardInterval ensures that interval is not smaller than ntpIntervalMin
 // if it is not valid, it will default to ntpIntervalMin.
-func guardInterval(interval uint) uint {
+func guardInterval(interval time.Duration) time.Duration {
 	if interval < ntpIntervalMin {
 		return ntpIntervalMin
 	}

diff --git a/pkg/metrics/ntp_test.go b/pkg/metrics/ntp_test.go
@@ -19,34 +19,34 @@ import (
 func TestNewNtp_Interval(t *testing.T) {
 	testCases := []struct {
 		name             string
-		interval         uint
+		interval         time.Duration
 		pool             []string
 		expectedInterval time.Duration
 		expectedPool     []string
 	}{
 		{
 			name:             "invalid interval fallbacks to minimum",
-			interval:         2,
-			expectedInterval: time.Duration(ntpIntervalMin) * time.Minute,
+			interval:         2 * time.Second,
+			expectedInterval: ntpIntervalMin,
 		},
 		{
 			name:             "pool is allowed to be empty",
-			expectedInterval: time.Duration(ntpIntervalMin) * time.Minute,
+			expectedInterval: ntpIntervalMin,
 		},
 		{
 			name:             "valid interval",
-			interval:         17,
+			interval:         17 * time.Minute,
 			expectedInterval: time.Duration(17) * time.Minute,
 		},
 		{
 			name:             "valid pool",
 			pool:             []string{"one", "two", "three"},
 			expectedPool:     []string{"one", "two", "three"},
-			expectedInterval: time.Duration(ntpIntervalMin) * time.Minute,
+			expectedInterval: ntpIntervalMin,
 		},
 	}
 
-	timeout := uint(100)
+	timeout := 100 * time.Minute
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
 			ntp := NewNtp(testCase.pool, timeout, testCase.interval)
@@ -60,13 +60,13 @@ func TestNewNtp_Interval(t *testing.T) {
 func TestNewNtp_Timeout(t *testing.T) {
 	testCases := []struct {
 		name            string
-		timeout         uint
+		timeout         time.Duration
 		pool            []string
 		expectedTimeout time.Duration
 	}{
 		{
 			name:            "valid timeout",
-			timeout:         125,
+			timeout:         125 * time.Minute,
 			expectedTimeout: time.Duration(125) * time.Second,
 		},
 		{
@@ -88,8 +88,8 @@ func TestNewNtp_Timeout(t *testing.T) {
 func TestOffset_Interval(t *testing.T) {
 	testCases := []struct {
 		name              string
-		interval          uint
-		timeout           uint
+		interval          time.Duration
+		timeout           time.Duration
 		offset            time.Duration
 		pool              []string
 		now               func() time.Time
@@ -155,8 +155,8 @@ func TestOffset_Interval(t *testing.T) {
 }
 
 func TestOffset_OffsetAverage(t *testing.T) {
-	timeout := uint(5000)
-	interval := uint(15)
+	timeout := 5000 * time.Millisecond
+	interval := 15 * time.Minute
 	ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval)
 	ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{
 		{
@@ -179,8 +179,8 @@ func TestOffset_OffsetAverage(t *testing.T) {
 }
 
 func TestOffset_AnyHostErrorShouldNotReturnError(t *testing.T) {
-	timeout := uint(5000)
-	interval := uint(15)
+	timeout := 5000 * time.Millisecond
+	interval := 15 * time.Minute
 	ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval)
 	ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{
 		{
@@ -202,8 +202,8 @@ func TestOffset_AnyHostErrorShouldNotReturnError(t *testing.T) {
 }
 
 func TestOffset_AllHostErrorShouldReturnError(t *testing.T) {
-	timeout := uint(5000)
-	interval := uint(15)
+	timeout := 5000 * time.Millisecond
+	interval := 15 * time.Minute
 	ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval)
 	ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{
 		{
@@ -225,8 +225,8 @@ func TestOffset_AllHostErrorShouldReturnError(t *testing.T) {
 func TestOffset_InvalidNtpResponse(t *testing.T) {
 	t.Parallel()
 
-	timeout := uint(5000)
-	interval := uint(15)
+	timeout := 5000 * time.Millisecond
+	interval := 15 * time.Minute
 	ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval)
 	ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{
 		{
@@ -251,8 +251,8 @@ func TestOffset_InvalidNtpResponse(t *testing.T) {
 func TestOffset_AnyHostInvalidShouldNotReturnError(t *testing.T) {
 	t.Parallel()
 
-	timeout := uint(5000)
-	interval := uint(15)
+	timeout := 5000 * time.Millisecond
+	interval := 15 * time.Minute
 	ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval)
 	ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{
 		{