From b99c66c8cc0771c3a89a5af8f8a3ebe22b20e9a4 Mon Sep 17 00:00:00 2001 From: Erick Fejta Date: Tue, 23 Jun 2020 12:20:46 -0700 Subject: [PATCH] Retry git fetch commands. The git fetch command waits at most 2s for dns to resolve with no retries. This can occasionally cause unnecessary flakiness. Retry these calls with exponential backoff to mitigate --- prow/pod-utils/clone/clone.go | 75 +++- prow/pod-utils/clone/clone_test.go | 532 +++++++++++++++++++---------- 2 files changed, 417 insertions(+), 190 deletions(-) diff --git a/prow/pod-utils/clone/clone.go b/prow/pod-utils/clone/clone.go index 8da0255cdc10..120802b8531c 100644 --- a/prow/pod-utils/clone/clone.go +++ b/prow/pod-utils/clone/clone.go @@ -34,6 +34,10 @@ import ( "k8s.io/test-infra/prow/logrusutil" ) +type runnable interface { + run() (string, string, error) +} + // Run clones the refs under the prescribed directory and optionally // configures the git username and email in the repository as well. func Run(refs prowapi.Refs, dir, gitUserName, gitUserEmail, cookiePath string, env []string, oauthToken string) Record { @@ -47,7 +51,7 @@ func Run(refs prowapi.Refs, dir, gitUserName, gitUserEmail, cookiePath string, e // This function runs the provided commands in order, logging them as they run, // aborting early and returning if any command fails. - runCommands := func(commands []cloneCommand) error { + runCommands := func(commands []runnable) error { for _, command := range commands { formattedCommand, output, err := command.run() log := logrus.WithFields(logrus.Fields{"command": formattedCommand, "output": output}) @@ -142,11 +146,32 @@ func (g *gitCtx) gitCommand(args ...string) cloneCommand { return cloneCommand{dir: g.cloneDir, env: g.env, command: "git", args: args} } +var ( + fetchRetries = []time.Duration{ + 100 * time.Millisecond, + 200 * time.Millisecond, + 400 * time.Millisecond, + 800 * time.Millisecond, + 2 * time.Second, + } +) + +func (g *gitCtx) gitFetch(fetchArgs ...string) retryCommand { + args := []string{"fetch"} + args = append(args, fetchArgs...) + + return retryCommand{ + runnable: g.gitCommand(args...), + retries: fetchRetries, + } +} + // commandsForBaseRef returns the list of commands needed to initialize and // configure a local git directory, as well as fetch and check out the provided // base ref. -func (g *gitCtx) commandsForBaseRef(refs prowapi.Refs, gitUserName, gitUserEmail, cookiePath string) []cloneCommand { - commands := []cloneCommand{{dir: "/", env: g.env, command: "mkdir", args: []string{"-p", g.cloneDir}}} +func (g *gitCtx) commandsForBaseRef(refs prowapi.Refs, gitUserName, gitUserEmail, cookiePath string) []runnable { + var commands []runnable + commands = append(commands, cloneCommand{dir: "/", env: g.env, command: "mkdir", args: []string{"-p", g.cloneDir}}) commands = append(commands, g.gitCommand("init")) if gitUserName != "" { @@ -160,11 +185,11 @@ func (g *gitCtx) commandsForBaseRef(refs prowapi.Refs, gitUserName, gitUserEmail } if refs.CloneDepth > 0 { - commands = append(commands, g.gitCommand("fetch", g.repositoryURI, "--tags", "--prune", "--depth", strconv.Itoa(refs.CloneDepth))) - commands = append(commands, g.gitCommand("fetch", "--depth", strconv.Itoa(refs.CloneDepth), g.repositoryURI, refs.BaseRef)) + commands = append(commands, g.gitFetch(g.repositoryURI, "--tags", "--prune", "--depth", strconv.Itoa(refs.CloneDepth))) + commands = append(commands, g.gitFetch("--depth", strconv.Itoa(refs.CloneDepth), g.repositoryURI, refs.BaseRef)) } else { - commands = append(commands, g.gitCommand("fetch", g.repositoryURI, "--tags", "--prune")) - commands = append(commands, g.gitCommand("fetch", g.repositoryURI, refs.BaseRef)) + commands = append(commands, g.gitFetch(g.repositoryURI, "--tags", "--prune")) + commands = append(commands, g.gitFetch(g.repositoryURI, refs.BaseRef)) } var target string if refs.BaseSHA != "" { @@ -231,14 +256,14 @@ func (g *gitCtx) gitRevParse() (string, error) { // It's recommended that fakeTimestamp be set to the timestamp of the base ref. // This enables reproducible timestamps and git tree digests every time the same // set of base and pull refs are used. -func (g *gitCtx) commandsForPullRefs(refs prowapi.Refs, fakeTimestamp int) []cloneCommand { - var commands []cloneCommand +func (g *gitCtx) commandsForPullRefs(refs prowapi.Refs, fakeTimestamp int) []runnable { + var commands []runnable for _, prRef := range refs.Pulls { ref := fmt.Sprintf("pull/%d/head", prRef.Number) if prRef.Ref != "" { ref = prRef.Ref } - commands = append(commands, g.gitCommand("fetch", g.repositoryURI, ref)) + commands = append(commands, g.gitFetch(g.repositoryURI, ref)) var prCheckout string if prRef.SHA != "" { prCheckout = prRef.SHA @@ -259,6 +284,30 @@ func (g *gitCtx) commandsForPullRefs(refs prowapi.Refs, fakeTimestamp int) []clo return commands } +type retryCommand struct { + runnable + retries []time.Duration +} + +func (rc retryCommand) run() (string, string, error) { + cmd, out, err := rc.runnable.run() + if err == nil { + return cmd, out, err + } + for _, dur := range rc.retries { + logrus.WithError(err).WithFields(logrus.Fields{ + "sleep": dur, + "command": cmd, + }).Info("Retrying after sleep") + time.Sleep(dur) + cmd, out, err = rc.runnable.run() + if err == nil { + break + } + } + return cmd, out, err +} + type cloneCommand struct { dir string env []string @@ -266,8 +315,8 @@ type cloneCommand struct { args []string } -func (c *cloneCommand) run() (string, string, error) { - output := bytes.Buffer{} +func (c cloneCommand) run() (string, string, error) { + var output bytes.Buffer cmd := exec.Command(c.command, c.args...) cmd.Dir = c.dir cmd.Env = append(cmd.Env, c.env...) @@ -277,6 +326,6 @@ func (c *cloneCommand) run() (string, string, error) { return strings.Join(append([]string{c.command}, c.args...), " "), output.String(), err } -func (c *cloneCommand) String() string { +func (c cloneCommand) String() string { return fmt.Sprintf("PWD=%s %s %s %s", c.dir, strings.Join(c.env, " "), c.command, strings.Join(c.env, " ")) } diff --git a/prow/pod-utils/clone/clone_test.go b/prow/pod-utils/clone/clone_test.go index 1589e1564e4a..d649ebd035a9 100644 --- a/prow/pod-utils/clone/clone_test.go +++ b/prow/pod-utils/clone/clone_test.go @@ -17,11 +17,13 @@ limitations under the License. package clone import ( + "fmt" "io/ioutil" "os" "os/exec" "reflect" "testing" + "time" "k8s.io/apimachinery/pkg/util/diff" prowapi "k8s.io/test-infra/prow/apis/prowjobs/v1" @@ -64,8 +66,8 @@ func TestCommandsForRefs(t *testing.T) { refs prowapi.Refs dir, gitUserName, gitUserEmail, cookiePath string env []string - expectedBase []cloneCommand - expectedPull []cloneCommand + expectedBase []runnable + expectedPull []runnable oauthToken string }{ { @@ -76,17 +78,23 @@ func TestCommandsForRefs(t *testing.T) { BaseRef: "master", }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -97,17 +105,23 @@ func TestCommandsForRefs(t *testing.T) { BaseRef: "master", }, dir: "/", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/src/github.com/org/repo"}}, - {dir: "/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/src/github.com/org/repo"}}, + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -119,18 +133,24 @@ func TestCommandsForRefs(t *testing.T) { }, gitUserName: "user", dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"config", "user.name", "user"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"config", "user.name", "user"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -142,18 +162,24 @@ func TestCommandsForRefs(t *testing.T) { }, gitUserEmail: "user@go.com", dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"config", "user.email", "user@go.com"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"config", "user.email", "user@go.com"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -165,18 +191,24 @@ func TestCommandsForRefs(t *testing.T) { }, cookiePath: "/cookie.txt", dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"config", "http.cookiefile", "/cookie.txt"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"config", "http.cookiefile", "/cookie.txt"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -188,14 +220,20 @@ func TestCommandsForRefs(t *testing.T) { SkipSubmodules: true, }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, }, expectedPull: nil, }, @@ -208,17 +246,23 @@ func TestCommandsForRefs(t *testing.T) { BaseRef: "master", }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }}, { name: "refs with clone URI override", @@ -229,17 +273,23 @@ func TestCommandsForRefs(t *testing.T) { CloneURI: "internet.com", }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "internet.com", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "internet.com", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "internet.com", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "internet.com", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -252,17 +302,23 @@ func TestCommandsForRefs(t *testing.T) { CloneURI: "https://internet.com", }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@internet.com", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@internet.com", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@internet.com", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://12345678:x-oauth-basic@internet.com", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -274,17 +330,23 @@ func TestCommandsForRefs(t *testing.T) { PathAlias: "my/favorite/dir", }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/my/favorite/dir"}}, - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"init"}}, - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/my/favorite/dir", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/my/favorite/dir"}}, + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/my/favorite/dir", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -296,17 +358,23 @@ func TestCommandsForRefs(t *testing.T) { BaseSHA: "abcdef", }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "abcdef"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "abcdef"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "abcdef"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "abcdef"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -320,19 +388,28 @@ func TestCommandsForRefs(t *testing.T) { }, }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/1/head"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/1/head"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -346,19 +423,28 @@ func TestCommandsForRefs(t *testing.T) { }, }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull-me"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull-me"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -372,19 +458,28 @@ func TestCommandsForRefs(t *testing.T) { }, }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/1/head"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "abcdef"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/1/head"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "abcdef"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, { @@ -399,21 +494,33 @@ func TestCommandsForRefs(t *testing.T) { }, }, dir: "/go", - expectedBase: []cloneCommand{ - {dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, - }, - expectedPull: []cloneCommand{ - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/1/head"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/2/head"}}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 2)}, - {dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, + expectedBase: []runnable{ + cloneCommand{dir: "/", command: "mkdir", args: []string{"-p", "/go/src/github.com/org/repo"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"init"}}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "--tags", "--prune"}}, + fetchRetries, + }, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "master"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"branch", "--force", "master", "FETCH_HEAD"}}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"checkout", "master"}}, + }, + expectedPull: []runnable{ + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/1/head"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 1)}, + retryCommand{ + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"fetch", "https://github.com/org/repo.git", "pull/2/head"}}, + fetchRetries, + }, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"merge", "--no-ff", "FETCH_HEAD"}, env: gitTimestampEnvs(fakeTimestamp + 2)}, + cloneCommand{dir: "/go/src/github.com/org/repo", command: "git", args: []string{"submodule", "update", "--init", "--recursive"}}, }, }, } @@ -565,3 +672,74 @@ func TestCensorGitCommand(t *testing.T) { }) } } + +// fakeRunner will pass run() if called when calls == 1, +// decrementing calls each time. +type fakeRunner struct { + calls int +} + +func (fr *fakeRunner) run() (string, string, error) { + fr.calls-- + if fr.calls == 0 { + return "command", "output", nil + } + return "command", "output", fmt.Errorf("calls: %d", fr.calls) +} + +func TestGitFetch(t *testing.T) { + const short = time.Nanosecond + command := func(calls int, retries ...time.Duration) retryCommand { + return retryCommand{ + runnable: &fakeRunner{calls}, + retries: retries, + } + } + cases := []struct { + name string + retryCommand + err bool + }{ + { + name: "works without retires", + retryCommand: command(1), + }, + { + name: "errors if first call fails without retries", + retryCommand: command(0), + err: true, + }, + { + name: "works with retries (without retrying)", + retryCommand: command(1, short), + }, + { + name: "works with retries (retrying)", + retryCommand: command(2, short), + }, + { + name: "errors without retries if first call fails", + retryCommand: command(2), + err: true, + }, + { + name: "errors with retries when all retries are consumed", + retryCommand: command(3, short), + err: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, _, err := tc.run() + switch { + case err != nil: + if !tc.err { + t.Errorf("unexpected error: %v", err) + } + case tc.err: + t.Error("failed to received expected error") + } + }) + } +}