Skip to content

Commit

Permalink
refactor: use IsRetriable interface for errors
Browse files Browse the repository at this point in the history
  • Loading branch information
masontikhonov committed Dec 2, 2024
1 parent 37e6304 commit 9a4aee3
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 17 deletions.
2 changes: 1 addition & 1 deletion venona/pkg/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func Test_executeAgentTask(t *testing.T) {
a := &Agent{
log: logger.New(logger.Options{}),
}
err := a.executeAgentTask(tt.task)
err := a.executeAgentTask(context.Background(), tt.task)
if err != nil || tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
}
Expand Down
44 changes: 44 additions & 0 deletions venona/pkg/codefresh/codefresh_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions venona/pkg/errors/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package errors

type RetriableError interface {
IsRetriable() bool
}

func IsRetriable(err error) bool {
e, ok := err.(RetriableError)
return ok && e.IsRetriable()
}
18 changes: 11 additions & 7 deletions venona/pkg/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ const (
type (
// Kubernetes API client
Kubernetes interface {
CreateResource(ctx context.Context, taskType task.Type, spec interface{}) *K8sError
DeleteResource(ctx context.Context, opts DeleteOptions) *K8sError
CreateResource(ctx context.Context, taskType task.Type, spec interface{}) error
DeleteResource(ctx context.Context, opts DeleteOptions) error
}

// Options for Kubernetes
Expand Down Expand Up @@ -76,7 +76,7 @@ type (

K8sError struct {
error
IsRetriable bool
isRetriable bool
}
)

Expand All @@ -86,7 +86,11 @@ var (
removeFinalizersJsonPatch = []byte(`[{ "op": "remove", "path": "/metadata/finalizers" }]`)
)

func NewK8sError(err error, operation K8sOperation) *K8sError {
func (e K8sError) IsRetriable() bool {
return e.isRetriable
}

func NewK8sError(err error, operation K8sOperation) error {
isNotRetriable := k8serrors.IsBadRequest(err) ||
k8serrors.IsForbidden(err) ||
k8serrors.IsMethodNotSupported(err) ||
Expand All @@ -99,7 +103,7 @@ func NewK8sError(err error, operation K8sOperation) *K8sError {

return &K8sError{
error: err,
IsRetriable: !isNotRetriable,
isRetriable: !isNotRetriable,
}
}

Expand Down Expand Up @@ -127,7 +131,7 @@ func New(opts Options) (Kubernetes, error) {
}, err
}

func (k kube) CreateResource(ctx context.Context, taskType task.Type, spec interface{}) *K8sError {
func (k kube) CreateResource(ctx context.Context, taskType task.Type, spec interface{}) error {
start := time.Now()
bytes, err := json.Marshal(spec)
if err != nil {
Expand Down Expand Up @@ -170,7 +174,7 @@ func (k kube) CreateResource(ctx context.Context, taskType task.Type, spec inter
return nil
}

func (k kube) DeleteResource(ctx context.Context, opts DeleteOptions) *K8sError {
func (k kube) DeleteResource(ctx context.Context, opts DeleteOptions) error {
start := time.Now()
switch opts.Kind {
case task.TypeDeletePVC:
Expand Down
5 changes: 3 additions & 2 deletions venona/pkg/queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/codefresh-io/go/venona/pkg/codefresh"
ierrors "github.com/codefresh-io/go/venona/pkg/errors"
"github.com/codefresh-io/go/venona/pkg/logger"
"github.com/codefresh-io/go/venona/pkg/metrics"
"github.com/codefresh-io/go/venona/pkg/monitoring"
Expand Down Expand Up @@ -185,15 +186,15 @@ func (wfq *wfQueueImpl) handleWorkflow(ctx context.Context, wf *workflow.Workflo
metrics.ObserveWorkflowMetrics(wf.Type, sinceCreation, inRunner, processed)
}

func (wfq *wfQueueImpl) reportTaskStatus(ctx context.Context, taskDef task.Task, err *runtime.HandleTaskError) {
func (wfq *wfQueueImpl) reportTaskStatus(ctx context.Context, taskDef task.Task, err error) {
status := task.TaskStatus{
OccurredAt: time.Now(),
StatusRevision: taskDef.Metadata.CurrentStatusRevision + 1,
}
if err != nil {
status.Status = task.StatusError
status.Reason = err.Error()
status.IsRetriable = err.IsRetriable
status.IsRetriable = ierrors.IsRetriable(err)
} else {
status.Status = task.StatusSuccess
}
Expand Down
19 changes: 12 additions & 7 deletions venona/pkg/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ import (
"encoding/json"
"fmt"

ierrors "github.com/codefresh-io/go/venona/pkg/errors"
"github.com/codefresh-io/go/venona/pkg/kubernetes"
"github.com/codefresh-io/go/venona/pkg/task"
)

type (
// Runtime API client
Runtime interface {
HandleTask(ctx context.Context, t *task.Task) *HandleTaskError
HandleTask(ctx context.Context, t *task.Task) error
}

// Options for runtime
Expand All @@ -40,14 +41,18 @@ type (

HandleTaskError struct {
error
IsRetriable bool
isRetriable bool
}
)

func NewHandleTaskError(err error, isRetriable bool) *HandleTaskError {
func (e HandleTaskError) IsRetriable() bool {
return e.isRetriable
}

func NewHandleTaskError(err error, isRetriable bool) error {
return &HandleTaskError{
error: err,
IsRetriable: isRetriable,
isRetriable: isRetriable,
}
}

Expand All @@ -58,12 +63,12 @@ func New(opts Options) Runtime {
}
}

func (r runtime) HandleTask(ctx context.Context, t *task.Task) *HandleTaskError {
func (r runtime) HandleTask(ctx context.Context, t *task.Task) error {
switch t.Type {
case task.TypeCreatePVC, task.TypeCreatePod:
err := r.client.CreateResource(ctx, t.Type, t.Spec)
if err != nil {
return NewHandleTaskError(fmt.Errorf("failed creating resource: %w", err), err.IsRetriable) // TODO: Return already executed tasks in order to terminate them
return NewHandleTaskError(fmt.Errorf("failed creating resource: %w", err), ierrors.IsRetriable(err)) // TODO: Return already executed tasks in order to terminate them
}
case task.TypeDeletePVC, task.TypeDeletePod:
opts := kubernetes.DeleteOptions{}
Expand All @@ -78,7 +83,7 @@ func (r runtime) HandleTask(ctx context.Context, t *task.Task) *HandleTaskError
}

if err := r.client.DeleteResource(ctx, opts); err != nil {
return NewHandleTaskError(fmt.Errorf("failed deleting resource: %w", err), err.IsRetriable)
return NewHandleTaskError(fmt.Errorf("failed deleting resource: %w", err), ierrors.IsRetriable(err))
}
default:
return NewHandleTaskError(fmt.Errorf("unknown task type \"%s\"", t.Type), false)
Expand Down

0 comments on commit 9a4aee3

Please sign in to comment.