commands/.../build.go: set exec command output to stdout, stderr #692
Conversation
openshift-ci-robot
added
the
size/M
|
|
||
| if enableTests { | ||
| testBinary := filepath.Join(wd, scaffold.BuildBinDir, filepath.Base(wd)+"-test") | ||
| buildTestCmd := exec.Command("go", "test", "-c", "-o", testBinary, testLocationBuild+"/...") | ||
| buildTestCmd.Env = goBuildEnv | ||
| o, err := buildTestCmd.CombinedOutput() | ||
| buildTestCmd.Stdout = os.Stdout |
There was a problem hiding this comment.
If we are going to print building the intermediate image, I don't see us log to the user that is what is happening. I think it might be nice to do that so when they see two docker builds happening they are not confused. Thoughts?
There was a problem hiding this comment.
We don't log anything right now unless there's a failure. If we add a log for one of the steps, do you think it makes sense to go ahead and log all of the steps?
There was a problem hiding this comment.
^^ Thoughts?
We're currently printing the intermediate image build output without a log saying what's happening, so this PR doesn't change that.
I agree that adding some logging for context would be helpful though. Would logging before every subprocess be unnecessarily verbose?
Without --enable-tests, the only output is the docker build output for the final image, so users should understand that with no additional context. With --enable-tests, the output is the docker build output for both the intermediate and final image. So maybe we only log both docker builds, but only when tests are enabled?
There was a problem hiding this comment.
I agree that there should be a log.Print per step. We can add that in a separate PR; I'm actually working on rewriting our log usage so I can add statements for each step.
There was a problem hiding this comment.
Sounds good to me. In that case, I'll merge this as is.
Description of the change:
Changing build output so that exec'ed build subprocesses use the main process stdout and stderr.
Motivation for the change:
Currently
operator-sdk buildprints subprocess output only after the subprocess has finished. This can be a poor user experience for long running subprocesses that output progress along the way (e.g.docker build).