From 528cdebd1fb94f05150e37a109ce4db9a083fcc5 Mon Sep 17 00:00:00 2001 From: Marc Cataford Date: Fri, 2 Aug 2024 16:38:18 -0400 Subject: [PATCH] feat: bubble up errors in shell steps at the job-level --- internal/runner/driver.go | 10 ++- internal/runner/podman_driver.go | 9 ++- internal/runner/runner.go | 29 ++++++-- internal/runner/runner_flow_test.go | 102 ++++++++++++++++++++++++---- 4 files changed, 129 insertions(+), 21 deletions(-) diff --git a/internal/runner/driver.go b/internal/runner/driver.go index 4d76068..4713075 100644 --- a/internal/runner/driver.go +++ b/internal/runner/driver.go @@ -8,7 +8,15 @@ type ContainerDriver interface { Pull(string) error Start(string, string) error Stop(string) error - Exec(string, string) error + Exec(string, string) CommandResult +} + +// Represents the outcome of a command call made by the driver. +type CommandResult struct { + // Exit code of the underlying exec.Command call. + ExitCode int + // Error (or nil) returned by the attempt to run the command. + Error error } func NewDriver(driverType string) (ContainerDriver, error) { diff --git a/internal/runner/podman_driver.go b/internal/runner/podman_driver.go index 39af49c..b3bed8e 100644 --- a/internal/runner/podman_driver.go +++ b/internal/runner/podman_driver.go @@ -46,10 +46,15 @@ func (d PodmanDriver) Stop(containerName string) error { return nil } -func (d PodmanDriver) Exec(containerId string, command string) error { +func (d PodmanDriver) Exec(containerId string, command string) CommandResult { cmd := exec.Command("podman", "exec", containerId, "bash", "-c", command) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - return cmd.Run() + err := cmd.Run() + + return CommandResult{ + Error: err, + ExitCode: cmd.ProcessState.ExitCode(), + } } diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 26609d8..1cd235e 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -1,6 +1,7 @@ package runner import ( + "errors" "fmt" "log" workflow "runner/internal/workflow" @@ -87,15 +88,20 @@ func (r *Runner) Execute(workflow workflow.Workflow) Task { // The container is started before the job steps are run and cleaned up after. func (r *Runner) RunJobInContainer(imageUri string, containerId string, job workflow.Job) error { r.StartContainer(imageUri, containerId) - defer r.StopContainer(containerId) + + defer func() { + log.Printf("Cleaning up %s", containerId) + r.StopContainer(containerId) + }() log.Printf("Started %s", containerId) for _, step := range job.Steps { log.Printf("Run: %s", step.Run) - r.RunCommandInContainer(containerId, step.Run) - } - log.Printf("Cleaning up %s", containerId) + if err := r.RunCommandInContainer(containerId, step.Run); err != nil { + return err + } + } return nil } @@ -111,8 +117,21 @@ func (r *Runner) StartContainer(uri string, containerName string) error { } // Executes a command within the given container. +// +// If the command raises an error while in the container or fails to run +// the command at all, an error is returned, otherwise nil. func (r *Runner) RunCommandInContainer(containerId string, command string) error { - return r.Driver.Exec(containerId, command) + result := r.Driver.Exec(containerId, command) + + if result.Error != nil { + return result.Error + } + + if result.ExitCode != 0 { + return errors.New(fmt.Sprintf("Command returned a non-zero exit code (%d).", result.ExitCode)) + } + + return nil } // Stops the given container. diff --git a/internal/runner/runner_flow_test.go b/internal/runner/runner_flow_test.go index 0d8f44c..b8e376c 100644 --- a/internal/runner/runner_flow_test.go +++ b/internal/runner/runner_flow_test.go @@ -1,6 +1,8 @@ package runner import ( + "errors" + workflow "runner/internal/workflow" "slices" "testing" ) @@ -11,33 +13,107 @@ type MockCall struct { } type MockDriver struct { - calls []MockCall + calls map[string][]MockCall + mockResult *CommandResult +} + +func NewMockDriver() MockDriver { + return MockDriver{ + calls: map[string][]MockCall{}, + } +} + +func (d *MockDriver) WithCommandResult(c *CommandResult) { + d.mockResult = c } func (d *MockDriver) Pull(uri string) error { - d.calls = append(d.calls, MockCall{fname: "Pull", args: []string{uri}}) + if _, init := d.calls["Pull"]; !init { + d.calls["Pull"] = []MockCall{} + } + + d.calls["Pull"] = append(d.calls["Pull"], MockCall{fname: "Pull", args: []string{uri}}) return nil } func (d *MockDriver) Start(uri string, containerName string) error { - d.calls = append(d.calls, MockCall{fname: "Start", args: []string{uri, containerName}}) + if _, init := d.calls["Start"]; !init { + d.calls["Start"] = []MockCall{} + } + d.calls["Start"] = append(d.calls["Start"], MockCall{fname: "Start", args: []string{uri, containerName}}) return nil } func (d *MockDriver) Stop(uri string) error { - d.calls = append(d.calls, MockCall{fname: "Stop", args: []string{uri}}) + if _, init := d.calls["Stop"]; !init { + d.calls["Stop"] = []MockCall{} + } + d.calls["Stop"] = append(d.calls["Stop"], MockCall{fname: "Stop", args: []string{uri}}) return nil } -func (d *MockDriver) Exec(containerName string, command string) error { - d.calls = append(d.calls, MockCall{fname: "Exec", args: []string{containerName, command}}) - return nil +func (d *MockDriver) Exec(containerName string, command string) CommandResult { + if _, init := d.calls["Exec"]; !init { + d.calls["Exec"] = []MockCall{} + } + d.calls["Exec"] = append(d.calls["Exec"], MockCall{fname: "Exec", args: []string{containerName, command}}) + + if d.mockResult != nil { + return *d.mockResult + } + + return CommandResult{} +} + +func TestRunnerRunCommandInContainerReturnsErrorFromDriver(t *testing.T) { + mockDriver := NewMockDriver() + mockDriver.WithCommandResult(&CommandResult{ExitCode: 0, Error: errors.New("test")}) + + runner := Runner{ + Driver: &mockDriver, + } + + err := runner.RunCommandInContainer("test-container", "test-command") + + if err == nil { + t.Errorf("Expected error, got nil.") + } +} + +func TestRunnerRunCommandInContainerReturnsErrorIfCommandExitCodeNonzero(t *testing.T) { + mockDriver := NewMockDriver() + mockDriver.WithCommandResult(&CommandResult{ExitCode: 1, Error: nil}) + + runner := Runner{ + Driver: &mockDriver, + } + + err := runner.RunCommandInContainer("test-container", "test-command") + + if err == nil { + t.Errorf("Expected error, got nil.") + } +} + +func TestRunJobInContainerStopsContainersOnExit(t *testing.T) { + mockDriver := NewMockDriver() + mockDriver.WithCommandResult(&CommandResult{ExitCode: 1, Error: nil}) + + runner := Runner{ + Driver: &mockDriver, + } + + runner.RunJobInContainer("uri", "container", workflow.Job{}) + + if len(mockDriver.calls["Stop"]) != 1 { + t.Errorf("Expected 1 call to Stop, found calls: %#v", mockDriver.calls["Stop"]) + } } func TestRunnerPullContainerCallsDriverPull(t *testing.T) { - mockDriver := MockDriver{} + mockDriver := NewMockDriver() runner := Runner{ Driver: &mockDriver, } @@ -49,14 +125,14 @@ func TestRunnerPullContainerCallsDriverPull(t *testing.T) { } expectedArgs := []string{"test-image"} - actualArgs := mockDriver.calls[0].args + actualArgs := mockDriver.calls["Pull"][0].args if !slices.Equal(actualArgs, expectedArgs) { t.Errorf("Expected call args to be %#v, got %#v instead.", expectedArgs, actualArgs) } } func TestRunnerStartContainerCallsDriverStart(t *testing.T) { - mockDriver := MockDriver{} + mockDriver := NewMockDriver() runner := Runner{ Driver: &mockDriver, } @@ -68,14 +144,14 @@ func TestRunnerStartContainerCallsDriverStart(t *testing.T) { } expectedArgs := []string{"test-image", "container"} - actualArgs := mockDriver.calls[0].args + actualArgs := mockDriver.calls["Start"][0].args if !slices.Equal(actualArgs, expectedArgs) { t.Errorf("Expected call args to be %#v, got %#v instead.", expectedArgs, actualArgs) } } func TestRunnerStopContainerCallsDriverStop(t *testing.T) { - mockDriver := MockDriver{} + mockDriver := NewMockDriver() runner := Runner{ Driver: &mockDriver, } @@ -87,7 +163,7 @@ func TestRunnerStopContainerCallsDriverStop(t *testing.T) { } expectedArgs := []string{"container"} - actualArgs := mockDriver.calls[0].args + actualArgs := mockDriver.calls["Stop"][0].args if !slices.Equal(actualArgs, expectedArgs) { t.Errorf("Expected call args to be %#v, got %#v instead.", expectedArgs, actualArgs) }