Extend retryCmd with configurable retry and bo factor

prow/git/git.go

@@ -23,6 +23,7 @@ import (
  "errors"
  "fmt"
  "io/ioutil"
+ "math"
  "os"
  "os/exec"
  "path/filepath"
@@ -164,15 +165,15 @@ func (c *Client) Clone(organization, repository string) (*Repo, error) {
  return nil, err
  }
  remote := fmt.Sprintf("%s/%s", base, repo)
- if b, err := retryCmd(c.logger, "", c.git, "clone", "--mirror", remote, cache); err != nil {
+ if b, err := retryCmd(c.logger, 5, 1, "", c.git, "clone", "--mirror", remote, cache); err != nil {
  return nil, fmt.Errorf("git cache clone error: %v. output: %s", err, string(b))
  }
  } else if err != nil {
  return nil, err
  } else {
  // Cache hit. Do a git fetch to keep updated.
  c.logger.Infof("Fetching %s.", repo)
- if b, err := retryCmd(c.logger, cache, c.git, "fetch"); err != nil {
+ if b, err := retryCmd(c.logger, 5, 1, cache, c.git, "fetch"); err != nil {
  return nil, fmt.Errorf("git fetch error: %v. output: %s", err, string(b))
  }
  }
@@ -410,7 +411,7 @@ func (r *Repo) Push(branch string) error {
 // CheckoutPullRequest does exactly that.
 func (r *Repo) CheckoutPullRequest(number int) error {
  r.logger.Infof("Fetching and checking out %s/%s#%d.", r.org, r.repo, number)
- if b, err := retryCmd(r.logger, r.dir, r.git, "fetch", r.base+"/"+r.org+"/"+r.repo, fmt.Sprintf("pull/%d/head:pull%d", number, number)); err != nil {
+ if b, err := retryCmd(r.logger, 5, 1, r.dir, r.git, "fetch", r.base+"/"+r.org+"/"+r.repo, fmt.Sprintf("pull/%d/head:pull%d", number, number)); err != nil {
  return fmt.Errorf("git fetch failed for PR %d: %v. output: %s", number, err, string(b))
  }
  co := r.gitCommand("checkout", fmt.Sprintf("pull%d", number))
@@ -429,20 +430,23 @@ func (r *Repo) Config(key, value string) error {
  return nil
 }
 
-// retryCmd will retry the command a few times with backoff. Use this for any
-// commands that will be talking to GitHub, such as clones or fetches.
-func retryCmd(l *logrus.Entry, dir, cmd string, arg ...string) ([]byte, error) {
+// GetBackoffTime returns a backoff time calculated using formula: `{backoff factor} * 2 ^ {# of retries}`
+func GetBackoffTime(factor float64, retry int) time.Duration {
+ return time.Duration(math.Max(factor*math.Exp2(float64(retry)), 0)) * time.Second
+}
+
+// retryCmd will try the command `tries` times with a backoff (in seconds). Use this for any commands that
+// will be talking to GitHub, such as clones or fetches.
+func retryCmd(l *logrus.Entry, tries int, backoffFactor float64, dir, cmd string, arg ...string) ([]byte, error) {
  var b []byte
  var err error
- sleepyTime := time.Second
- for i := 0; i < 3; i++ {
+ for i := 0; i < tries; i++ {
  c := exec.Command(cmd, arg...)
  c.Dir = dir
  b, err = c.CombinedOutput()
  if err != nil {
  l.Warningf("Running %s %v returned error %v with output %s.", cmd, arg, err, string(b))
- time.Sleep(sleepyTime)
- sleepyTime *= 2
+ time.Sleep(GetBackoffTime(backoffFactor, i))
  continue
  }
  break

prow/git/git_test.go

@@ -23,9 +23,10 @@ import (
  "os/exec"
  "path/filepath"
  "testing"
+ "time"
 
  "github.com/sirupsen/logrus"
-
+ "k8s.io/test-infra/prow/git"
  "k8s.io/test-infra/prow/git/localgit"
  "k8s.io/test-infra/prow/github"
 )
@@ -482,3 +483,52 @@ func TestMerging(t *testing.T) {
  })
  }
 }
+
+func TestBackoff(t *testing.T) {
+ testCases := []struct {
+ name string
+ retry int
+ backoffFactor float64
+ expected time.Duration
+ }{
+ {
+ name: "Retry 0 (i.e. try 1) with a backoff factor 1 should backoff for 1 seconds",
+ retry: 0,
+ backoffFactor: 1,
+ expected: 1 * time.Second,
+ },
+ {
+ name: "Retry 4 with a backoff factor 1 should backoff for 16 seconds",
+ retry: 4,
+ backoffFactor: 1,
+ expected: 16 * time.Second,
+ },
+ {
+ name: "Retry 2 with a backoff factor 0.5 should backoff for 2 seconds",
+ retry: 2,
+ backoffFactor: 0.5,
+ expected: 2 * time.Second,
+ },
+ {
+ name: "Negative backoff factor should return 0 backoff time",
+ retry: 3,
+ backoffFactor: -2,
+ expected: 0,
+ },
+ {
+ name: "Negative retry number should return 0 backoff time",
+ retry: -5,
+ backoffFactor: 1,
+ expected: 0,
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ actual := git.GetBackoffTime(tc.backoffFactor, tc.retry)
+ if tc.expected != actual {
+ t.Errorf("expected: %v != actual: %v", tc.expected, actual)
+ }
+ })
+ }
+}