worker: mark builds as failed based on osbuild's output

osbuild reports failing builds in two ways: it sets the "success" field
in its output to `false` and it returns with a non-zero exit status. The
worker used both, returning an `OSBuildError` when osbuild return
non-zero, but also forwarding the resulting object with the "success"
field.

Change this to only use the "success" field and ignore the return value.
The latter is useful for people running osbuild in a terminal or script,
but is redundant for this use-case.

This makes error reporting more consistent: `RunOSBuild` only returns an
error when *running* osbuild failed, not when the build fails.
This commit is contained in:
Lars Karlitski 2020-10-25 08:45:10 +01:00
parent b3c7548697
commit 89814c3107
2 changed files with 14 additions and 18 deletions

View file

@ -133,6 +133,11 @@ func RunJob(job worker.Job, store string, kojiServers map[string]koji.GSSAPICred
end_time := time.Now()
// Don't run uploaders if osbuild failed
if !result.Success {
return result, nil
}
var r []error
for _, t := range targets {
@ -478,18 +483,13 @@ func main() {
var status common.ImageBuildState
result, err := RunJob(job, store, kojiServers)
if err != nil {
if err != nil || result.Success == false {
log.Printf(" Job failed: %v", err)
status = common.IBFailed
// Fail the jobs in any targets that expects it
FailJob(job, kojiServers)
// If the error comes from osbuild, retrieve the result
if osbuildError, ok := err.(*OSBuildError); ok {
result = osbuildError.Result
}
// Ensure we always have a non-nil result, composer doesn't like nils.
// This can happen in cases when OSBuild crashes and doesn't produce
// a meaningful output. E.g. when the machine runs of disk space.

View file

@ -10,15 +10,11 @@ import (
"github.com/osbuild/osbuild-composer/internal/osbuild"
)
type OSBuildError struct {
Message string
Result *osbuild.Result
}
func (e *OSBuildError) Error() string {
return e.Message
}
// Run an instance of osbuild, returning a parsed osbuild.Result.
//
// Note that osbuild returns non-zero when the pipeline fails. This function
// does not return an error in this case. Instead, the failure is communicated
// with its corresponding logs through osbuild.Result.
func RunOSBuild(manifest distro.Manifest, store, outputDirectory string, errorWriter io.Writer) (*osbuild.Result, error) {
cmd := exec.Command(
"osbuild",
@ -58,9 +54,9 @@ func RunOSBuild(manifest distro.Manifest, store, outputDirectory string, errorWr
err = cmd.Wait()
if err != nil {
return nil, &OSBuildError{
Message: fmt.Sprintf("running osbuild failed: %v", err),
Result: &result,
// ignore ExitError if output could be decoded correctly
if _, isExitError := err.(*exec.ExitError); !isExitError {
return nil, fmt.Errorf("running osbuild failed: %v", err)
}
}