Merge pull request #1193 from lavalamp/ff5

File issues for failures even in non-blocking jobs

Author: eparis

hack/verify-flags/known-flags.txt

@@ -49,6 +49,7 @@ max-pr-number
 max-sync-failures
 min-pr-number
 network-config
+nonblocking-jenkins-jobs
 nginx-configmap
 number-of-old-test-results
 on-change

mungegithub/mungers/e2e/e2e.go

@@ -49,8 +49,9 @@ type BuildInfo struct {
 // RealE2ETester is the object which will get status from a google bucket
 // information about recent jobs
 type RealE2ETester struct {
-	JobNames           []string
-	WeakStableJobNames []string
+	BlockingJobNames    []string
+	NonBlockingJobNames []string
+	WeakStableJobNames  []string
 
 	sync.Mutex
 	BuildStatus          map[string]BuildInfo // protect by mutex
@@ -143,6 +144,11 @@ func (e *RealE2ETester) getGCSResult(j cache.Job, n cache.Number) (*cache.Result
 	}
 	if len(thisFailures) == 0 {
 		r.Status = cache.ResultFailed
+		// We add a "flake" just to make sure this appears in the flake
+		// cache as something that needs to be synced.
+		r.Flakes = map[cache.Test]string{
+			cache.RunBrokenTestName: "Unable to get data-- please look at the logs",
+		}
 		return r, nil
 	}
 
@@ -159,7 +165,7 @@ func (e *RealE2ETester) getGCSResult(j cache.Job, n cache.Number) (*cache.Result
 func (e *RealE2ETester) GCSBasedStable() (allStable, ignorableFlakes bool) {
 	allStable = true
 
-	for _, job := range e.JobNames {
+	for _, job := range e.BlockingJobNames {
 		lastBuildNumber, err := e.GoogleGCSBucketUtils.GetLastestBuildNumberFromJenkinsGoogleBucket(job)
 		glog.V(4).Infof("Checking status of %v, %v", job, lastBuildNumber)
 		if err != nil {
@@ -217,6 +223,23 @@ func (e *RealE2ETester) GCSBasedStable() (allStable, ignorableFlakes bool) {
 		e.setBuildStatus(job, "Not Stable", strconv.Itoa(lastBuildNumber))
 	}
 
+	// Also get status for non-blocking jobs
+	for _, job := range e.NonBlockingJobNames {
+		lastBuildNumber, err := e.GoogleGCSBucketUtils.GetLastestBuildNumberFromJenkinsGoogleBucket(job)
+		glog.V(4).Infof("Checking status of %v, %v", job, lastBuildNumber)
+		if err != nil {
+			glog.Errorf("Error while getting data for %v: %v", job, err)
+			e.setBuildStatus(job, "[nonblocking] Not Stable", strconv.Itoa(lastBuildNumber))
+			continue
+		}
+
+		if thisResult, err := e.GetBuildResult(job, lastBuildNumber); err != nil || thisResult.Status != cache.ResultStable {
+			e.setBuildStatus(job, "[nonblocking] Not Stable", strconv.Itoa(lastBuildNumber))
+		} else {
+			e.setBuildStatus(job, "[nonblocking] Stable", strconv.Itoa(lastBuildNumber))
+		}
+	}
+
 	return allStable, ignorableFlakes
 }
 

mungegithub/mungers/e2e/e2e_test.go

@@ -47,18 +47,19 @@ func marshalOrDie(obj interface{}, t *testing.T) []byte {
 }
 
 func genMockGCSListResponse(files ...string) []byte {
-	resptemplate := "{\"items\":[%s]}"
-	itemTempalte := "{\"name\":\"%s\"}"
+	respTemplate := "{\"items\":[%s]}"
+	itemTemplate := "{\"name\":\"%s\"}"
 	items := []string{}
 	for _, file := range files {
-		items = append(items, fmt.Sprintf(itemTempalte, file))
+		items = append(items, fmt.Sprintf(itemTemplate, file))
 	}
-	return []byte(fmt.Sprintf(resptemplate, strings.Join(items, ",")))
+	return []byte(fmt.Sprintf(respTemplate, strings.Join(items, ",")))
 }
 
 func TestCheckGCSBuilds(t *testing.T) {
 	latestBuildNumberFoo := 42
 	latestBuildNumberBar := 44
+	latestBuildNumberBaz := 99
 	tests := []struct {
 		paths             map[string][]byte
 		expectStable      bool
@@ -77,12 +78,18 @@ func TestCheckGCSBuilds(t *testing.T) {
 					Result:    "SUCCESS",
 					Timestamp: 1234,
 				}, t),
+				"/baz/latest-build.txt": []byte(strconv.Itoa(latestBuildNumberBaz)),
+				fmt.Sprintf("/baz/%v/finished.json", latestBuildNumberBaz): marshalOrDie(utils.FinishedFile{
+					Result:    "UNSTABLE",
+					Timestamp: 1234,
+				}, t),
 				"/": genMockGCSListResponse(),
 			},
 			expectStable: true,
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Stable", ID: "42"},
 				"bar": {Status: "Stable", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "99"},
 			},
 		},
 		{
@@ -97,12 +104,18 @@ func TestCheckGCSBuilds(t *testing.T) {
 					Result:    "UNSTABLE",
 					Timestamp: 1234,
 				}, t),
+				"/baz/latest-build.txt": []byte(strconv.Itoa(latestBuildNumberBaz)),
+				fmt.Sprintf("/baz/%v/finished.json", latestBuildNumberBaz): marshalOrDie(utils.FinishedFile{
+					Result:    "SUCCESS",
+					Timestamp: 1234,
+				}, t),
 				"/": genMockGCSListResponse(),
 			},
 			expectStable: false,
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Stable", ID: "42"},
 				"bar": {Status: "Not Stable", ID: "44"},
+				"baz": {Status: "[nonblocking] Stable", ID: "99"},
 			},
 		},
 		{
@@ -140,6 +153,7 @@ func TestCheckGCSBuilds(t *testing.T) {
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Stable", ID: "42"},
 				"bar": {Status: "Ignorable flake", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "-1"},
 			},
 		},
 		{
@@ -177,6 +191,7 @@ func TestCheckGCSBuilds(t *testing.T) {
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Stable", ID: "42"},
 				"bar": {Status: "Ignorable flake", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "-1"},
 			},
 		},
 		{
@@ -214,6 +229,7 @@ func TestCheckGCSBuilds(t *testing.T) {
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Stable", ID: "42"},
 				"bar": {Status: "Not Stable", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "-1"},
 			},
 		},
 
@@ -235,6 +251,7 @@ func TestCheckGCSBuilds(t *testing.T) {
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Stable", ID: "42"},
 				"bar": {Status: "Not Stable", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "-1"},
 			},
 		},
 		{
@@ -255,6 +272,7 @@ func TestCheckGCSBuilds(t *testing.T) {
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Not Stable", ID: "42"},
 				"bar": {Status: "Not Stable", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "-1"},
 			},
 		},
 		{
@@ -275,6 +293,7 @@ func TestCheckGCSBuilds(t *testing.T) {
 			expectedStatus: map[string]BuildInfo{
 				"foo": {Status: "Not Stable", ID: "42"},
 				"bar": {Status: "Stable", ID: "44"},
+				"baz": {Status: "[nonblocking] Not Stable", ID: "-1"},
 			},
 		},
 	}
