Skip to content

Commit

Permalink
adds a context options in tide config
Browse files Browse the repository at this point in the history
  • Loading branch information
sebastienvas committed May 12, 2018
1 parent f0383ae commit 0653446
Show file tree
Hide file tree
Showing 11 changed files with 945 additions and 53 deletions.
43 changes: 43 additions & 0 deletions prow/cmd/tide/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,38 @@ The field to search token correspondence is based on the following mapping:

Every PR that need to be rebased is filtered from the pool before processing


### Context Policy Options

A PR will be merged when all checks are passing. With this option you can customize
which contexts are required or optional.

By default, required and optional contexts will be derived from Prow Job Config.
This allows to find if required checks are missing from the github combined status.

If `branch-protection` config is defined, it can be used to know which test needs
be passing to merge a PR.

When branch protection is not used, required and optional contexts can be defined
globally, or at the org, repo or branch level.

If we want to skip unknown checks (ie checks that are not defined in Prow Config), we can set
`skip-unknown-contexts` to true. This option can be set globally or per org,
repo and branch.

**Important**: If this option is not set and no prow jobs are defined tide will trust the github
combined status and will assume that all checks are required (except tide).


### Example

```yaml
tide:
merge_method:
kubeflow/community: squash
target_url: https://prow.k8s.io/tide.html
queries:
- repos:
- kubeflow/community
Expand All @@ -113,6 +138,24 @@ tide:
- do-not-merge/work-in-progress
- needs-ok-to-test
- needs-rebase
context_options:
policy:
# Use branch protection options to define required and optional contexts
from-branch-protection: true
# Treat unknown contexts as optional
skip-unknown-contexts: true
orgs:
org:
repo:
branch:
policy:
from-branch-protection: false
required-contexts:
- "required_test"
optional-contexts:
- "optional_test"
```

**Explanation**: The component starts periodically querying all PRs in `github.com/kubeflow/community` and
Expand Down
21 changes: 13 additions & 8 deletions prow/config/branch_protection.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func (c *Config) GetBranchProtection(org, repo, branch string) (*Policy, error)
}

