Merge pull request #9124 from csrwng/git_check_backoff

Merged by openshift-bot

Author: OpenShift Bot

pkg/build/builder/docker.go

@@ -48,7 +48,7 @@ func NewDockerBuilder(dockerClient DockerClient, buildsClient client.BuildInterf
 		build:        build,
 		gitClient:    gitClient,
 		tar:          tar.New(),
-		urlTimeout:   urlCheckTimeout,
+		urlTimeout:   initialURLCheckTimeout,
 		client:       buildsClient,
 		cgLimits:     cgLimits,
 	}

pkg/build/builder/source.go

@@ -21,10 +21,15 @@ import (
 )
 
 const (
-	// urlCheckTimeout is the timeout used to check the source URL
-	// If fetching the URL exceeds the timeout, then the build will
-	// not proceed further and stop
-	urlCheckTimeout = 16 * time.Second
+	// initialURLCheckTimeout is the initial timeout used to check the
+	// source URL.  If fetching the URL exceeds the timeout, then a longer
+	// timeout will be tried until the fetch either succeeds or the build
+	// itself times out.
+	initialURLCheckTimeout = 16 * time.Second
+
+	// timeoutIncrementFactor is the factor to use when increasing
+	// the timeout after each unsuccessful try
+	timeoutIncrementFactor = 4
 )
 
 type gitAuthError string
@@ -104,35 +109,42 @@ func fetchSource(dockerClient DockerClient, dir string, build *api.Build, urlTim
 // remote repository failed to authenticate.
 // Since this is calling the 'git' binary, the proxy settings should be
 // available for this command.
-func checkRemoteGit(gitClient GitClient, url string, timeout time.Duration) error {
-	glog.V(4).Infof("git ls-remote --heads %s", url)
+func checkRemoteGit(gitClient GitClient, url string, initialTimeout time.Duration) error {
 
 	var (
 		out    string
 		errOut string
 		err    error
 	)
 
-	out, errOut, err = gitClient.TimedListRemote(timeout, url, "--heads")
-	if _, ok := err.(*git.TimeoutError); err != nil && ok {
-		return fmt.Errorf("timeout while waiting for remote repository %q", url)
-	}
-
-	if len(out) != 0 {
-		glog.V(4).Infof(out)
-	}
-	if len(errOut) != 0 {
-		glog.V(4).Infof(errOut)
+	timeout := initialTimeout
+	for {
+		glog.V(4).Infof("git ls-remote --heads %s", url)
+		out, errOut, err = gitClient.TimedListRemote(timeout, url, "--heads")
+		if len(out) != 0 {
+			glog.V(4).Infof(out)
+		}
+		if len(errOut) != 0 {
+			glog.V(4).Infof(errOut)
+		}
+		if err != nil {
+			if _, ok := err.(*git.TimeoutError); ok {
+				timeout = timeout * timeoutIncrementFactor
+				glog.Infof("WARNING: timed out waiting for git server, will wait %s", timeout)
+				continue
+			}
+		}
+		break
 	}
-
-	combinedOut := out + errOut
-	switch {
-	case strings.Contains(combinedOut, "Authentication failed"):
-		return gitAuthError(url)
-	case strings.Contains(combinedOut, "not found"):
-		return gitNotFoundError(url)
+	if err != nil {
+		combinedOut := out + errOut
+		switch {
+		case strings.Contains(combinedOut, "Authentication failed"):
+			return gitAuthError(url)
+		case strings.Contains(combinedOut, "not found"):
+			return gitNotFoundError(url)
+		}
 	}
-
 	return err
 }
 

pkg/build/builder/source_test.go

@@ -32,16 +32,6 @@ func TestCheckRemoteGit(t *testing.T) {
 		t.Errorf("expected gitAuthError, got %q", v)
 	}
 
-	t0 := time.Now()
-	err = checkRemoteGit(gitRepo, "https://254.254.254.254/foo/bar", 4*time.Second)
-	t1 := time.Now()
-	if err == nil || (err != nil && !strings.Contains(fmt.Sprintf("%s", err), "timeout")) {
-		t.Errorf("expected timeout error, got %q", err)
-	}
-	if t1.Sub(t0) > 5*time.Second {
-		t.Errorf("expected timeout in 4 seconds, it took %v", t1.Sub(t0))
-	}
-
 	err = checkRemoteGit(gitRepo, "https://github.com/openshift/origin", 10*time.Second)
 	if err != nil {
 		t.Errorf("unexpected error %q", err)

pkg/build/builder/sti.go

@@ -115,7 +115,7 @@ func (s *S2IBuilder) Build() error {
 	download := &downloader{
 		s:       s,
 		in:      os.Stdin,
-		timeout: urlCheckTimeout,
+		timeout: initialURLCheckTimeout,
 
 		dir:        srcDir,
 		contextDir: contextDir,