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
raystack · Sep 9, 2022 · 1e9a7b9 · 1e9a7b9
1 parent 7808743
commit 1e9a7b9
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
 		}
 	}