Improve tot request code

Handling some cases where `err` asignment would not have happened
correctly and the error would have been dropped, as well as using an
exponential backoff and reducing the total number of retries done.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

prow/pjutil/jobspec.go

@@ -22,7 +22,9 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
+	"path"
 	"strconv"
 	"time"
 
@@ -63,21 +65,30 @@ func NewJobSpec(spec kube.ProwJobSpec, buildId, prowJobId string) JobSpec {
 // to vend build identifier for the job
 func GetBuildID(name, totURL string) (string, error) {
 	var err error
-	url := totURL + "/vend/" + name
-	for retries := 0; retries < 60; retries++ {
+	url, err := url.Parse(totURL)
+	if err != nil {
+		return "", fmt.Errorf("invalid tot url: %v", err)
+	}
+	url.Path = path.Join(url.Path, "vend", name)
+	sleep := 100 * time.Millisecond
+	for retries := 0; retries < 10; retries++ {
 		if retries > 0 {
-			time.Sleep(2 * time.Second)
+			time.Sleep(sleep)
+			sleep = sleep * 2
 		}
 		var resp *http.Response
-		resp, err = http.Get(url)
+		resp, err = http.Get(url.String())
 		if err != nil {
 			continue
 		}
 		defer resp.Body.Close()
 		if resp.StatusCode != 200 {
+			err = fmt.Errorf("got unexpected response from tot: %v", resp.Status)
 			continue
 		}
-		if buf, err := ioutil.ReadAll(resp.Body); err == nil {
+		var buf []byte
+		buf, err = ioutil.ReadAll(resp.Body)
+		if err == nil {
 			return string(buf), nil
 		}
 		return "", err

prow/pjutil/jobspec_test.go

@@ -17,6 +17,9 @@ limitations under the License.
 package pjutil
 
 import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"reflect"
 	"testing"
 
@@ -190,3 +193,82 @@ func TestEnvironmentForSpec(t *testing.T) {
 		}
 	}
 }
+
+type responseVendor struct {
+	codes []int
+	data  []string
+
+	position int
+}
+
+func (r *responseVendor) next() (int, string) {
+	code := r.codes[r.position]
+	datum := r.data[r.position]
+
+	r.position = r.position + 1
+	if r.position == len(r.codes) {
+		r.position = 0
+	}
+
+	return code, datum
+}
+
+func parrotServer(codes []int, data []string) *httptest.Server {
+	vendor := responseVendor{
+		codes: codes,
+		data:  data,
+	}
+
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		code, datum := vendor.next()
+		w.WriteHeader(code)
+		fmt.Fprint(w, datum)
+	}))
+}
+
+func TestGetBuildID(t *testing.T) {
+	var testCases = []struct {
+		name        string
+		codes       []int
+		data        []string
+		expected    string
+		expectedErr bool
+	}{
+		{
+			name:        "all good",
+			codes:       []int{200},
+			data:        []string{"yay"},
+			expected:    "yay",
+			expectedErr: false,
+		},
+		{
+			name:        "fail then success",
+			codes:       []int{500, 200},
+			data:        []string{"boo", "yay"},
+			expected:    "yay",
+			expectedErr: false,
+		},
+		{
+			name:        "fail",
+			codes:       []int{500},
+			data:        []string{"boo"},
+			expected:    "boo",
+			expectedErr: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		totServ := parrotServer(testCase.codes, testCase.data)
+
+		actual, actualErr := GetBuildID("dummy", totServ.URL)
+		if testCase.expectedErr && actualErr == nil {
+			t.Errorf("%s: expected an error but got none", testCase.name)
+		} else if !testCase.expectedErr && actualErr != nil {
+			t.Errorf("%s: expected no error but got one: %v", testCase.name, actualErr)
+		} else if !testCase.expectedErr && actual != testCase.expected {
+			t.Errorf("%s: expected response %v but got: %v", testCase.name, testCase.expected, actual)
+		}
+
+		totServ.Close()
+	}
+}