Merge lp:~axwalk/juju-core/lp1237736-bootstrap-context into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Introduce BootstrapContext

Environ.Bootstrap now takes an additional
parameter, a *BootstrapContext. This is a
structure that permits access to the
caller's context. It currently comprises
stdio handles and a signal handler.

Fixes #1237736

https://codereview.appspot.com/38240043/

Introduce BootstrapContext

Environ.Bootstrap now takes an additional
parameter, a *BootstrapContext. This is a
structure that permits access to the
caller's context. It currently comprises
stdio handles and a signal handler.

Fixes #1237736

https://codereview.appspot.com/38240043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+197999_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce BootstrapContext

Environ.Bootstrap now takes an additional
parameter, a *BootstrapContext. This is a
structure that permits access to the
caller's context. It currently comprises
stdio handles and a signal handler.

Fixes #1237736

https://code.launchpad.net/~axwalk/juju-core/lp1237736-bootstrap-context/+merge/197999

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/38240043/

Affected files (+397, -174 lines):
   A [revision details]
   M cloudinit/sshinit/configure.go
   M cmd/juju/addmachine.go
   M cmd/juju/bootstrap.go
   M environs/bootstrap/bootstrap.go
   M environs/bootstrap/bootstrap_test.go
   A environs/bootstrapcontext.go
   A environs/bootstrapcontext_test.go
   M environs/interface.go
   M environs/jujutest/livetests.go
   M environs/jujutest/tests.go
   M environs/manual/bootstrap.go
   M environs/manual/bootstrap_test.go
   M environs/manual/provisioner.go
   M environs/open_test.go
   M environs/sshstorage/storage.go
   M environs/sshstorage/storage_test.go
   M environs/testing/bootstrap.go
   M juju/apiconn_test.go
   M juju/conn_test.go
   M juju/testing/conn.go
   M provider/azure/environ.go
   M provider/common/bootstrap.go
   M provider/common/bootstrap_test.go
   M provider/dummy/environs.go
   M provider/ec2/ec2.go
   M provider/ec2/local_test.go
   M provider/joyent/environ.go
   M provider/local/environ.go
   M provider/maas/environ.go
   M provider/maas/environ_test.go
   M provider/null/environ.go
   M provider/null/environ_test.go
   M provider/openstack/local_test.go
   M provider/openstack/provider.go

[William Reade (fwereade)]

Looks generally clean, but I'd like rog's input on the interrupt
handling.

https://codereview.appspot.com/38240043/diff/1/cloudinit/sshinit/configure.go
File cloudinit/sshinit/configure.go (right):

https://codereview.appspot.com/38240043/diff/1/cloudinit/sshinit/configure.go#newcode26
cloudinit/sshinit/configure.go:26: Config *cloudinit.Config
to follow on from the discussion earlier, should this maybe actually be
a *instance.Config? Not necessarily actionable, just trying to have a
discussion.

https://codereview.appspot.com/38240043/diff/1/cloudinit/sshinit/configure.go#newcode28
cloudinit/sshinit/configure.go:28: // Stdin is required to solicit sudo
prompts,
s/solicit/respond to/?

https://codereview.appspot.com/38240043/diff/1/environs/bootstrap/bootstrap.go
File environs/bootstrap/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/1/environs/bootstrap/bootstrap.go#newcode22
environs/bootstrap/bootstrap.go:22: func Bootstrap(ctx
*environs.BootstrapContext, environ environs.Environ, cons
constraints.Value) error {
Random thought. Might it be good to make this func a method on
BootstrapContext?

https://codereview.appspot.com/38240043/diff/1/environs/bootstrapcontext_test.go
File environs/bootstrapcontext_test.go (right):

https://codereview.appspot.com/38240043/diff/1/environs/bootstrapcontext_test.go#newcode71
environs/bootstrapcontext_test.go:71: }
I seem to recall rog wanted to do things differently here. Rog?

https://codereview.appspot.com/38240043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/38240043/diff/1/cloudinit/sshinit/configure.go
File cloudinit/sshinit/configure.go (right):

https://codereview.appspot.com/38240043/diff/1/cloudinit/sshinit/configure.go#newcode26
cloudinit/sshinit/configure.go:26: Config *cloudinit.Config
On 2013/12/11 08:36:46, fwereade wrote:
> to follow on from the discussion earlier, should this maybe actually
be a
> *instance.Config? Not necessarily actionable, just trying to have a
discussion.

I agree that environs/cloudinit should not be called what it is, but I'm
not sure what the right to call it is. I'm not super keen on putting it
in instance, as the configuration brings a lot of things together that
aren't relevant at the instance.Instance level after provisioning.

https://codereview.appspot.com/38240043/diff/1/cloudinit/sshinit/configure.go#newcode28
cloudinit/sshinit/configure.go:28: // Stdin is required to solicit sudo
prompts,
On 2013/12/11 08:36:46, fwereade wrote:
> s/solicit/respond to/?

Done.

https://codereview.appspot.com/38240043/diff/1/environs/bootstrap/bootstrap.go
File environs/bootstrap/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/1/environs/bootstrap/bootstrap.go#newcode22
environs/bootstrap/bootstrap.go:22: func Bootstrap(ctx
*environs.BootstrapContext, environ environs.Environ, cons
constraints.Value) error {
On 2013/12/11 08:36:46, fwereade wrote:
> Random thought. Might it be good to make this func a method on
BootstrapContext?

I'd rather not, it changes the meaning of what the BootstrapContext is.

My intention is just to capture information (stdio handles) about the
context in which Environ.Bootstrap is being called, and provide a way of
manipulating that context (i.e. installing signal handlers).

https://codereview.appspot.com/38240043/

[Roger Peppe (rogpeppe)]

Looks reasonable in general, but some concerns outlined below.

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go
File environs/bootstrapcontext.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go#newcode49
environs/bootstrapcontext.go:49: func (ctx *BootstrapContext)
SetInterruptHandler(f func()) (old func()) {
I'm not keen on this approach, as it doesn't work well in a concurrent
situation. It's quite possible that we might have several concurrent
goroutines all wanting to know when they've been interrupted. This
scheme means each one may trample on another's handler.

The underlying os/signal mechanism works fine in that situation. Why not
use something more like that here?

e.g.
type BootstrapContext struct {
         Stdin io.Reader
         Stdout io.Writer
 Stderr io.Writer
 InterruptNotify func(sig chan<- os.Signal)
 StopInterruptNotify func(chan<- os.Signal)
}

func DefaultBootstrapContext() *BootstrapContext {
 return &BootstrapContext{
  Stdin: os.Stdin,
  Stdout: os.Stdout,
  Stderr: os.Stderr,
  InterruptNotify: func(c chan<- os.Signal) {
   signal.Notify(c, os.Interrupt)
  },
  StopInterruptNotify: signal.Stop,
 }
}

One thought - perhaps it might be nice to specify it as an interface
rather
than a concrete type; then cmd.Context could be made to satisfy it.
Something like this, perhaps?

type Stdio struct {
         Stdin io.Reader
         Stdout io.Writer
 Stderr io.Writer
}

type BootstrapContext interface {
     Stdio() Stdio
     InterruptNotify(sig chan<- os.Signal)
     StopInterruptNotify(chan<- os.Signal)
}

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go
File environs/manual/provisioner.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go#newcode50
environs/manual/provisioner.go:50: // and must be a terminal (except in
tests)
I know this is how it already is, but this doesn't make me feel happy.
Somehow it feels like the wrong level of abstraction, and feels like it
makes things hard to script.

This is just a handwave - I can't currently think of concrete
suggestions for anything better, sorry.

https://codereview.appspot.com/38240043/diff/20001/environs/sshstorage/storage.go
File environs/sshstorage/storage.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/sshstorage/storage.go#newcode68
environs/sshstorage/storage.go:68: // TmpDir is the remote temporary
directory for storage.
Why do we need this? Can't we use a directory inside StorageDir?

https://codereview.appspot.com/38240043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go
File environs/bootstrapcontext.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go#newcode49
environs/bootstrapcontext.go:49: func (ctx *BootstrapContext)
SetInterruptHandler(f func()) (old func()) {
On 2013/12/11 18:53:24, rog wrote:
> I'm not keen on this approach, as it doesn't work well in a concurrent
> situation. It's quite possible that we might have several concurrent
goroutines
> all wanting to know when they've been interrupted. This scheme means
each one
> may trample on another's handler.

> The underlying os/signal mechanism works fine in that situation. Why
not use
> something more like that here?

> e.g.
> type BootstrapContext struct {
> Stdin io.Reader
> Stdout io.Writer
> Stderr io.Writer
> InterruptNotify func(sig chan<- os.Signal)
> StopInterruptNotify func(chan<- os.Signal)
> }

> func DefaultBootstrapContext() *BootstrapContext {
> return &BootstrapContext{
> Stdin: os.Stdin,
> Stdout: os.Stdout,
> Stderr: os.Stderr,
> InterruptNotify: func(c chan<- os.Signal) {
> signal.Notify(c, os.Interrupt)
> },
> StopInterruptNotify: signal.Stop,
> }
> }

> One thought - perhaps it might be nice to specify it as an interface
rather
> than a concrete type; then cmd.Context could be made to satisfy it.
> Something like this, perhaps?

> type Stdio struct {
> Stdin io.Reader
> Stdout io.Writer
> Stderr io.Writer
> }

> type BootstrapContext interface {
> Stdio() Stdio
> InterruptNotify(sig chan<- os.Signal)
> StopInterruptNotify(chan<- os.Signal)
> }

I went with something very similar: Stdio(), Stdout(), and Stderr()
instead of Stdio(). Returning a struct makes for awkward package
dependencies. This does mean having a wrapper struct, as changing all
the fields of cmd.Context to methods is painful.

I've rewritten SetInterruptHandler, made it internal to
provider/common/bootstrap.go, and based it on the
InterruptNotify/StopInterruptNotify methods of BootstrapContext.

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go
File environs/manual/provisioner.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go#newcode50
environs/manual/provisioner.go:50: // and must be a terminal (except in
tests)
On 2013/12/11 18:53:24, rog wrote:
> I know this is how it already is, but this doesn't make me feel happy.
> Somehow it feels like the wrong level of abstraction, and feels like
it makes
> things hard to script.

> This is just a handwave - I can't currently think of concrete
suggestions for
> anything better, sorry.

I don't think it's necessarily that bad; perhaps my wording threw you.
It should read "must be a terminal (except in tests, or if the remote
user has passwordless sudo enabled)". If passwordless sudo is enabled,
then you can script it.

https://codereview.appspot.com/38240043/diff/20001/environs/sshstorage/storage.go
File environs/sshstorage/storage.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/sshstorage/storage.go#newcode6...

[Andrew Wilkins (axwalk)]

