worker/uniter: make logger predictable

worker/uniter: make logger predictable

The stdout pipe is closed too early by os/exec;
do the work ourselves to prevent this.

https://code.launchpad.net/~rogpeppe/juju-core/136-uniter-fix-logger/+merge/131580

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-10-26 10:57:11]

I think this is part of the solution that logger.stop() should become
logger.wait(), because the hook is not complete until the output is fully
drained.

[dave_cheney.net, 2012-10-26 13:05:34]

Ignore previous comment, SGTM, lets try it.

[rog, 2012-10-29 17:23:50]

Please take a look.

[dave_cheney.net, 2012-10-29 17:24:26]

Fixes issue https://bugs.launchpad.net/juju-core/+bug/1071320

On Tue, Oct 30, 2012 at 4:23 AM,  <rogpeppe@gmail.com> wrote:
> Reviewers: mp+131580_code.launchpad.net, dfc,
>
> Message:
> Please take a look.
>
> Description:
> worker/uniter: make logger predictable
>
> The stdout pipe is closed too early by os/exec;
> do the work ourselves to prevent this.
>
>
https://code.launchpad.net/~rogpeppe/juju-core/136-uniter-fix-logger/+merge/1...
>
> (do not edit description out of merge proposal)
>
>
> Please review this at https://codereview.appspot.com/6774050/
>
> Affected files:
>   A [revision details]
>   M worker/uniter/context.go
>
>
> Index: [revision details]
> === added file '[revision details]'
> --- [revision details]  2012-01-01 00:00:00 +0000
> +++ [revision details]  2012-01-01 00:00:00 +0000
> @@ -0,0 +1,2 @@
> +Old revision: fwereade@gmail.com-20121025145338-0iop27ld9s0etx8l
> +New revision: roger.peppe@canonical.com-20121026104721-a13k00rlv2eeqi6z
>
> Index: worker/uniter/context.go
> === modified file 'worker/uniter/context.go'
> --- worker/uniter/context.go    2012-10-10 21:46:49 +0000
> +++ worker/uniter/context.go    2012-10-26 10:47:21 +0000
> @@ -151,17 +151,22 @@
>         ps := exec.Command(filepath.Join(charmDir, "hooks", hookName))
>         ps.Env = ctx.hookVars(charmDir, toolsDir, socketPath)
>         ps.Dir = charmDir
> -       outReader, err := ps.StdoutPipe()
> +       outReader, outWriter, err := os.Pipe()
>         if err != nil {
> -               return err
> +               return fmt.Errorf("cannot make logging pipe: %v", err)
>         }
> -       ps.Stderr = ps.Stdout
> +       ps.Stdout = outWriter
> +       ps.Stderr = outWriter
>         logger := &hookLogger{
>                 r:    outReader,
>                 done: make(chan struct{}),
>         }
>         go logger.run()
> -       err = ps.Run()
> +       err = ps.Start()
> +       outWriter.Close()
> +       if err == nil {
> +               err = ps.Wait()
> +       }
>         logger.stop()
>         if ee, ok := err.(*exec.Error); ok && err != nil {
>                 if os.IsNotExist(ee.Err) {
>
>
>
>

[niemeyer, 2012-10-30 08:32:33]

LGTM

[rog, 2012-10-30 09:03:56]

*** Submitted:

worker/uniter: make logger predictable

The stdout pipe is closed too early by os/exec;
do the work ourselves to prevent this.

R=dfc, niemeyer
CC=
https://codereview.appspot.com/6774050