Block SIGINT during bootstrap

Block SIGINT during bootstrap

This change is to catch SIGINT during bootstrap
for all providers. Also, if SIGINT is delivered
while tools are being uploaded, the upload will
be cancelled.

If the user Ctrl-C's while the local provider's
bootstrap script is being executed, it will be
interrupted and will return and error to juju.
Juju will then attempt to destroy the environment
and remove the .jenv file.

We will now also send some very basic feedback to
stderr when uploading tools.

Fixes lp:1296475

https://code.launchpad.net/~axwalk/juju-core/lp1296475-block-sigint-bootstrap/+merge/212558

(do not edit description out of merge proposal)

[axw, 2014-03-25 08:22:36]

Please take a look.

[axw, 2014-03-25 08:52:10]

Please take a look.

[thumper, 2014-03-26 02:41:47]

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/bootstrap.go#newc...
cmd/juju/bootstrap.go:112: fmt.Fprintln(ctx.GetStderr(), "Interrupt signalled:
waiting for bootstrap to exit")
You could use the new awesome:
  ctx.Infof("Interrupt signalled: waiting for bootstrap to exit")

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/common.go
File cmd/juju/common.go (right):

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/common.go#newcode44
cmd/juju/common.go:44: fmt.Fprintf(ctx.GetStderr(), "%s failed, destroying
environment\n", action)
ctx.Infof("%s failed, destroying environment", action)

A new line is added to the end if missing.

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/interru...
File environs/bootstrap/interruptiblestorage.go (right):

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/interru...
environs/bootstrap/interruptiblestorage.go:48: n, err = r.Reader.Read(p)
Is it expected that this will continue until finished even if interrupted?

Is there a race condition here between the return 0, interruptedError below and
this assignment?

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/synctoo...
File environs/bootstrap/synctools.go (right):

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/synctoo...
environs/bootstrap/synctools.go:35: logger.Infof("checking that upload is
possible")
we could change this now to use the Verbosef method (if we add it to the
BootstrapContext interface)

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/synctoo...
environs/bootstrap/synctools.go:56: fmt.Fprintln(ctx.GetStderr(), "cancelling
tools upload")
we should put Infof and Verbosef onto the bootstrap context I think.

[axw, 2014-03-26 03:35:29]

Please take a look.

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/bootstrap.go#newc...
cmd/juju/bootstrap.go:112: fmt.Fprintln(ctx.GetStderr(), "Interrupt signalled:
waiting for bootstrap to exit")
On 2014/03/26 02:41:47, thumper wrote:
> You could use the new awesome:
>   ctx.Infof("Interrupt signalled: waiting for bootstrap to exit")

Done.

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/common.go
File cmd/juju/common.go (right):

https://codereview.appspot.com/76160046/diff/20001/cmd/juju/common.go#newcode44
cmd/juju/common.go:44: fmt.Fprintf(ctx.GetStderr(), "%s failed, destroying
environment\n", action)
On 2014/03/26 02:41:47, thumper wrote:
> ctx.Infof("%s failed, destroying environment", action)
> 
> A new line is added to the end if missing.

Done.

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/interru...
File environs/bootstrap/interruptiblestorage.go (right):

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/interru...
environs/bootstrap/interruptiblestorage.go:48: n, err = r.Reader.Read(p)
On 2014/03/26 02:41:47, thumper wrote:
> Is it expected that this will continue until finished even if interrupted?
> 
> Is there a race condition here between the return 0, interruptedError below
and
> this assignment?

Yes, good call. Updated to use locals rather than named results.

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/synctoo...
File environs/bootstrap/synctools.go (right):

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/synctoo...
environs/bootstrap/synctools.go:35: logger.Infof("checking that upload is
possible")
On 2014/03/26 02:41:47, thumper wrote:
> we could change this now to use the Verbosef method (if we add it to the
> BootstrapContext interface)

I don't think this particular one should be logged to stderr, but I have added
Infof/Verbosef to BootstrapContext and changed the other ones over.

https://codereview.appspot.com/76160046/diff/20001/environs/bootstrap/synctoo...
environs/bootstrap/synctools.go:56: fmt.Fprintln(ctx.GetStderr(), "cancelling
tools upload")
On 2014/03/26 02:41:47, thumper wrote:
> we should put Infof and Verbosef onto the bootstrap context I think.

Done.

[thumper, 2014-03-26 03:40:55]

LGTM

[wwitzel3, 2014-03-26 11:43:53]

LGTM