Skip to content

Commit

Permalink
progress: fix missing build log output on errors in progress
Browse files Browse the repository at this point in the history
This commit fixes a silly mistake from PR#810. The issue is that
in #810 we started to collect the osbuild stdout/stderr so that
we can show crashes from osbuild or other unexpected output.

However when a stage fails this is reported via the json progress
and not directly on stdout/stderr - this was missed when #810
was done.

This commit does a short term fix by collecting the buildlog again
and showing it in the error and also updates the test to be more
realistic. However we really need a test that actually tests
the real behavior, ideally a real osbuild run with a stage error
so that we can be sure we capture this (criticial!) functionality.
  • Loading branch information
mvo5 committed Feb 5, 2025
1 parent 4f86598 commit db7b418
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
11 changes: 10 additions & 1 deletion bib/pkg/progress/progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
}
wp.Close()

var tracesMsgs []string
var statusErrs []error
for {
st, err := osbuildStatus.Status()
Expand All @@ -389,10 +390,18 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
if st.Message != "" {
pb.SetMessagef(st.Message)
}

// keep all messages/traces for better error reporting
if st.Message != "" {
tracesMsgs = append(tracesMsgs, st.Message)
}
if st.Trace != "" {
tracesMsgs = append(tracesMsgs, st.Trace)
}
}

if err := cmd.Wait(); err != nil {
return fmt.Errorf("error running osbuild: %w\nOutput:\n%s", err, stdio.String())
return fmt.Errorf("error running osbuild: %w\nBuildLog:\n%s\nOutput:\n%s", err, strings.Join(tracesMsgs, "\n"), stdio.String())
}
if len(statusErrs) > 0 {
return fmt.Errorf("errors parsing osbuild status:\n%w", errors.Join(statusErrs...))
Expand Down
11 changes: 10 additions & 1 deletion bib/pkg/progress/progress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package progress_test
import (
"bytes"
"fmt"
"io"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -141,7 +142,13 @@ func makeFakeOsbuild(t *testing.T, content string) string {
}

func TestRunOSBuildWithProgressErrorReporting(t *testing.T) {
restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `echo osbuild-stdout-output
restore := progress.MockOsStderr(io.Discard)
defer restore()

restore = progress.MockOsbuildCmd(makeFakeOsbuild(t, `
>&3 echo '{"message": "osbuild-stage-message"}'
echo osbuild-stdout-output
>&2 echo osbuild-stderr-output
exit 112
`))
Expand All @@ -151,6 +158,8 @@ exit 112
assert.NoError(t, err)
err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), "", "", nil, nil)
assert.EqualError(t, err, `error running osbuild: exit status 112
BuildLog:
osbuild-stage-message
Output:
osbuild-stdout-output
osbuild-stderr-output
Expand Down

0 comments on commit db7b418

Please sign in to comment.