osbuild/osbuild-exec: extract building the osbuild cmd to helper

cmd/build/main.go

@@ -136,7 +136,11 @@ func run() error {
 	fmt.Printf("Building manifest: %s\n", manifestPath)
 
 	jobOutput := filepath.Join(outputDir, buildName)
-	_, err = osbuild.RunOSBuild(mf, osbuildStore, jobOutput, imgType.Exports(), checkpoints, nil, false, os.Stderr)
+	_, err = osbuild.RunOSBuild(mf, imgType.Exports(), checkpoints, os.Stderr, &osbuild.OSBuildOptions{
+		StoreDir:   osbuildStore,
+		OutputDir:  jobOutput,
+		JSONOutput: false,
+	})
 	if err != nil {
 		return err
 	}

cmd/osbuild-playground/playground.go

@@ -57,7 +57,11 @@ func RunPlayground(img image.ImageKind, d distro.Distro, arch distro.Arch, repos
 
 	store := path.Join(state_dir, "osbuild-store")
 
-	_, err = osbuild.RunOSBuild(bytes, store, "./", manifest.GetExports(), manifest.GetCheckpoints(), nil, false, os.Stdout)
+	_, err = osbuild.RunOSBuild(bytes, manifest.GetExports(), manifest.GetCheckpoints(), os.Stdout, &osbuild.OSBuildOptions{
+		StoreDir:   store,
+		OutputDir:  "./",
+		JSONOutput: false,
+	})
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "could not run osbuild: %s", err.Error())
 	}

pkg/osbuild/osbuild-exec.go

@@ -9,29 +9,48 @@ import (
 	"os/exec"
 	"strings"
 
+	"github.com/hashicorp/go-version"
+
 	"github.com/osbuild/images/data/dependencies"
+	"github.com/osbuild/images/internal/common"
 	"github.com/osbuild/images/pkg/datasizes"
+)
 
-	"github.com/hashicorp/go-version"
+type MonitorType string
+
+const (
+	MonitorJSONSeq = "JSONSeqMonitor"
+	MonitorNull    = "NullMonitor"
+	MonitorLog     = "LogMonitor"
 )
 
