From 84c8bc50acdef481b328b80e3f5254016dc73df5 Mon Sep 17 00:00:00 2001 From: Cuong Nguyen <128072568+can-anyscale@users.noreply.github.com> Date: Tue, 28 May 2024 14:18:05 -0700 Subject: [PATCH] [rayci] add buildkite notification (#237) Add buildkite notification upon build failures. Notification is sent to the owner of the build (buildkite syntax: https://buildkite.com/docs/pipelines/notifications). Enable notification for microcheck for now. This will allow folks to know build issues faster. Test: - CI - I added a failure into rayci CI in a previous commit and check that I get notified Signed-off-by: can --- raycicmd/bk_pipeline.go | 15 ++++++++++++++- raycicmd/build_info.go | 9 +++++---- raycicmd/config.go | 11 ++++++++++- raycicmd/main.go | 13 +++++++++---- raycicmd/main_test.go | 27 +++++++++++++++------------ raycicmd/make.go | 4 ++++ raycicmd/make_test.go | 32 ++++++++++++++++++++++++++++++++ 7 files changed, 89 insertions(+), 22 deletions(-) diff --git a/raycicmd/bk_pipeline.go b/raycicmd/bk_pipeline.go index 6ce44ca..c0b114b 100644 --- a/raycicmd/bk_pipeline.go +++ b/raycicmd/bk_pipeline.go @@ -5,6 +5,11 @@ import ( "time" ) +type bkNotify struct { + Email string `yaml:"email,omitempty"` + If string `yaml:"if,omitempty"` +} + type bkPipelineGroup struct { Group string `yaml:"group,omitempty"` Key string `yaml:"key,omitempty"` @@ -13,7 +18,8 @@ type bkPipelineGroup struct { } type bkPipeline struct { - Steps []*bkPipelineGroup `yaml:"steps,omitempty"` + Steps []*bkPipelineGroup `yaml:"steps,omitempty"` + Notify []*bkNotify `yaml:"notify,omitempty"` } func (p *bkPipeline) totalSteps() int { @@ -28,6 +34,13 @@ func newBkAgents(queue string) map[string]any { return map[string]any{"queue": queue} } +func makeBuildFailureBkNotify(email string) *bkNotify { + return &bkNotify{ + Email: email, + If: `build.state == "failing"`, + } +} + func makeNoopBkPipeline(q string) *bkPipeline { step := map[string]any{"command": "echo no pipeline steps"} if q != "" { diff --git a/raycicmd/build_info.go b/raycicmd/build_info.go index f4e129b..6f6cb3e 100644 --- a/raycicmd/build_info.go +++ b/raycicmd/build_info.go @@ -10,10 +10,11 @@ import ( ) type buildInfo struct { - buildID string - launcherBranch string - gitCommit string - selects []string + buildID string + buildAuthorEmail string + launcherBranch string + gitCommit string + selects []string } func makeBuildID(envs Envs) (string, error) { diff --git a/raycicmd/config.go b/raycicmd/config.go index 6970375..6e78167 100644 --- a/raycicmd/config.go +++ b/raycicmd/config.go @@ -126,6 +126,12 @@ type config struct { // Optional. MaxParallelism int `yaml:"max_parallelism"` + // NotifyOwnerOnFailure sets if the owner of the build should be notified + // when a build fails. + // + // Optional. + NotifyOwnerOnFailure bool `yaml:"notify_owner_on_failure"` + // DockerPlugin contains additional docker plugin configs, to fine tune // the docker plugin's behavior. // @@ -283,11 +289,14 @@ func ciDefaultConfig(envs Envs) *config { case rayPRPipeline, rayV2PremergePipeline, rayDevPipeline: return prPipelineConfig("ray-pr", nil, -1) case rayV2MicrocheckPipeline: - return prPipelineConfig( + mcPipelineConfig := prPipelineConfig( "ray-pr-microcheck", map[string]string{"RAYCI_MICROCHECK_RUN": "1"}, 1, ) + mcPipelineConfig.NotifyOwnerOnFailure = true + + return mcPipelineConfig } // By default, assume it is less privileged. diff --git a/raycicmd/main.go b/raycicmd/main.go index c64d522..1a07195 100644 --- a/raycicmd/main.go +++ b/raycicmd/main.go @@ -104,14 +104,19 @@ func makeBuildInfo(flags *Flags, envs Envs) (*buildInfo, error) { } rayciBranch, _ := envs.Lookup("RAYCI_BRANCH") + + // buildAuthorEmail is the email of the user who triggered the buildkite webhook + // event; for most parts, it is the same as the github author email. + buildAuthorEmail, _ := envs.Lookup("BUILDKITE_BUILD_CREATOR_EMAIL") commit := gitCommit(envs) selects := stepSelects(flags.Select, envs) return &buildInfo{ - buildID: buildID, - launcherBranch: rayciBranch, - gitCommit: commit, - selects: selects, + buildID: buildID, + buildAuthorEmail: buildAuthorEmail, + launcherBranch: rayciBranch, + gitCommit: commit, + selects: selects, }, nil } diff --git a/raycicmd/main_test.go b/raycicmd/main_test.go index cf1562c..6bcb0e0 100644 --- a/raycicmd/main_test.go +++ b/raycicmd/main_test.go @@ -58,10 +58,11 @@ func TestExecWithInput(t *testing.T) { func TestMakeBuildInfo(t *testing.T) { flags := &Flags{} envs := newEnvsMap(map[string]string{ - "RAYCI_BUILD_ID": "fake-build-id", - "BUILDKITE_COMMIT": "abc123", - "RAYCI_BRANCH": "foobar", - "RAYCI_SELECT": "foo,bar,taz", + "RAYCI_BUILD_ID": "fake-build-id", + "BUILDKITE_COMMIT": "abc123", + "BUILDKITE_BUILD_CREATOR_EMAIL": "reef@anyscale.com", + "RAYCI_BRANCH": "foobar", + "RAYCI_SELECT": "foo,bar,taz", }) info, err := makeBuildInfo(flags, envs) @@ -70,10 +71,11 @@ func TestMakeBuildInfo(t *testing.T) { } want := &buildInfo{ - buildID: "fake-build-id", - launcherBranch: "foobar", - gitCommit: "abc123", - selects: []string{"foo", "bar", "taz"}, + buildID: "fake-build-id", + buildAuthorEmail: "reef@anyscale.com", + launcherBranch: "foobar", + gitCommit: "abc123", + selects: []string{"foo", "bar", "taz"}, } if !reflect.DeepEqual(info, want) { t.Errorf("got %+v, want %+v", info, want) @@ -85,10 +87,11 @@ func TestMakeBuildInfo(t *testing.T) { t.Fatal("make build info with selects overwrite: ", err) } want = &buildInfo{ - buildID: "fake-build-id", - launcherBranch: "foobar", - gitCommit: "abc123", - selects: []string{"gee", "goo"}, + buildID: "fake-build-id", + buildAuthorEmail: "reef@anyscale.com", + launcherBranch: "foobar", + gitCommit: "abc123", + selects: []string{"gee", "goo"}, } if !reflect.DeepEqual(info, want) { t.Errorf("got %+v, want %+v", info, want) diff --git a/raycicmd/make.go b/raycicmd/make.go index 6a3f31d..1c41673 100644 --- a/raycicmd/make.go +++ b/raycicmd/make.go @@ -141,5 +141,9 @@ func makePipeline(repoDir string, config *config, info *buildInfo) ( return makeNoopBkPipeline(q), nil } + if email := info.buildAuthorEmail; email != "" && config.NotifyOwnerOnFailure { + pl.Notify = append(pl.Notify, makeBuildFailureBkNotify(email)) + } + return pl, nil } diff --git a/raycicmd/make_test.go b/raycicmd/make_test.go index 053cbec..6da63b4 100644 --- a/raycicmd/make_test.go +++ b/raycicmd/make_test.go @@ -293,6 +293,38 @@ func TestMakePipeline(t *testing.T) { t.Errorf("got step keys %v, want %v", keys, want) } }) + + t.Run("notify", func(t *testing.T) { + buildID := "fakebuild" + info := &buildInfo{ + buildID: buildID, + } + + config := *commonConfig + config.NotifyOwnerOnFailure = false + + got, err := makePipeline(tmp, &config, info) + if err != nil { + t.Fatalf("makePipeline: %v", err) + } + if len(got.Notify) != 0 { + t.Errorf("got %d notify, want 0", len(got.Notify)) + } + + const email = "reef@anyscale.com" + infoWithEmail := &buildInfo{ + buildID: buildID, + buildAuthorEmail: email, + } + config.NotifyOwnerOnFailure = true + got, err = makePipeline(tmp, &config, infoWithEmail) + if err != nil { + t.Fatalf("makePipeline: %v", err) + } + if len(got.Notify) == 0 || got.Notify[0].Email != email || got.Notify[0].If != `build.state == "failing"` { + t.Errorf(`got %v, want email %v, want if build.state == "failing"`, got.Notify, email) + } + }) } func TestSortPipelineGroups(t *testing.T) {