provider/common/boostrap.go: timeout sensibly

provider/common/boostrap.go: timeout sensibly

The new synchronous bootstrap tries indefinitely until it is able to
get a DNSName and then connect to the machine that should be starting
up (bug #1257427)

This changes it to use the same timeout that we have for connecting to
the DB or API (10 minutes). It adds some bits that make it possible to
set the timeout to other values (which makes nice tests), and then
changes a few of the types in use into smaller interfaces
(instance.Instance into DNSNamer and os.File into io.Writer), which
also makes it easier to test.

https://code.launchpad.net/~jameinel/juju-core/dont-wait-forever-1257427/+merge/197683

(do not edit description out of merge proposal)

[jameinel, 2013-12-04 10:54:41]

Please take a look.

[fwereade, 2013-12-04 14:23:04]

LGTM, just trivials.

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

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap.go#n...
provider/common/bootstrap.go:151: }
Tech-debt bug if there's commonality that should consolidated? I accept it's
low...

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap.go#n...
provider/common/bootstrap.go:162: // All we need for waitSSH is to ask for the
DNS name
orphan comment?

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap.go#n...
provider/common/bootstrap.go:205: connectionAddress := dnsName + ":22"
define connectionAddress 4 lines up and use it in the message?

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

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap_test...
provider/common/bootstrap_test.go:285: }
For completeness' sake, how about a test that kills the tomb?

[jameinel, 2013-12-08 09:59:36]

Please take a look.

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

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap.go#n...
provider/common/bootstrap.go:151: }
On 2013/12/04 14:23:05, fwereade wrote:
> Tech-debt bug if there's commonality that should consolidated? I accept it's
> low...

I ended up needing another field (DNSNameDelay) to test it appropriately. So I
don't know if it is actually a tech-debt. It is localized enough that I'd only
consider it when we get to allowing users to configure it.

Anyway, filed and docstring updated.
http://pad.lv/1258889

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap.go#n...
provider/common/bootstrap.go:162: // All we need for waitSSH is to ask for the
DNS name
On 2013/12/04 14:23:05, fwereade wrote:
> orphan comment?

Done.

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap.go#n...
provider/common/bootstrap.go:205: connectionAddress := dnsName + ":22"
On 2013/12/04 14:23:05, fwereade wrote:
> define connectionAddress 4 lines up and use it in the message?

Done.

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

https://codereview.appspot.com/37100043/diff/1/provider/common/bootstrap_test...
provider/common/bootstrap_test.go:285: }
On 2013/12/04 14:23:05, fwereade wrote:
> For completeness' sake, how about a test that kills the tomb?

Done. I added one while waiting for Dial and one while waiting for DNSName.