On 2013/12/19 08:44:33, axw wrote:
> Please take a look.

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go
> File environs/bootstrapcontext.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go#newcode49
> environs/bootstrapcontext.go:49: func (ctx *BootstrapContext)
> SetInterruptHandler(f func()) (old func()) {
> On 2013/12/11 18:53:24, rog wrote:
> > I'm not keen on this approach, as it doesn't work well in a
concurrent
> > situation. It's quite possible that we might have several concurrent
> goroutines
> > all wanting to know when they've been interrupted. This scheme means
each one
> > may trample on another's handler.
> >
> > The underlying os/signal mechanism works fine in that situation. Why
not use
> > something more like that here?
> >
> > e.g.
> > type BootstrapContext struct {
> > Stdin io.Reader
> > Stdout io.Writer
> > Stderr io.Writer
> > InterruptNotify func(sig chan<- os.Signal)
> > StopInterruptNotify func(chan<- os.Signal)
> > }
> >
> >
> > func DefaultBootstrapContext() *BootstrapContext {
> > return &BootstrapContext{
> > Stdin: os.Stdin,
> > Stdout: os.Stdout,
> > Stderr: os.Stderr,
> > InterruptNotify: func(c chan<- os.Signal) {
> > signal.Notify(c, os.Interrupt)
> > },
> > StopInterruptNotify: signal.Stop,
> > }
> > }
> >
> > One thought - perhaps it might be nice to specify it as an interface
rather
> > than a concrete type; then cmd.Context could be made to satisfy it.
> > Something like this, perhaps?
> >
> > type Stdio struct {
> > Stdin io.Reader
> > Stdout io.Writer
> > Stderr io.Writer
> > }
> >
> > type BootstrapContext interface {
> > Stdio() Stdio
> > InterruptNotify(sig chan<- os.Signal)
> > StopInterruptNotify(chan<- os.Signal)
> > }
> >

> I went with something very similar: Stdio(), Stdout(), and Stderr()
instead of
> Stdio(). Returning a struct makes for awkward package dependencies.
This does
> mean having a wrapper struct, as changing all the fields of
cmd.Context to
> methods is painful.

> I've rewritten SetInterruptHandler, made it internal to
> provider/common/bootstrap.go, and based it on the
> InterruptNotify/StopInterruptNotify methods of BootstrapContext.

Actually, in the end I got rid of that, got rid of tomb, and just used
the channel directly. Much simpler now, I think.

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go
> File environs/manual/provisioner.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go#newcode50
> environs/manual/provisioner.go:50: // and must be a terminal (except
in tests)
> On 2013/12/11 18:53:24, rog wrote:
> > I know this is how it already is, but this doesn't make me feel
happy.
> > Somehow it feels like the wrong level of abstraction, and feels like
it makes
> > things hard to script.
> >
> > This is just a handwave - I can't currently think of concrete
suggestions for
> > anything better, sorry.

> I don't think it's necessarily that bad; perhaps my wording threw you.
It should
> read "must be a terminal (except in tests, or if the remote user has
> pa...

[Andrew Wilkins (axwalk)]

On 2013/12/19 08:44:33, axw wrote:
> Please take a look.

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go
> File environs/bootstrapcontext.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/bootstrapcontext.go#newcode49
> environs/bootstrapcontext.go:49: func (ctx *BootstrapContext)
> SetInterruptHandler(f func()) (old func()) {
> On 2013/12/11 18:53:24, rog wrote:
> > I'm not keen on this approach, as it doesn't work well in a
concurrent
> > situation. It's quite possible that we might have several concurrent
> goroutines
> > all wanting to know when they've been interrupted. This scheme means
each one
> > may trample on another's handler.
> >
> > The underlying os/signal mechanism works fine in that situation. Why
not use
> > something more like that here?
> >
> > e.g.
> > type BootstrapContext struct {
> > Stdin io.Reader
> > Stdout io.Writer
> > Stderr io.Writer
> > InterruptNotify func(sig chan<- os.Signal)
> > StopInterruptNotify func(chan<- os.Signal)
> > }
> >
> >
> > func DefaultBootstrapContext() *BootstrapContext {
> > return &BootstrapContext{
> > Stdin: os.Stdin,
> > Stdout: os.Stdout,
> > Stderr: os.Stderr,
> > InterruptNotify: func(c chan<- os.Signal) {
> > signal.Notify(c, os.Interrupt)
> > },
> > StopInterruptNotify: signal.Stop,
> > }
> > }
> >
> > One thought - perhaps it might be nice to specify it as an interface
rather
> > than a concrete type; then cmd.Context could be made to satisfy it.
> > Something like this, perhaps?
> >
> > type Stdio struct {
> > Stdin io.Reader
> > Stdout io.Writer
> > Stderr io.Writer
> > }
> >
> > type BootstrapContext interface {
> > Stdio() Stdio
> > InterruptNotify(sig chan<- os.Signal)
> > StopInterruptNotify(chan<- os.Signal)
> > }
> >

> I went with something very similar: Stdio(), Stdout(), and Stderr()
instead of
> Stdio(). Returning a struct makes for awkward package dependencies.
This does
> mean having a wrapper struct, as changing all the fields of
cmd.Context to
> methods is painful.

> I've rewritten SetInterruptHandler, made it internal to
> provider/common/bootstrap.go, and based it on the
> InterruptNotify/StopInterruptNotify methods of BootstrapContext.

Actually, in the end I got rid of that, got rid of tomb, and just used
the channel directly. Much simpler now, I think.

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go
> File environs/manual/provisioner.go (right):

https://codereview.appspot.com/38240043/diff/20001/environs/manual/provisioner.go#newcode50
> environs/manual/provisioner.go:50: // and must be a terminal (except
in tests)
> On 2013/12/11 18:53:24, rog wrote:
> > I know this is how it already is, but this doesn't make me feel
happy.
> > Somehow it feels like the wrong level of abstraction, and feels like
it makes
> > things hard to script.
> >
> > This is just a handwave - I can't currently think of concrete
suggestions for
> > anything better, sorry.

> I don't think it's necessarily that bad; perhaps my wording threw you.
It should
> read "must be a terminal (except in tests, or if the remote user has
> pa...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/38240043/

[Roger Peppe (rogpeppe)]

LGTM, thanks, with a few thoughts below.

Also, with respect to this:
> I don't think it's necessarily that bad; perhaps my wording threw you.
It should
> read "must be a terminal (except in tests, or if the remote user has
> passwordless sudo enabled)". If passwordless sudo is enabled, then you
can
> script it.

I think the feeling I'm wary of these changes is that it
feels like a gradual creeping of command-line-specific
interaction into some of the core logic. I would like it
if our core logic was independent of that (I wish we didn't
have to shell out to ssh at all FWIW).

For example, previously it would have been possible
to call Bootstrap inside a web service, but with the new dependency
on stdio, that's not really viable any more.

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

