From caff96bd4f7f636e1dfc0f50637aba94ddf4bf2e Mon Sep 17 00:00:00 2001
From: Tom Gundersen <teg@jklm.no>
Date: Wed, 27 Nov 2019 00:18:50 +0100
Subject: [PATCH] job/run: never panic on failed job

Return the error code of the osbuild run, and an array of errors,
one for each target provided. If a target fails, all other targets
are still attempted.

If either osbuild or one of the targets retursn an error, the worker
notifies osbuild-composer that the job failed.

Signed-off-by: Tom Gundersen <teg@jklm.no>
---
 cmd/osbuild-worker/main.go | 39 ++++++++++++++++++++++++++------------
 internal/jobqueue/job.go   | 37 ++++++++++++++++++++++--------------
 2 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go
index ab05ab862..40b37650a 100644
--- a/cmd/osbuild-worker/main.go
+++ b/cmd/osbuild-worker/main.go
@@ -74,21 +74,36 @@ func (c *ComposerClient) UpdateJob(job *jobqueue.Job, status string) error {
 	return nil
 }
 
+func handleJob(client *ComposerClient) {
+	fmt.Println("Waiting for a new job...")
+	job, err := client.AddJob()
+	if err != nil {
+		panic(err)
+	}
+
+	client.UpdateJob(job, "RUNNING")
+
+	fmt.Printf("Running job %s\n", job.ID.String())
+	err, errs := job.Run()
+	if err != nil {
+		client.UpdateJob(job, "FAILED")
+		return
+	}
+
+	for _, err := range errs {
+		if err != nil {
+			client.UpdateJob(job, "FAILED")
+			return
+		}
+	}
+
+	client.UpdateJob(job, "FINISHED")
+}
+
 func main() {
 	client := NewClient()
 
 	for {
-		fmt.Println("Waiting for a new job...")
-		job, err := client.AddJob()
-		if err != nil {
-			panic(err)
-		}
-
-		client.UpdateJob(job, "RUNNING")
-
-		fmt.Printf("Running job %s\n", job.ID.String())
-		job.Run()
-
-		client.UpdateJob(job, "FINISHED")
+		handleJob(client)
 	}
 }
diff --git a/internal/jobqueue/job.go b/internal/jobqueue/job.go
index acabe47ac..e220465a0 100644
--- a/internal/jobqueue/job.go
+++ b/internal/jobqueue/job.go
@@ -2,6 +2,7 @@ package jobqueue
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 	"os/exec"
 
@@ -21,28 +22,28 @@ type JobStatus struct {
 	Status string `json:"status"`
 }
 
-func (job *Job) Run() error {
+func (job *Job) Run() (error, []error) {
 	cmd := exec.Command("osbuild", "--store", "/var/cache/osbuild-composer/store", "--json", "-")
 	cmd.Stderr = os.Stderr
 
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
-		return err
+		return err, nil
 	}
 
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
-		return err
+		return err, nil
 	}
 
 	err = cmd.Start()
 	if err != nil {
-		return err
+		return err, nil
 	}
 
 	err = json.NewEncoder(stdin).Encode(job.Pipeline)
 	if err != nil {
-		return err
+		return err, nil
 	}
 	stdin.Close()
 
@@ -52,20 +53,23 @@ func (job *Job) Run() error {
 	}
 	err = json.NewDecoder(stdout).Decode(&result)
 	if err != nil {
-		return err
+		return err, nil
 	}
 
 	err = cmd.Wait()
 	if err != nil {
-		return err
+		return err, nil
 	}
 
+	var r []error
+
 	for _, t := range job.Targets {
 		switch options := t.Options.(type) {
 		case *target.LocalTargetOptions:
 			err = os.MkdirAll(options.Location, 0755)
 			if err != nil {
-				panic(err)
+				r = append(r, err)
+				continue
 			}
 
 			cp := exec.Command("cp", "-a", "-L", "/var/cache/osbuild-composer/store/refs/"+result.OutputID+"/.", options.Location)
@@ -73,29 +77,34 @@ func (job *Job) Run() error {
 			cp.Stdout = os.Stdout
 			err = cp.Run()
 			if err != nil {
-				panic(err)
+				r = append(r, err)
+				continue
 			}
 		case *target.AWSTargetOptions:
 			a, err := awsupload.New(options.Region, options.AccessKeyID, options.SecretAccessKey)
 			if err != nil {
-				panic(err)
+				r = append(r, err)
+				continue
 			}
 
 			_, err = a.Upload("/var/cache/osbuild-composer/store/refs/"+result.OutputID+"/image.ami", options.Bucket, options.Key)
 			if err != nil {
-				panic(err)
+				r = append(r, err)
+				continue
 			}
 
 			/* TODO: communicate back the AMI */
 			_, err = a.Register(t.ImageName, options.Bucket, options.Key)
 			if err != nil {
-				panic(err)
+				r = append(r, err)
+				continue
 			}
 		case *target.AzureTargetOptions:
 		default:
-			panic("foo")
+			r = append(r, fmt.Errorf("invalid target type"))
 		}
+		r = append(r, nil)
 	}
 
-	return nil
+	return nil, r
 }