From a6bcdd6158c1582c7b13726041991247dec55c31 Mon Sep 17 00:00:00 2001 From: slchase Date: Tue, 21 May 2019 17:05:32 -0700 Subject: [PATCH] Creation of Entomologist A new controller that pins together issues and test targets Issue #10701 --- prow/github/client.go | 29 ++++ prow/github/client_test.go | 36 ++++ prow/github/fakegithub/fakegithub.go | 10 ++ testgrid/BUILD.bazel | 1 + testgrid/cmd/entomologist/BUILD.bazel | 50 ++++++ testgrid/cmd/entomologist/README.md | 34 ++++ testgrid/cmd/entomologist/main.go | 221 +++++++++++++++++++++++ testgrid/cmd/entomologist/main_test.go | 232 +++++++++++++++++++++++++ 8 files changed, 613 insertions(+) create mode 100644 testgrid/cmd/entomologist/BUILD.bazel create mode 100644 testgrid/cmd/entomologist/README.md create mode 100644 testgrid/cmd/entomologist/main.go create mode 100644 testgrid/cmd/entomologist/main_test.go diff --git a/prow/github/client.go b/prow/github/client.go index 0d6c82ed294ed..008bb1976ca8f 100644 --- a/prow/github/client.go +++ b/prow/github/client.go @@ -98,6 +98,7 @@ type IssueClient interface { CloseIssue(org, repo string, number int) error ReopenIssue(org, repo string, number int) error FindIssues(query, sort string, asc bool) ([]Issue, error) + ListOpenIssues(org, repo string) ([]Issue, error) GetIssue(org, repo string, number int) (*Issue, error) EditIssue(org, repo string, number int, issue *Issue) (*Issue, error) } @@ -1221,6 +1222,34 @@ func (c *client) ListIssueComments(org, repo string, number int) ([]IssueComment return comments, nil } +// ListOpenIssues returns all open issues, including pull requests +// +// Each page of results consumes one API token. +// +// See https://developer.github.com/v3/issues/#list-issues-for-a-repository +func (c *client) ListOpenIssues(org, repo string) ([]Issue, error) { + c.log("ListOpenIssues", org, repo) + if c.fake { + return nil, nil + } + path := fmt.Sprintf("/repos/%s/%s/issues", org, repo) + var issues []Issue + err := c.readPaginatedResults( + path, + acceptNone, + func() interface{} { + return &[]Issue{} + }, + func(obj interface{}) { + issues = append(issues, *(obj.(*[]Issue))...) + }, + ) + if err != nil { + return nil, err + } + return issues, nil +} + // GetPullRequests get all open pull requests for a repo. // // See https://developer.github.com/v3/pulls/#list-pull-requests diff --git a/prow/github/client_test.go b/prow/github/client_test.go index a70b6c251519e..8d060a4d21c6c 100644 --- a/prow/github/client_test.go +++ b/prow/github/client_test.go @@ -414,6 +414,42 @@ func TestCreateStatus(t *testing.T) { } } +func TestListIssues(t *testing.T) { + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("Bad method: %s", r.Method) + } + if r.URL.Path == "/repos/k8s/kuber/issues" { + ics := []Issue{{Number: 1}} + b, err := json.Marshal(ics) + if err != nil { + t.Fatalf("Didn't expect error: %v", err) + } + w.Header().Set("Link", fmt.Sprintf(`; rel="first", ; rel="next"`, r.Host)) + fmt.Fprint(w, string(b)) + } else if r.URL.Path == "/someotherpath" { + ics := []Issue{{Number: 2}} + b, err := json.Marshal(ics) + if err != nil { + t.Fatalf("Didn't expect error: %v", err) + } + fmt.Fprint(w, string(b)) + } else { + t.Errorf("Bad request path: %s", r.URL.Path) + } + })) + defer ts.Close() + c := getClient(ts.URL) + ics, err := c.ListOpenIssues("k8s", "kuber") + if err != nil { + t.Errorf("Didn't expect error: %v", err) + } else if len(ics) != 2 { + t.Errorf("Expected two issues, found %d: %v", len(ics), ics) + } else if ics[0].Number != 1 || ics[1].Number != 2 { + t.Errorf("Wrong issue IDs: %v", ics) + } +} + func TestListIssueComments(t *testing.T) { ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { diff --git a/prow/github/fakegithub/fakegithub.go b/prow/github/fakegithub/fakegithub.go index 2fa3811ea9890..a95b468a66d26 100644 --- a/prow/github/fakegithub/fakegithub.go +++ b/prow/github/fakegithub/fakegithub.go @@ -116,6 +116,16 @@ func (f *FakeClient) IsMember(org, user string) (bool, error) { return false, nil } +// ListOpenIssues returns f.issues +// To mock a mix of issues and pull requests, see github.Issue.PullRequest +func (f *FakeClient) ListOpenIssues(org, repo string) ([]github.Issue, error) { + var issues []github.Issue + for _, issue := range f.Issues { + issues = append(issues, *issue) + } + return issues, nil +} + // ListIssueComments returns comments. func (f *FakeClient) ListIssueComments(owner, repo string, number int) ([]github.IssueComment, error) { return append([]github.IssueComment{}, f.IssueComments[number]...), nil diff --git a/testgrid/BUILD.bazel b/testgrid/BUILD.bazel index 5a9b1308a9b95..7e14465eb9e46 100644 --- a/testgrid/BUILD.bazel +++ b/testgrid/BUILD.bazel @@ -20,6 +20,7 @@ filegroup( ":package-srcs", "//testgrid/cluster:all-srcs", "//testgrid/cmd/configurator:all-srcs", + "//testgrid/cmd/entomologist:all-srcs", "//testgrid/cmd/updater:all-srcs", "//testgrid/config:all-srcs", "//testgrid/images:all-srcs", diff --git a/testgrid/cmd/entomologist/BUILD.bazel b/testgrid/cmd/entomologist/BUILD.bazel new file mode 100644 index 0000000000000..baec755e79d17 --- /dev/null +++ b/testgrid/cmd/entomologist/BUILD.bazel @@ -0,0 +1,50 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["main.go"], + importpath = "k8s.io/test-infra/testgrid/cmd/entomologist", + visibility = ["//visibility:private"], + deps = [ + "//pkg/io:go_default_library", + "//prow/config/secret:go_default_library", + "//prow/flagutil:go_default_library", + "//prow/github:go_default_library", + "//testgrid/issue_state:go_default_library", + "//vendor/github.com/golang/protobuf/proto:go_default_library", + "//vendor/github.com/sirupsen/logrus:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", + ], +) + +go_binary( + name = "entomologist", + embed = [":go_default_library"], + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = ["main_test.go"], + embed = [":go_default_library"], + deps = [ + "//prow/flagutil:go_default_library", + "//prow/github:go_default_library", + "//prow/github/fakegithub:go_default_library", + "//testgrid/issue_state:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/testgrid/cmd/entomologist/README.md b/testgrid/cmd/entomologist/README.md new file mode 100644 index 0000000000000..831c0ed4f5555 --- /dev/null +++ b/testgrid/cmd/entomologist/README.md @@ -0,0 +1,34 @@ +# Entomologist + +Entomologist collects bugs from GitHub and pins them to TestGrid tests. When an issue is "pinned" +to a TestGrid target, a hyperlink to the issue will appear on that row. + +## Pinning Behavior + +Entomologist is expecting targets to be explicitly called out in issues. You can do this by writing +"target:" at the start of a new line, and then the target you want to pin to. + + +>Some update caused these tests to start failing! +> +>target: [sig-storage] ConfigMap binary data should be reflected in volume [NodeConformance] [Conformance] [coreos-beta] +> +>target: [sig-storage] ConfigMap should be consumable from pods in volume as non-root [LinuxOnly] [NodeConformance] [Conformance] [coreos-beta] +> +>/help + + +GitHub calls Pull Requests "Issues", but Entomologist doesn't. +Targets in Pull Requests won't be pinned. + +Entomologist can be configured with a caching proxy, such as [ghProxy](https://github.com/kubernetes/test-infra/tree/master/ghproxy), +to minimize API token usage. +Entomologist is writing an [issue_state.proto](https://github.com/kubernetes/test-infra/blob/master/testgrid/issue_state/issue_state.proto) +file to Google Cloud Storage (GCS). TestGrid consumes the information placed there. + +## Required Flags + +- `--github-org/--github-repo`: The organization/repository you're looking through for issues.\ + For example, this org/repo is `--github-org=kubernetes --github-repo=test-infra` +- `--output`: The location of the issue_state.proto file that Entomologist will write to +- `--gcs-credentials-file`: GCS requires credentials to write to a GCS location \ No newline at end of file diff --git a/testgrid/cmd/entomologist/main.go b/testgrid/cmd/entomologist/main.go new file mode 100644 index 0000000000000..86d00c14d3b1e --- /dev/null +++ b/testgrid/cmd/entomologist/main.go @@ -0,0 +1,221 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "k8s.io/test-infra/prow/config/secret" + "k8s.io/test-infra/prow/flagutil" + "os" + "os/signal" + "regexp" + "strconv" + "strings" + "syscall" + "time" + + "github.com/golang/protobuf/proto" + "github.com/sirupsen/logrus" + + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/test-infra/pkg/io" + "k8s.io/test-infra/prow/github" + "k8s.io/test-infra/testgrid/issue_state" +) + +type githubClient interface { + ListOpenIssues(org, repo string) ([]github.Issue, error) + ListIssueComments(org, repo string, number int) ([]github.IssueComment, error) +} + +const defaultPollTime = 1 * time.Hour + +type options struct { + github flagutil.GitHubOptions + organization string + repository string + gcsPath string + gcsCredentials string + oneshot bool +} + +func (o *options) parseArgs(fs *flag.FlagSet, args []string) error { + + fs.StringVar(&o.organization, "github-org", "", "GitHub organization") + fs.StringVar(&o.repository, "github-repo", "", "GitHub repository") + fs.StringVar(&o.gcsPath, "output", "", "GCS output (gs://bucket)") + fs.StringVar(&o.gcsCredentials, "gcs-credentials-file", "", "/path/to/service/account/credentials (as .json)") + fs.BoolVar(&o.oneshot, "oneshot", false, "Write proto once and exit instead of monitoring GitHub for changes") + o.github.AddFlags(fs) + if err := fs.Parse(args); err != nil { + return err + } + + return o.validate() +} + +func (o *options) validate() error { + if o.organization == "" { + return errors.New("--github-org is required") + } + if o.repository == "" { + return errors.New("--github-repo is required") + } + if o.gcsPath == "" || !strings.HasPrefix(o.gcsPath, "gs://") { + return errors.New("invalid or missing --output") + } + if o.gcsCredentials == "" { + return errors.New("--gcs-credentials-file required for write operations") + } + return o.github.Validate(false) +} + +func parseOptions() options { + var o options + + if err := o.parseArgs(flag.CommandLine, os.Args[1:]); err != nil { + logrus.WithError(err).Fatal("Invalid Flags") + } + + return o +} + +func main() { + opt := parseOptions() + + secretAgent := &secret.Agent{} + if err := secretAgent.Start([]string{opt.github.TokenPath}); err != nil { + logrus.WithError(err).Fatal("Error starting secrets agent.") + } + + ghct, err := opt.github.GitHubClient(secretAgent, false) + if err != nil { + logrus.WithError(err).Fatal("Error starting GitHub client") + } + + ctx, cancelFunc := context.WithCancel(context.Background()) + defer cancelFunc() + + client, _ := io.NewOpener(ctx, opt.gcsCredentials) + if client == nil { + logrus.Fatalf("Empty credentials (at %s) not allowed for write operation", opt.gcsCredentials) + } + + // testgrid expects issue_state.proto files for an org/repo to be named this way + gcsFile := fmt.Sprintf("/bugs-%s-%s", opt.organization, opt.repository) + writer, _ := client.Writer(ctx, opt.gcsPath+gcsFile) + defer writer.Close() + + // kick off goroutines + poll := func() { + issueState, err := pinIssues(ghct, opt) + if err != nil { + logrus.Fatalf("Could not get issues: %e", err) + } + + out, _ := proto.Marshal(issueState) + n, _ := writer.Write(out) + + logrus.Infof("Sending %d characters to GCS", n) + } + + if opt.oneshot { + poll() + return + } + + stopCh := make(chan struct{}) + defer close(stopCh) + wait.Until(poll, defaultPollTime, stopCh) + + sigTerm := make(chan os.Signal, 1) + signal.Notify(sigTerm, syscall.SIGTERM) + signal.Notify(sigTerm, syscall.SIGINT) + <-sigTerm + + logrus.Info("Entomologist is closing...") +} + +var targetRegExp = regexp.MustCompile(`(?m)^target:(.+)$`) + +func getIssues(client githubClient, o options) ([]github.Issue, error) { + issuesAndPRs, err := client.ListOpenIssues(o.organization, o.repository) + + issues := issuesAndPRs[:0] + for _, issue := range issuesAndPRs { + if !issue.IsPullRequest() { + issues = append(issues, issue) + } + } + + return issues, err +} + +// Algorithm for detecting and formatting test targets in issues +func pinIssues(client githubClient, o options) (*issue_state.IssueState, error) { + issues, err := getIssues(client, o) + + logrus.Infof("Found %d open issues in %s/%s", len(issues), o.organization, o.repository) + + if err != nil { + return nil, err + } + + var results []*issue_state.IssueInfo + + for _, issue := range issues { + + var targets []string + + matches := targetRegExp.FindAllStringSubmatch(issue.Body, -1) + for _, match := range matches { + targets = append(targets, strings.TrimSpace(match[1])) + } + + // TODO(chases2): separate the comments API calls into their own goroutines + comments, err := client.ListIssueComments(o.organization, o.repository, issue.Number) + if err != nil { + return nil, err + } + for _, comment := range comments { + matches := targetRegExp.FindAllStringSubmatch(comment.Body, -1) + for _, match := range matches { + targets = append(targets, strings.TrimSpace(match[1])) + } + } + + if len(targets) != 0 { + newResult := issue_state.IssueInfo{ + IssueId: strconv.Itoa(issue.Number), + Title: issue.Title, + RowIds: targets, + } + + results = append(results, &newResult) + } + } + + logrus.Printf("Pinning %d issues", len(results)) + file := &issue_state.IssueState{ + IssueInfo: results, + } + + return file, nil +} diff --git a/testgrid/cmd/entomologist/main_test.go b/testgrid/cmd/entomologist/main_test.go new file mode 100644 index 0000000000000..6eece75a57be0 --- /dev/null +++ b/testgrid/cmd/entomologist/main_test.go @@ -0,0 +1,232 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "flag" + "k8s.io/test-infra/prow/flagutil" + "reflect" + "strconv" + "testing" + + "k8s.io/test-infra/prow/github" + "k8s.io/test-infra/prow/github/fakegithub" + "k8s.io/test-infra/testgrid/issue_state" +) + +func TestOptions(t *testing.T) { + + testCases := []struct { + name string + args []string + expected *options + }{ + { + name: "no args, reject", + args: []string{}, + }, + { + name: "missing GCS credentials, reject", + args: []string{"--github-org=testorg", "--github-repo=testrepo", "--output=gs://foo/bar"}, + }, + { + name: "non-gcs output, reject", + args: []string{"--github-org=testorg", "--github-repo=testrepo", "--output=/usr/src/foo/bar"}, + }, + { + name: "required options", + args: []string{ + "--github-org=testorg", + "--github-repo=testrepo", + "--output=gs://foo/bar", + "--gcs-credentials-file=/usr/foo/creds.json", + }, + expected: &options{ + organization: "testorg", + repository: "testrepo", + gcsPath: "gs://foo/bar", + gcsCredentials: "/usr/foo/creds.json", + }, + }, + { + name: "oneshot with many options", + args: []string{ + "--github-org=testorg", + "--github-repo=testrepo", + "--github-endpoint=https://127.0.0.1:8888", + "--github-token-path=/usr/foo/tkpath", + "--output=gs://foo/bar", + "--gcs-credentials-file=/usr/foo/creds.json", + "--oneshot", + }, + expected: &options{ + organization: "testorg", + repository: "testrepo", + gcsPath: "gs://foo/bar", + gcsCredentials: "/usr/foo/creds.json", + oneshot: true, + }, + }, + } + + for _, test := range testCases { + flags := flag.NewFlagSet(test.name, flag.ContinueOnError) + var actual options + err := actual.parseArgs(flags, test.args) + actual.github = flagutil.GitHubOptions{} + switch { + case err == nil && test.expected == nil: + t.Errorf("%s: failed to return an error", test.name) + case err != nil && test.expected != nil: + t.Errorf("%s: unexpected error: %v", test.name, err) + case test.expected != nil && !reflect.DeepEqual(*test.expected, actual): + t.Errorf("%s: actual %#v != expected %#v", test.name, actual, *test.expected) + } + } +} + +func TestPinIssues(t *testing.T) { + testCases := []struct { + issue github.Issue + issueComments []github.IssueComment + expectedPins []string + }{ + { + issue: github.Issue{ + Title: "Body With No Target; ignore", + Number: 2, + Body: "Don't capture this target: it's wrong", + }, + }, + { + issue: github.Issue{ + Title: "Body with Single Target", + Number: 3, + Body: "target:[Organization] Testing Name [sig-testing]", + }, + expectedPins: []string{ + "[Organization] Testing Name [sig-testing]", + }, + }, + { + issue: github.Issue{ + Title: "Body with Multiple Targets", + Number: 4, + Body: "target://project:test\r\ntarget://project:example", + }, + expectedPins: []string{ + "//project:test", + "//project:example", + }, + }, + { + issue: github.Issue{ + Title: "Tolerate Whitespace", + Number: 5, + Body: "target:tolerateWindowsCR\r\ntarget:tolerateLinuxCR\ntarget:\t\ttrim-tabs\t", + }, + issueComments: []github.IssueComment{ + {Body: "target: trim leading space"}, + {Body: "target:trim trailing space "}, + }, + expectedPins: []string{ + "tolerateWindowsCR", + "tolerateLinuxCR", + "trim-tabs", + "trim leading space", + "trim trailing space", + }, + }, + { + issue: github.Issue{ + Title: "Target in Comment", + Number: 6, + }, + issueComments: []github.IssueComment{ + {Body: "target://project:test"}, + }, + expectedPins: []string{ + "//project:test", + }, + }, + { + issue: github.Issue{ + Title: "Multiple Comments", + Number: 7, + }, + issueComments: []github.IssueComment{ + {Body: "target://project:first\r\ntarget://project:second"}, + {Body: "target://project:third"}, + {Body: "This is a false target: it's not at the beginning of the line"}, + }, + expectedPins: []string{ + "//project:first", + "//project:second", + "//project:third", + }, + }, + { + issue: github.Issue{ + Title: "Pull Request; ignore", + Number: 8, + PullRequest: &struct{}{}, + Body: "target://project:no", + }, + }, + } + + for _, test := range testCases { + t.Run(test.issue.Title, func(t *testing.T) { + + g := fakegithub.FakeClient{ + Issues: map[int]*github.Issue{ + test.issue.Number: &test.issue, + }, + IssueComments: map[int][]github.IssueComment{ + test.issue.Number: test.issueComments, + }, + } + var o options + + result, err := pinIssues(&g, o) + if err != nil { + t.Errorf("Error in Issue Pinning: %e", err) + } + + if test.expectedPins == nil { + if len(result.IssueInfo) != 0 { + t.Errorf("Expected no issue, but got %v", result.IssueInfo) + } + } else { + if len(result.IssueInfo) != 1 { + t.Errorf("Returned %d issues from one issue: %v", len(result.IssueInfo), result.IssueInfo) + } + + expected := issue_state.IssueInfo{ + Title: test.issue.Title, + IssueId: strconv.Itoa(test.issue.Number), + RowIds: test.expectedPins, + } + + if !reflect.DeepEqual(*result.IssueInfo[0], expected) { + t.Errorf("Targeter: Failed with %s, got %v, expected %v", + expected.Title, *result.IssueInfo[0], expected) + } + } + }) + } +}