https://codereview.appspot.com/38240043/diff/60001/cmd/juju/bootstrap.go#newcode81
cmd/juju/bootstrap.go:81: type bootstrapContext struct {
nice

https://codereview.appspot.com/38240043/diff/60001/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/38240043/diff/60001/environs/interface.go#newcode200
environs/interface.go:200: // Environ.Bootstrap, providing it a means of
obtaining
s/it // ?

https://codereview.appspot.com/38240043/diff/60001/environs/testing/bootstrap.go
File environs/testing/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/environs/testing/bootstrap.go#newcode50
environs/testing/bootstrap.go:50: func NewBootstrapContext(c *gc.C)
BootstrapContext {
Wouldn't it be better if this returned environs.BootstrapContext ?
(the local type could be unexported)

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode91
provider/common/bootstrap.go:91: for {
for _ = range ch {
      fmt.Fprintln(ctx.Stderr(), "Cleaning up failed bootstrap")
}
return
?

I wonder if we should allow the user to abort if they interrupt more
than 3 times...

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode154
provider/common/bootstrap.go:154: // the terminal, which will be the ssh
subprocess at this
For the record, I don't believe this is actually true.
The interrupt gets sent to all processes in the foreground
process group of the terminal.

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode372
provider/common/bootstrap.go:372: case <-interrupted:
Much more direct - I like.

https://codereview.appspot.com/38240043/

[Andrew Wilkins (axwalk)]

On 2013/12/19 11:29:57, rog wrote:
> LGTM, thanks, with a few thoughts below.

> Also, with respect to this:
> > I don't think it's necessarily that bad; perhaps my wording threw
you. It
> should
> > read "must be a terminal (except in tests, or if the remote user has
> > passwordless sudo enabled)". If passwordless sudo is enabled, then
you can
> > script it.

> I think the feeling I'm wary of these changes is that it
> feels like a gradual creeping of command-line-specific
> interaction into some of the core logic. I would like it
> if our core logic was independent of that (I wish we didn't
> have to shell out to ssh at all FWIW).

That's fair, although I do think bootstrap is somewhat removed from the
"core logic". Your point about web services is a good one.

> For example, previously it would have been possible
> to call Bootstrap inside a web service, but with the new dependency
> on stdio, that's not really viable any more.

As mentioned on IRC, I don't think that's true. We push authorized_keys
to the cloud, and passwordless sudo is enabled by default; there's no
need for any interactivity. The exception is manual provisioning *iff* a
prompt is required (it may not be), which has never been supported
outside of a non-interactive terminal session.

We can do it all now with $SSHPASS, but that's not really ideal due to
its process-global nature. We could change that to add some programmable
prompts, or use PTYs directly to do the same, but it's starting to get
quite complicated. Or we could do it with go.crypto/ssh, which is
something I hope to introduce in the not too distant future.

https://codereview.appspot.com/38240043/diff/60001/cmd/juju/bootstrap.go
> File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/cmd/juju/bootstrap.go#newcode81
> cmd/juju/bootstrap.go:81: type bootstrapContext struct {
> nice

https://codereview.appspot.com/38240043/diff/60001/environs/interface.go
> File environs/interface.go (right):

https://codereview.appspot.com/38240043/diff/60001/environs/interface.go#newcode200
> environs/interface.go:200: // Environ.Bootstrap, providing it a means
of
> obtaining
> s/it // ?

https://codereview.appspot.com/38240043/diff/60001/environs/testing/bootstrap.go
> File environs/testing/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/environs/testing/bootstrap.go#newcode50
> environs/testing/bootstrap.go:50: func NewBootstrapContext(c *gc.C)
> BootstrapContext {
> Wouldn't it be better if this returned environs.BootstrapContext ?
> (the local type could be unexported)

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go
> File provider/common/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode91
> provider/common/bootstrap.go:91: for {
> for _ = range ch {
> fmt.Fprintln(ctx.Stderr(), "Cleaning up failed bootstrap")
> }
> return
> ?

> I wonder if we should allow the user to abort if they interrupt more
than 3
> times...

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode154
> provider/common/bootstrap.go:154: // the termina...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/38240043/diff/60001/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/38240043/diff/60001/environs/interface.go#newcode200
environs/interface.go:200: // Environ.Bootstrap, providing it a means of
obtaining
On 2013/12/19 11:29:57, rog wrote:
> s/it // ?

I meant provided to Environ.Bootstrap. It makes sense either way to me;
I'm not fussed, I'll change it.

https://codereview.appspot.com/38240043/diff/60001/environs/testing/bootstrap.go
File environs/testing/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/environs/testing/bootstrap.go#newcode50
environs/testing/bootstrap.go:50: func NewBootstrapContext(c *gc.C)
BootstrapContext {
On 2013/12/19 11:29:57, rog wrote:
> Wouldn't it be better if this returned environs.BootstrapContext ?
> (the local type could be unexported)

I had planned to do that, but some tests want to change the stdio
objects. Alternatively, NewBootstrapContext could just take a
*cmd.Context. I'll do that before landing.

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode91
provider/common/bootstrap.go:91: for {
On 2013/12/19 11:29:57, rog wrote:
> for _ = range ch {
> fmt.Fprintln(ctx.Stderr(), "Cleaning up failed bootstrap")
> }
> return
> ?

Wow, that was dumb. Thanks.

> I wonder if we should allow the user to abort if they interrupt more
than 3
> times...

Sounds reasonable, but I'd like to stick with what we've got for now and
tweak it based on feedback after it's been used a bit.

https://codereview.appspot.com/38240043/diff/60001/provider/common/bootstrap.go#newcode154
provider/common/bootstrap.go:154: // the terminal, which will be the ssh
subprocess at this
On 2013/12/19 11:29:57, rog wrote:
> For the record, I don't believe this is actually true.
> The interrupt gets sent to all processes in the foreground
> process group of the terminal.

My statement wasn't meant imply that it was exclusive of all other
processes in the group. That's why the signal remains blocked; so that
when it gets delivered to the child and this one, this one ignores it
and simply waits on the child's death.

If I'm still wrong, I'd be happy for some schooling :)

https://codereview.appspot.com/38240043/

=== modified file 'cloudinit/sshinit/configure.go'
--- cloudinit/sshinit/configure.go	2013-12-16 07:20:01 +0000
+++ cloudinit/sshinit/configure.go	2013-12-20 02:40:15 +0000
@@ -6,7 +6,7 @@
 import (
 	"encoding/base64"
 	"fmt"
-	"os"
+	"io"
 	"strings"
 
 	"launchpad.net/loggo"
@@ -18,24 +18,42 @@
 
 var logger = loggo.GetLogger("juju.cloudinit.sshinit")
 
+type ConfigureParams struct {
+	// Host is the host to configure, in the format [user@]hostname.
+	Host string
+
+	// Config is the cloudinit config to carry out.
+	Config *cloudinit.Config
+
+	// Stdin is required to respond to sudo prompts,
+	// and must be a terminal (except in tests)
+	Stdin io.Reader
+
+	// Stdout is required to present sudo prompts to the user.
+	Stdout io.Writer
+
+	// Stderr is required to present bootstrap progress to the user.
+	Stderr io.Writer
+}
+
 // Configure connects to the specified host over SSH,
 // and executes a script that carries out cloud-config.
-func Configure(host string, cfg *cloudinit.Config) error {
-	logger.Infof("Provisioning machine agent on %s", host)
-	script, err := generateScript(cfg)
+func Configure(params ConfigureParams) error {
+	logger.Infof("Provisioning machine agent on %s", params.Host)
+	script, err := generateScript(params.Config)
 	if err != nil {
 		return err
 	}
 	scriptBase64 := base64.StdEncoding.EncodeToString([]byte(script))
 	script = fmt.Sprintf(`F=$(mktemp); echo %s | base64 -d > $F; . $F`, scriptBase64)
 	cmd := ssh.Command(
-		host,
+		params.Host,
 		[]string{"sudo", fmt.Sprintf("bash -c '%s'", script)},
 		ssh.AllocateTTY,
 	)
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	cmd.Stdin = os.Stdin
+	cmd.Stdout = params.Stdout
+	cmd.Stderr = params.Stderr
+	cmd.Stdin = params.Stdin
 	return cmd.Run()
 }
 

=== modified file 'cmd/cmd.go'
--- cmd/cmd.go	2013-12-13 04:56:01 +0000
+++ cmd/cmd.go	2013-12-20 02:40:15 +0000
@@ -11,6 +11,7 @@
 	"io/ioutil"
 	"net/http"
 	"os"
+	"os/signal"
 	"path/filepath"
 	"strings"
 
@@ -111,6 +112,16 @@
 	return filepath.Join(ctx.Dir, path)
 }
 
+// InterruptNotify partially satisfies environs/bootstrap.BootstrapContext
+func (ctx *Context) InterruptNotify(c chan<- os.Signal) {
+	signal.Notify(c, os.Interrupt)
+}
+
+// StopInterruptNotify partially satisfies environs/bootstrap.BootstrapContext
+func (ctx *Context) StopInterruptNotify(c chan<- os.Signal) {
+	signal.Stop(c)
+}
+
 // Info holds some of the usage documentation of a Command.
 type Info struct {
 	// Name is the Command's name.

=== modified file 'cmd/juju/addmachine.go'
--- cmd/juju/addmachine.go	2013-12-19 14:19:21 +0000
+++ cmd/juju/addmachine.go	2013-12-20 02:40:15 +0000
@@ -141,11 +141,14 @@
 	return m.String(), err
 }
 
-func (c *AddMachineCommand) Run(_ *cmd.Context) error {
+func (c *AddMachineCommand) Run(ctx *cmd.Context) error {
 	if c.SSHHost != "" {
 		args := manual.ProvisionMachineArgs{
 			Host:    c.SSHHost,
 			EnvName: c.EnvName,
+			Stdin:   ctx.Stdin,
+			Stdout:  ctx.Stdout,
+			Stderr:  ctx.Stderr,
 		}
 		_, err := manual.ProvisionMachine(args)
 		return err

=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2013-12-03 11:40:28 +0000
+++ cmd/juju/bootstrap.go	2013-12-20 02:40:15 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"fmt"
+	"io"
 	"strings"
 
 	"launchpad.net/gnuflag"
@@ -77,6 +78,22 @@
 	return cmd.CheckEmpty(args)
 }
 
+type bootstrapContext struct {
+	*cmd.Context
+}
+
+func (c bootstrapContext) Stdin() io.Reader {
+	return c.Context.Stdin
+}
+
+func (c bootstrapContext) Stdout() io.Writer {
+	return c.Context.Stdout
+}
+
+func (c bootstrapContext) Stderr() io.Writer {
+	return c.Context.Stderr
+}
+
 // Run connects to the environment specified on the command line and bootstraps
 // a juju in that environment if none already exists. If there is as yet no environments.yaml file,
 // the user is informed how to create one.
@@ -89,10 +106,11 @@
 	if err != nil {
 		return err
 	}
+	bootstrapContext := bootstrapContext{ctx}
 	// If the environment has a special bootstrap Storage, use it wherever
 	// we'd otherwise use environ.Storage.
 	if bs, ok := environ.(environs.BootstrapStorager); ok {
-		if err := bs.EnableBootstrapStorage(); err != nil {
+		if err := bs.EnableBootstrapStorage(bootstrapContext); err != nil {
 			return fmt.Errorf("failed to enable bootstrap storage: %v", err)
 		}
 	}
@@ -116,7 +134,7 @@
 			return err
 		}
 	}
-	return bootstrap.Bootstrap(environ, c.Constraints)
+	return bootstrap.Bootstrap(bootstrapContext, environ, c.Constraints)
 }
 
 func (c *BootstrapCommand) uploadTools(environ environs.Environ) error {

=== modified file 'environs/bootstrap/bootstrap.go'
--- environs/bootstrap/bootstrap.go	2013-12-20 00:30:15 +0000
+++ environs/bootstrap/bootstrap.go	2013-12-20 02:40:15 +0000
@@ -20,7 +20,7 @@
 // Bootstrap bootstraps the given environment. The supplied constraints are
 // used to provision the instance, and are also set within the bootstrapped
 // environment.
-func Bootstrap(environ environs.Environ, cons constraints.Value) error {
+func Bootstrap(ctx environs.BootstrapContext, environ environs.Environ, cons constraints.Value) error {
 	cfg := environ.Config()
 	if secret := cfg.AdminSecret(); secret == "" {
 		return fmt.Errorf("environment configuration has no admin-secret")
@@ -44,7 +44,7 @@
 		return err
 	}
 	logger.Infof("bootstrapping environment %q", environ.Name())
-	return environ.Bootstrap(cons)
+	return environ.Bootstrap(ctx, cons)
 }
 
 // SetBootstrapTools returns the newest tools from the given tools list,

=== modified file 'environs/bootstrap/bootstrap_test.go'
--- environs/bootstrap/bootstrap_test.go	2013-11-18 05:41:36 +0000
+++ environs/bootstrap/bootstrap_test.go	2013-12-20 02:40:15 +0000
@@ -53,6 +53,11 @@
 	s.LoggingSuite.TearDownTest(c)
 }
 
+func bootstrapContext(c *gc.C) environs.BootstrapContext {
+	ctx := coretesting.Context(c)
+	return envtesting.NewBootstrapContext(ctx)
+}
+
 func (s *bootstrapSuite) TestBootstrapNeedsSettings(c *gc.C) {
 	env := newEnviron("bar", noKeysDefined)
 	s.setDummyStorage(c, env)
@@ -64,20 +69,20 @@
 		env.cfg = cfg
 	}
 
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "environment configuration has no admin-secret")
 
 	fixEnv("admin-secret", "whatever")
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "environment configuration has no ca-cert")
 
 	fixEnv("ca-cert", coretesting.CACert)
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "environment configuration has no ca-private-key")
 
 	fixEnv("ca-private-key", coretesting.CAKey)
 	uploadTools(c, env)
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 }
 
@@ -90,7 +95,7 @@
 func (s *bootstrapSuite) TestBootstrapEmptyConstraints(c *gc.C) {
 	env := newEnviron("foo", useDefaultKeys)
 	s.setDummyStorage(c, env)
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	c.Assert(env.bootstrapCount, gc.Equals, 1)
 	c.Assert(env.constraints, gc.DeepEquals, constraints.Value{})
@@ -100,7 +105,7 @@
 	env := newEnviron("foo", useDefaultKeys)
 	s.setDummyStorage(c, env)
 	cons := constraints.MustParse("cpu-cores=2 mem=4G")
-	err := bootstrap.Bootstrap(env, cons)
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, cons)
 	c.Assert(err, gc.IsNil)
 	c.Assert(env.bootstrapCount, gc.Equals, 1)
 	c.Assert(env.constraints, gc.DeepEquals, cons)
@@ -163,7 +168,7 @@
 		if test.Arch != "" {
 			cons = constraints.MustParse("arch=" + test.Arch)
 		}
-		err = bootstrap.Bootstrap(env, cons)
+		err = bootstrap.Bootstrap(bootstrapContext(c), env, cons)
 		if test.Err != nil {
 			c.Check(err, gc.ErrorMatches, ".*"+test.Err.Error())
 			continue
@@ -186,7 +191,7 @@
 	env := newEnviron("foo", useDefaultKeys)
 	s.setDummyStorage(c, env)
 	envtesting.RemoveFakeTools(c, env.Storage())
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	// bootstrap.Bootstrap leaves it to the provider to
 	// locate bootstrap tools.
 	c.Assert(err, gc.IsNil)
@@ -252,7 +257,7 @@
 	return e.name
 }
 
-func (e *bootstrapEnviron) Bootstrap(cons constraints.Value) error {
+func (e *bootstrapEnviron) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
 	e.bootstrapCount++
 	e.constraints = cons
 	return nil

=== modified file 'environs/interface.go'
--- environs/interface.go	2013-11-18 05:41:36 +0000
+++ environs/interface.go	2013-12-20 02:40:15 +0000
@@ -4,6 +4,9 @@
 package environs
 
 import (
+	"io"
+	"os"
+
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/storage"
@@ -71,7 +74,7 @@
 	// EnableBootstrapStorage enables bootstrap storage, returning an
 	// error if enablement failed. If nil is returned, then calling
 	// this again will have no effect and will return nil.
-	EnableBootstrapStorage() error
+	EnableBootstrapStorage(BootstrapContext) error
 }
 
 // ConfigGetter implements access to an environments configuration.
@@ -136,7 +139,7 @@
 	// Bootstrap is responsible for selecting the appropriate tools,
 	// and setting the agent-version configuration attribute prior to
 	// bootstrapping the environment.
-	Bootstrap(cons constraints.Value) error
+	Bootstrap(ctx BootstrapContext, cons constraints.Value) error
 
 	// StateInfo returns information on the state initialized
 	// by Bootstrap.
@@ -192,3 +195,24 @@
 	// Provider returns the EnvironProvider that created this Environ.
 	Provider() EnvironProvider
 }
+
+// BootstrapContext is an interface that is passed to
+// Environ.Bootstrap, providing a means of obtaining
+// information about and manipulating the context in which
+// it is being invoked.
+type BootstrapContext interface {
+	Stdin() io.Reader
+	Stdout() io.Writer
+	Stderr() io.Writer
+
+	// InterruptNotify starts watching for interrupt signals
+	// on behalf of the caller, sending them to the supplied
+	// channel.
+	InterruptNotify(sig chan<- os.Signal)
+
+	// StopInterruptNotify undoes the effects of a previous
+	// call to InterruptNotify with the same channel. After
+	// StopInterruptNotify returns, no more signals will be
+	// delivered to the channel.
+	StopInterruptNotify(chan<- os.Signal)
+}

=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2013-12-19 19:54:23 +0000
+++ environs/jujutest/livetests.go	2013-12-20 02:40:15 +0000
@@ -140,7 +140,7 @@
 	envtesting.UploadFakeTools(c, t.Env.Storage())
 	err := common.EnsureNotBootstrapped(t.Env)
 	c.Assert(err, gc.IsNil)
-	err = bootstrap.Bootstrap(t.Env, cons)
+	err = bootstrap.Bootstrap(bootstrapContext(c), t.Env, cons)
 	c.Assert(err, gc.IsNil)
 	t.bootstrapped = true
 }
@@ -924,7 +924,7 @@
 	err = storageCopy(dummyStorage, currentName, envStorage, otherName)
 	c.Assert(err, gc.IsNil)
 
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	conn, err := juju.NewConn(env)

=== modified file 'environs/jujutest/tests.go'
--- environs/jujutest/tests.go	2013-10-15 04:38:40 +0000
+++ environs/jujutest/tests.go	2013-12-20 02:40:15 +0000
@@ -78,6 +78,10 @@
 	t.LoggingSuite.TearDownTest(c)
 }
 
+func bootstrapContext(c *gc.C) environs.BootstrapContext {
+	return envtesting.NewBootstrapContext(coretesting.Context(c))
+}
+
 func (t *Tests) TestStartStop(c *gc.C) {
 	e := t.Prepare(c)
 	envtesting.UploadFakeTools(c, e.Storage())
@@ -134,7 +138,7 @@
 	envtesting.UploadFakeTools(c, e.Storage())
 	err := common.EnsureNotBootstrapped(e)
 	c.Assert(err, gc.IsNil)
-	err = bootstrap.Bootstrap(e, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), e, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	info, apiInfo, err := e.StateInfo()
@@ -162,7 +166,7 @@
 
 	err = common.EnsureNotBootstrapped(e3)
 	c.Assert(err, gc.IsNil)
-	err = bootstrap.Bootstrap(e3, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), e3, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	err = common.EnsureNotBootstrapped(e3)

=== modified file 'environs/manual/bootstrap.go'
--- environs/manual/bootstrap.go	2013-11-20 02:31:56 +0000
+++ environs/manual/bootstrap.go	2013-12-20 02:40:15 +0000
@@ -30,6 +30,7 @@
 	DataDir       string
 	Environ       LocalStorageEnviron
 	PossibleTools tools.List
+	Context       environs.BootstrapContext
 
 	// If series and hardware characteristics
 	// are known ahead of time, they can be
@@ -134,5 +135,5 @@
 	for k, v := range agentEnv {
 		mcfg.AgentEnvironment[k] = v
 	}
-	return provisionMachineAgent(args.Host, mcfg)
+	return provisionMachineAgent(args.Host, mcfg, args.Context.Stdin(), args.Context.Stdout(), args.Context.Stderr())
 }

=== modified file 'environs/manual/bootstrap_test.go'
--- environs/manual/bootstrap_test.go	2013-11-18 04:53:44 +0000
+++ environs/manual/bootstrap_test.go	2013-12-20 02:40:15 +0000
@@ -12,9 +12,11 @@
 	"launchpad.net/juju-core/environs/bootstrap"
 	"launchpad.net/juju-core/environs/filestorage"
 	"launchpad.net/juju-core/environs/storage"
+	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
+	coretesting "launchpad.net/juju-core/testing"
 )
 
 type bootstrapSuite struct {
@@ -74,6 +76,7 @@
 		DataDir:       "/var/lib/juju",
 		Environ:       s.env,
 		PossibleTools: toolsList,
+		Context:       envtesting.NewBootstrapContext(coretesting.Context(c)),
 	}
 }
 