// Automatically require any required prow jobs
if prowContexts := branchRequirements(org, repo, branch, c.Presubmits); len(prowContexts) > 0 {
if prowContexts, _ := BranchRequirements(org, repo, branch, c.Presubmits); len(prowContexts) > 0 {
// Error if protection is disabled
if policy.Protect != nil && !*policy.Protect {
return nil, fmt.Errorf("required prow jobs require branch protection")
Expand Down Expand Up @@ -297,8 +297,8 @@ func (c *Config) GetBranchProtection(org, repo, branch string) (*Policy, error)
return &policy, nil
}

func jobRequirements(jobs []Presubmit, branch string, after bool) []string {
var required []string
func jobRequirements(jobs []Presubmit, branch string, after bool) ([]string, []string) {
var required, optional []string
for _, j := range jobs {
if !j.Brancher.RunsAgainstBranch(branch) {
continue
Expand All @@ -307,19 +307,24 @@ func jobRequirements(jobs []Presubmit, branch string, after bool) []string {
if !after && !j.AlwaysRun && j.RunIfChanged == "" {
continue // No
}
if !j.SkipReport && !j.Optional { // This job needs a context
if j.ContextRequired() { // This job needs a context
required = append(required, j.Context)
} else {
optional = append(optional, j.Context)
}
// Check which children require contexts
required = append(required, jobRequirements(j.RunAfterSuccess, branch, true)...)
r, o := jobRequirements(j.RunAfterSuccess, branch, true)
required = append(required, r...)
optional = append(optional, o...)
}
return required
return required, optional
}

func branchRequirements(org, repo, branch string, presubmits map[string][]Presubmit) []string {
// BranchRequirements returns required and optional presubmits prow jobs for a given org, repo branch.
func BranchRequirements(org, repo, branch string, presubmits map[string][]Presubmit) ([]string, []string) {
p, ok := presubmits[org+"/"+repo]
if !ok {
return nil
return nil, nil
}
return jobRequirements(p, branch, false)
}
44 changes: 35 additions & 9 deletions prow/config/branch_protection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ func TestJobRequirements(t *testing.T) {
name string
config []Presubmit
masterExpected, otherExpected []string
masterOptional, otherOptional []string
}{
{
name: "basic",
Expand All @@ -312,22 +313,29 @@ func TestJobRequirements(t *testing.T) {
SkipReport: false,
},
{
Context: "optional",
Context: "not-always",
AlwaysRun: false,
SkipReport: false,
},
{
Context: "optional",
Context: "skip-report",
AlwaysRun: true,
SkipReport: false,
Optional: true,
SkipReport: true,
Brancher: Brancher{
SkipBranches: []string{"master"},
},
},
{
Context: "optional",
AlwaysRun: true,
SkipReport: false,
Optional: true,
},
},
masterExpected: []string{"always-run", "run-if-changed"},
masterOptional: []string{"optional"},
otherExpected: []string{"always-run", "run-if-changed"},
otherOptional: []string{"skip-report", "optional"},
},
{
name: "children",
Expand Down Expand Up @@ -379,9 +387,12 @@ func TestJobRequirements(t *testing.T) {
SkipReport: true,
RunAfterSuccess: []Presubmit{
{
Context: "hidden-parent",
SkipReport: true,
AlwaysRun: false,
Context: "hidden-parent",
Optional: true,
AlwaysRun: false,
Brancher: Brancher{
Branches: []string{"master"},
},
RunAfterSuccess: []Presubmit{
{
Context: "visible-kid",
Expand All @@ -400,23 +411,38 @@ func TestJobRequirements(t *testing.T) {
"also-me-3",
"visible-kid",
},
masterOptional: []string{
"run-if-changed",
"run-and-skip",
"hidden-grandpa",
"hidden-parent"},
otherExpected: []string{
"always-run", "include-me",
"me2",
"also-me-3",
},
otherOptional: []string{
"run-if-changed",
"run-and-skip",
"hidden-grandpa"},
},
}

for _, tc := range cases {
masterActual := jobRequirements(tc.config, "master", false)
masterActual, masterOptional := jobRequirements(tc.config, "master", false)
if !reflect.DeepEqual(masterActual, tc.masterExpected) {
t.Errorf("branch: master - %s: actual %v != expected %v", tc.name, masterActual, tc.masterExpected)
}
otherActual := jobRequirements(tc.config, "other", false)
if !reflect.DeepEqual(masterOptional, tc.masterOptional) {
t.Errorf("branch: master - optional - %s: actual %v != expected %v", tc.name, masterOptional, tc.masterOptional)
}
otherActual, otherOptional := jobRequirements(tc.config, "other", false)
if !reflect.DeepEqual(masterActual, tc.masterExpected) {
t.Errorf("branch: other - %s: actual %v != expected %v", tc.name, otherActual, tc.otherExpected)
}
if !reflect.DeepEqual(otherOptional, tc.otherOptional) {
t.Errorf("branch: other - optional - %s: actual %v != expected %v", tc.name, otherOptional, tc.otherOptional)
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions prow/config/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,14 @@ func (ps Presubmit) TriggerMatches(body string) bool {
return ps.re.MatchString(body)
}

// ContextRequired checks whether a context is required from github points of view (required check).
func (ps Presubmit) ContextRequired() bool {
if ps.Optional || ps.SkipReport {
return false
}
return true
}

type ChangedFilesProvider func() ([]string, error)

func matching(j Presubmit, body string, testAll bool) []Presubmit {
Expand Down
101 changes: 101 additions & 0 deletions prow/config/tide.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package config

import (
"errors"
"fmt"
"strings"
"time"
Expand All @@ -31,6 +32,31 @@ const timeFormatISO8601 = "2006-01-02T15:04:05Z"

type TideQueries []TideQuery

type TideContextPolicy struct {
// whether to consider unknown contexts optional (skip) or required.
SkipUnknownContexts *bool `json:"skip-unknown-contexts,omitempty"`
RequiredContexts []string `json:"required-contexts,omitempty"`
OptionalContexts []string `json:"optional-contexts,omitempty"`
// Infer required and optional jobs from Branch Protection configuration
FromBranchProtection *bool `json:"from-branch-protection,omitempty"`
}

type TideOrgContextPolicy struct {
Policy TideContextPolicy `json:"policy,omitempty"`
Repos map[string]TideRepoContextPolicy `json:"repos,omitempty"`
}

type TideRepoContextPolicy struct {
Policy TideContextPolicy `json:"policy,omitempty"`
Branches map[string]TideContextPolicy `json:"branches,omitempty"`
}

type TideContextPolicyOptions struct {
Policy TideContextPolicy `json:"policy,omitempty"`
// Github Orgs
Orgs map[string]TideOrgContextPolicy `json:"orgs,omitempty"`
}

// Tide is config for the tide pool.
type Tide struct {
// SyncPeriodString compiles into SyncPeriod at load time.
Expand Down Expand Up @@ -66,6 +92,12 @@ type Tide struct {
// controller to handle org/repo:branch pools. Defaults to 20. Needs to be a
// positive number.
MaxGoroutines int `json:"max_goroutines,omitempty"`

// TideContextPolicyOptions defines merge options for context. If not set it will infer
// the required and optional contexts from the prow jobs configured and use the github
// combined status; otherwise it may apply the branch protection setting or let user
// define their own options in case branch protection is not used.
ContextOptions TideContextPolicyOptions `json:"context_options,omitempty"`
}

// MergeMethod returns the merge method to use for a repo. The default of merge is
Expand Down Expand Up @@ -216,3 +248,72 @@ func (tq *TideQuery) Validate() error {

return nil
}

func mergeTideContextPolicyConfig(a, b TideContextPolicy) TideContextPolicy {
mergeBool := func(a, b *bool) *bool {
if b == nil {
return a
}
return b
}
c := TideContextPolicy{}
c.FromBranchProtection = mergeBool(c.FromBranchProtection, b.FromBranchProtection)
c.SkipUnknownContexts = mergeBool(a.SkipUnknownContexts, b.SkipUnknownContexts)
required := sets.NewString(a.RequiredContexts...)
optional := sets.NewString(a.OptionalContexts...)
required.Insert(b.RequiredContexts...)
optional.Insert(b.OptionalContexts...)
c.RequiredContexts = required.List()
c.OptionalContexts = optional.List()
return c
}

func parseTideContextPolicyOptions(org, repo, branch string, options TideContextPolicyOptions) TideContextPolicy {
option := options.Policy
if o, ok := options.Orgs[org]; ok {
option = mergeTideContextPolicyConfig(option, o.Policy)
if r, ok := o.Repos[repo]; ok {
option = mergeTideContextPolicyConfig(option, r.Policy)
if b, ok := r.Branches[branch]; ok {
option = mergeTideContextPolicyConfig(option, b)
}
}
}
return option
}

// GetTideContextPolicy parses the prow config to find context merge options.
// If none are set, it will use the prow jobs configured and use the default github combined status.
// Otherwise if set it will use the branch protection setting, or the listed jobs.
func (c Config) GetTideContextPolicy(org, repo, branch string) (TideContextPolicy, error) {
options := parseTideContextPolicyOptions(org, repo, branch, c.Tide.ContextOptions)
// Adding required and optional contexts from options
required := sets.NewString(options.RequiredContexts...)
optional := sets.NewString(options.OptionalContexts...)

// automatically generate required and optional entries for Prow Jobs
prowRequired, prowOptional := BranchRequirements(org, repo, branch, c.Presubmits)
required.Insert(prowRequired...)
optional.Insert(prowOptional...)

// Using Branch protection configuration
if options.FromBranchProtection != nil && *options.FromBranchProtection {
bp, err := c.GetBranchProtection(org, repo, branch)
if err != nil {
return TideContextPolicy{}, err
}
if bp == nil {
return TideContextPolicy{}, errors.New("branch protection is not set")
}
if bp.Protect == nil || !*bp.Protect || bp.RequiredStatusChecks == nil {
return TideContextPolicy{}, errors.New("branch protection is invalid")
}
required.Insert(bp.RequiredStatusChecks.Contexts...)
}

return TideContextPolicy{
RequiredContexts: required.List(),
OptionalContexts: optional.List(),
SkipUnknownContexts: options.SkipUnknownContexts,
}, nil
}
Loading

0 comments on commit 0653446

Please sign in to comment.