From 1e9a7b98c6dd5e7595244aca4878cb8f20c04e8a Mon Sep 17 00:00:00 2001 From: Yash Bhardwaj Date: Fri, 9 Sep 2022 16:38:54 +0530 Subject: [PATCH] Fix: handle job failure event msg block builder (#582) * fix: handle multiple job failure error types * fix: handle lint issues for workerErrChan in slack_test --- ext/notify/slack/slack.go | 13 ++++++------- ext/notify/slack/slack_test.go | 4 +++- job/event.go | 2 +- models/job.go | 12 ++++++------ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/ext/notify/slack/slack.go b/ext/notify/slack/slack.go index d165744f66..0ec6142ee4 100644 --- a/ext/notify/slack/slack.go +++ b/ext/notify/slack/slack.go @@ -141,7 +141,7 @@ func (s *Notifier) queueNotification(receiverIDs []string, oauthSecret string, a } // accumulate messages -func buildMessageBlocks(events []event) []api.Block { +func buildMessageBlocks(events []event, workerErrChan chan error) []api.Block { var blocks []api.Block // core details related to event @@ -150,8 +150,7 @@ func buildMessageBlocks(events []event) []api.Block { fieldSlice = append(fieldSlice, api.NewTextBlockObject("mrkdwn", fmt.Sprintf("*Job:*\n%s", evt.jobName), false, false)) fieldSlice = append(fieldSlice, api.NewTextBlockObject("mrkdwn", fmt.Sprintf("*Owner:*\n%s", evt.owner), false, false)) - switch evt.meta.Type { - case models.SLAMissEvent: + if evt.meta.Type.IsOfType(models.SLAMissEvent) { heading := api.NewTextBlockObject("plain_text", fmt.Sprintf("[Job] SLA Breached | %s/%s", evt.projectName, evt.namespaceName), true, false) blocks = append(blocks, api.NewHeaderBlock(heading)) @@ -179,7 +178,7 @@ func buildMessageBlocks(events []event) []api.Block { } } } - case models.JobFailureEvent: + } else if evt.meta.Type.IsOfType(models.JobFailureEvent) { heading := api.NewTextBlockObject("plain_text", fmt.Sprintf("[Job] Failure | %s/%s", evt.projectName, evt.namespaceName), true, false) blocks = append(blocks, api.NewHeaderBlock(heading)) @@ -193,8 +192,8 @@ func buildMessageBlocks(events []event) []api.Block { if taskID, ok := evt.meta.Value["task_id"]; ok && taskID.GetStringValue() != "" { fieldSlice = append(fieldSlice, api.NewTextBlockObject("mrkdwn", fmt.Sprintf("*Task ID:*\n%s", taskID.GetStringValue()), false, false)) } - default: - // unknown event + } else { + workerErrChan <- fmt.Errorf("worker_buildMessageBlocks: unknown event type: %v", evt.meta.Type) continue } @@ -249,7 +248,7 @@ func (s *Notifier) Worker(ctx context.Context) { continue } var messageOptions []api.MsgOption - messageOptions = append(messageOptions, api.MsgOptionBlocks(buildMessageBlocks(events)...)) + messageOptions = append(messageOptions, api.MsgOptionBlocks(buildMessageBlocks(events, s.workerErrChan)...)) messageOptions = append(messageOptions, api.MsgOptionAsUser(true)) client := api.New(route.authToken, api.OptionAPIURL(s.slackURL)) diff --git a/ext/notify/slack/slack_test.go b/ext/notify/slack/slack_test.go index 094af89daa..af1b86fced 100644 --- a/ext/notify/slack/slack_test.go +++ b/ext/notify/slack/slack_test.go @@ -272,11 +272,13 @@ func TestBuildMessages(t *testing.T) { }, } for _, tt := range tests { + workerErrChan := make(chan error) t.Run(tt.name, func(t *testing.T) { - got := buildMessageBlocks(tt.args.events) + got := buildMessageBlocks(tt.args.events, workerErrChan) b, err := json.MarshalIndent(got, "", " ") assert.Nil(t, err) assert.Equal(t, tt.want, string(b)) }) + assert.Equal(t, len(workerErrChan), 0) } } diff --git a/job/event.go b/job/event.go index ca4831b52f..ebd0a618e4 100644 --- a/job/event.go +++ b/job/event.go @@ -34,7 +34,7 @@ func (e *eventService) Register(ctx context.Context, namespace models.NamespaceS evt models.JobEvent) error { var err error for _, notify := range jobSpec.Behavior.Notify { - if notify.ShouldNotify(evt.Type) { + if evt.Type.IsOfType(notify.On) { for _, channel := range notify.Channels { chanParts := strings.Split(channel, "://") scheme := chanParts[0] diff --git a/models/job.go b/models/job.go index 3640ffaf32..7ba79ee662 100644 --- a/models/job.go +++ b/models/job.go @@ -173,18 +173,18 @@ type JobSpecNotifier struct { Channels []string } -func (n *JobSpecNotifier) ShouldNotify(eventType JobEventType) bool { - var faiulreEvents = []JobEventType{JobFailureEvent, JobFailEvent, TaskFailEvent, HookFailEvent, SensorFailEvent} +func (incomingEvent JobEventType) IsOfType(targetEvent JobEventType) bool { + var failureEvents = []JobEventType{JobFailureEvent, JobFailEvent, TaskFailEvent, HookFailEvent, SensorFailEvent} - switch n.On { + switch targetEvent { case JobFailureEvent: - for _, event := range faiulreEvents { - if eventType == event { + for _, event := range failureEvents { + if incomingEvent == event { return true } } case SLAMissEvent: - if eventType == SLAMissEvent { + if incomingEvent == SLAMissEvent { return true } }