Merge branch '435-use-exponential-backoff' into 'master'

Resolve "Use exponential back-off for polling status endpoint"

Closes #435

See merge request gitlab-org/gitlab-pages!307

5 files changed, 38 insertions, 31 deletions

diff --git a/go.mod b/go.mod
index c46a657e..d726d664 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,7 @@ module gitlab.com/gitlab-org/gitlab-pages
 go 1.13
 
 require (
+	github.com/cenkalti/backoff/v4 v4.0.2
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/fzipp/gocyclo v0.0.0-20150627053110-6acd4345c835
 	github.com/golang/mock v1.3.1
diff --git a/go.sum b/go.sum
index 550d6a7d..5a517c8a 100644
--- a/go.sum
+++ b/go.sum
@@ -38,6 +38,8 @@ github.com/beorn7/perks v1.0.0 h1:HWo1m869IqiPhD389kmkxeTalrjNbbJTC8LXupb+sl0=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
+github.com/cenkalti/backoff/v4 v4.0.2 h1:JIufpQLbh4DkbQoii76ItQIUFzevQSqOLZca4eamEDs=
+github.com/cenkalti/backoff/v4 v4.0.2/go.mod h1:eEew/i+1Q6OrCDZh3WiXYv3+nJwBASZ8Bog/87DQnVg=
 github.com/certifi/gocertifi v0.0.0-20180905225744-ee1a9a0726d2/go.mod h1:GJKEexRPVJrBSOjoqN5VNOIKJ5Q3RViH6eu3puDRwx4=
 github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
 github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index 2635d864..67c4eb6b 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -8,6 +8,8 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/cenkalti/backoff/v4"
+
 	"gitlab.com/gitlab-org/gitlab-pages/internal/domain"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/serving"
@@ -36,7 +38,7 @@ func New(config client.Config) (*Gitlab, error) {
 		mu:     &sync.RWMutex{},
 	}
 
-	go g.poll(defaultPollingMaxRetries, defaultPollingInterval)
+	go g.poll(backoff.DefaultInitialInterval, maxPollingTime)
 
 	// using nil for cache config will use the default values specified in internal/source/gitlab/cache/cache.go#12
 	return g, nil
diff --git a/internal/source/gitlab/gitlab_poll.go b/internal/source/gitlab/gitlab_poll.go
index 9fce7250..bc1611ef 100644
--- a/internal/source/gitlab/gitlab_poll.go
+++ b/internal/source/gitlab/gitlab_poll.go
@@ -3,37 +3,38 @@ package gitlab
 import (
 	"time"
 
+	"github.com/cenkalti/backoff/v4"
 	log "github.com/sirupsen/logrus"
 )
 
 const (
-	// defaultPollingMaxRetries to be used by poll
-	defaultPollingMaxRetries = 30
-	// defaultPollingInterval to be used by poll
-	defaultPollingInterval = time.Minute
+	// maxPollingTime is the maximum duration to try to call the Status API
+	maxPollingTime = 60 * time.Minute
 )
 
-// poll tries to call the /internal/pages/status API endpoint once plus
-// `retries` every `interval`.
-// It updates the `isReady` value when successful.
+// Poll tries to call the /internal/pages/status API endpoint once plus
+// for `maxElapsedTime`
 // TODO: Remove in https://gitlab.com/gitlab-org/gitlab/-/issues/218357
-func (g *Gitlab) poll(retries int, interval time.Duration) {
-	var err error
-	for i := 0; i <= retries; i++ {
+func (g *Gitlab) poll(interval, maxElapsedTime time.Duration) {
+	backOff := backoff.NewExponentialBackOff()
+	backOff.InitialInterval = interval
+	backOff.MaxElapsedTime = maxElapsedTime
+
+	operation := func() error {
 		log.Info("Checking GitLab internal API availability")
-		err = g.client.Status()
-		if err == nil {
-			log.Info("GitLab internal pages status API connected successfully")
-			g.mu.Lock()
-			g.isReady = true
-			g.mu.Unlock()
-
-			// return as soon as we connect to the API
-			return
-		}
-
-		time.Sleep(interval)
+
+		return g.client.Status()
 	}
 
-	log.WithError(err).Errorf("Failed to connect to the internal GitLab API after %d tries every %.2fs", retries+1, interval.Seconds())
+	err := backoff.Retry(operation, backOff)
+	if err != nil {
+		log.WithError(err).Errorf("Failed to connect to the internal GitLab API after %.2fs", maxElapsedTime.Seconds())
+		return
+	}
+
+	g.mu.Lock()
+	g.isReady = true
+	g.mu.Unlock()
+
+	log.Info("GitLab internal pages status API connected successfully")
 }
diff --git a/internal/source/gitlab/gitlab_poll_test.go b/internal/source/gitlab/gitlab_poll_test.go
index 01f13846..8eecf210 100644
--- a/internal/source/gitlab/gitlab_poll_test.go
+++ b/internal/source/gitlab/gitlab_poll_test.go
@@ -17,31 +17,31 @@ func TestClient_Poll(t *testing.T) {
 	tests := []struct {
 		name         string
 		retries      int
-		interval     time.Duration
+		maxTime      time.Duration
 		expectedFail bool
 	}{
 		{
 			name:         "success_with_no_retry",
 			retries:      0,
-			interval:     5 * time.Millisecond,
+			maxTime:      10 * time.Millisecond,
 			expectedFail: false,
 		},
 		{
 			name:         "success_after_N_retries",
 			retries:      3,
-			interval:     10 * time.Millisecond,
+			maxTime:      30 * time.Millisecond,
 			expectedFail: false,
 		},
 		{
 			name:         "fail_with_no_retries",
 			retries:      0,
-			interval:     5 * time.Millisecond,
+			maxTime:      10 * time.Millisecond,
 			expectedFail: true,
 		},
 		{
 			name:         "fail_after_N_retries",
 			retries:      3,
-			interval:     5 * time.Millisecond,
+			maxTime:      30 * time.Millisecond,
 			expectedFail: true,
 		},
 	}
@@ -64,10 +64,11 @@ func TestClient_Poll(t *testing.T) {
 
 			glClient := Gitlab{client: client, mu: &sync.RWMutex{}}
 
-			glClient.poll(tt.retries, tt.interval)
+			glClient.poll(3*time.Millisecond, tt.maxTime)
 			if tt.expectedFail {
 				require.False(t, glClient.isReady)
-				s := fmt.Sprintf("Failed to connect to the internal GitLab API after %d tries every %.2fs", tt.retries+1, tt.interval.Seconds())
+
+				s := fmt.Sprintf("Failed to connect to the internal GitLab API after %.2fs", tt.maxTime.Seconds())
 				require.Equal(t, s, hook.LastEntry().Message)
 				return
 			}