@@ -292,10 +311,13 @@ func TestCheckGCSBuilds(t *testing.T) {
 			},
 		})
 		e2e := &RealE2ETester{
-			JobNames: []string{
+			BlockingJobNames: []string{
 				"foo",
 				"bar",
 			},
+			NonBlockingJobNames: []string{
+				"baz",
+			},
 			BuildStatus:          map[string]BuildInfo{},
 			GoogleGCSBucketUtils: utils.NewTestUtils(server.URL),
 		}

mungegithub/mungers/flake-manager.go

@@ -124,18 +124,23 @@ func (p *FlakeManager) isIndividualFlake(f cache.Flake) bool {
 		return false
 	}
 
+	if len(f.Result.Flakes) > 0 {
+		// If this flake really represents an entire suite failure,
+		// this key will be present.
+		if _, ok := f.Result.Flakes[cache.RunBrokenTestName]; ok {
+			return false
+		}
+	}
+
 	return true
 }
 
 func (p *FlakeManager) listPreviousIssues(title string) []string {
-	if previousIssues := p.finder.AllIssuesForKey(title); len(previousIssues) > 0 {
-		s := []string{}
-		for _, i := range previousIssues {
-			s = append(s, fmt.Sprintf("#%v", i))
-		}
-		return s
+	s := []string{}
+	for _, i := range p.finder.AllIssuesForKey(title) {
+		s = append(s, fmt.Sprintf("#%v", i))
 	}
-	return nil
+	return s
 }
 
 // makeGubernatorLink returns a URL to view the build results in a GCS path.

mungegithub/mungers/flakesync/cache.go

@@ -44,6 +44,10 @@ const (
 	// ResultFailed means it failed without generating readable JUnit
 	// files, and introspection is not possible.
 	ResultFailed ResultStatus = "failed"
+
+	// RunBrokenTestName names a "flake" which really represents the fact
+	// that the entire run was broken.
+	RunBrokenTestName Test = "Suite so broken it failed to produce JUnit output"
 )
 
 // Result records a test job completion.

mungegithub/mungers/submit-queue.go

@@ -133,11 +133,12 @@ type submitQueueInterruptedObject struct {
 // SubmitQueue will merge PR which meet a set of requirements.
 //  PR must have LGTM after the last commit
 //  PR must have passed all github CI checks
-//  The google internal jenkins instance must be passing the JobNames e2e tests
+//  The google internal jenkins instance must be passing the BlockingJobNames e2e tests
 type SubmitQueue struct {
-	githubConfig       *github.Config
-	JobNames           []string
-	WeakStableJobNames []string
+	githubConfig        *github.Config
+	BlockingJobNames    []string
+	NonBlockingJobNames []string
+	WeakStableJobNames  []string
 
 	// If FakeE2E is true, don't try to connect to JenkinsHost, all jobs are passing.
 	FakeE2E bool
@@ -302,7 +303,8 @@ func (sq *SubmitQueue) internalInitialize(config *github.Config, features *featu
 	defer sq.Unlock()
 
 	// Clean up all of our flags which we wish --flag="" to mean []string{}
-	sq.JobNames = cleanStringSlice(sq.JobNames)
+	sq.BlockingJobNames = cleanStringSlice(sq.BlockingJobNames)
+	sq.NonBlockingJobNames = cleanStringSlice(sq.NonBlockingJobNames)
 	sq.WeakStableJobNames = cleanStringSlice(sq.WeakStableJobNames)
 	sq.RequiredStatusContexts = cleanStringSlice(sq.RequiredStatusContexts)
 	sq.RequiredRetestContexts = cleanStringSlice(sq.RequiredRetestContexts)
@@ -313,7 +315,7 @@ func (sq *SubmitQueue) internalInitialize(config *github.Config, features *featu
 	// TODO: This is not how injection for tests should work.
 	if sq.FakeE2E {
 		sq.e2e = &fake_e2e.FakeE2ETester{
-			JobNames:           sq.JobNames,
+			JobNames:           sq.BlockingJobNames,
 			WeakStableJobNames: sq.WeakStableJobNames,
 		}
 	} else {
@@ -325,7 +327,8 @@ func (sq *SubmitQueue) internalInitialize(config *github.Config, features *featu
 		}
 
 		sq.e2e = (&e2e.RealE2ETester{
-			JobNames:             sq.JobNames,
+			BlockingJobNames:     sq.BlockingJobNames,
+			NonBlockingJobNames:  sq.NonBlockingJobNames,
 			WeakStableJobNames:   sq.WeakStableJobNames,
 			BuildStatus:          map[string]e2e.BuildInfo{},
 			GoogleGCSBucketUtils: gcs,
@@ -387,7 +390,13 @@ func (sq *SubmitQueue) EachLoop() error {
 
 // AddFlags will add any request flags to the cobra `cmd`
 func (sq *SubmitQueue) AddFlags(cmd *cobra.Command, config *github.Config) {
-	cmd.Flags().StringSliceVar(&sq.JobNames, "jenkins-jobs", []string{
+	cmd.Flags().StringSliceVar(&sq.NonBlockingJobNames, "nonblocking-jenkins-jobs", []string{
+		"kubernetes-e2e-gke-prod",
+		"kubernetes-e2e-gke-subnet",
+		"kubernetes-e2e-gke-test",
+		"kubernetes-e2e-gce-examples",
+	}, "Comma separated list of jobs that don't block merges, but will have status reported and issues filed.")
+	cmd.Flags().StringSliceVar(&sq.BlockingJobNames, "jenkins-jobs", []string{
 		"kubelet-gce-e2e-ci",
 		"kubernetes-build",
 		"kubernetes-test-go",
@@ -399,7 +408,7 @@ func (sq *SubmitQueue) AddFlags(cmd *cobra.Command, config *github.Config) {
 		"kubernetes-e2e-gke-slow",
 		"kubernetes-e2e-gce-scalability",
 		"kubernetes-kubemark-5-gce",
-	}, "Comma separated list of jobs in Jenkins to use for stability testing")
+	}, "Comma separated list of jobs in Jenkins that should block merges if failing.")
 	cmd.Flags().StringSliceVar(&sq.WeakStableJobNames, "weak-stable-jobs",
 		[]string{},
 		"Comma separated list of jobs in Jenkins to use for stability testing that needs only weak success")
@@ -426,7 +435,8 @@ func (sq *SubmitQueue) updateHealth() {
 		Jobs:    map[string]bool{},
 	}
 	for job, status := range sq.e2e.GetBuildStatus() {
-		newEntry.Jobs[job] = status.Status == "Stable"
+		// Ignore flakes.
+		newEntry.Jobs[job] = status.Status != "Not Stable"
 	}
 	sq.healthHistory = append(sq.healthHistory, newEntry)
 	// Now compute the health structure so we don't have to do it on page load

mungegithub/mungers/submit-queue_test.go

@@ -182,7 +182,7 @@ func getTestSQ(startThreads bool, config *github_util.Config, server *httptest.S
 	sq := new(SubmitQueue)
 	sq.RequiredStatusContexts = []string{notRequiredReTestContext1, notRequiredReTestContext2}
 	sq.RequiredRetestContexts = []string{requiredReTestContext1, requiredReTestContext2}
-	sq.JobNames = []string{"foo"}
+	sq.BlockingJobNames = []string{"foo"}
 	sq.WeakStableJobNames = []string{"bar"}
 	sq.githubE2EQueue = map[int]*github_util.MungeObject{}
 	sq.githubE2EPollTime = 50 * time.Millisecond
@@ -199,7 +199,7 @@ func getTestSQ(startThreads bool, config *github_util.Config, server *httptest.S
 	sq.doNotMergeMilestones = []string{doNotMergeMilestone}
 
 	sq.e2e = &fake_e2e.FakeE2ETester{
-		JobNames:           sq.JobNames,
+		JobNames:           sq.BlockingJobNames,
 		WeakStableJobNames: sq.WeakStableJobNames,
 	}