This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
ci: add owners awareness to deployment-notifier #33968
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "bufio" | ||
| "bytes" | ||
| "os" | ||
| "os/exec" | ||
| "strings" | ||
| "sync" | ||
|
|
||
| "github.com/sourcegraph/codenotify" | ||
| "github.com/sourcegraph/sourcegraph/lib/errors" | ||
| ) | ||
|
|
||
| // CodeOwnerResolver returns the list of GitHub users who subscribed themselves | ||
| // to changes on particular paths, through the OWNERS file. | ||
| // | ||
| // See https://github.com/sourcegraph/codenotify for more informations. | ||
| type CodeOwnerResolver interface { | ||
| Resolve(ref string) ([]string, error) | ||
| } | ||
|
|
||
| func NewMockCodeOwnerResolver(mapping map[string][]string) CodeOwnerResolver { | ||
| if mapping == nil { | ||
| mapping = map[string][]string{} | ||
| } | ||
| return &mockCodeOwnerResolver{ | ||
| mapping: mapping, | ||
| } | ||
| } | ||
|
|
||
| type mockCodeOwnerResolver struct { | ||
| mapping map[string][]string | ||
| } | ||
|
|
||
| func (m *mockCodeOwnerResolver) Resolve(ref string) ([]string, error) { | ||
| return m.mapping[ref], nil | ||
| } | ||
|
|
||
| func NewGitCodeOwnerResolver(cloneURL string, clonePath string) CodeOwnerResolver { | ||
| return &gitCodeOwnerResolver{ | ||
| cloneURL: cloneURL, | ||
| clonePath: clonePath, | ||
| } | ||
| } | ||
|
|
||
| type gitCodeOwnerResolver struct { | ||
| cloneURL string | ||
| clonePath string | ||
| once sync.Once | ||
| } | ||
|
|
||
| func (g *gitCodeOwnerResolver) Resolve(ref string) ([]string, error) { | ||
| var err error | ||
| g.once.Do(func() { | ||
| _, err = exec.Command("git", "clone", "--quiet", "[email protected]:sourcegraph/sourcegraph.git", g.clonePath).Output() | ||
| }) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| cwd, err := os.Getwd() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| os.Chdir(g.clonePath) | ||
| defer func() { _ = os.Chdir(cwd) }() | ||
|
|
||
| owners := []string{} | ||
| diff, err := exec.Command("git", "show", "--name-only", ref).CombinedOutput() | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "error getting diff for %s", ref) | ||
| } | ||
| paths, err := readLines(diff) | ||
| if err != nil { | ||
| return nil, errors.Errorf("error scanning lines from diff %s: %s\n%s", ref, err, string(diff)) | ||
| } | ||
| for _, path := range paths { | ||
| fs := codenotify.NewGitFS(g.clonePath, ref) | ||
| ownersList, err := codenotify.Subscribers(fs, path, "CODENOTIFY") | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "error computing the subscribers for %s", path) | ||
| } | ||
| owners = append(owners, ownersList...) | ||
| } | ||
| return uniqAndSanitize(owners), nil | ||
| } | ||
|
|
||
| func uniqAndSanitize(strs []string) []string { | ||
| set := map[string]struct{}{} | ||
| for _, str := range strs { | ||
| set[str] = struct{}{} | ||
| } | ||
| uniqStrs := make([]string, 0, len(set)) | ||
| for str := range set { | ||
| uniqStrs = append(uniqStrs, strings.TrimLeft(str, "@")) | ||
| } | ||
| return uniqStrs | ||
| } | ||
|
|
||
| func readLines(b []byte) ([]string, error) { | ||
| lines := []string{} | ||
| scanner := bufio.NewScanner(bytes.NewBuffer(b)) | ||
| for scanner.Scan() { | ||
| lines = append(lines, scanner.Text()) | ||
| } | ||
| return lines, scanner.Err() | ||
| } | ||
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import ( | |
| type Flags struct { | ||
| GitHubToken string | ||
| DryRun bool | ||
| CodeOwners bool | ||
| Environment string | ||
| SlackToken string | ||
| SlackAnnounceWebhook string | ||
|
|
@@ -34,6 +35,7 @@ func (f *Flags) Parse() { | |
| flag.StringVar(&f.GitHubToken, "github.token", os.Getenv("GITHUB_TOKEN"), "mandatory github token") | ||
| flag.StringVar(&f.Environment, "environment", "", "Environment being deployed") | ||
| flag.BoolVar(&f.DryRun, "dry", false, "Pretend to post notifications, printing to stdout instead") | ||
| flag.BoolVar(&f.CodeOwners, "codeowners", false, "Mention code owners for each PR on Slack") | ||
| flag.StringVar(&f.SlackToken, "slack.token", "", "mandatory slack api token") | ||
| flag.StringVar(&f.SlackAnnounceWebhook, "slack.webhook", "", "Slack Webhook URL to post the results on") | ||
| flag.StringVar(&f.HoneycombToken, "honeycomb.token", "", "mandatory honeycomb api token") | ||
|
|
@@ -71,10 +73,24 @@ func main() { | |
| log.Fatal(err) | ||
| } | ||
|
|
||
| var or CodeOwnerResolver | ||
| if flags.CodeOwners { | ||
| tmpDir, err := os.MkdirTemp("", "sourcegraph-clone-xxxxx") | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| defer func() { | ||
| os.RemoveAll(tmpDir) | ||
| }() | ||
| or = NewGitCodeOwnerResolver("[email protected]:sourcegraph/sourcegraph.git", tmpDir) | ||
| } else { | ||
| or = NewMockCodeOwnerResolver(nil) | ||
| } | ||
|
|
||
| dn := NewDeploymentNotifier( | ||
| ghc, | ||
| NewManifestDeploymentDiffer(changedFiles), | ||
| or, | ||
| flags.Environment, | ||
| manifestRevision, | ||
| ) | ||
|
|
@@ -97,13 +113,14 @@ func main() { | |
| } | ||
| } | ||
|
|
||
| // Notifications | ||
| slc := slack.New(flags.SlackToken) | ||
| teammates := team.NewTeammateResolver(ghc, slc) | ||
| if flags.DryRun { | ||
| fmt.Println("Github\n---") | ||
| for _, pr := range report.PullRequests { | ||
| fmt.Println("-", pr.GetNumber()) | ||
| fmt.Println(" - code owners:", strings.Join(report.PullRequestsCodeOwners[pr.GetNumber()], ", ")) | ||
| } | ||
| out, err := renderComment(report, traceURL) | ||
| if err != nil { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,9 @@ var slackTemplate = `:arrow_left: *{{.Environment}}* deployment (<{{.BuildURL}}| | |
| - Pull Requests: | ||
| {{- range .PullRequests }} | ||
| - <{{ .WebURL }}|{{ .Name }}> {{ .AuthorSlackID }} | ||
| {{- if .Owners }} | ||
| - Owners: {{- range .Owners}}{{ . }}{{" "}}{{- end}} | ||
| {{- end }} | ||
|
Comment on lines
+27
to
+29
There was a problem hiding this comment. This makes me nervous because there is no opt-out, and with staggered deploys this can be several pings a day There was a problem hiding this comment. @bobheadxi What do you think about adding in the handbook's data the following additional fields for each team member? - Corgi MacDog
github: "woofwoof"
notifications:
deployments:
preprod:
author: true
codeowners: false
cloud:
author: true
codeowners: trueWith the following defaults, that could be overridden by the GitHub labels on PRs? Side note: presently, there are very few codeowners file in the repo and the team aliases do not ping, therefore they would ping even less. There was a problem hiding this comment. That might overload the intent of There was a problem hiding this comment. For now, can we only notify OWNERS if we have There was a problem hiding this comment. I agree, but right now I don't see an immediate solution other than this :/ Maybe another file possibly? There was a problem hiding this comment.
This would defeat the purpose of giving more visibility to what happens in Cloud, that was mentioned over Slack (https://sourcegraph.slack.com/archives/C89KCDK5J/p1649869394862269). If we were to decide to revert the changes to the I also want to run a spike on guessing how a given PR changed the services, that would make the signal VS noise much higher and notifications would be much more useful. There was a problem hiding this comment. Let's try this then, and see how it goes. I wonder if that means we should revert There was a problem hiding this comment. or introduce the |
||
| {{- end }}` | ||
|
|
||
| type slackSummaryPresenter struct { | ||
|
|
@@ -37,6 +40,7 @@ type pullRequestPresenter struct { | |
| Name string | ||
| AuthorSlackID string | ||
| WebURL string | ||
| Owners []string | ||
| } | ||
|
|
||
| func slackSummary(ctx context.Context, teammates team.TeammateResolver, report *DeploymentReport, traceURL string) (string, error) { | ||
|
|
@@ -63,10 +67,23 @@ func slackSummary(ctx context.Context, teammates team.TeammateResolver, report * | |
| } | ||
| } | ||
|
|
||
| ghOwners := report.PullRequestsCodeOwners[pr.GetNumber()] | ||
| slackOwners := make([]string, 0, len(ghOwners)) | ||
| for _, ghOwner := range ghOwners { | ||
| teammate, err := teammates.ResolveByGitHubHandle(ctx, ghOwner) | ||
| if err != nil { | ||
| // TODO handle teams | ||
| slackOwners = append(slackOwners, "`"+ghOwner+"`") | ||
| } else { | ||
| slackOwners = append(slackOwners, fmt.Sprintf("<@%s>", teammate.SlackID)) | ||
| } | ||
| } | ||
|
|
||
| presenter.PullRequests = append(presenter.PullRequests, pullRequestPresenter{ | ||
| Name: pr.GetTitle(), | ||
| WebURL: pr.GetHTMLURL(), | ||
| AuthorSlackID: authorSlackID, | ||
| Owners: slackOwners, | ||
| }) | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
temp thing, because there aren't much OWNERS files, so this makes testing easier.