From 7b629fcbe0fcd9c5e2c36abbb43164306a9a6984 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:18:54 -0500 Subject: [PATCH] Extend hostid test backoff (#49193) 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 2ea22c4e71e7f..62073f1c44a63 100644 --- a/lib/utils/hostid/hostid_test.go +++ b/lib/utils/hostid/hostid_test.go @@ -24,12 +24,14 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/assert" "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" ) @@ -53,7 +55,16 @@ func TestReadOrCreate(t *testing.T) { i := 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 df91eec76b83d..73b51a0dc537d 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.NewFullJitter, + }, + 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.NewFullJitter(), - }) + + 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) {