From 0e65d2cdfa4b25cff77c10839724584f5e19e280 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Tue, 19 Nov 2024 13:58:14 -0500 Subject: [PATCH] Extend hostid test backoff (#49214) Attempts to reduce any flakiness caused by contention producing a host id by allowing tests to use a longer exponential backoff and retrying more times. --- lib/utils/hostid/hostid_test.go | 13 ++++++++- lib/utils/hostid/hostid_unix.go | 51 ++++++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/lib/utils/hostid/hostid_test.go b/lib/utils/hostid/hostid_test.go index 3a4a97552ac8b..208b95c292f6d 100644 --- a/lib/utils/hostid/hostid_test.go +++ b/lib/utils/hostid/hostid_test.go @@ -25,11 +25,13 @@ import ( "slices" "strings" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" + "github.com/gravitational/teleport/api/utils/retryutils" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/hostid" ) @@ -52,7 +54,16 @@ func TestReadOrCreate(t *testing.T) { for i := 0; i < concurrency; i++ { wg.Go(func() error { <-barrier - id, err := hostid.ReadOrCreateFile(dir) + id, err := hostid.ReadOrCreateFile( + dir, + hostid.WithBackoff(retryutils.RetryV2Config{ + First: 50 * time.Millisecond, + Driver: retryutils.NewExponentialDriver(100 * time.Millisecond), + Max: 15 * time.Second, + Jitter: retryutils.FullJitter, + }), + hostid.WithIterationLimit(10), + ) ids[i] = id return err }) diff --git a/lib/utils/hostid/hostid_unix.go b/lib/utils/hostid/hostid_unix.go index df43aa4ab851f..cee2ee0b2df58 100644 --- a/lib/utils/hostid/hostid_unix.go +++ b/lib/utils/hostid/hostid_unix.go @@ -44,23 +44,52 @@ func WriteFile(dataDir string, id string) error { return nil } +type options struct { + retryConfig retryutils.RetryV2Config + iterationLimit int +} + +// WithBackoff overrides the default backoff configuration of +// [ReadOrCreateFile]. +func WithBackoff(cfg retryutils.RetryV2Config) func(*options) { + return func(o *options) { + o.retryConfig = cfg + } +} + +// WithIterationLimit overrides the default number of time +// [ReadOrCreateFile] will attempt to produce a hostid. +func WithIterationLimit(limit int) func(*options) { + return func(o *options) { + o.iterationLimit = limit + } +} + // ReadOrCreateFile looks for a hostid file in the data dir. If present, -// returns the UUID from it, otherwise generates one -func ReadOrCreateFile(dataDir string) (string, error) { +// returns the UUID from it, otherwise generates one. +func ReadOrCreateFile(dataDir string, opts ...func(*options)) (string, error) { + o := options{ + retryConfig: retryutils.RetryV2Config{ + First: 100 * time.Millisecond, + Driver: retryutils.NewLinearDriver(100 * time.Millisecond), + Max: time.Second, + Jitter: retryutils.FullJitter, + }, + iterationLimit: 3, + } + + for _, opt := range opts { + opt(&o) + } + hostUUIDFileLock := GetPath(dataDir) + ".lock" - const iterationLimit = 3 - - backoff, err := retryutils.NewRetryV2(retryutils.RetryV2Config{ - First: 100 * time.Millisecond, - Driver: retryutils.NewLinearDriver(100 * time.Millisecond), - Max: time.Second, - Jitter: retryutils.FullJitter, - }) + + backoff, err := retryutils.NewRetryV2(o.retryConfig) if err != nil { return "", trace.Wrap(err) } - for i := 0; i < iterationLimit; i++ { + for i := 0; i < o.iterationLimit; i++ { if read, err := ReadFile(dataDir); err == nil { return read, nil } else if !trace.IsNotFound(err) {