diff --git a/pkg/keeper/keeper.go b/pkg/keeper/keeper.go index d67f57909..26d58ada3 100644 --- a/pkg/keeper/keeper.go +++ b/pkg/keeper/keeper.go @@ -24,6 +24,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/jenkins-x/lighthouse/pkg/labels" "net/http" "os" "sort" @@ -79,6 +80,7 @@ type scmProviderClient interface { GetFile(string, string, string, string) ([]byte, error) ListFiles(string, string, string, string) ([]*scm.FileEntry, error) GetIssueLabels(string, string, int, bool) ([]*scm.Label, error) + AddLabel(string, string, int, string, bool) error } type contextChecker interface { @@ -603,16 +605,20 @@ func (c *DefaultController) initSubpoolData(sp *subpool) error { // should be deleted. func filterSubpool(spc scmProviderClient, sp *subpool) *subpool { var toKeep []PullRequest + var conflicting []int for _, pr := range sp.prs { p := pr if !filterPR(spc, sp, &p) { toKeep = append(toKeep, pr) + } else if pr.Mergeable == githubql.MergeableStateConflicting && !hasLabel(&pr, labels.HasConflicts) { + conflicting = append(conflicting, int(pr.Number)) } } if len(toKeep) == 0 { return nil } sp.prs = toKeep + sp.conflictingPRs = conflicting return sp } @@ -841,7 +847,7 @@ func accumulate(presubmits map[int][]job.Presubmit, prs []PullRequest, pjs []v1a missingTests = map[int][]job.Presubmit{} for _, pr := range prs { // Accumulate the best result for each job (Passing > Pending > Failing/Unknown) - // We can ignore the baseSHA here because the subPool only contains PipelineActivitys with the correct baseSHA + // We can ignore the baseSHA here because the subPool only contains LighthouseJobs with the correct baseSHA psStates := make(map[string]simpleState) for _, pj := range pjs { if pj.Spec.Type != job.PresubmitJob { @@ -957,6 +963,11 @@ func (c *DefaultController) pickBatch(sp subpool, cc contextChecker) ([]PullRequ } } } + for _, prNumber := range sp.conflictingPRs { + if err := c.spc.AddLabel(sp.org, sp.repo, prNumber, labels.HasConflicts, true); err != nil { + sp.log.Warnf("error while adding Label %q: %v", labels.HasConflicts, err) + } + } return res, nil } @@ -1486,13 +1497,13 @@ func prMeta(prs ...PullRequest) []v1alpha1.Pull { func sortPools(pools []Pool) { sort.Slice(pools, func(i, j int) bool { - if string(pools[i].Org) != string(pools[j].Org) { - return string(pools[i].Org) < string(pools[j].Org) + if pools[i].Org != pools[j].Org { + return pools[i].Org < pools[j].Org } - if string(pools[i].Repo) != string(pools[j].Repo) { - return string(pools[i].Repo) < string(pools[j].Repo) + if pools[i].Repo != pools[j].Repo { + return pools[i].Repo < pools[j].Repo } - return string(pools[i].Branch) < string(pools[j].Branch) + return pools[i].Branch < pools[j].Branch }) sortPRs := func(prs []PullRequest) { @@ -1517,8 +1528,9 @@ type subpool struct { // ljs contains all LighthouseJobs of type Presubmit or Batch // that have the same baseSHA as the subpool - ljs []v1alpha1.LighthouseJob - prs []PullRequest + ljs []v1alpha1.LighthouseJob + prs []PullRequest + conflictingPRs []int cc contextChecker // presubmit contains all required presubmits for each PR @@ -1907,13 +1919,13 @@ func scmPRToGraphQLPR(scmPR *scm.PullRequest, scmRepo *scm.Repository) *PullRequ mergeable = githubql.MergeableStateConflicting } - labels := struct { + qlLabels := struct { Nodes []struct { Name githubql.String } }{} for _, l := range scmPR.Labels { - labels.Nodes = append(labels.Nodes, struct{ Name githubql.String }{Name: githubql.String(l.Name)}) + qlLabels.Nodes = append(qlLabels.Nodes, struct{ Name githubql.String }{Name: githubql.String(l.Name)}) } return &PullRequest{ @@ -1924,7 +1936,7 @@ func scmPRToGraphQLPR(scmPR *scm.PullRequest, scmRepo *scm.Repository) *PullRequ HeadRefOID: githubql.String(scmPR.Head.Sha), Mergeable: mergeable, Repository: scmRepoToGraphQLRepo(scmRepo), - Labels: labels, + Labels: qlLabels, Body: githubql.String(scmPR.Body), Title: githubql.String(scmPR.Title), UpdatedAt: githubql.DateTime{Time: scmPR.Updated}, diff --git a/pkg/keeper/status.go b/pkg/keeper/status.go index 1a043bdfe..0ef0eb7b9 100644 --- a/pkg/keeper/status.go +++ b/pkg/keeper/status.go @@ -161,14 +161,7 @@ func requirementDiff(pr *PullRequest, q *keeper.Query, cc contextChecker) (strin // Weight incorrect labels and statues with low (normal) diff values. var missingLabels []string for _, l1 := range q.Labels { - var found bool - for _, l2 := range pr.Labels.Nodes { - if strings.EqualFold(string(l2.Name), string(l1)) { - found = true - break - } - } - if !found { + if !hasLabel(pr, l1) { missingLabels = append(missingLabels, l1) } } @@ -185,11 +178,8 @@ func requirementDiff(pr *PullRequest, q *keeper.Query, cc contextChecker) (strin var presentLabels []string for _, l1 := range q.MissingLabels { - for _, l2 := range pr.Labels.Nodes { - if string(l2.Name) == l1 { - presentLabels = append(presentLabels, l1) - break - } + if hasLabel(pr, l1) { + presentLabels = append(presentLabels, l1) } } diff += len(presentLabels) @@ -229,6 +219,15 @@ func requirementDiff(pr *PullRequest, q *keeper.Query, cc contextChecker) (strin return desc, diff } +func hasLabel(pr *PullRequest, l1 string) bool { + for _, l2 := range pr.Labels.Nodes { + if strings.EqualFold(string(l2.Name), l1) { + return true + } + } + return false +} + // Returns expected status state and description. // If a PR is not mergeable, we have to select a KeeperQuery to compare it against // in order to generate a diff for the status description. We choose the query diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 3918b19d2..4d4adc4df 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -26,6 +26,7 @@ const ( CpApproved = "cherry-pick-approved" CpUnapproved = "do-not-merge/cherry-pick-not-approved" GoodFirstIssue = "good first issue" + HasConflicts = "has conflicts" Help = "help wanted" Hold = "do-not-merge/hold" InvalidOwners = "do-not-merge/invalid-owners-file"