diff --git a/pkg/command/command.go b/pkg/command/command.go index 26caa80b440..02fc10af7c7 100644 --- a/pkg/command/command.go +++ b/pkg/command/command.go @@ -30,7 +30,8 @@ import ( // A generic command abstraction type Command struct { - cmds []*command + cmds []*command + stdErrWriters, stdOutWriters []io.Writer } // The internal command representation @@ -64,6 +65,8 @@ func NewWithWorkDir(workDir, cmd string, args ...string) *Command { Cmd: cmdWithDir(workDir, cmd, args...), pipeWriter: nil, }}, + stdErrWriters: []io.Writer{}, + stdOutWriters: []io.Writer{}, } } @@ -88,6 +91,29 @@ func (c *Command) Pipe(cmd string, args ...string) *Command { return c } +// AddWriter can be used to add an additional output (stdout) and error +// (stderr) writer to the command, for example when having the need to log to +// files. +func (c *Command) AddWriter(writer io.Writer) *Command { + c.AddOutputWriter(writer) + c.AddErrorWriter(writer) + return c +} + +// AddErrorWriter can be used to add an additional error (stderr) writer to the +// command, for example when having the need to log to files. +func (c *Command) AddErrorWriter(writer io.Writer) *Command { + c.stdErrWriters = append(c.stdErrWriters, writer) + return c +} + +// AddOutputWriter can be used to add an additional output (stdout) writer to +// the command, for example when having the need to log to files. +func (c *Command) AddOutputWriter(writer io.Writer) *Command { + c.stdOutWriters = append(c.stdOutWriters, writer) + return c +} + // Run starts the command and waits for it to finish. It returns an error if // the command execution was not possible at all, otherwise the Status. // This method prints the commands output during execution @@ -190,8 +216,12 @@ func (c *Command) run(printOutput bool) (res *Status, err error) { var stdOutWriter, stdErrWriter io.Writer if printOutput { - stdOutWriter = io.MultiWriter(os.Stdout, stdOutBuffer) - stdErrWriter = io.MultiWriter(os.Stderr, stdErrBuffer) + stdOutWriter = io.MultiWriter(append( + []io.Writer{os.Stdout, stdOutBuffer}, c.stdOutWriters..., + )...) + stdErrWriter = io.MultiWriter(append( + []io.Writer{os.Stderr, stdErrBuffer}, c.stdErrWriters..., + )...) } else { stdOutWriter = stdOutBuffer stdErrWriter = stdErrBuffer diff --git a/pkg/command/command_test.go b/pkg/command/command_test.go index 1ead592b4b2..0115f0f6ff4 100644 --- a/pkg/command/command_test.go +++ b/pkg/command/command_test.go @@ -17,6 +17,9 @@ limitations under the License. package command import ( + "bytes" + "io/ioutil" + "os" "testing" "github.com/stretchr/testify/require" @@ -197,3 +200,103 @@ func TestFailureRunSilentSuccessOutput(t *testing.T) { require.NotNil(t, err) require.Nil(t, res) } + +func TestSuccessLogWriter(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + + res, err := New("echo", "Hello World").AddWriter(f).RunSuccessOutput() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Equal(t, res.Output(), string(content)) +} + +func TestSuccessLogWriterMultiple(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + b := &bytes.Buffer{} + + res, err := New("echo", "Hello World"). + AddWriter(f). + AddWriter(b). + RunSuccessOutput() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Equal(t, res.Output(), string(content)) + require.Equal(t, res.Output(), b.String()) +} + +func TestSuccessLogWriterSilent(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + + err = New("echo", "Hello World").AddWriter(f).RunSilentSuccess() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Empty(t, content) +} + +func TestSuccessLogWriterStdErr(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + + res, err := New("bash", "-c", ">&2 echo error"). + AddWriter(f).RunSuccessOutput() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Equal(t, res.Error(), string(content)) +} + +func TestSuccessLogWriterStdErrAndStdOut(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + + res, err := New("bash", "-c", ">&2 echo stderr; echo stdout"). + AddWriter(f).RunSuccessOutput() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Equal(t, res.Output()+res.Error(), string(content)) +} + +func TestSuccessLogWriterStdErrAndStdOutOnlyStdErr(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + + res, err := New("bash", "-c", ">&2 echo stderr; echo stdout"). + AddErrorWriter(f).RunSuccessOutput() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Equal(t, res.Error(), string(content)) +} + +func TestSuccessLogWriterStdErrAndStdOutOnlyStdOut(t *testing.T) { + f, err := ioutil.TempFile("", "log") + require.Nil(t, err) + defer func() { require.Nil(t, os.Remove(f.Name())) }() + + res, err := New("bash", "-c", ">&2 echo stderr; echo stdout"). + AddOutputWriter(f).RunSuccessOutput() + require.Nil(t, err) + + content, err := ioutil.ReadFile(f.Name()) + require.Nil(t, err) + require.Equal(t, res.Output(), string(content)) +} diff --git a/pkg/gcp/build/build.go b/pkg/gcp/build/build.go index b68ee433879..ef6d6d340a3 100644 --- a/pkg/gcp/build/build.go +++ b/pkg/gcp/build/build.go @@ -20,7 +20,6 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" "path" "path/filepath" "strconv" @@ -232,7 +231,7 @@ func RunSingleJob(o *Options, jobName, uploaded, version string, subs map[string args = append(args, diskSizeArg) } - cmd := exec.Command("gcloud", args...) + cmd := command.New("gcloud", args...) if o.LogDir != "" { p := path.Join(o.LogDir, strings.ReplaceAll(jobName, "/", "-")+".log") @@ -243,16 +242,11 @@ func RunSingleJob(o *Options, jobName, uploaded, version string, subs map[string } defer f.Close() - - cmd.Stdout = f - cmd.Stderr = f - } else { - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + cmd.AddWriter(f) } - if err := cmd.Run(); err != nil { - return errors.Wrapf(err, "error running %s", cmd.Args) + if err := cmd.RunSuccess(); err != nil { + return errors.Wrapf(err, "error running %s", cmd.String()) } return nil