diff --git a/cfsetup.yaml b/cfsetup.yaml index d8d80c49..e30467c1 100644 --- a/cfsetup.yaml +++ b/cfsetup.yaml @@ -194,6 +194,7 @@ stretch: &stretch builddeps: - *pinned_go_fips - build-essential + - gotest-to-teamcity post-cache: - export GOOS=linux - export GOARCH=amd64 @@ -201,7 +202,7 @@ stretch: &stretch # cd to a non-module directory: https://github.com/golang/go/issues/24250 - (cd / && go get github.com/BurntSushi/go-sumtype) - export PATH="$HOME/go/bin:$PATH" - - make test + - make test | gotest-to-teamcity update-homebrew: builddeps: - openssh-client diff --git a/cmd/cloudflared/main.go b/cmd/cloudflared/main.go index 71495db9..0d28cbc7 100644 --- a/cmd/cloudflared/main.go +++ b/cmd/cloudflared/main.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "math/rand" "strings" "time" @@ -43,6 +44,7 @@ var ( ) func main() { + rand.Seed(time.Now().UnixNano()) metrics.RegisterBuildInfo(BuildTime, Version) raven.SetRelease(Version) diff --git a/origin/backoffhandler.go b/origin/backoffhandler.go index e99605e1..96b3326c 100644 --- a/origin/backoffhandler.go +++ b/origin/backoffhandler.go @@ -2,6 +2,7 @@ package origin import ( "context" + "math/rand" "time" ) @@ -30,7 +31,7 @@ type BackoffHandler struct { resetDeadline time.Time } -func (b BackoffHandler) GetBackoffDuration(ctx context.Context) (time.Duration, bool) { +func (b BackoffHandler) GetMaxBackoffDuration(ctx context.Context) (time.Duration, bool) { // Follows the same logic as Backoff, but without mutating the receiver. // This select has to happen first to reflect the actual behaviour of the Backoff function. select { @@ -45,7 +46,8 @@ func (b BackoffHandler) GetBackoffDuration(ctx context.Context) (time.Duration, if b.retries >= b.MaxRetries && !b.RetryForever { return time.Duration(0), false } - return time.Duration(b.GetBaseTime() * 1 << b.retries), true + maxTimeToWait := b.GetBaseTime() * 1 << (b.retries + 1) + return maxTimeToWait, true } // BackoffTimer returns a channel that sends the current time when the exponential backoff timeout expires. @@ -62,7 +64,9 @@ func (b *BackoffHandler) BackoffTimer() <-chan time.Time { } else { b.retries++ } - return timeAfter(time.Duration(b.GetBaseTime() * 1 << (b.retries - 1))) + maxTimeToWait := time.Duration(b.GetBaseTime() * 1 << (b.retries)) + timeToWait := time.Duration(rand.Int63n(maxTimeToWait.Nanoseconds())) + return timeAfter(timeToWait) } // Backoff is used to wait according to exponential backoff. Returns false if the @@ -83,7 +87,9 @@ func (b *BackoffHandler) Backoff(ctx context.Context) bool { // Sets a grace period within which the the backoff timer is maintained. After the grace // period expires, the number of retries & backoff duration is reset. func (b *BackoffHandler) SetGracePeriod() { - b.resetDeadline = timeNow().Add(time.Duration(b.GetBaseTime() * 2 << b.retries)) + maxTimeToWait := b.GetBaseTime() * 2 << (b.retries + 1) + timeToWait := time.Duration(rand.Int63n(maxTimeToWait.Nanoseconds())) + b.resetDeadline = timeNow().Add(timeToWait) } func (b BackoffHandler) GetBaseTime() time.Duration { diff --git a/origin/backoffhandler_test.go b/origin/backoffhandler_test.go index 6c8e4e9c..8add3f0b 100644 --- a/origin/backoffhandler_test.go +++ b/origin/backoffhandler_test.go @@ -40,7 +40,7 @@ func TestBackoffCancel(t *testing.T) { if backoff.Backoff(ctx) { t.Fatalf("backoff allowed after cancel") } - if _, ok := backoff.GetBackoffDuration(ctx); ok { + if _, ok := backoff.GetMaxBackoffDuration(ctx); ok { t.Fatalf("backoff allowed after cancel") } } @@ -69,24 +69,24 @@ func TestBackoffGracePeriod(t *testing.T) { } } -func TestGetBackoffDurationRetries(t *testing.T) { +func TestGetMaxBackoffDurationRetries(t *testing.T) { // make backoff return immediately timeAfter = immediateTimeAfter ctx := context.Background() backoff := BackoffHandler{MaxRetries: 3} - if _, ok := backoff.GetBackoffDuration(ctx); !ok { + if _, ok := backoff.GetMaxBackoffDuration(ctx); !ok { t.Fatalf("backoff failed immediately") } backoff.Backoff(ctx) // noop - if _, ok := backoff.GetBackoffDuration(ctx); !ok { + if _, ok := backoff.GetMaxBackoffDuration(ctx); !ok { t.Fatalf("backoff failed after 1 retry") } backoff.Backoff(ctx) // noop - if _, ok := backoff.GetBackoffDuration(ctx); !ok { + if _, ok := backoff.GetMaxBackoffDuration(ctx); !ok { t.Fatalf("backoff failed after 2 retry") } backoff.Backoff(ctx) // noop - if _, ok := backoff.GetBackoffDuration(ctx); ok { + if _, ok := backoff.GetMaxBackoffDuration(ctx); ok { t.Fatalf("backoff allowed after 3 (max) retries") } if backoff.Backoff(ctx) { @@ -94,25 +94,25 @@ func TestGetBackoffDurationRetries(t *testing.T) { } } -func TestGetBackoffDuration(t *testing.T) { +func TestGetMaxBackoffDuration(t *testing.T) { // make backoff return immediately timeAfter = immediateTimeAfter ctx := context.Background() backoff := BackoffHandler{MaxRetries: 3} - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second { - t.Fatalf("backoff didn't return 1 second on first retry") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*2 { + t.Fatalf("backoff (%s) didn't return < 2 seconds on first retry", duration) } backoff.Backoff(ctx) // noop - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second*2 { - t.Fatalf("backoff didn't return 2 seconds on second retry") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*4 { + t.Fatalf("backoff (%s) didn't return < 4 seconds on second retry", duration) } backoff.Backoff(ctx) // noop - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second*4 { - t.Fatalf("backoff didn't return 4 seconds on third retry") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*8 { + t.Fatalf("backoff (%s) didn't return < 8 seconds on third retry", duration) } backoff.Backoff(ctx) // noop - if duration, ok := backoff.GetBackoffDuration(ctx); ok || duration != 0 { - t.Fatalf("backoff didn't return 0 seconds on fourth retry (exceeding limit)") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); ok || duration != 0 { + t.Fatalf("backoff (%s) didn't return 0 seconds on fourth retry (exceeding limit)", duration) } } @@ -121,27 +121,27 @@ func TestBackoffRetryForever(t *testing.T) { timeAfter = immediateTimeAfter ctx := context.Background() backoff := BackoffHandler{MaxRetries: 3, RetryForever: true} - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second { - t.Fatalf("backoff didn't return 1 second on first retry") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*2 { + t.Fatalf("backoff (%s) didn't return < 2 seconds on first retry", duration) } backoff.Backoff(ctx) // noop - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second*2 { - t.Fatalf("backoff didn't return 2 seconds on second retry") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*4 { + t.Fatalf("backoff (%s) didn't return < 4 seconds on second retry", duration) } backoff.Backoff(ctx) // noop - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second*4 { - t.Fatalf("backoff didn't return 4 seconds on third retry") + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*8 { + t.Fatalf("backoff (%s) didn't return < 8 seconds on third retry", duration) } if !backoff.Backoff(ctx) { t.Fatalf("backoff refused on fourth retry despire RetryForever") } - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second*8 { + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*16 { t.Fatalf("backoff returned %v instead of 8 seconds on fourth retry", duration) } if !backoff.Backoff(ctx) { t.Fatalf("backoff refused on fifth retry despire RetryForever") } - if duration, ok := backoff.GetBackoffDuration(ctx); !ok || duration != time.Second*8 { + if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*16 { t.Fatalf("backoff returned %v instead of 8 seconds on fifth retry", duration) } } diff --git a/origin/reconnect.go b/origin/reconnect.go index 5b64f38c..c05c66e2 100644 --- a/origin/reconnect.go +++ b/origin/reconnect.go @@ -108,7 +108,7 @@ func (cm *reconnectCredentialManager) RefreshAuth( authOutcome, err := authenticate(ctx, backoff.Retries()) if err != nil { cm.authFail.WithLabelValues(err.Error()).Inc() - if _, ok := backoff.GetBackoffDuration(ctx); ok { + if _, ok := backoff.GetMaxBackoffDuration(ctx); ok { return backoff.BackoffTimer(), nil } return nil, err diff --git a/origin/reconnect_test.go b/origin/reconnect_test.go index d3cf6dbe..9b9dfa8c 100644 --- a/origin/reconnect_test.go +++ b/origin/reconnect_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" tunnelpogs "github.com/cloudflare/cloudflared/tunnelrpc/pogs" ) @@ -28,13 +29,14 @@ func TestRefreshAuthBackoff(t *testing.T) { // authentication failures should consume the backoff for i := uint(0); i < backoff.MaxRetries; i++ { retryChan, err := rcm.RefreshAuth(context.Background(), backoff, auth) - assert.NoError(t, err) - assert.NotNil(t, retryChan) - assert.Equal(t, (1<