=== modified file 'environs/manual/provisioner.go'
--- environs/manual/provisioner.go	2013-12-11 09:14:12 +0000
+++ environs/manual/provisioner.go	2013-12-20 02:40:15 +0000
@@ -6,6 +6,7 @@
 import (
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"strings"
 
@@ -44,6 +45,16 @@
 	// Tools to install on the machine. If nil, tools will be automatically
 	// chosen using environs/tools FindInstanceTools.
 	Tools *tools.Tools
+
+	// Stdin is required to respond to sudo prompts,
+	// and must be a terminal (except in tests)
+	Stdin io.Reader
+
+	// Stdout is required to present sudo prompts to the user.
+	Stdout io.Writer
+
+	// Stderr is required to present machine provisioning progress to the user.
+	Stderr io.Writer
 }
 
 // ErrProvisioned is returned by ProvisionMachine if the target
@@ -91,7 +102,7 @@
 	}
 
 	// Finally, provision the machine agent.
-	err = provisionMachineAgent(args.Host, mcfg)
+	err = provisionMachineAgent(args.Host, mcfg, args.Stdin, args.Stdout, args.Stderr)
 	if err != nil {
 		return machineId, err
 	}
@@ -205,7 +216,7 @@
 	return mcfg, nil
 }
 
-func provisionMachineAgent(host string, mcfg *cloudinit.MachineConfig) error {
+func provisionMachineAgent(host string, mcfg *cloudinit.MachineConfig, stdin io.Reader, stdout, stderr io.Writer) error {
 	cloudcfg := coreCloudinit.New()
 	if err := cloudinit.ConfigureJuju(mcfg, cloudcfg); err != nil {
 		return err
@@ -213,5 +224,11 @@
 	// Explicitly disabling apt_upgrade so as not to trample
 	// the target machine's existing configuration.
 	cloudcfg.SetAptUpgrade(false)
-	return sshinit.Configure(host, cloudcfg)
+	return sshinit.Configure(sshinit.ConfigureParams{
+		Host:   host,
+		Config: cloudcfg,
+		Stdin:  stdin,
+		Stdout: stdout,
+		Stderr: stderr,
+	})
 }

=== modified file 'environs/open_test.go'
--- environs/open_test.go	2013-10-04 12:18:17 +0000
+++ environs/open_test.go	2013-12-20 02:40:15 +0000
@@ -40,7 +40,9 @@
 	env, err := environs.Prepare(cfg, configstore.NewMem())
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, env.Storage())
-	c.Assert(bootstrap.Bootstrap(env, constraints.Value{}), gc.IsNil)
+	ctx := envtesting.NewBootstrapContext(testing.Context(c))
+	err = bootstrap.Bootstrap(ctx, env, constraints.Value{})
+	c.Assert(err, gc.IsNil)
 }
 
 func (*OpenSuite) TestNewUnknownEnviron(c *gc.C) {

=== modified file 'environs/sshstorage/storage.go'
--- environs/sshstorage/storage.go	2013-12-03 05:20:25 +0000
+++ environs/sshstorage/storage.go	2013-12-20 02:40:15 +0000
@@ -11,7 +11,6 @@
 	"fmt"
 	"io"
 	"io/ioutil"
-	"os"
 	"os/exec"
 	"path"
 	"sort"
@@ -59,32 +58,49 @@
 	flockExclusive flockmode = "-x"
 )
 
+type NewSSHStorageParams struct {
+	// Host is the host to connect to, in the format [user@]hostname.
+	Host string
+
+	// StorageDir is the root of the remote storage directory.
+	StorageDir string
+
+	// TmpDir is the remote temporary directory for storage.
+	// A temporary directory must be specified, and should be located on the
+	// same filesystem as the storage directory to ensure atomic writes.
+	// The temporary directory will be created when NewSSHStorage is invoked
+	// if it doesn't already exist; it will never be removed. NewSSHStorage
+	// will attempt to reassign ownership to the login user, and will return
+	// an error if it cannot do so.
+	TmpDir string
+
+	// Stdin in required to respond to sudo passwords,
+	// and must be a terminal (except in tests).
+	Stdin io.Reader
+
+	// Stdout is required to present sudo prompts to the user.
+	Stdout io.Writer
+}
+
 // NewSSHStorage creates a new SSHStorage, connected to the
 // specified host, managing state under the specified remote path.
