From 629a23a0173ead1efc7ed471d1fd81f38e05e1ba Mon Sep 17 00:00:00 2001 From: Jules Casteran Date: Fri, 10 Mar 2023 17:06:48 +0100 Subject: [PATCH 1/5] bump sdk-go --- go.sum | 1 + 1 file changed, 1 insertion(+) diff --git a/go.sum b/go.sum index 76be72212c..446955ba06 100644 --- a/go.sum +++ b/go.sum @@ -49,6 +49,7 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kubernetes-client/go-base v0.0.0-20190205182333-3d0e39759d98 h1:ZMIkOkl/Bg5H4EJI7zbjVXAo4rV0QJOGz2U5A0xUmZU= github.com/kubernetes-client/go-base v0.0.0-20190205182333-3d0e39759d98/go.mod h1:HPlr4uJEfrxar3JUY9cmXs3oooPjTLO6nEaEAIt5LI8= +github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.7/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= From 4e60a15d9abd3a6832d3732d3ef5ed77178145cf Mon Sep 17 00:00:00 2001 From: Jules Casteran Date: Mon, 13 Mar 2023 15:03:31 +0100 Subject: [PATCH 2/5] feat(tasks): change api of internal task library --- internal/tasks/tasks.go | 90 +++++++++++++++++++------------ internal/tasks/tasks_test.go | 101 ++++++++++++++++++----------------- 2 files changed, 109 insertions(+), 82 deletions(-) diff --git a/internal/tasks/tasks.go b/internal/tasks/tasks.go index 3590101800..557828122e 100644 --- a/internal/tasks/tasks.go +++ b/internal/tasks/tasks.go @@ -5,49 +5,62 @@ import ( "fmt" "os" "os/signal" + "reflect" ) -type Task func(ctx context.Context, args interface{}) (nextArgs interface{}, err error) -type TaskWithCleanup[T any] func(ctx context.Context, args interface{}) (nextArgs interface{}, cleanupArgs T, err error) -type Cleanup[T any] func(ctx context.Context, cleanupArgs T) error +type TaskFunc[T any, U any] func(t *Task, args T) (nextArgs U, err error) +type CleanupFunc func(ctx context.Context) error -type taskInfo struct { - Name string - function TaskWithCleanup[any] - cleanFunction Cleanup[any] - cleanupArgs interface{} +type Task struct { + Name string + Ctx context.Context + + taskFunction TaskFunc[any, any] + argType reflect.Type + returnType reflect.Type + cleanFunctions []CleanupFunc } type Tasks struct { - tasks []taskInfo + tasks []Task } func Begin() *Tasks { return &Tasks{} } -// Add a task that does not need cleanup -func (ts *Tasks) Add(name string, task Task) { - ts.tasks = append(ts.tasks, taskInfo{ - Name: name, - function: func(ctx context.Context, i interface{}) (passedData interface{}, cleanUpData interface{}, err error) { - passedData, err = task(ctx, i) +func Add[TaskArg any, TaskReturn any](ts *Tasks, name string, taskFunc TaskFunc[TaskArg, TaskReturn]) { + var argValue TaskArg + var returnValue TaskReturn + argType := reflect.TypeOf(argValue) + returnType := reflect.TypeOf(returnValue) + + tasksAmount := len(ts.tasks) + if tasksAmount > 0 { + lastTask := &ts.tasks[tasksAmount-1] + if argType != lastTask.returnType { + panic(fmt.Errorf("invalid task declared, wait for %s, previous task returns %s", argType.Name(), lastTask.returnType.Name())) + } + } + + ts.tasks = append(ts.tasks, Task{ + Name: name, + argType: argType, + returnType: returnType, + taskFunction: func(t *Task, i interface{}) (passedData interface{}, err error) { + if i == nil { + var zero TaskArg + passedData, err = taskFunc(t, zero) + } else { + passedData, err = taskFunc(t, i.(TaskArg)) + } return }, }) } -// AddWithCleanUp adds a task to the list with a cleanup function in case of fail during tasks execution -func AddWithCleanUp[T any](ts *Tasks, name string, task TaskWithCleanup[T], clean Cleanup[T]) { - ts.tasks = append(ts.tasks, taskInfo{ - Name: name, - function: func(ctx context.Context, args interface{}) (nextArgs interface{}, cleanUpArgs any, err error) { - return task(ctx, args) - }, - cleanFunction: func(ctx context.Context, cleanupArgs any) error { - return clean(ctx, cleanupArgs.(T)) - }, - }) +func (t *Task) AddToCleanUp(cleanupFunc CleanupFunc) { + t.cleanFunctions = append(t.cleanFunctions, cleanupFunc) } // setupContext return a contextWithCancel that will cancel on os interrupt (Ctrl-C) @@ -73,14 +86,17 @@ func (ts *Tasks) Cleanup(ctx context.Context, failed int) { default: } - if task.cleanFunction != nil { + if len(task.cleanFunctions) != 0 { fmt.Printf("[%d/%d] Cleaning task %q\n", i+1, totalTasks, task.Name) loader.Start() - err := task.cleanFunction(cancelableCtx, task.cleanupArgs) - if err != nil { - fmt.Printf("task %d failed to cleanup, there may be dangling resources: %s\n", i+1, err.Error()) + for _, cleanUpFunc := range task.cleanFunctions { + err := cleanUpFunc(cancelableCtx) + if err != nil { + fmt.Printf("task %d failed to cleanup, there may be dangling resources: %s\n", i+1, err.Error()) + } } + loader.Stop() } } @@ -97,19 +113,27 @@ func (ts *Tasks) Execute(ctx context.Context, data interface{}) (interface{}, er for i := range ts.tasks { task := &ts.tasks[i] + // Add context and reset cleanup functions, allows to execute multiple times + task.Ctx = cancelableCtx + task.cleanFunctions = []CleanupFunc(nil) + fmt.Printf("[%d/%d] %s\n", i+1, totalTasks, task.Name) loader.Start() - data, task.cleanupArgs, err = task.function(cancelableCtx, data) + data, err = task.taskFunction(task, data) taskIsCancelled := false select { case <-cancelableCtx.Done(): taskIsCancelled = true default: } - if err != nil || taskIsCancelled { + if err != nil && taskIsCancelled { loader.Stop() - fmt.Println("task failed, cleaning up created resources") + if taskIsCancelled { + fmt.Println("task canceled, cleaning up created resources") + } else { + fmt.Println("task failed, cleaning up created resources") + } ts.Cleanup(ctx, i) return nil, fmt.Errorf("task %d %q failed: %w", i+1, task.Name, err) } diff --git a/internal/tasks/tasks_test.go b/internal/tasks/tasks_test.go index b4912d6f85..4a0308a51a 100644 --- a/internal/tasks/tasks_test.go +++ b/internal/tasks/tasks_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "runtime" + "strings" "testing" "time" @@ -17,24 +18,28 @@ func TestCleanup(t *testing.T) { clean := 0 - tasks.AddWithCleanUp(ts, "Task 1", func(context.Context, interface{}) (interface{}, string, error) { - return nil, "", nil - }, func(context.Context, string) error { - clean++ - return nil + tasks.Add(ts, "TaskFunc 1", func(task *tasks.Task, args interface{}) (nextArgs interface{}, err error) { + task.AddToCleanUp(func(ctx context.Context) error { + clean++ + return nil + }) + return nil, nil }) - tasks.AddWithCleanUp(ts, "Task 2", func(context.Context, interface{}) (interface{}, string, error) { - return nil, "", nil - }, func(context.Context, string) error { - clean++ - return nil + tasks.Add(ts, "TaskFunc 2", func(task *tasks.Task, args interface{}) (nextArgs interface{}, err error) { + task.AddToCleanUp(func(ctx context.Context) error { + clean++ + return nil + }) + return nil, nil }) - tasks.AddWithCleanUp(ts, "Task 3", func(context.Context, interface{}) (interface{}, string, error) { - return nil, "", fmt.Errorf("fail") - }, func(context.Context, string) error { - clean++ - return nil + tasks.Add(ts, "TaskFunc 3", func(task *tasks.Task, args interface{}) (nextArgs interface{}, err error) { + task.AddToCleanUp(func(ctx context.Context) error { + clean++ + return nil + }) + return nil, fmt.Errorf("fail") }) + _, err := ts.Execute(context.Background(), nil) assert.NotNil(t, err, "Execute should return error after cleanup") assert.Equal(t, clean, 2, "2 task cleanup should have been executed") @@ -49,48 +54,46 @@ func TestCleanupOnContext(t *testing.T) { clean := 0 ctx := context.Background() - tasks.AddWithCleanUp(ts, "Task 1", - func(context.Context, interface{}) (interface{}, string, error) { - return nil, "", nil - }, func(context.Context, string) error { + tasks.Add(ts, "TaskFunc 1", func(task *tasks.Task, args interface{}) (nextArgs interface{}, err error) { + task.AddToCleanUp(func(ctx context.Context) error { clean++ return nil - }, - ) - tasks.AddWithCleanUp(ts, "Task 2", - func(context.Context, interface{}) (interface{}, string, error) { - return nil, "", nil - }, func(context.Context, string) error { + }) + return nil, nil + }) + tasks.Add(ts, "TaskFunc 2", func(task *tasks.Task, args interface{}) (nextArgs interface{}, err error) { + task.AddToCleanUp(func(ctx context.Context) error { clean++ return nil - }, - ) - tasks.AddWithCleanUp(ts, "Task 3", - func(ctx context.Context, _ interface{}) (interface{}, string, error) { - p, err := os.FindProcess(os.Getpid()) - if err != nil { - return nil, "", err - } - - // Interrupt tasks, as done with Ctrl-C - err = p.Signal(os.Interrupt) - if err != nil { - t.Fatal(err) - } - - select { - case <-time.After(time.Second): - return nil, "", nil - case <-ctx.Done(): - return nil, "", fmt.Errorf("interrupted") - } - }, func(context.Context, string) error { + }) + return nil, nil + }) + tasks.Add(ts, "TaskFunc 3", func(task *tasks.Task, args interface{}) (nextArgs interface{}, err error) { + task.AddToCleanUp(func(ctx context.Context) error { clean++ return nil - }, - ) + }) + p, err := os.FindProcess(os.Getpid()) + if err != nil { + return nil, err + } + + // Interrupt tasks, as done with Ctrl-C + err = p.Signal(os.Interrupt) + if err != nil { + t.Fatal(err) + } + + select { + case <-task.Ctx.Done(): + return nil, fmt.Errorf("interrupted") + case <-time.After(time.Second * 3): + return nil, nil + } + }) _, err := ts.Execute(ctx, nil) assert.NotNil(t, err, "context should have been interrupted") + assert.True(t, strings.Contains(err.Error(), "interrupted"), "error is not interrupted: %s", err) assert.Equal(t, clean, 2, "2 task cleanup should have been executed") } From f7da8e145b19f735bc6780d578c5b8ea1cd78ea4 Mon Sep 17 00:00:00 2001 From: Jules Casteran Date: Mon, 13 Mar 2023 15:42:00 +0100 Subject: [PATCH 3/5] fix condition --- internal/tasks/tasks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/tasks/tasks.go b/internal/tasks/tasks.go index 557828122e..bdee8531a6 100644 --- a/internal/tasks/tasks.go +++ b/internal/tasks/tasks.go @@ -127,7 +127,7 @@ func (ts *Tasks) Execute(ctx context.Context, data interface{}) (interface{}, er taskIsCancelled = true default: } - if err != nil && taskIsCancelled { + if err != nil { loader.Stop() if taskIsCancelled { fmt.Println("task canceled, cleaning up created resources") From 36db9c029d1559b866f1fb329b69eb7e0548afde Mon Sep 17 00:00:00 2001 From: Jules Casteran Date: Mon, 13 Mar 2023 16:04:20 +0100 Subject: [PATCH 4/5] add tests for generic arguments --- internal/tasks/tasks_test.go | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/internal/tasks/tasks_test.go b/internal/tasks/tasks_test.go index 4a0308a51a..c79d270140 100644 --- a/internal/tasks/tasks_test.go +++ b/internal/tasks/tasks_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "runtime" + "strconv" "strings" "testing" "time" @@ -13,6 +14,42 @@ import ( "github.com/scaleway/scaleway-cli/v2/internal/tasks" ) +func TestGeneric(t *testing.T) { + ts := tasks.Begin() + + tasks.Add(ts, "convert int to string", func(t *tasks.Task, args int) (nextArgs string, err error) { + return fmt.Sprintf("%d", args), nil + }) + tasks.Add(ts, "convert string to int and divide by 4", func(t *tasks.Task, args string) (nextArgs int, err error) { + i, err := strconv.ParseInt(args, 10, 32) + if err != nil { + return 0, err + } + return int(i) / 4, nil + }) + + res, err := ts.Execute(context.Background(), 12) + assert.Nil(t, err) + assert.Equal(t, 3, res) +} + +func TestInvalidGeneric(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Errorf("The code did not panic") + } + }() + + ts := tasks.Begin() + + tasks.Add(ts, "convert int to string", func(t *tasks.Task, args int) (nextArgs string, err error) { + return fmt.Sprintf("%d", args), nil + }) + tasks.Add(ts, "divide by 4", func(t *tasks.Task, args int) (nextArgs int, err error) { + return args / 4, nil + }) +} + func TestCleanup(t *testing.T) { ts := tasks.Begin() From 60af3f883b8d0a719cae3e9bcfa4a960ce5b2d03 Mon Sep 17 00:00:00 2001 From: Jules Casteran Date: Mon, 13 Mar 2023 16:06:58 +0100 Subject: [PATCH 5/5] go mod tidy --- go.sum | 1 - 1 file changed, 1 deletion(-) diff --git a/go.sum b/go.sum index 446955ba06..76be72212c 100644 --- a/go.sum +++ b/go.sum @@ -49,7 +49,6 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kubernetes-client/go-base v0.0.0-20190205182333-3d0e39759d98 h1:ZMIkOkl/Bg5H4EJI7zbjVXAo4rV0QJOGz2U5A0xUmZU= github.com/kubernetes-client/go-base v0.0.0-20190205182333-3d0e39759d98/go.mod h1:HPlr4uJEfrxar3JUY9cmXs3oooPjTLO6nEaEAIt5LI8= -github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.7/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=