-// 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 []byte, store, outputDirectory string, exports, checkpoints, extraEnv []string, result bool, errorWriter io.Writer) (*Result, error) {
-	if err := CheckMinimumOSBuildVersion(); err != nil {
-		return nil, err
-	}
+type OSBuildOptions struct {
+	StoreDir  string
+	OutputDir string
+	ExtraEnv  []string
 
-	var stdoutBuffer bytes.Buffer
-	var res Result
+	Monitor   MonitorType
+	MonitorFD uintptr
+
+	JSONOutput bool
+
+	CacheMaxSize int64
+}
+
+func NewOSBuildCmd(manifest []byte, exports, checkpoints []string, optsPtr *OSBuildOptions) *exec.Cmd {
+	opts := common.ValueOrEmpty(optsPtr)
 
+	cacheMaxSize := int64(20 * datasizes.GiB)
+	if opts.CacheMaxSize != 0 {
+		cacheMaxSize = opts.CacheMaxSize
+	}
+
+	// nolint: gosec
 	cmd := exec.Command(
 		"osbuild",
-		"--store", store,
-		"--output-directory", outputDirectory,
+		"--store", opts.StoreDir,
+		"--output-directory", opts.OutputDir,
+		fmt.Sprintf("--cache-max-size=%v", cacheMaxSize),
 		"-",
 	)
 
@@ -43,46 +62,49 @@ func RunOSBuild(manifest []byte, store, outputDirectory string, exports, checkpo
 		cmd.Args = append(cmd.Args, "--checkpoint", checkpoint)
 	}
 
-	if len(checkpoints) > 0 {
-		// set the cache-max-size to a reasonable size that the checkpoints actually get stored
-		cmd.Args = append(cmd.Args, "--cache-max-size", fmt.Sprint(20*datasizes.GiB))
+	if opts.Monitor != "" {
+		cmd.Args = append(cmd.Args, fmt.Sprintf("--monitor=%s", opts.Monitor))
 	}
-
-	if result {
+	if opts.MonitorFD != 0 {
+		cmd.Args = append(cmd.Args, fmt.Sprintf("--monitor-fd=%d", opts.MonitorFD))
+	}
+	if opts.JSONOutput {
 		cmd.Args = append(cmd.Args, "--json")
-		cmd.Stdout = &stdoutBuffer
-	} else {
-		cmd.Stdout = os.Stdout
 	}
 
-	if len(extraEnv) > 0 {
-		cmd.Env = append(os.Environ(), extraEnv...)
-	}
+	cmd.Env = append(os.Environ(), opts.ExtraEnv...)
+	cmd.Stdin = bytes.NewBuffer(manifest)
+	return cmd
+}
 
-	cmd.Stderr = errorWriter
-	stdin, err := cmd.StdinPipe()
-	if err != nil {
-		return nil, fmt.Errorf("error setting up stdin for osbuild: %v", err)
+// 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 []byte, exports, checkpoints []string, errorWriter io.Writer, opts *OSBuildOptions) (*Result, error) {
+	if err := CheckMinimumOSBuildVersion(); err != nil {
+		return nil, err
 	}
 
-	err = cmd.Start()
-	if err != nil {
-		return nil, fmt.Errorf("error starting osbuild: %v", err)
-	}
+	var stdoutBuffer bytes.Buffer
+	var res Result
+	cmd := NewOSBuildCmd(manifest, exports, checkpoints, opts)
 
-	_, err = stdin.Write(manifest)
-	if err != nil {
-		return nil, fmt.Errorf("error writing osbuild manifest: %v", err)
+	if opts.JSONOutput {
+		cmd.Stdout = &stdoutBuffer
+	} else {
+		cmd.Stdout = os.Stdout
 	}
+	cmd.Stderr = errorWriter
 
-	err = stdin.Close()
+	err := cmd.Start()
 	if err != nil {
-		return nil, fmt.Errorf("error closing osbuild's stdin: %v", err)
+		return nil, fmt.Errorf("error starting osbuild: %v", err)
 	}
 
 	err = cmd.Wait()
-
-	if result {
+	if opts.JSONOutput {
 		// try to decode the output even though the job could have failed
 		if stdoutBuffer.Len() == 0 {
 			return nil, fmt.Errorf("osbuild did not return any output")
@@ -95,7 +117,7 @@ func RunOSBuild(manifest []byte, store, outputDirectory string, exports, checkpo
 
 	if err != nil {
 		// ignore ExitError if output could be decoded correctly (only if running with --json)
-		if _, isExitError := err.(*exec.ExitError); !isExitError || !result {
+		if _, isExitError := err.(*exec.ExitError); !isExitError || !opts.JSONOutput {
 			return nil, fmt.Errorf("running osbuild failed: %v", err)
 		}
 	}

pkg/osbuild/osbuild-exec_test.go

@@ -0,0 +1,93 @@
+package osbuild_test
+
+import (
+	"fmt"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/osbuild/images/pkg/datasizes"
+	"github.com/osbuild/images/pkg/osbuild"
+)
+
+func TestNewOSBuildCmdNilOptions(t *testing.T) {
+	mf := []byte(`{"real": "manifest"}`)
+	cmd := osbuild.NewOSBuildCmd(mf, nil, nil, nil)
+	assert.NotNil(t, cmd)
+
+	assert.Equal(
+		t,
+		[]string{
+			"osbuild",
+			"--store",
+			"",
+			"--output-directory",
+			"",
+			fmt.Sprintf("--cache-max-size=%d", int64(20*datasizes.GiB)),
+			"-",
+		},
+		cmd.Args,
+	)
+
+	stdin, err := io.ReadAll(cmd.Stdin)
+	assert.NoError(t, err)
+	assert.Equal(t, mf, stdin)
+}
+
+func TestNewOSBuildCmdFullOptions(t *testing.T) {
+	mf := []byte(`{"real": "manifest"}`)
+	cmd := osbuild.NewOSBuildCmd(
+		mf,
+		[]string{
+			"export-1",
+			"export-2",
+		},
+		[]string{
+			"checkpoint-1",
+			"checkpoint-2",
+		},
+		&osbuild.OSBuildOptions{
+			StoreDir:     "store",
+			OutputDir:    "output",
+			ExtraEnv:     []string{"EXTRA_ENV_1=1", "EXTRA_ENV_2=2"},
+			Monitor:      osbuild.MonitorLog,
+			MonitorFD:    10,
+			JSONOutput:   true,
+			CacheMaxSize: 10 * datasizes.GiB,
+		},
+	)
+	assert.NotNil(t, cmd)
+
+	assert.Equal(
+		t,
+		[]string{
+			"osbuild",
+			"--store",
+			"store",
+			"--output-directory",
+			"output",
+			fmt.Sprintf("--cache-max-size=%d", int64(10*datasizes.GiB)),
+			"-",
+			"--export",
+			"export-1",
+			"--export",
+			"export-2",
+			"--checkpoint",
+			"checkpoint-1",
+			"--checkpoint",
+			"checkpoint-2",
+			"--monitor=LogMonitor",
+			"--monitor-fd=10",
+			"--json",
+		},
+		cmd.Args,
+	)
+
+	assert.Contains(t, cmd.Env, "EXTRA_ENV_1=1")
+	assert.Contains(t, cmd.Env, "EXTRA_ENV_2=2")
+
+	stdin, err := io.ReadAll(cmd.Stdin)
+	assert.NoError(t, err)
+	assert.Equal(t, mf, stdin)
+}