Code review comment for lp:~axwalk/juju-core/synchronous-bootstrap

This looks pretty solid. I like how manual provisioning and cloud
provisioning now share more code.

Before landing, please test live on each of the providers.

I have a nagging concern about the finishBootstrap step. WaitDNS and the
ssh connect retry should work most if not all of the time, but there are
times when canonistack for example is reaaaally slow to start an
instance. It worries me that we could needlessly kill bootstrap if we
think we have waited long enough but the provider is just slow.
Thoughts?

https://codereview.appspot.com/30190043/diff/1/cloudinit/sshinit/configure_test.go
File cloudinit/sshinit/configure_test.go (left):

https://codereview.appspot.com/30190043/diff/1/cloudinit/sshinit/configure_test.go#oldcode3
cloudinit/sshinit/configure_test.go:3:
Is there a test that "apt-get -y upgrade" is only used when asked for? I
can't see it if there is. I think this is a pretty important change so
warrants an explicit test.

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

https://codereview.appspot.com/30190043/diff/1/cmd/juju/bootstrap.go#newcode126
cmd/juju/bootstrap.go:126: signal.Notify(make(chan os.Signal),
os.Interrupt)
Just a thought. One other option is to install a handler which prints a
message informing the user that their Ctrl-C cannot terminate the cmd
because cleanup is being done.

https://codereview.appspot.com/30190043/diff/1/environs/cloudinit.go
File environs/cloudinit.go (right):

https://codereview.appspot.com/30190043/diff/1/environs/cloudinit.go#newcode135
environs/cloudinit.go:135: // and setup the SSH keys. The rest we leave
to environs/manual.
environs/manual or cloudinit/sshinit?

https://codereview.appspot.com/30190043/diff/1/environs/cloudinit_test.go
File environs/cloudinit_test.go (right):

https://codereview.appspot.com/30190043/diff/1/environs/cloudinit_test.go#newcode207
environs/cloudinit_test.go:207: // for MAAS. MAAS needs to configure and
thren bounce the
typo

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

https://codereview.appspot.com/30190043/diff/1/provider/common/bootstrap.go#newcode35
provider/common/bootstrap.go:35: defer func() {
Could we break this logic out into a separate function outside of
Bootstrap for clarity. The func could be

handleBootstrapError(err error, inst instance.Instance)

https://codereview.appspot.com/30190043/diff/1/provider/common/bootstrap.go#newcode95
provider/common/bootstrap.go:95: func TestingDisableFinishBootstrap()
func() {
I was going to suggest move this to an export_tests.go file but I see
it's called outside this package. Bollocks.

https://codereview.appspot.com/30190043/diff/1/provider/common/bootstrap.go#newcode104
provider/common/bootstrap.go:104: func TestingPatchFinishBootstrap(f
func(instance.Instance, *cloudinit.MachineConfig) error) func() {
Does this need to be exported? It's only called from the above.

https://codereview.appspot.com/30190043/