Merge pull request #1205 from freehan/autopriority

add auto prioritization for flake issues

Author: lavalamp

mungegithub/mungers/flake-manager.go

@@ -27,8 +27,10 @@ import (
 	"k8s.io/contrib/mungegithub/mungers/testowner"
 	"k8s.io/contrib/test-utils/utils"
 
-	// "github.com/golang/glog"
+	"github.com/golang/glog"
+	libgithub "github.com/google/go-github/github"
 	"github.com/spf13/cobra"
+	"time"
 )
 
 // issueFinder finds an issue for a given key.
@@ -97,6 +99,7 @@ func (p *FlakeManager) EachLoop() error {
 		return fmt.Errorf("submit queue not initialized yet")
 	}
 	if !p.finder.Synced() {
+		glog.V(3).Infof("issue-cache is not synced. flake-manager is skipping this run.")
 		return nil
 	}
 	p.sq.e2e.GCSBasedStable()
@@ -207,7 +210,17 @@ func (p *individualFlakeSource) Body(newIssue bool) string {
 
 // Labels implements IssueSource
 func (p *individualFlakeSource) Labels() []string {
-	return []string{"kind/flake"}
+	return []string{"kind/flake", sync.PriorityP2.String()}
+}
+
+// Priority implements IssueSource
+func (p *individualFlakeSource) Priority(obj *github.MungeObject) (sync.Priority, error) {
+	comments, err := obj.ListComments()
+	if err != nil {
+		return sync.PriorityP2, fmt.Errorf("Failed to list comment of issue: %v", err)
+	}
+	// Different IssueSource's Priority calculation may differ
+	return autoPrioritize(comments, obj.Issue.CreatedAt), nil
 }
 
 type brokenJobSource struct {
@@ -270,5 +283,56 @@ func (p *brokenJobSource) Body(newIssue bool) string {
 
 // Labels implements IssueSource
 func (p *brokenJobSource) Labels() []string {
-	return []string{"kind/flake", "team/test-infra"}
+	return []string{"kind/flake", "team/test-infra", sync.PriorityP2.String()}
+}
+
+// Priority implements IssueSource
+func (p *brokenJobSource) Priority(obj *github.MungeObject) (sync.Priority, error) {
+	comments, err := obj.ListComments()
+	if err != nil {
+		return sync.PriorityP2, fmt.Errorf("Failed to list comment of issue: %v", err)
+	}
+	// Different IssueSource's Priority calculation may differ
+	return autoPrioritize(comments, obj.Issue.CreatedAt), nil
+}
+
+// autoPrioritize prioritize flake issue based on the number of flakes
+func autoPrioritize(comments []libgithub.IssueComment, issueCreatedAt *time.Time) sync.Priority {
+	occurence := []*time.Time{issueCreatedAt}
+	lastMonth := time.Now().Add(-1 * 30 * 24 * time.Hour)
+	lastWeek := time.Now().Add(-1 * 7 * 24 * time.Hour)
+	// number of flakes happened in this month
+	monthCount := 0
+	// number of flakes happened in this week
+	weekCount := 0
+
+	for _, c := range comments {
+		// TODO: think of a better way to identify flake comments
+		// "Failed:" is a special string contained in flake issue filed by flake-manager
+		// Please make sure it matches the body generated by IssueSource.Body()
+		if !sync.RobotUser.Has(*c.User.Login) || !strings.Contains(*c.Body, "Failed:") {
+			continue
+		}
+		occurence = append(occurence, c.CreatedAt)
+	}
+
+	for _, o := range occurence {
+		if lastMonth.Before(*o) {
+			monthCount += 1
+		}
+		if lastWeek.Before(*o) {
+			weekCount += 1
+		}
+	}
+
+	// P2: By default
+	// P1: Flake happens more than once in last month.
+	// P0: Flake happens more than twice in last week.
+	p := sync.PriorityP2
+	if weekCount >= 3 {
+		p = sync.PriorityP0
+	} else if monthCount >= 2 {
+		p = sync.PriorityP1
+	}
+	return p
 }

mungegithub/mungers/flake-manager_test.go

@@ -20,9 +20,12 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-github/github"
+	github_testing "k8s.io/contrib/mungegithub/github/testing"
 	cache "k8s.io/contrib/mungegithub/mungers/flakesync"
 	"k8s.io/contrib/mungegithub/mungers/sync"
 	"k8s.io/contrib/test-utils/utils"
+	"time"
 )
 
 func makeTestFlakeManager() *FlakeManager {
@@ -82,3 +85,70 @@ func TestBrokenJobSource(t *testing.T) {
 	expect(t, source.Title(), "e2e-gce: broken test run")
 	checkCommon(t, &source)
 }
+
+func flakecomment(id int, createdAt time.Time) github.IssueComment {
+	return github_testing.Comment(id, "k8s-bot", createdAt, "Failed:")
+}
+
+func TestAutoPrioritize(t *testing.T) {
+	testcases := []struct {
+		comments       []github.IssueComment
+		issueCreatedAt time.Time
+		expectPriority int
+	}{
+		// New flake issue
+		{
+			comments:       []github.IssueComment{},
+			issueCreatedAt: time.Now(),
+			expectPriority: 2,
+		},
+		{
+			comments: []github.IssueComment{
+				flakecomment(1, time.Now()),
+			},
+			issueCreatedAt: time.Now().Add(-1 * 29 * 24 * time.Hour),
+			expectPriority: 1,
+		},
+		{
+			comments: []github.IssueComment{
+				flakecomment(1, time.Now()),
+				flakecomment(1, time.Now().Add(-1*3*24*time.Hour)),
+				flakecomment(1, time.Now().Add(-1*6*24*time.Hour)),
+			},
+			issueCreatedAt: time.Now().Add(-1 * 30 * 24 * time.Hour),
+			expectPriority: 0,
+		},
+		{
+			comments: []github.IssueComment{
+				flakecomment(1, time.Now()),
+				flakecomment(1, time.Now().Add(-8*24*time.Hour)),
+			},
+			issueCreatedAt: time.Now().Add(-1 * 29 * 24 * time.Hour),
+			expectPriority: 1,
+		},
+		{
+			comments: []github.IssueComment{
+				flakecomment(1, time.Now()),
+				flakecomment(1, time.Now().Add(-8*24*time.Hour)),
+				flakecomment(1, time.Now().Add(-15*24*time.Hour)),
+				flakecomment(1, time.Now().Add(-20*24*time.Hour)),
+			},
+			issueCreatedAt: time.Now().Add(-1 * 29 * 24 * time.Hour),
+			expectPriority: 1,
+		},
+		{
+			comments: []github.IssueComment{
+				flakecomment(1, time.Now()),
+				flakecomment(1, time.Now().Add(-1*3*24*time.Hour)),
+			},
+			issueCreatedAt: time.Now().Add(-1 * 6 * 24 * time.Hour),
+			expectPriority: 0,
+		},
+	}
+	for _, tc := range testcases {
+		p := autoPrioritize(tc.comments, &tc.issueCreatedAt)
+		if p.Priority() != tc.expectPriority {
+			t.Errorf("Expected priority: %d, But got: %d", tc.expectPriority, p.Priority())
+		}
+	}
+}

mungegithub/mungers/sync/issue-sync.go

@@ -25,6 +25,38 @@ import (
 	"k8s.io/kubernetes/pkg/util/sets"
 )
 
+const (
+	// BotName is the name of merge-bot
+	BotName = "k8s-merge-robot"
+	// JenkinsBotName is the name of kubekins bot
+	JenkinsBotName = "k8s-bot"
+	priorityPrefix = "priority/P"
+	// PriorityP0 represents Priority P0
+	PriorityP0 = Priority(0)
+	// PriorityP1 represents Priority P1
+	PriorityP1 = Priority(1)
+	// PriorityP2 represents Priority P2
+	PriorityP2 = Priority(2)
+	// PriorityP3 represents Priority P3
+	PriorityP3 = Priority(3)
+)
+
+// RobotUser is a set of name of robot user
+var RobotUser = sets.NewString(JenkinsBotName, BotName)
+
+// Priority represents the priority label in an issue
+type Priority int
+
+// String return the priority label in string
+func (p Priority) String() string {
+	return fmt.Sprintf(priorityPrefix+"%d", p)
+}
+
+// Priority returns the priority in int
+func (p Priority) Priority() int {
+	return int(p)
+}
+
 // OwnerMapper finds an owner for a given test name.
 type OwnerMapper interface {
 	// TestOwner returns a GitHub username for a test, or "" if none are found.
@@ -59,6 +91,9 @@ type IssueSource interface {
 
 	// If an issue is filed, these labels will be applied.
 	Labels() []string
+
+	// Priority calculates and returns the priority of an flake issue
+	Priority(obj *github.MungeObject) (Priority, error)
 }
 
 // IssueSyncer implements robust issue syncing logic and won't file duplicates etc.
@@ -105,23 +140,25 @@ func (s *IssueSyncer) Sync(source IssueSource) error {
 		return nil
 	}
 
+	var obj *github.MungeObject
 	// Update an issue if possible.
 	if len(updatableIssues) > 0 {
-		obj := updatableIssues[0]
+		obj = updatableIssues[0]
 		// Update the chosen issue
-		if err := s.updateIssue(obj, source); err != nil {
+		if err = s.updateIssue(obj, source); err != nil {
 			return fmt.Errorf("error updating issue %v for %v: %v", *obj.Issue.Number, source.ID(), err)
 		}
 		s.synced.Insert(source.ID())
 		return nil
 	}
 
 	// No issue could be updated, create a new issue.
-	n, err := s.createIssue(source)
+	obj, err = s.createIssue(source)
 	if err != nil {
 		return fmt.Errorf("error making issue for %v: %v", source.ID, err)
 	}
-	s.finder.Created(source.Title(), n)
+	issueNum := *obj.Issue.Number
+	s.finder.Created(source.Title(), issueNum)
 	s.synced.Insert(source.ID())
 	return nil
 }
@@ -196,12 +233,20 @@ func (s *IssueSyncer) updateIssue(obj *github.MungeObject, source IssueSource) e
 		panic(fmt.Errorf("Programmer error: %v does not contain %v!", body, id))
 	}
 	glog.Infof("Updating issue %v with item %v", *obj.Issue.Number, source.ID())
-	return obj.WriteComment(body)
+	if err := obj.WriteComment(body); err != nil {
+		return err
+	}
+	p, err := source.Priority(obj)
+	if err != nil {
+		return err
+	}
+	return s.syncPriority(obj, p)
+
 }
 
 // createIssue makes a new issue for the given item. If we know about other
 // issues for the item, then they'll be referenced.
-func (s *IssueSyncer) createIssue(source IssueSource) (issueNumber int, err error) {
+func (s *IssueSyncer) createIssue(source IssueSource) (*github.MungeObject, error) {
 	body := source.Body(true)
 	id := source.ID()
 	if !strings.Contains(body, source.ID()) {
@@ -221,8 +266,27 @@ func (s *IssueSyncer) createIssue(source IssueSource) (issueNumber int, err erro
 		owner,
 	)
 	if err != nil {
-		return 0, err
+		return nil, err
 	}
 	glog.Infof("Created issue %v:\n%v", *obj.Issue.Number, body)
-	return *obj.Issue.Number, nil
+	return obj, nil
+}
+
+// syncPriority will sync the input priority to the issue if the input priority is higher than the existing ones
+func (s *IssueSyncer) syncPriority(obj *github.MungeObject, priority Priority) error {
+	if obj.Priority() <= priority.Priority() {
+		return nil
+	}
+	plabels := github.GetLabelsWithPrefix(obj.Issue.Labels, priorityPrefix)
+	err := obj.AddLabel(priority.String())
+	if err != nil {
+		return nil
+	}
+	for _, l := range plabels {
+		err = obj.RemoveLabel(l)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
 }