Skip to content
This repository was archived by the owner on Apr 17, 2019. It is now read-only.

Commit dba64c5

Browse files
authored
Merge pull request #1152 from rmmh/flakeowner
Automatically assign newly created flake issues to test owners.
2 parents 61f4581 + 44659f6 commit dba64c5

File tree

6 files changed

+295
-8
lines changed

6 files changed

+295
-8
lines changed

hack/verify-flags/known-flags.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ ssl-cert
8383
start-from
8484
stats-port
8585
sync-period
86+
test-owners-csv
8687
tcp-services
8788
tcp-services-configmap
8889
token-file

mungegithub/github/github.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -491,16 +491,22 @@ func (config *Config) GetObject(num int) (*MungeObject, error) {
491491
}
492492

493493
// NewIssue will file a new issue and return an object for it.
494-
func (config *Config) NewIssue(title, body string, labels []string) (*MungeObject, error) {
494+
// If "owner" is not empty, the issue will be assigned to "owner".
495+
func (config *Config) NewIssue(title, body string, labels []string, owner string) (*MungeObject, error) {
495496
config.analytics.CreateIssue.Call(config, nil)
496497
glog.Infof("Creating an issue: %q", title)
497498
if config.DryRun {
498499
return nil, fmt.Errorf("can't make issues in dry-run mode")
499500
}
501+
var assignee *string
502+
if owner != "" {
503+
assignee = &owner
504+
}
500505
issue, _, err := config.client.Issues.Create(config.Org, config.Project, &github.IssueRequest{
501-
Title: &title,
502-
Body: &body,
503-
Labels: &labels,
506+
Title: &title,
507+
Body: &body,
508+
Labels: &labels,
509+
Assignee: assignee,
504510
})
505511
if err != nil {
506512
glog.Errorf("createIssue: %v", err)

mungegithub/mungers/flake-manager.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"k8s.io/contrib/mungegithub/github"
2525
cache "k8s.io/contrib/mungegithub/mungers/flakesync"
2626
"k8s.io/contrib/mungegithub/mungers/sync"
27+
"k8s.io/contrib/mungegithub/mungers/testowner"
2728
"k8s.io/contrib/test-utils/utils"
2829

2930
// "github.com/golang/glog"
@@ -44,7 +45,8 @@ type FlakeManager struct {
4445
config *github.Config
4546
googleGCSBucketUtils *utils.Utils
4647

47-
syncer *sync.IssueSyncer
48+
syncer *sync.IssueSyncer
49+
ownerPath string
4850
}
4951

5052
func init() {
@@ -76,7 +78,16 @@ func (p *FlakeManager) Initialize(config *github.Config, features *features.Feat
7678
}
7779
p.config = config
7880
p.googleGCSBucketUtils = utils.NewUtils(utils.KubekinsBucket, utils.LogDir)
79-
p.syncer = sync.NewIssueSyncer(config, p.finder)
81+
82+
var owner *testowner.ReloadingOwnerList
83+
var err error
84+
if p.ownerPath != "" {
85+
owner, err = testowner.NewReloadingOwnerList(p.ownerPath)
86+
if err != nil {
87+
return err
88+
}
89+
}
90+
p.syncer = sync.NewIssueSyncer(config, p.finder, owner)
8091
return nil
8192
}
8293

@@ -96,7 +107,9 @@ func (p *FlakeManager) EachLoop() error {
96107
}
97108

98109
// AddFlags will add any request flags to the cobra `cmd`
99-
func (p *FlakeManager) AddFlags(cmd *cobra.Command, config *github.Config) {}
110+
func (p *FlakeManager) AddFlags(cmd *cobra.Command, config *github.Config) {
111+
cmd.Flags().StringVar(&p.ownerPath, "test-owners-csv", "", "file containing a CSV-exported test-owners spreadsheet")
112+
}
100113

101114
// Munge is unused by this munger.
102115
func (p *FlakeManager) Munge(obj *github.MungeObject) {}

mungegithub/mungers/sync/issue-sync.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ import (
2525
"k8s.io/kubernetes/pkg/util/sets"
2626
)
2727

28+
// OwnerMapper finds an owner for a given test name.
29+
type OwnerMapper interface {
30+
// TestOwner returns a GitHub username for a test, or "" if none are found.
31+
TestOwner(testName string) string
32+
}
33+
2834
// IssueFinder finds an issue for a given key.
2935
type IssueFinder interface {
3036
AllIssuesForKey(key string) []int
@@ -60,14 +66,16 @@ type IssueSyncer struct {
6066
config *github.Config
6167
finder IssueFinder
6268
synced sets.String
69+
owner OwnerMapper // 'owner' may be nil, disabling issue assignment.
6370
}
6471

6572
// NewIssueSyncer constructs an issue syncer.
66-
func NewIssueSyncer(config *github.Config, finder IssueFinder) *IssueSyncer {
73+
func NewIssueSyncer(config *github.Config, finder IssueFinder, owner OwnerMapper) *IssueSyncer {
6774
return &IssueSyncer{
6875
config: config,
6976
finder: finder,
7077
synced: sets.NewString(),
78+
owner: owner,
7179
}
7280
}
7381

@@ -201,10 +209,16 @@ func (s *IssueSyncer) createIssue(source IssueSource) (issueNumber int, err erro
201209
panic(fmt.Errorf("Programmer error: %v does not contain %v!", body, id))
202210
}
203211

212+
var owner string
213+
if s.owner != nil {
214+
owner = s.owner.TestOwner(source.Title())
215+
}
216+
204217
obj, err := s.config.NewIssue(
205218
source.Title(),
206219
body,
207220
source.Labels(),
221+
owner,
208222
)
209223
if err != nil {
210224
return 0, err
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/*
2+
Copyright 2016 The Kubernetes Authors All rights reserved.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package testowner
18+
19+
import (
20+
"encoding/csv"
21+
"io"
22+
"os"
23+
"regexp"
24+
"strings"
25+
"time"
26+
27+
"github.com/golang/glog"
28+
)
29+
30+
var tagRegex = regexp.MustCompile(`\[.*?\]|\{.*?\}`)
31+
var whiteSpaceRegex = regexp.MustCompile(`\s+`)
32+
33+
// Turn a test name into a canonical form (without tags, lowercase, etc.)
34+
func normalize(name string) string {
35+
tagLess := tagRegex.ReplaceAll([]byte(name), []byte(""))
36+
squeezed := whiteSpaceRegex.ReplaceAll(tagLess, []byte(" "))
37+
return strings.ToLower(strings.TrimSpace(string(squeezed)))
38+
}
39+
40+
// OwnerList uses a map to get owners for a given test name.
41+
type OwnerList struct {
42+
mapping map[string]string
43+
}
44+
45+
// TestOwner returns the owner for a test, or the empty string if none is found.
46+
func (o *OwnerList) TestOwner(testName string) string {
47+
name := normalize(testName)
48+
owner, _ := o.mapping[name]
49+
return owner
50+
}
51+
52+
// NewOwnerList constructs an OwnerList given a mapping from test names to test owners.
53+
func NewOwnerList(mapping map[string]string) *OwnerList {
54+
list := OwnerList{}
55+
list.mapping = make(map[string]string)
56+
for input, output := range mapping {
57+
list.mapping[normalize(input)] = output
58+
}
59+
return &list
60+
}
61+
62+
// NewOwnerListFromCsv constructs an OwnerList given a CSV file that includes
63+
// 'owner' and 'test name' columns.
64+
func NewOwnerListFromCsv(r io.Reader) (*OwnerList, error) {
65+
reader := csv.NewReader(r)
66+
records, err := reader.ReadAll()
67+
if err != nil {
68+
return nil, err
69+
}
70+
mapping := make(map[string]string)
71+
ownerCol := -1
72+
nameCol := -1
73+
for _, record := range records {
74+
if ownerCol == -1 || nameCol == -1 {
75+
for col, val := range record {
76+
switch strings.ToLower(val) {
77+
case "owner":
78+
ownerCol = col
79+
case "test name":
80+
nameCol = col
81+
}
82+
}
83+
} else {
84+
mapping[record[nameCol]] = record[ownerCol]
85+
}
86+
}
87+
return NewOwnerList(mapping), nil
88+
}
89+
90+
// ReloadingOwnerList maps test names to owners, reloading the mapping when the
91+
// underlying file is changed.
92+
type ReloadingOwnerList struct {
93+
path string
94+
mtime time.Time
95+
ownerList *OwnerList
96+
}
97+
98+
// NewReloadingOwnerList creates a ReloadingOwnerList given a path to a CSV
99+
// file containing owner mapping information.
100+
func NewReloadingOwnerList(path string) (*ReloadingOwnerList, error) {
101+
ownerList := &ReloadingOwnerList{path: path}
102+
err := ownerList.reload()
103+
if err != nil {
104+
return nil, err
105+
}
106+
return ownerList, nil
107+
}
108+
109+
// TestOwner returns the owner for a test, or the empty string if none is found.
110+
func (o *ReloadingOwnerList) TestOwner(testName string) string {
111+
err := o.reload()
112+
if err != nil {
113+
glog.Errorf("Unable to reload test owners at %s: %v", o.path, err)
114+
// Process using the previous data.
115+
}
116+
return o.ownerList.TestOwner(testName)
117+
}
118+
119+
func (o *ReloadingOwnerList) reload() error {
120+
info, err := os.Stat(o.path)
121+
if err != nil {
122+
return err
123+
}
124+
if info.ModTime() == o.mtime {
125+
return nil
126+
}
127+
file, err := os.Open(o.path)
128+
if err != nil {
129+
return err
130+
}
131+
defer file.Close()
132+
ownerList, err := NewOwnerListFromCsv(file)
133+
if err != nil {
134+
return err
135+
}
136+
o.ownerList = ownerList
137+
o.mtime = info.ModTime()
138+
return nil
139+
}
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
Copyright 2016 The Kubernetes Authors All rights reserved.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package testowner
18+
19+
import (
20+
"bufio"
21+
"bytes"
22+
"io/ioutil"
23+
"os"
24+
"testing"
25+
"time"
26+
)
27+
28+
func TestNormalize(t *testing.T) {
29+
tests := map[string]string{
30+
"A": "a",
31+
"Perf [Performance]": "perf",
32+
"[k8s.io] test [performance] stuff": "test stuff",
33+
"[k8s.io] blah {Kubernetes e2e suite}": "blah",
34+
}
35+
for input, output := range tests {
36+
result := normalize(input)
37+
if result != output {
38+
t.Errorf("normalize(%s) != %s (got %s)", input, output, result)
39+
}
40+
}
41+
}
42+
43+
func TestOwnerList(t *testing.T) {
44+
list := NewOwnerList(map[string]string{"Perf [performane]": "me"})
45+
owner := list.TestOwner("perf [flaky]")
46+
if owner != "me" {
47+
t.Error("Unexpected return value ", owner)
48+
}
49+
owner = list.TestOwner("Unknown test")
50+
if owner != "" {
51+
t.Errorf("Unexpected return value ", owner)
52+
}
53+
}
54+
55+
func TestOwnerListFromCsv(t *testing.T) {
56+
r := bytes.NewReader([]byte(",,header nonsense,\n" +
57+
",owner,suggested owner,test name\n" +
58+
",foo,other,Test name\n" +
59+
",bar,foo,other test\n"))
60+
list, err := NewOwnerListFromCsv(r)
61+
if err != nil {
62+
t.Error(err)
63+
}
64+
if owner := list.TestOwner("test name"); owner != "foo" {
65+
t.Error("unexpected return value ", owner)
66+
}
67+
if owner := list.TestOwner("other test"); owner != "bar" {
68+
t.Error("unexpected return value ", owner)
69+
}
70+
}
71+
72+
func TestReloadingOwnerList(t *testing.T) {
73+
tempfile, err := ioutil.TempFile(os.TempDir(), "ownertest")
74+
if err != nil {
75+
t.Error(err)
76+
}
77+
defer os.Remove(tempfile.Name())
78+
defer tempfile.Close()
79+
writer := bufio.NewWriter(tempfile)
80+
_, err = writer.WriteString("owner,test name\nfoo,flake\n")
81+
if err != nil {
82+
t.Error(err)
83+
}
84+
err = writer.Flush()
85+
if err != nil {
86+
t.Error(err)
87+
}
88+
list, err := NewReloadingOwnerList(tempfile.Name())
89+
if err != nil {
90+
t.Error(err)
91+
}
92+
if owner := list.TestOwner("flake"); owner != "foo" {
93+
t.Error("unexpected owner for 'flake': ", owner)
94+
}
95+
96+
// Assuming millisecond resolution on our FS, this sleep
97+
// ensures the mtime will change with the next write.
98+
time.Sleep(5 * time.Millisecond)
99+
100+
tempfile.Seek(0, os.SEEK_SET)
101+
writer.Reset(tempfile)
102+
_, err = writer.WriteString("owner,test name\nbar,flake\n")
103+
if err != nil {
104+
t.Error(err)
105+
}
106+
err = writer.Flush()
107+
if err != nil {
108+
t.Error(err)
109+
}
110+
111+
if owner := list.TestOwner("flake"); owner != "bar" {
112+
t.Error("unexpected owner for 'flake': ", owner)
113+
}
114+
}

0 commit comments

Comments
 (0)