-//
-// A temporary directory must be specified, and should be located on the
-// same filesystem as the storage directory to ensure atomic writes.
-// The temporary directory will be created when NewSSHStorage is invoked
-// if it doesn't already exist; it will never be removed. NewSSHStorage
-// will attempt to reassign ownership to the login user, and will return
-// an error if it cannot do so.
-func NewSSHStorage(host, storagedir, tmpdir string) (*SSHStorage, error) {
-	if storagedir == "" {
+func NewSSHStorage(params NewSSHStorageParams) (*SSHStorage, error) {
+	if params.StorageDir == "" {
 		return nil, errors.New("storagedir must be specified and non-empty")
 	}
-	if tmpdir == "" {
+	if params.TmpDir == "" {
 		return nil, errors.New("tmpdir must be specified and non-empty")
 	}
 
 	script := fmt.Sprintf(
 		"install -d -g $SUDO_GID -o $SUDO_UID %s %s",
-		utils.ShQuote(storagedir),
-		utils.ShQuote(tmpdir),
+		utils.ShQuote(params.StorageDir),
+		utils.ShQuote(params.TmpDir),
 	)
 
-	cmd := sshCommand(host, true, fmt.Sprintf("sudo bash -c %s", utils.ShQuote(script)))
-	cmd.Stdin = os.Stdin
-	cmd.Stdout = os.Stdout // for sudo prompts/output
+	cmd := sshCommand(params.Host, true, fmt.Sprintf("sudo bash -c %s", utils.ShQuote(script)))
+	cmd.Stdin = params.Stdin
+	cmd.Stdout = params.Stdout // for sudo prompts/output
 	var stderr bytes.Buffer
 	cmd.Stderr = &stderr
 	if err := cmd.Run(); err != nil {
@@ -95,7 +111,7 @@
 	// We could use sftp, but then we'd be at the mercy of
 	// sftp's output messages for checking errors. Instead,
 	// we execute an interactive bash shell.
-	cmd = sshCommand(host, false, "bash")
+	cmd = sshCommand(params.Host, false, "bash")
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
 		return nil, err
@@ -109,9 +125,9 @@
 	// get at catastrophic failure messages.
 	cmd.Stderr = cmd.Stdout
 	stor := &SSHStorage{
-		host:       host,
-		remotepath: storagedir,
-		tmpdir:     tmpdir,
+		host:       params.Host,
+		remotepath: params.StorageDir,
+		tmpdir:     params.TmpDir,
 		cmd:        cmd,
 		stdin:      stdin,
 		stdout:     stdout,
@@ -120,7 +136,7 @@
 	cmd.Start()
 
 	// Verify we have write permissions.
-	_, err = stor.runf(flockExclusive, "touch %s", utils.ShQuote(storagedir))
+	_, err = stor.runf(flockExclusive, "touch %s", utils.ShQuote(params.StorageDir))
 	if err != nil {
 		stdin.Close()
 		stdout.Close()

=== modified file 'environs/sshstorage/storage_test.go'
--- environs/sshstorage/storage_test.go	2013-10-07 03:40:52 +0000
+++ environs/sshstorage/storage_test.go	2013-12-20 02:40:15 +0000
@@ -40,6 +40,17 @@
 	return cmd
 }
 
+func newSSHStorage(host, storageDir, tmpDir string) (*SSHStorage, error) {
+	params := NewSSHStorageParams{
+		Host:       host,
+		StorageDir: storageDir,
+		TmpDir:     tmpDir,
+		Stdin:      &bytes.Buffer{},
+		Stdout:     ioutil.Discard,
+	}
+	return NewSSHStorage(params)
+}
+
 // flockBin is the path to the original "flock" binary.
 var flockBin string
 
@@ -68,7 +79,7 @@
 
 func (s *storageSuite) makeStorage(c *gc.C) (storage *SSHStorage, storageDir string) {
 	storageDir = c.MkDir()
-	storage, err := NewSSHStorage("example.com", storageDir, storageDir+"-tmp")
+	storage, err := newSSHStorage("example.com", storageDir, storageDir+"-tmp")
 	c.Assert(err, gc.IsNil)
 	c.Assert(storage, gc.NotNil)
 	s.AddCleanup(func(*gc.C) { storage.Close() })
@@ -89,12 +100,12 @@
 	}
 }
 
-func (s *storageSuite) TestNewSSHStorage(c *gc.C) {
+func (s *storageSuite) TestnewSSHStorage(c *gc.C) {
 	storageDir := c.MkDir()
-	// Run this block twice to ensure NewSSHStorage can reuse
+	// Run this block twice to ensure newSSHStorage can reuse
 	// an existing storage location.
 	for i := 0; i < 2; i++ {
-		stor, err := NewSSHStorage("example.com", storageDir, storageDir+"-tmp")
+		stor, err := newSSHStorage("example.com", storageDir, storageDir+"-tmp")
 		c.Assert(err, gc.IsNil)
 		c.Assert(stor, gc.NotNil)
 		c.Assert(stor.Close(), gc.IsNil)
@@ -106,7 +117,7 @@
 	storageDir = c.MkDir()
 	err = os.Chmod(storageDir, 0555)
 	c.Assert(err, gc.IsNil)
-	_, err = NewSSHStorage("example.com", filepath.Join(storageDir, "subdir"), storageDir+"-tmp")
+	_, err = newSSHStorage("example.com", filepath.Join(storageDir, "subdir"), storageDir+"-tmp")
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*cannot change owner and permissions of(.|\n)*")
 }
 
@@ -177,13 +188,13 @@
 	}
 	s.PatchValue(&sshCommand, badSshCommand)
 
-	stor, err := NewSSHStorage("example.com", c.MkDir(), c.MkDir())
+	stor, err := newSSHStorage("example.com", c.MkDir(), c.MkDir())
 	c.Assert(err, gc.IsNil)
 	defer stor.Close()
 	err = stor.Put("whatever", bytes.NewBuffer(nil), 0)
 	c.Assert(err, gc.ErrorMatches, `failed to write input: write \|1: broken pipe \(output: "blah blah\\nmore"\)`)
 
-	_, err = NewSSHStorage("example.com", c.MkDir(), c.MkDir())
+	_, err = newSSHStorage("example.com", c.MkDir(), c.MkDir())
 	c.Assert(err, gc.ErrorMatches, `failed to locate "JUJU-RC: " \(output: "Hey it's JUJU-RC: , but not at the beginning of the line\\nmore"\)`)
 }
 
@@ -292,7 +303,7 @@
 	//
 	// flock exits with exit code 1 if it can't acquire the
 	// lock immediately in non-blocking mode (which the tests force).
-	_, err := NewSSHStorage("example.com", storageDir, storageDir+"-tmp")
+	_, err := newSSHStorage("example.com", storageDir, storageDir+"-tmp")
 	c.Assert(err, gc.ErrorMatches, "exit code 1")
 }
 
@@ -338,13 +349,13 @@
 
 func (s *storageSuite) TestStorageDirBlank(c *gc.C) {
 	tmpdir := c.MkDir()
-	_, err := NewSSHStorage("example.com", "", tmpdir)
+	_, err := newSSHStorage("example.com", "", tmpdir)
 	c.Assert(err, gc.ErrorMatches, "storagedir must be specified and non-empty")
 }
 
 func (s *storageSuite) TestTmpDirBlank(c *gc.C) {
 	storageDir := c.MkDir()
-	_, err := NewSSHStorage("example.com", storageDir, "")
+	_, err := newSSHStorage("example.com", storageDir, "")
 	c.Assert(err, gc.ErrorMatches, "tmpdir must be specified and non-empty")
 }
 
@@ -354,7 +365,7 @@
 	storageDir := c.MkDir()
 	tmpdirs := []string{storageDir, filepath.Join(storageDir, "subdir")}
 	for _, tmpdir := range tmpdirs {
-		stor, err := NewSSHStorage("example.com", storageDir, tmpdir)
+		stor, err := newSSHStorage("example.com", storageDir, tmpdir)
 		defer stor.Close()
 		c.Assert(err, gc.IsNil)
 		err = stor.Put("test-write", bytes.NewReader(nil), 0)
@@ -363,13 +374,13 @@
 }
 
 func (s *storageSuite) TestTmpDirPermissions(c *gc.C) {
-	// NewSSHStorage will fail if it can't create or change the
+	// newSSHStorage will fail if it can't create or change the
 	// permissions of the temporary directory.
 	storageDir := c.MkDir()
 	tmpdir := c.MkDir()
 	os.Chmod(tmpdir, 0400)
 	defer os.Chmod(tmpdir, 0755)
-	_, err := NewSSHStorage("example.com", storageDir, filepath.Join(tmpdir, "subdir2"))
+	_, err := newSSHStorage("example.com", storageDir, filepath.Join(tmpdir, "subdir2"))
 	c.Assert(err, gc.ErrorMatches, ".*install: cannot create directory.*Permission denied.*")
 }
 
@@ -379,6 +390,6 @@
 	tmpdir := filepath.Join(storageDirBase, `"`)
 	c.Assert(os.Mkdir(storageDir, 0755), gc.IsNil)
 	c.Assert(os.Mkdir(tmpdir, 0755), gc.IsNil)
-	_, err := NewSSHStorage("example.com", storageDir, tmpdir)
+	_, err := newSSHStorage("example.com", storageDir, tmpdir)
 	c.Assert(err, gc.IsNil)
 }

=== modified file 'environs/testing/bootstrap.go'
--- environs/testing/bootstrap.go	2013-12-03 05:20:25 +0000
+++ environs/testing/bootstrap.go	2013-12-20 02:40:15 +0000
@@ -4,8 +4,12 @@
 package testing
 
 import (
+	"io"
+
 	"launchpad.net/loggo"
 
+	"launchpad.net/juju-core/cmd"
+	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/provider/common"
@@ -18,9 +22,29 @@
 // do not attempt to SSH to non-existent machines. The result is a function
 // that restores finishBootstrap.
 func DisableFinishBootstrap() func() {
-	f := func(*common.BootstrapContext, instance.Instance, *cloudinit.MachineConfig) error {
+	f := func(environs.BootstrapContext, instance.Instance, *cloudinit.MachineConfig) error {
 		logger.Warningf("provider/common.FinishBootstrap is disabled")
 		return nil
 	}
 	return testbase.PatchValue(&common.FinishBootstrap, f)
 }
+
+type bootstrapContext struct {
+	*cmd.Context
+}
+
+func (c bootstrapContext) Stdin() io.Reader {
+	return c.Context.Stdin
+}
+
+func (c bootstrapContext) Stdout() io.Writer {
+	return c.Context.Stdout
+}
+
+func (c bootstrapContext) Stderr() io.Writer {
+	return c.Context.Stderr
+}
+
+func NewBootstrapContext(ctx *cmd.Context) environs.BootstrapContext {
+	return bootstrapContext{ctx}
+}

=== modified file 'juju/apiconn_test.go'
--- juju/apiconn_test.go	2013-12-04 10:18:57 +0000
+++ juju/apiconn_test.go	2013-12-20 02:40:15 +0000
@@ -49,7 +49,7 @@
 	c.Assert(err, gc.IsNil)
 
 	envtesting.UploadFakeTools(c, env.Storage())
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	cfg = env.Config()

=== modified file 'juju/conn_test.go'
--- juju/conn_test.go	2013-12-19 19:54:23 +0000
+++ juju/conn_test.go	2013-12-20 02:40:15 +0000
@@ -56,13 +56,17 @@
 	cs.LoggingSuite.TearDownTest(c)
 }
 
+func bootstrapContext(c *gc.C) environs.BootstrapContext {
+	return envtesting.NewBootstrapContext(coretesting.Context(c))
+}
+
 func (*NewConnSuite) TestNewConnWithoutAdminSecret(c *gc.C) {
 	cfg, err := config.New(config.NoDefaults, dummy.SampleConfig())
 	c.Assert(err, gc.IsNil)
 	env, err := environs.Prepare(cfg, configstore.NewMem())
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	attrs := env.Config().AllAttrs()
@@ -81,7 +85,7 @@
 	env, err := environs.PrepareFromName(envName, store)
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 }
 
@@ -126,7 +130,7 @@
 	env, err := environs.Prepare(cfg, configstore.NewMem())
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	info, _, err := env.StateInfo()
 	c.Assert(err, gc.IsNil)
@@ -174,7 +178,7 @@
 	env, err := environs.Prepare(cfg, configstore.NewMem())
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	// Make a new Conn, which will push the secrets.
@@ -210,7 +214,7 @@
 	env, err := environs.Prepare(cfg, configstore.NewMem())
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	// Check that Bootstrap has correctly used a hash
@@ -264,7 +268,7 @@
 	environ, err := environs.Prepare(cfg, configstore.NewMem())
 	c.Assert(err, gc.IsNil)
 	envtesting.UploadFakeTools(c, environ.Storage())
-	err = bootstrap.Bootstrap(environ, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), environ, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	s.conn, err = juju.NewConn(environ)
 	c.Assert(err, gc.IsNil)

=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go	2013-12-03 14:19:47 +0000
+++ juju/testing/conn.go	2013-12-20 02:40:15 +0000
@@ -198,7 +198,8 @@
 	c.Assert(environ.Name(), gc.Equals, "dummyenv")
 
 	envtesting.MustUploadFakeTools(environ.Storage())
-	c.Assert(bootstrap.Bootstrap(environ, constraints.Value{}), gc.IsNil)
+	ctx := envtesting.NewBootstrapContext(testing.Context(c))
+	c.Assert(bootstrap.Bootstrap(ctx, environ, constraints.Value{}), gc.IsNil)
 
 	s.BackingState = environ.(GetStater).GetStateInAPIServer()
 

=== modified file 'provider/azure/environ.go'
--- provider/azure/environ.go	2013-11-18 05:41:36 +0000
+++ provider/azure/environ.go	2013-12-20 02:40:15 +0000
@@ -231,7 +231,7 @@
 }
 
 // Bootstrap is specified in the Environ interface.
-func (env *azureEnviron) Bootstrap(cons constraints.Value) (err error) {
+func (env *azureEnviron) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) (err error) {
 	// The creation of the affinity group and the virtual network is specific to the Azure provider.
 	err = env.createAffinityGroup()
 	if err != nil {
@@ -253,7 +253,7 @@
 			env.deleteVirtualNetwork()
 		}
 	}()
-	err = common.Bootstrap(env, cons)
+	err = common.Bootstrap(ctx, env, cons)
 	return err
 }
 

=== modified file 'provider/common/bootstrap.go'
--- provider/common/bootstrap.go	2013-12-18 10:11:12 +0000
+++ provider/common/bootstrap.go	2013-12-20 02:40:15 +0000
@@ -7,14 +7,11 @@
 	"fmt"
 	"io"
 	"os"
-	"os/signal"
 	"path"
 	"strings"
-	"sync"
 	"time"
 
 	"launchpad.net/loggo"
-	"launchpad.net/tomb"
 
 	coreCloudinit "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/cloudinit/sshinit"
@@ -34,19 +31,13 @@
 // Bootstrap is a common implementation of the Bootstrap method defined on
 // environs.Environ; we strongly recommend that this implementation be used
 // when writing a new provider.
-func Bootstrap(env environs.Environ, cons constraints.Value) (err error) {
+func Bootstrap(ctx environs.BootstrapContext, env environs.Environ, cons constraints.Value) (err error) {
 	// TODO make safe in the case of racing Bootstraps
 	// If two Bootstraps are called concurrently, there's
 	// no way to make sure that only one succeeds.
 
-	// TODO(axw) 2013-11-22 #1237736
-	// Modify environs/Environ Bootstrap method signature
-	// to take a new context structure, which contains
-	// Std{in,out,err}, and interrupt signal handling.
-	ctx := BootstrapContext{Stderr: os.Stderr}
-
 	var inst instance.Instance
-	defer func() { handleBootstrapError(err, &ctx, inst, env) }()
+	defer func() { handleBootstrapError(err, ctx, inst, env) }()
 
 	// Create an empty bootstrap state file so we can get its URL.
 	// It will be updated with the instance id and hardware characteristics
@@ -62,12 +53,12 @@
 		return err
 	}
 
-	fmt.Fprintln(ctx.Stderr, "Launching instance")
+	fmt.Fprintln(ctx.Stderr(), "Launching instance")
 	inst, hw, err := env.StartInstance(cons, selectedTools, machineConfig)
 	if err != nil {
 		return fmt.Errorf("cannot start bootstrap instance: %v", err)
 	}
-	fmt.Fprintf(ctx.Stderr, " - %s\n", inst.Id())
+	fmt.Fprintf(ctx.Stderr(), " - %s\n", inst.Id())
 
 	var characteristics []instance.HardwareCharacteristics
 	if hw != nil {
@@ -82,20 +73,28 @@
 	if err != nil {
 		return fmt.Errorf("cannot save state: %v", err)
 	}
-	return FinishBootstrap(&ctx, inst, machineConfig)
+	return FinishBootstrap(ctx, inst, machineConfig)
 }
 
 // handelBootstrapError cleans up after a failed bootstrap.
-func handleBootstrapError(err error, ctx *BootstrapContext, inst instance.Instance, env environs.Environ) {
+func handleBootstrapError(err error, ctx environs.BootstrapContext, inst instance.Instance, env environs.Environ) {
 	if err == nil {
 		return
 	}
+
 	logger.Errorf("bootstrap failed: %v", err)
-	ctx.SetInterruptHandler(func() {
-		fmt.Fprintln(ctx.Stderr, "Cleaning up failed bootstrap")
-	})
+	ch := make(chan os.Signal, 1)
+	ctx.InterruptNotify(ch)
+	defer ctx.StopInterruptNotify(ch)
+	defer close(ch)
+	go func() {
+		for _ = range ch {
+			fmt.Fprintln(ctx.Stderr(), "Cleaning up failed bootstrap")
+		}
+	}()
+
 	if inst != nil {
-		fmt.Fprintln(ctx.Stderr, "Stopping instance...")
+		fmt.Fprintln(ctx.Stderr(), "Stopping instance...")
 		if stoperr := env.StopInstances([]instance.Instance{inst}); stoperr != nil {
 			logger.Errorf("cannot stop failed bootstrap instance %q: %v", inst.Id(), stoperr)
 		} else {
@@ -110,16 +109,16 @@
 			logger.Errorf("cannot delete bootstrap state file: %v", rmerr)
 		}
 	}
-	ctx.SetInterruptHandler(nil)
 }
 
 // FinishBootstrap completes the bootstrap process by connecting
 // to the instance via SSH and carrying out the cloud-config.
 //
 // Note: FinishBootstrap is exposed so it can be replaced for testing.
-var FinishBootstrap = func(ctx *BootstrapContext, inst instance.Instance, machineConfig *cloudinit.MachineConfig) error {
-	var t tomb.Tomb
-	ctx.SetInterruptHandler(func() { t.Killf("interrupted") })
+var FinishBootstrap = func(ctx environs.BootstrapContext, inst instance.Instance, machineConfig *cloudinit.MachineConfig) error {
+	interrupted := make(chan os.Signal, 1)
+	ctx.InterruptNotify(interrupted)
+	defer ctx.StopInterruptNotify(interrupted)
 	// Each attempt to connect to an address must verify the machine is the
 	// bootstrap machine by checking its nonce file exists and contains the
 	// nonce in the MachineConfig. This also blocks sshinit from proceeding
@@ -141,21 +140,27 @@
 	// TODO: jam 2013-12-04 bug #1257649
 	// It would be nice if users had some controll over their bootstrap
 	// timeout, since it is unlikely to be a perfect match for all clouds.
-	addr, err := waitSSH(ctx, checkNonceCommand, inst, &t, DefaultBootstrapSSHTimeout())
+	addr, err := waitSSH(ctx, interrupted, checkNonceCommand, inst, DefaultBootstrapSSHTimeout())
 	if err != nil {
 		return err
 	}
 	// Bootstrap is synchronous, and will spawn a subprocess
 	// to complete the procedure. If the user hits Ctrl-C,
 	// SIGINT is sent to the foreground process attached to
-	// the terminal, which will be the ssh subprocess at that
-	// point.
-	ctx.SetInterruptHandler(func() {})
+	// the terminal, which will be the ssh subprocess at this
+	// point. For that reason, we do not call StopInterruptNotify
+	// until this function completes.
 	cloudcfg := coreCloudinit.New()
 	if err := cloudinit.ConfigureJuju(machineConfig, cloudcfg); err != nil {
 		return err
 	}
-	return sshinit.Configure("ubuntu@"+addr, cloudcfg)
+	return sshinit.Configure(sshinit.ConfigureParams{
+		Host:   "ubuntu@" + addr,
+		Config: cloudcfg,
+		Stdin:  ctx.Stdin(),
+		Stdout: ctx.Stdout(),
+		Stderr: ctx.Stderr(),
+	})
 }
 
 // SSHTimeoutOpts lists the amount of time we will wait for various parts of
@@ -316,8 +321,7 @@
 // the presence of a file on the machine that contains the
 // machine's nonce. The "checkHostScript" is a bash script
 // that performs this file check.
-func waitSSH(ctx *BootstrapContext, checkHostScript string, inst addresser, t *tomb.Tomb, timeout SSHTimeoutOpts) (addr string, err error) {
-	defer t.Done()
+func waitSSH(ctx environs.BootstrapContext, interrupted <-chan os.Signal, checkHostScript string, inst addresser, timeout SSHTimeoutOpts) (addr string, err error) {
 	globalTimeout := time.After(timeout.Timeout)
 	pollAddresses := time.NewTimer(0)
 
@@ -326,24 +330,24 @@
 	// or the tomb is killed.
 	checker := parallelHostChecker{
 		Try:             parallel.NewTry(0, nil),
-		stderr:          ctx.Stderr,
+		stderr:          ctx.Stderr(),
 		active:          make(map[instance.Address]chan struct{}),
 		checkDelay:      timeout.ConnectDelay,
 		checkHostScript: checkHostScript,
 	}
 	defer checker.Kill()
 
-	fmt.Fprintln(ctx.Stderr, "Waiting for address")
+	fmt.Fprintln(ctx.Stderr(), "Waiting for address")
 	for {
 		select {
 		case <-pollAddresses.C:
 			pollAddresses.Reset(timeout.AddressesDelay)
 			if err := inst.Refresh(); err != nil {
-				return "", t.Killf("refreshing addresses: %v", err)
+				return "", fmt.Errorf("refreshing addresses: %v", err)
 			}
 			addresses, err := inst.Addresses()
 			if err != nil {
-				return "", t.Killf("getting addresses: %v", err)
+				return "", fmt.Errorf("getting addresses: %v", err)
 			}
 			checker.UpdateAddresses(addresses)
 		case <-globalTimeout:
@@ -361,8 +365,8 @@
 				args = append(args, lastErr)
 			}
 			return "", fmt.Errorf(format, args...)
-		case <-t.Dying():
-			return "", t.Err()
+		case <-interrupted:
+			return "", fmt.Errorf("interrupted")
 		case <-checker.Dead():
 			result, err := checker.Result()
 			if err != nil {
@@ -373,49 +377,6 @@
 	}
 }
 
-// TODO(axw) move this to environs; see
-// comment near the top of common.Bootstrap.
-type BootstrapContext struct {
-	once        sync.Once
-	handlerchan chan func()
-
-	Stderr io.Writer
-}
-
-func (ctx *BootstrapContext) SetInterruptHandler(f func()) {
-	ctx.once.Do(ctx.initHandler)
-	ctx.handlerchan <- f
-}
-
-func (ctx *BootstrapContext) initHandler() {
-	ctx.handlerchan = make(chan func())
-	go ctx.handleInterrupt()
-}
-
-func (ctx *BootstrapContext) handleInterrupt() {
-	signalchan := make(chan os.Signal, 1)
-	var s chan os.Signal
-	var handler func()
-	for {
-		select {
-		case handler = <-ctx.handlerchan:
-			if handler == nil {
-				if s != nil {
-					signal.Stop(signalchan)
-					s = nil
-				}
-			} else {
-				if s == nil {
-					s = signalchan
-					signal.Notify(signalchan, os.Interrupt)
-				}
-			}
-		case <-s:
-			handler()
-		}
-	}
-}
-
 // EnsureBootstrapTools finds tools, syncing with an external tools source as
 // necessary; it then selects the newest tools to bootstrap with, and sets
 // agent-version.

=== modified file 'provider/common/bootstrap_test.go'
--- provider/common/bootstrap_test.go	2013-12-18 03:13:19 +0000
+++ provider/common/bootstrap_test.go	2013-12-20 02:40:15 +0000
@@ -6,11 +6,11 @@
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"time"
 
 	gc "launchpad.net/gocheck"
 	"launchpad.net/loggo"
-	"launchpad.net/tomb"
 
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
@@ -75,13 +75,23 @@
 	return func() *config.Config { return cfg }
 }
 
+// bootstrapContext creates a BootstrapContext which
+// writes stderr to the bytes.Buffer returned.
+func bootstrapContext(c *gc.C) (ctx environs.BootstrapContext, stderr *bytes.Buffer) {
+	cmdContext := coretesting.Context(c)
+	stderr = &bytes.Buffer{}
+	cmdContext.Stderr = stderr
+	return envtesting.NewBootstrapContext(cmdContext), stderr
+}
+
 func (s *BootstrapSuite) TestCannotWriteStateFile(c *gc.C) {
 	brokenStorage := &mockStorage{
 		Storage: newStorage(s, c),
 		putErr:  fmt.Errorf("noes!"),
 	}
 	env := &mockEnviron{storage: brokenStorage}
-	err := common.Bootstrap(env, constraints.Value{})
+	ctx, _ := bootstrapContext(c)
+	err := common.Bootstrap(ctx, env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "cannot create initial state file: noes!")
 }
 
@@ -107,7 +117,8 @@
 		config:        configGetter(c),
 	}
 
-	err = common.Bootstrap(env, checkCons)
+	ctx, _ := bootstrapContext(c)
+	err = common.Bootstrap(ctx, env, checkCons)
 	c.Assert(err, gc.ErrorMatches, "cannot start bootstrap instance: meh, not started")
 }
 
@@ -137,7 +148,8 @@
 		config:        configGetter(c),
 	}
 
-	err := common.Bootstrap(env, constraints.Value{})
+	ctx, _ := bootstrapContext(c)
+	err := common.Bootstrap(ctx, env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "cannot save state: suddenly a wild blah")
 	c.Assert(stopped, gc.HasLen, 1)
 	c.Assert(stopped[0].Id(), gc.Equals, instance.Id("i-blah"))
@@ -173,7 +185,8 @@
 		config:        configGetter(c),
 	}
 
-	err := common.Bootstrap(env, constraints.Value{})
+	ctx, _ := bootstrapContext(c)
+	err := common.Bootstrap(ctx, env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "cannot save state: suddenly a wild blah")
 	c.Assert(stopped, gc.HasLen, 1)
 	c.Assert(stopped[0].Id(), gc.Equals, instance.Id("i-blah"))
@@ -211,7 +224,8 @@
 		startInstance: startInstance,
 		config:        getConfig,
 	}
-	err := common.Bootstrap(env, constraints.Value{})
+	ctx, _ := bootstrapContext(c)
+	err := common.Bootstrap(ctx, env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	savedState, err := bootstrap.LoadStateFromURL(checkURL)
@@ -244,27 +258,22 @@
 }
 
 func (s *BootstrapSuite) TestWaitSSHTimesOutWaitingForAddresses(c *gc.C) {
-	ctx := &common.BootstrapContext{}
-	buf := &bytes.Buffer{}
-	ctx.Stderr = buf
-	var t tomb.Tomb
-	_, err := common.WaitSSH(ctx, "/bin/true", neverAddresses{}, &t, testSSHTimeout)
+	ctx, stderr := bootstrapContext(c)
+	_, err := common.WaitSSH(ctx, nil, "/bin/true", neverAddresses{}, testSSHTimeout)
 	c.Check(err, gc.ErrorMatches, "waited for 10ms without getting any addresses")
-	c.Check(buf.String(), gc.Matches, "Waiting for address\n")
+	c.Check(stderr.String(), gc.Matches, "Waiting for address\n")
 }
 
 func (s *BootstrapSuite) TestWaitSSHKilledWaitingForAddresses(c *gc.C) {
-	ctx := &common.BootstrapContext{}
-	buf := &bytes.Buffer{}
-	ctx.Stderr = buf
-	var t tomb.Tomb
+	ctx, stderr := bootstrapContext(c)
+	interrupted := make(chan os.Signal, 1)
 	go func() {
 		<-time.After(2 * time.Millisecond)
-		t.Killf("stopping WaitSSH during Addresses")
+		interrupted <- os.Interrupt
 	}()
-	_, err := common.WaitSSH(ctx, "/bin/true", neverAddresses{}, &t, testSSHTimeout)
-	c.Check(err, gc.ErrorMatches, "stopping WaitSSH during Addresses")
-	c.Check(buf.String(), gc.Matches, "Waiting for address\n")
+	_, err := common.WaitSSH(ctx, interrupted, "/bin/true", neverAddresses{}, testSSHTimeout)
+	c.Check(err, gc.ErrorMatches, "interrupted")
+	c.Check(stderr.String(), gc.Matches, "Waiting for address\n")
 }
 
 type brokenAddresses struct {
@@ -276,13 +285,10 @@
 }
 
 func (s *BootstrapSuite) TestWaitSSHStopsOnBadError(c *gc.C) {
-	ctx := &common.BootstrapContext{}
-	buf := &bytes.Buffer{}
-	ctx.Stderr = buf
-	var t tomb.Tomb
-	_, err := common.WaitSSH(ctx, "/bin/true", brokenAddresses{}, &t, testSSHTimeout)
+	ctx, stderr := bootstrapContext(c)
+	_, err := common.WaitSSH(ctx, nil, "/bin/true", brokenAddresses{}, testSSHTimeout)
 	c.Check(err, gc.ErrorMatches, "getting addresses: Addresses will never work")
-	c.Check(buf.String(), gc.Equals, "Waiting for address\n")
+	c.Check(stderr.String(), gc.Equals, "Waiting for address\n")
 }
 
 type neverOpensPort struct {
@@ -295,47 +301,42 @@
 }
 
 func (s *BootstrapSuite) TestWaitSSHTimesOutWaitingForDial(c *gc.C) {
-	ctx := &common.BootstrapContext{}
-	buf := &bytes.Buffer{}
-	ctx.Stderr = buf
-	var t tomb.Tomb
+	ctx, stderr := bootstrapContext(c)
 	// 0.x.y.z addresses are always invalid
-	_, err := common.WaitSSH(ctx, "/bin/true", &neverOpensPort{addr: "0.1.2.3"}, &t, testSSHTimeout)
+	_, err := common.WaitSSH(ctx, nil, "/bin/true", &neverOpensPort{addr: "0.1.2.3"}, testSSHTimeout)
 	c.Check(err, gc.ErrorMatches,
 		`waited for 10ms without being able to connect: mock connection failure to 0.1.2.3`)
-	c.Check(buf.String(), gc.Matches,
+	c.Check(stderr.String(), gc.Matches,
 		"Waiting for address\n"+
 			"(Attempting to connect to 0.1.2.3:22\n)+")
 }
 
-type killOnDial struct {
+type interruptOnDial struct {
 	neverRefreshes
-	name     string
-	tomb     *tomb.Tomb
-	returned bool
+	name        string
+	interrupted chan os.Signal
+	returned    bool
 }
 
-func (k *killOnDial) Addresses() ([]instance.Address, error) {
+func (i *interruptOnDial) Addresses() ([]instance.Address, error) {
 	// kill the tomb the second time Addresses is called
-	if !k.returned {
-		k.returned = true
+	if !i.returned {
+		i.returned = true
 	} else {
-		k.tomb.Killf("stopping WaitSSH during Dial")
+		i.interrupted <- os.Interrupt
 	}
-	return []instance.Address{instance.NewAddress(k.name)}, nil
+	return []instance.Address{instance.NewAddress(i.name)}, nil
 }
 
 func (s *BootstrapSuite) TestWaitSSHKilledWaitingForDial(c *gc.C) {
-	ctx := &common.BootstrapContext{}
-	buf := &bytes.Buffer{}
-	ctx.Stderr = buf
-	var t tomb.Tomb
+	ctx, stderr := bootstrapContext(c)
 	timeout := testSSHTimeout
 	timeout.Timeout = 1 * time.Minute
-	_, err := common.WaitSSH(ctx, "", &killOnDial{name: "0.1.2.3", tomb: &t}, &t, timeout)
-	c.Check(err, gc.ErrorMatches, "stopping WaitSSH during Dial")
+	interrupted := make(chan os.Signal, 1)
+	_, err := common.WaitSSH(ctx, interrupted, "", &interruptOnDial{name: "0.1.2.3", interrupted: interrupted}, timeout)
+	c.Check(err, gc.ErrorMatches, "interrupted")
 	// Exact timing is imprecise but it should have tried a few times before being killed
-	c.Check(buf.String(), gc.Matches,
+	c.Check(stderr.String(), gc.Matches,
 		"Waiting for address\n"+
 			"(Attempting to connect to 0.1.2.3:22\n)+")
 }
@@ -360,25 +361,22 @@
 }
 
 func (s *BootstrapSuite) TestWaitSSHRefreshAddresses(c *gc.C) {
-	ctx := &common.BootstrapContext{}
-	buf := &bytes.Buffer{}
-	ctx.Stderr = buf
-	var t tomb.Tomb
-	_, err := common.WaitSSH(ctx, "", &addressesChange{addrs: [][]string{
+	ctx, stderr := bootstrapContext(c)
+	_, err := common.WaitSSH(ctx, nil, "", &addressesChange{addrs: [][]string{
 		nil,
 		nil,
 		[]string{"0.1.2.3"},
 		[]string{"0.1.2.3"},
 		nil,
 		[]string{"0.1.2.4"},
-	}}, &t, testSSHTimeout)
+	}}, testSSHTimeout)
 	// Not necessarily the last one in the list, due to scheduling.
 	c.Check(err, gc.ErrorMatches,
 		`waited for 10ms without being able to connect: mock connection failure to 0.1.2.[34]`)
-	c.Check(buf.String(), gc.Matches,
+	c.Check(stderr.String(), gc.Matches,
 		"Waiting for address\n"+
 			"(.|\n)*(Attempting to connect to 0.1.2.3:22\n)+(.|\n)*")
-	c.Check(buf.String(), gc.Matches,
+	c.Check(stderr.String(), gc.Matches,
 		"Waiting for address\n"+
 			"(.|\n)*(Attempting to connect to 0.1.2.4:22\n)+(.|\n)*")
 }

=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go	2013-12-19 09:50:26 +0000
+++ provider/dummy/environs.go	2013-12-20 02:40:15 +0000
@@ -102,6 +102,7 @@
 }
 
 type OpBootstrap struct {
+	Context     environs.BootstrapContext
 	Env         string
 	Constraints constraints.Value
 }
@@ -522,7 +523,7 @@
 		storage.NewStorageSimpleStreamsDataSource(e.Storage(), storage.BaseToolsPath)}, nil
 }
 
-func (e *environ) Bootstrap(cons constraints.Value) error {
+func (e *environ) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
 	selectedTools, err := common.EnsureBootstrapTools(e, e.Config().DefaultSeries(), cons.Arch)
 	if err != nil {
 		return err
@@ -592,7 +593,7 @@
 		estate.apiState = st
 	}
 	estate.bootstrapped = true
-	estate.ops <- OpBootstrap{Env: e.name, Constraints: cons}
+	estate.ops <- OpBootstrap{Context: ctx, Env: e.name, Constraints: cons}
 	return nil
 }
 

=== modified file 'provider/ec2/ec2.go'
--- provider/ec2/ec2.go	2013-12-18 14:16:37 +0000
+++ provider/ec2/ec2.go	2013-12-20 02:40:15 +0000
@@ -325,8 +325,8 @@
 	return stor
 }
 
-func (e *environ) Bootstrap(cons constraints.Value) error {
-	return common.Bootstrap(e, cons)
+func (e *environ) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
+	return common.Bootstrap(ctx, e, cons)
 }
 
 func (e *environ) StateInfo() (*state.Info, *api.Info, error) {

=== modified file 'provider/ec2/local_test.go'
--- provider/ec2/local_test.go	2013-12-16 07:20:01 +0000
+++ provider/ec2/local_test.go	2013-12-20 02:40:15 +0000
@@ -191,6 +191,10 @@
 	t.srv.stopServer(c)
 }
 
+func bootstrapContext(c *gc.C) environs.BootstrapContext {
+	return envtesting.NewBootstrapContext(coretesting.Context(c))
+}
+
 func (t *localServerSuite) TestPrecheck(c *gc.C) {
 	env := t.Prepare(c)
 	prechecker, ok := env.(environs.Prechecker)
@@ -205,7 +209,7 @@
 func (t *localServerSuite) TestBootstrapInstanceUserDataAndState(c *gc.C) {
 	env := t.Prepare(c)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	// check that the state holds the id of the bootstrap machine.
@@ -277,7 +281,7 @@
 func (t *localServerSuite) TestInstanceStatus(c *gc.C) {
 	env := t.Prepare(c)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	t.srv.ec2srv.SetInitialInstanceState(ec2test.Terminated)
 	inst, _ := testing.AssertStartInstance(c, env, "1")
@@ -288,7 +292,7 @@
 func (t *localServerSuite) TestStartInstanceHardwareCharacteristics(c *gc.C) {
 	env := t.Prepare(c)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	_, hc := testing.AssertStartInstance(c, env, "1")
 	c.Check(*hc.Arch, gc.Equals, "amd64")
@@ -300,7 +304,7 @@
 func (t *localServerSuite) TestAddresses(c *gc.C) {
 	env := t.Prepare(c)
 	envtesting.UploadFakeTools(c, env.Storage())
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	inst, _ := testing.AssertStartInstance(c, env, "1")
 	c.Assert(err, gc.IsNil)

=== modified file 'provider/joyent/environ.go'
--- provider/joyent/environ.go	2013-11-20 18:15:47 +0000
+++ provider/joyent/environ.go	2013-12-20 02:40:15 +0000
@@ -81,8 +81,8 @@
 	return environs.EmptyStorage
 }
 
-func (env *environ) Bootstrap(cons constraints.Value) error {
-	return common.Bootstrap(env, cons)
+func (env *environ) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
+	return common.Bootstrap(ctx, env, cons)
 }
 
 func (env *environ) StateInfo() (*state.Info, *api.Info, error) {

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2013-12-05 22:32:47 +0000
+++ provider/local/environ.go	2013-12-20 02:40:15 +0000
@@ -104,7 +104,7 @@
 }
 
 // Bootstrap is specified in the Environ interface.
-func (env *localEnviron) Bootstrap(cons constraints.Value) error {
+func (env *localEnviron) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
 	if !env.config.runningAsRoot {
 		return fmt.Errorf("bootstrapping a local environment must be done as root")
 	}

=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go	2013-12-16 09:53:26 +0000
+++ provider/maas/environ.go	2013-12-20 02:40:15 +0000
@@ -77,8 +77,8 @@
 }
 
 // Bootstrap is specified in the Environ interface.
-func (env *maasEnviron) Bootstrap(cons constraints.Value) error {
-	return common.Bootstrap(env, cons)
+func (env *maasEnviron) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
+	return common.Bootstrap(ctx, env, cons)
 }
 
 // StateInfo is specified in the Environ interface.

=== modified file 'provider/maas/environ_whitebox_test.go'
--- provider/maas/environ_whitebox_test.go	2013-12-16 09:53:26 +0000
+++ provider/maas/environ_whitebox_test.go	2013-12-20 02:40:15 +0000
@@ -25,6 +25,7 @@
 	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
+	coretesting "launchpad.net/juju-core/testing"
 	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
@@ -148,12 +149,16 @@
 	return utils.Gunzip(data)
 }
 
+func bootstrapContext(c *gc.C) environs.BootstrapContext {
+	return envtesting.NewBootstrapContext(coretesting.Context(c))
+}
+
 func (suite *environSuite) TestStartInstanceStartsInstance(c *gc.C) {
 	suite.setupFakeTools(c)
 	env := suite.makeEnviron()
 	// Create node 0: it will be used as the bootstrap node.
 	suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	// The bootstrap node has been acquired and started.
 	operations := suite.testMAASObject.TestServer.NodeOperations()
@@ -367,7 +372,7 @@
 	suite.setupFakeTools(c)
 	env := suite.makeEnviron()
 	suite.testMAASObject.TestServer.NewNode(`{"system_id": "thenode", "hostname": "host"}`)
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 }
 
@@ -383,14 +388,14 @@
 	c.Assert(err, gc.IsNil)
 	err = env.SetConfig(cfg)
 	c.Assert(err, gc.IsNil)
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Check(err, gc.ErrorMatches, "cannot find bootstrap tools.*")
 }
 
 func (suite *environSuite) TestBootstrapFailsIfNoNodes(c *gc.C) {
 	suite.setupFakeTools(c)
 	env := suite.makeEnviron()
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	// Since there are no nodes, the attempt to allocate one returns a
 	// 409: Conflict.
 	c.Check(err, gc.ErrorMatches, ".*409.*")
@@ -402,7 +407,7 @@
 	suite.testMAASObject.TestServer.NewNode(`{"system_id": "bootstrapnode", "hostname": "host"}`)
 
 	// bootstrap.Bootstrap calls Environ.Bootstrap.  This works.
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 }
 

=== modified file 'provider/null/environ.go'
--- provider/null/environ.go	2013-11-19 06:43:27 +0000
+++ provider/null/environ.go	2013-12-20 02:40:15 +0000
@@ -85,7 +85,7 @@
 	return e.envConfig().Name()
 }
 
-func (e *nullEnviron) Bootstrap(cons constraints.Value) error {
+func (e *nullEnviron) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
 	envConfig := e.envConfig()
 	hc, series, err := manual.DetectSeriesAndHardwareCharacteristics(envConfig.sshHost())
 	if err != nil {
@@ -96,6 +96,7 @@
 		return err
 	}
 	return manual.Bootstrap(manual.BootstrapArgs{
+		Context:                 ctx,
 		Host:                    e.envConfig().sshHost(),
 		DataDir:                 dataDir,
 		Environ:                 e,
@@ -144,7 +145,7 @@
 }
 
 // Implements environs.BootstrapStorager.
-func (e *nullEnviron) EnableBootstrapStorage() error {
+func (e *nullEnviron) EnableBootstrapStorage(ctx environs.BootstrapContext) error {
 	e.bootstrapStorageMutex.Lock()
 	defer e.bootstrapStorageMutex.Unlock()
 	if e.bootstrapStorage != nil {
@@ -153,7 +154,14 @@
 	cfg := e.envConfig()
 	storageDir := e.StorageDir()
 	storageTmpdir := path.Join(dataDir, storageTmpSubdir)
-	bootstrapStorage, err := sshstorage.NewSSHStorage(cfg.sshHost(), storageDir, storageTmpdir)
+	params := sshstorage.NewSSHStorageParams{
+		Host:       cfg.sshHost(),
+		StorageDir: storageDir,
+		TmpDir:     storageTmpdir,
+		Stdin:      ctx.Stdin(),
+		Stdout:     ctx.Stdout(),
+	}
+	bootstrapStorage, err := sshstorage.NewSSHStorage(params)
 	if err != nil {
 		return err
 	}

=== modified file 'provider/null/environ_test.go'
--- provider/null/environ_test.go	2013-12-12 06:57:36 +0000
+++ provider/null/environ_test.go	2013-12-20 02:40:15 +0000
@@ -14,8 +14,10 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/manual"
 	"launchpad.net/juju-core/environs/sshstorage"
+	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
+	coretesting "launchpad.net/juju-core/testing"
 	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/testing/testbase"
 )
@@ -116,17 +118,18 @@
 	s.PatchEnvironment("PATH", bin+":"+os.Getenv("PATH"))
 
 	s.PatchEnvironment("RC", "99") // simulate ssh failure
-	err = s.env.EnableBootstrapStorage()
+	ctx := envtesting.NewBootstrapContext(coretesting.Context(c))
+	err = s.env.EnableBootstrapStorage(ctx)
 	c.Assert(err, gc.ErrorMatches, "exit code 99")
 	c.Assert(s.env.Storage(), gc.Not(gc.FitsTypeOf), new(sshstorage.SSHStorage))
 
 	s.PatchEnvironment("RC", "0")
-	err = s.env.EnableBootstrapStorage()
+	err = s.env.EnableBootstrapStorage(ctx)
 	c.Assert(err, gc.IsNil)
 	c.Assert(s.env.Storage(), gc.FitsTypeOf, new(sshstorage.SSHStorage))
 
 	// Check idempotency
-	err = s.env.EnableBootstrapStorage()
+	err = s.env.EnableBootstrapStorage(ctx)
 	c.Assert(err, gc.IsNil)
 	c.Assert(s.env.Storage(), gc.FitsTypeOf, new(sshstorage.SSHStorage))
 }

=== modified file 'provider/openstack/local_test.go'
--- provider/openstack/local_test.go	2013-12-10 04:25:06 +0000
+++ provider/openstack/local_test.go	2013-12-20 02:40:15 +0000
@@ -252,6 +252,10 @@
 	s.LoggingSuite.TearDownTest(c)
 }
 
+func bootstrapContext(c *gc.C) environs.BootstrapContext {
+	return envtesting.NewBootstrapContext(coretesting.Context(c))
+}
+
 func (s *localServerSuite) TestPrecheck(c *gc.C) {
 	var cons constraints.Value
 	env := s.Prepare(c)
@@ -281,7 +285,7 @@
 	c.Assert(err, gc.IsNil)
 	env, err := environs.New(cfg)
 	c.Assert(err, gc.IsNil)
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*cannot allocate a public IP as needed(.|\n)*")
 }
 
@@ -310,7 +314,7 @@
 	c.Assert(err, gc.IsNil)
 	env, err := environs.Prepare(cfg, s.ConfigStore)
 	c.Assert(err, gc.IsNil)
-	err = bootstrap.Bootstrap(env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	inst, _ := testing.AssertStartInstance(c, env, "100")
 	err = env.StopInstances([]instance.Instance{inst})
@@ -319,7 +323,7 @@
 
 func (s *localServerSuite) TestStartInstanceHardwareCharacteristics(c *gc.C) {
 	env := s.Prepare(c)
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	_, hc := testing.AssertStartInstanceWithConstraints(c, env, "100", constraints.MustParse("mem=1024"))
 	c.Check(*hc.Arch, gc.Equals, "amd64")
@@ -476,7 +480,7 @@
 // It should be moved to environs.jujutests.Tests.
 func (s *localServerSuite) TestBootstrapInstanceUserDataAndState(c *gc.C) {
 	env := s.Prepare(c)
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	// check that the state holds the id of the bootstrap machine.
@@ -757,7 +761,7 @@
 	openstack.UseTestImageData(metadataStorage, s.cred)
 	defer openstack.RemoveTestImageData(metadataStorage)
 
-	err = bootstrap.Bootstrap(s.env, constraints.Value{})
+	err = bootstrap.Bootstrap(bootstrapContext(c), s.env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 }
 
@@ -900,7 +904,7 @@
 
 func (s *localServerSuite) TestAllInstancesIgnoresOtherMachines(c *gc.C) {
 	env := s.Prepare(c)
-	err := bootstrap.Bootstrap(env, constraints.Value{})
+	err := bootstrap.Bootstrap(bootstrapContext(c), env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 
 	// Check that we see 1 instance in the environment

=== modified file 'provider/openstack/provider.go'
--- provider/openstack/provider.go	2013-12-18 10:11:12 +0000
+++ provider/openstack/provider.go	2013-12-20 02:40:15 +0000
@@ -489,7 +489,7 @@
 	return stor
 }
 
-func (e *environ) Bootstrap(cons constraints.Value) error {
+func (e *environ) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
 	// The client's authentication may have been reset when finding tools if the agent-version
 	// attribute was updated so we need to re-authenticate. This will be a no-op if already authenticated.
 	// An authenticated client is needed for the URL() call below.
@@ -497,7 +497,7 @@
 	if err != nil {
 		return err
 	}
-	return common.Bootstrap(e, cons)
+	return common.Bootstrap(ctx, e, cons)
 }
 
 func (e *environ) StateInfo() (*state.Info, *api.Info, error) {