Skip to content

Commit

Permalink
fix(trigger-argo-workflow): prevent unnecessary retries if a permanen…
Browse files Browse the repository at this point in the history
…t error is encountered (#631)

* prevent unnecessary retries if a permanent error is encountered

* added unit test for isFatalError
  • Loading branch information
dblinkhorn authored Dec 4, 2024
1 parent c4c7060 commit 81c3771
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
25 changes: 23 additions & 2 deletions actions/trigger-argo-workflow/cmd/trigger-argo-workflow/argo.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type App struct {
}

var instanceToHost = map[string]string{
"dev": "argo-workflows-dev.grafana.net:443",
"ops": "argo-workflows.grafana.net:443",
"dev": "argo-workflows-dev.grafana.net:443",
"ops": "argo-workflows.grafana.net:443",
}

func (a App) server() string {
Expand Down Expand Up @@ -126,6 +126,23 @@ func (a *App) openGitHubOutput() io.WriteCloser {
return f
}

var fatalErrors = []string{
"AlreadyExists",
}

func isFatalError(err error) bool {
if err == nil {
return false
}

for _, fatalError := range fatalErrors {
if strings.Contains(err.Error(), fatalError) {
return true
}
}
return false
}

func (a *App) Run(md GitHubActionsMetadata) error {
bo := backoff.WithMaxRetries(backoff.NewExponentialBackOff(), a.retries)

Expand All @@ -136,6 +153,10 @@ func (a *App) Run(md GitHubActionsMetadata) error {
var err error
uri, out, err = a.runCmd(md)

if isFatalError(err) {
return backoff.Permanent(err)
}

return err
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"bytes"
"context"
"errors"
"fmt"
"log/slog"
"testing"
Expand Down Expand Up @@ -153,3 +154,34 @@ func TestSetURIAsJobOutput(t *testing.T) {
})
}
}

func TestIsFatalError(t *testing.T) {
tests := []struct {
name string
err error
expected bool
}{
{
name: "nil error",
err: nil,
expected: false,
},
{
name: "error without 'AlreadyExists'",
err: errors.New("rpc error: code = InvalidName desc = workflows.argoproj.io \"my-workflow-@#$#$\" is invalid name"),
expected: false,
},
{
name: "error containing 'AlreadyExists'",
err: errors.New("rpc error: code = AlreadyExists desc = workflows.argoproj.io \"my-workflow-1\" already exists"),
expected: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isFatalError(tt.err)
require.Equal(t, result, tt.expected)
})
}
}

0 comments on commit 81c3771

Please sign in to comment.