From 5bfd64c866b23728d4172d5676b00f51e8050ade Mon Sep 17 00:00:00 2001 From: Igor Ignatyev Date: Wed, 13 Dec 2023 13:27:53 +0300 Subject: [PATCH 1/4] #17: Failling actions do not return an error code --- app.go | 15 ++++++++++++++- pkg/action/env.container.go | 24 ++++++++++++++++++++++-- pkg/action/env.container_test.go | 3 ++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/app.go b/app.go index dee7712..2f3d377 100644 --- a/app.go +++ b/app.go @@ -1,6 +1,7 @@ package launchr import ( + "errors" "fmt" "os" "path/filepath" @@ -218,9 +219,21 @@ func (app *appImpl) Execute() int { return 125 } if err = app.exec(); err != nil { + var status int + var execError action.ContainerExecError + fmt.Fprintln(os.Stderr, "Error:", err) - return 1 + + switch { + case errors.As(err, &execError): + status = execError.GetCode() + default: + status = 1 + } + + return status } + return 0 } diff --git a/pkg/action/env.container.go b/pkg/action/env.container.go index 6605384..c14da42 100644 --- a/pkg/action/env.container.go +++ b/pkg/action/env.container.go @@ -32,6 +32,21 @@ const ( containerFlagRemoveImage = "remove-image" ) +// ContainerExecError is an execution error also containing command exit code. +type ContainerExecError struct { + code int + msg string +} + +func (e ContainerExecError) Error() string { + return e.msg +} + +// GetCode returns executions exit code. +func (e ContainerExecError) GetCode() int { + return e.code +} + type containerEnv struct { driver driver.ContainerRunner imgres ChainImageBuildResolver @@ -220,7 +235,12 @@ func (c *containerEnv) Execute(ctx context.Context, a *Action) (err error) { status := <-statusCh // @todo maybe we should note that SIG was sent to the container. Code 130 is sent on Ctlr+C. - log.Info("action %q finished with the exit code %d", a.ID, status) + msg := fmt.Sprintf("action %q finished with the exit code %d", a.ID, status) + if status != 0 { + err = ContainerExecError{code: status, msg: msg} + } else { + log.Info(msg) + } // Copy back the result from the volume. // @todo it's a bad implementation considering consequential runs, need to find a better way to sync with remote. @@ -248,7 +268,7 @@ func (c *containerEnv) Execute(ctx context.Context, a *Action) (err error) { } } - return nil + return err } func getCurrentUser() string { diff --git a/pkg/action/env.container_test.go b/pkg/action/env.container_test.go index 6f4eb56..db5eded 100644 --- a/pkg/action/env.container_test.go +++ b/pkg/action/env.container_test.go @@ -548,6 +548,7 @@ func Test_ContainerExec(t *testing.T) { errAny := errors.New("any") errAttach := errors.New("attach error") errStart := errors.New("start error") + errExecError := ContainerExecError{code: 2, msg: "action \"test\" finished with the exit code 2"} successSteps := []mockCallInfo{ { @@ -676,7 +677,7 @@ func Test_ContainerExec(t *testing.T) { []interface{}{nil}, }, ), - nil, + errExecError, }, } From 6c9829524132b5480605b61d394649d58fbcb46b Mon Sep 17 00:00:00 2001 From: Igor Ignatyev Date: Wed, 13 Dec 2023 17:52:29 +0300 Subject: [PATCH 2/4] #17: review updates --- app.go | 6 +++--- pkg/action/env.container.go | 20 ++------------------ pkg/action/env.container_test.go | 2 +- pkg/action/env.go | 15 +++++++++++++++ 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/app.go b/app.go index 2f3d377..d85e737 100644 --- a/app.go +++ b/app.go @@ -220,13 +220,13 @@ func (app *appImpl) Execute() int { } if err = app.exec(); err != nil { var status int - var execError action.ContainerExecError + var stError action.ActionStatusError fmt.Fprintln(os.Stderr, "Error:", err) switch { - case errors.As(err, &execError): - status = execError.GetCode() + case errors.As(err, &stError): + status = stError.GetCode() default: status = 1 } diff --git a/pkg/action/env.container.go b/pkg/action/env.container.go index c14da42..d808c7e 100644 --- a/pkg/action/env.container.go +++ b/pkg/action/env.container.go @@ -32,21 +32,6 @@ const ( containerFlagRemoveImage = "remove-image" ) -// ContainerExecError is an execution error also containing command exit code. -type ContainerExecError struct { - code int - msg string -} - -func (e ContainerExecError) Error() string { - return e.msg -} - -// GetCode returns executions exit code. -func (e ContainerExecError) GetCode() int { - return e.code -} - type containerEnv struct { driver driver.ContainerRunner imgres ChainImageBuildResolver @@ -236,10 +221,9 @@ func (c *containerEnv) Execute(ctx context.Context, a *Action) (err error) { status := <-statusCh // @todo maybe we should note that SIG was sent to the container. Code 130 is sent on Ctlr+C. msg := fmt.Sprintf("action %q finished with the exit code %d", a.ID, status) + log.Info(msg) if status != 0 { - err = ContainerExecError{code: status, msg: msg} - } else { - log.Info(msg) + err = ActionStatusError{code: status, msg: msg} } // Copy back the result from the volume. diff --git a/pkg/action/env.container_test.go b/pkg/action/env.container_test.go index db5eded..f9ccf42 100644 --- a/pkg/action/env.container_test.go +++ b/pkg/action/env.container_test.go @@ -548,7 +548,7 @@ func Test_ContainerExec(t *testing.T) { errAny := errors.New("any") errAttach := errors.New("attach error") errStart := errors.New("start error") - errExecError := ContainerExecError{code: 2, msg: "action \"test\" finished with the exit code 2"} + errExecError := ActionStatusError{code: 2, msg: "action \"test\" finished with the exit code 2"} successSteps := []mockCallInfo{ { diff --git a/pkg/action/env.go b/pkg/action/env.go index 93cfb28..64cfdc2 100644 --- a/pkg/action/env.go +++ b/pkg/action/env.go @@ -8,6 +8,21 @@ import ( "github.com/launchrctl/launchr/pkg/types" ) +// ActionStatusError is an execution error also containing command exit code. +type ActionStatusError struct { + code int + msg string +} + +func (e ActionStatusError) Error() string { + return e.msg +} + +// GetCode returns executions exit code. +func (e ActionStatusError) GetCode() int { + return e.code +} + // RunEnvironment is a common interface for all action run environments. type RunEnvironment interface { // Init prepares the run environment. From ab0cdf4c8ff250a1771d97324f592f789f083503 Mon Sep 17 00:00:00 2001 From: Igor Ignatyev Date: Wed, 13 Dec 2023 18:00:58 +0300 Subject: [PATCH 3/4] #17: review updates --- app.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app.go b/app.go index d85e737..ec8921c 100644 --- a/app.go +++ b/app.go @@ -222,13 +222,12 @@ func (app *appImpl) Execute() int { var status int var stError action.ActionStatusError - fmt.Fprintln(os.Stderr, "Error:", err) - switch { case errors.As(err, &stError): status = stError.GetCode() default: status = 1 + fmt.Fprintln(os.Stderr, "Error:", err) } return status From f3595e64020f9d542b7b31f652d9c5016e9f28fc Mon Sep 17 00:00:00 2001 From: Igor Ignatyev Date: Wed, 13 Dec 2023 18:08:04 +0300 Subject: [PATCH 4/4] #17: review updates --- app.go | 6 +++--- pkg/action/env.container.go | 2 +- pkg/action/env.container_test.go | 2 +- pkg/action/env.go | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app.go b/app.go index ec8921c..e9a3fc9 100644 --- a/app.go +++ b/app.go @@ -220,11 +220,11 @@ func (app *appImpl) Execute() int { } if err = app.exec(); err != nil { var status int - var stError action.ActionStatusError + var stErr action.RunStatusError switch { - case errors.As(err, &stError): - status = stError.GetCode() + case errors.As(err, &stErr): + status = stErr.GetCode() default: status = 1 fmt.Fprintln(os.Stderr, "Error:", err) diff --git a/pkg/action/env.container.go b/pkg/action/env.container.go index d808c7e..d73d607 100644 --- a/pkg/action/env.container.go +++ b/pkg/action/env.container.go @@ -223,7 +223,7 @@ func (c *containerEnv) Execute(ctx context.Context, a *Action) (err error) { msg := fmt.Sprintf("action %q finished with the exit code %d", a.ID, status) log.Info(msg) if status != 0 { - err = ActionStatusError{code: status, msg: msg} + err = RunStatusError{code: status, msg: msg} } // Copy back the result from the volume. diff --git a/pkg/action/env.container_test.go b/pkg/action/env.container_test.go index f9ccf42..4681b29 100644 --- a/pkg/action/env.container_test.go +++ b/pkg/action/env.container_test.go @@ -548,7 +548,7 @@ func Test_ContainerExec(t *testing.T) { errAny := errors.New("any") errAttach := errors.New("attach error") errStart := errors.New("start error") - errExecError := ActionStatusError{code: 2, msg: "action \"test\" finished with the exit code 2"} + errExecError := RunStatusError{code: 2, msg: "action \"test\" finished with the exit code 2"} successSteps := []mockCallInfo{ { diff --git a/pkg/action/env.go b/pkg/action/env.go index 64cfdc2..9a51076 100644 --- a/pkg/action/env.go +++ b/pkg/action/env.go @@ -8,18 +8,18 @@ import ( "github.com/launchrctl/launchr/pkg/types" ) -// ActionStatusError is an execution error also containing command exit code. -type ActionStatusError struct { +// RunStatusError is an execution error also containing command exit code. +type RunStatusError struct { code int msg string } -func (e ActionStatusError) Error() string { +func (e RunStatusError) Error() string { return e.msg } // GetCode returns executions exit code. -func (e ActionStatusError) GetCode() int { +func (e RunStatusError) GetCode() int { return e.code }