feat: bubble up errors in shell steps at the job-level

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) {

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(),
+	}
 }

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.

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)
 	}