Merge lp:~axwalk/juju-core/lp1258240-bootstrap-refresh-dnsname-take2 into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

provider/common: attempt all addresses in waitSSH

We now attempt all addresses during waitSSH, and
refresh the addresses periodically. Providers may
return private addresses; for some clouds this is
what we want to use, and for others it is not.

As private addresses are relative, one may be able
to connect to the private address and yet be
connecting to a machine other than the intended
one. For this reason, we emit the machine's nonce
during cloud-init to a file (/var/lib/juju/nonce.txt),
and verify it during waitSSH.

The nonce verification also serves to synchronise
cloud-init and sshinit. Since the file is written
as the last thing cloud-init does, we can safely
run sshinit once it has been verified.

Fixes #1258240
Fixes #1259942

https://codereview.appspot.com/40860048/

provider/common: attempt all addresses in waitSSH

We now attempt all addresses during waitSSH, and
refresh the addresses periodically. Providers may
return private addresses; for some clouds this is
what we want to use, and for others it is not.

As private addresses are relative, one may be able
to connect to the private address and yet be
connecting to a machine other than the intended
one. For this reason, we emit the machine's nonce
during cloud-init to a file (/var/lib/juju/nonce.txt),
and verify it during waitSSH.

The nonce verification also serves to synchronise
cloud-init and sshinit. Since the file is written
as the last thing cloud-init does, we can safely
run sshinit once it has been verified.

Fixes #1258240
Fixes #1259942

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Reviewers: mp+198867_code.launchpad.net,

Message:
Please take a look.

Description:
provider/common: refresh DNS name during waitSSH

We now refresh the Instance prior to calls to DNSName,
to circument address caching. This will lead to an
increase in communication with the provider API, so
I have increased the refresh interval to 10s.

Fixes #1258240

https://code.launchpad.net/~axwalk/juju-core/lp1258240-bootstrap-refresh-dnsname-take2/+merge/198867

(do not edit description out of merge proposal)

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

Affected files (+57, -3 lines):
   A [revision details]
   M cmd/jujud/machine_test.go
   M provider/common/bootstrap.go
   M provider/common/bootstrap_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20131213032430-fx3t7fjahpr3dob2
+New revision: <email address hidden>

Index: cmd/jujud/machine_test.go
=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go 2013-12-13 02:06:53 +0000
+++ cmd/jujud/machine_test.go 2013-12-13 04:02:20 +0000
@@ -26,8 +26,8 @@
   "launchpad.net/juju-core/state/api/params"
   statetesting "launchpad.net/juju-core/state/testing"
   "launchpad.net/juju-core/state/watcher"
+ "launchpad.net/juju-core/testing"
   coretesting "launchpad.net/juju-core/testing"
- "launchpad.net/juju-core/testing"
   jc "launchpad.net/juju-core/testing/checkers"
   "launchpad.net/juju-core/testing/testbase"
   "launchpad.net/juju-core/tools"

Index: provider/common/bootstrap.go
=== modified file 'provider/common/bootstrap.go'
--- provider/common/bootstrap.go 2013-12-12 12:39:20 +0000
+++ provider/common/bootstrap.go 2013-12-13 04:02:20 +0000
@@ -78,6 +78,7 @@
   if err != nil {
    return fmt.Errorf("cannot save state: %v", err)
   }
+ inst = &refreshingInstance{Instance: inst, env: env}
   return FinishBootstrap(&ctx, inst, machineConfig)
  }

@@ -109,6 +110,26 @@
   ctx.SetInterruptHandler(nil)
  }

+// refreshingInstance is one that refreshes the
+// underlying Instance before each call to DNSName.
+type refreshingInstance struct {
+ instance.Instance
+ env environs.Environ
+ refresh bool
+}
+
+func (i *refreshingInstance) DNSName() (string, error) {
+ if i.refresh {
+ instances, err := i.env.Instances([]instance.Id{i.Instance.Id()})
+ if err != nil {
+ return "", err
+ }
+ i.Instance = instances[0]
+ }
+ i.refresh = true
+ return i.Instance.DNSName()
+}
+
  // FinishBootstrap completes the bootstrap process by connecting
  // to the instance via SSH and carrying out the cloud-config.
  //
@@ -151,8 +172,10 @@
  // DefaultBootstrapSSHTimeout is the time we'll wait for SSH to come up on
the bootstrap node
  func DefaultBootstrapSSHTimeout() SSHTimeoutOpts {
   return SSHTimeoutOpts{
- Timeout: 10 * time.Minute,
- DNSNameDelay: 1 * time.Second,
+ Timeout: 10 * time.Minute,
+
+ // Not too frequent, as we refresh addresses from the provider each time.
+ DNSNameDelay: 10 * time.Second,
   }
  }

Index: provider/common/bootstrap_test.go
=== mod...

[Andrew Wilkins (axwalk)]

On 2013/12/13 04:13:23, axw wrote:
> Please take a look.

Hm, actually, I think this may break when a floating/public IP comes
along that we can't connect to from an internal network.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

On 2013/12/13 04:25:05, axw wrote:
> On 2013/12/13 04:13:23, axw wrote:
> > Please take a look.

> Hm, actually, I think this may break when a floating/public IP comes
along that
> we can't connect to from an internal network.

I played around with a dynamic select (reflect.Select) allowing the loop
to receive results from old/ongoing attempts. I think combined with a
check for some file we write to disk in cloud-init this could be
workable. I'll continue on with it next week.

https://codereview.appspot.com/40860048/

[John A Meinel (jameinel)]

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

https://codereview.appspot.com/40860048/diff/1/provider/common/bootstrap.go#newcode121
provider/common/bootstrap.go:121: func (i *refreshingInstance) DNSName()
(string, error) {
Doing this trick of reloading the Instance so that we don't cache sounds
like something we would do if we didn't control the whole library.

Is there a reason we can't just change the behavior. Either add a method
to indicate we want the uncached DNSName or a flag or something else to
Instance? Adding a proxy instance just to create them on the fly seems
like a strange workaround to me.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

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

https://codereview.appspot.com/40860048/diff/1/provider/common/bootstrap.go#newcode121
provider/common/bootstrap.go:121: func (i *refreshingInstance) DNSName()
(string, error) {
On 2013/12/15 12:36:38, jameinel wrote:
> Doing this trick of reloading the Instance so that we don't cache
sounds like
> something we would do if we didn't control the whole library.

> Is there a reason we can't just change the behavior. Either add a
method to
> indicate we want the uncached DNSName or a flag or something else to
Instance?
> Adding a proxy instance just to create them on the fly seems like a
strange
> workaround to me.

I will investigate this. In the mean time, I have updated the CL so that
it:
  - works on HP Cloud
  - works on Canonistack (i.e. where you really do want to use the
private IP)
  - verifies hosts by checking a file created by cloud-init contains the
machine's nonce

It works by attempting *all* addresses in parallel. I did away with the
initial port 22 check, and went straight to SSH; it's not slow relative
to the rest of the bootstrap process, and it's necessary anyway to
verify the nonce.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

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

https://codereview.appspot.com/40860048/diff/20001/provider/common/bootstrap.go#newcode161
provider/common/bootstrap.go:161: `, nonceFile,
utils.ShQuote(machineConfig.MachineNonce))
The bootstrap machine's nonce is static, so it's conceivable that the
private address points to another machine that has Juju on it and is
also a bootstrap node. Unlikely, perhaps, but conceivable.

If this CL is okay otherwise, I'll change Juju to use a UUID for the
bootstrap nonce too.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

On 2013/12/16 09:56:56, axw wrote:
> Please take a look.

I had intended to branch and prereq, but I forgot the vital branch step.
Oh well. If it's too much, let me know and I can try to fix it.
Otherwise... there's a Refresh method on instance.Instance now.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Roger Peppe (rogpeppe)]

Looks great in general, with some comments and suggestions below.

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

https://codereview.appspot.com/40860048/diff/60001/provider/common/bootstrap.go#newcode320
provider/common/bootstrap.go:320: // wait for the tasks to complete. If
not
s/not/no/

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go#newcode163
environs/cloudinit/cloudinit.go:163: c.AddFile(path.Join(cfg.DataDir,
NonceFile), cfg.MachineNonce, 0644)
Is this really the last thing that happens in cloudinit?
What if there are some scripts in the cloudinit.Config?

I think this should probably be an added script.

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

https://codereview.appspot.com/40860048/diff/120001/provider/common/bootstrap.go#newcode237
provider/common/bootstrap.go:237: time.Sleep(hc.checkDelay)
This should probably allow itself to be stopped when closed.j

https://codereview.appspot.com/40860048/diff/120001/provider/common/bootstrap.go#newcode288
provider/common/bootstrap.go:288: for _, addr := range addresses {
I was wondering if some of this code might look nicer in a separate type
that wraps the Try.

type parallelHostChecker struct {
     active make(map[instance.Address]chan struct{}
     *parallel.Try
}

func newParallelHostChecker(checkDelay, checkHostScript)
*parallelHostChecker

func (g *parallelHostChecker) SetAddresses(addrs []instance.Address)

func (g *parallelHostChecker) Close() error {
     g.Try.Close()
     for _, ch := range active {
         close(ch)
     }
     return nil
}

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

https://codereview.appspot.com/40860048/diff/120001/provider/common/bootstrap_test.go#newcode364
provider/common/bootstrap_test.go:364: buf := &bytes.Buffer{}
Might be worth running go test -race just to make sure that it's ok to
use this, assuming you haven't already.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go
File provider/ec2/ec2.go (right):

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode71
provider/ec2/ec2.go:71: instanceMu sync.Mutex
just "mu" should be fine here.
conventionally it's situated above the fields that it guards.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode75
provider/ec2/ec2.go:75: return inst.InstanceId
shouldn't this use the mutex?

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode81
provider/ec2/ec2.go:81: return instance.Id(inst.InstanceId)
ditto

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode85
provider/ec2/ec2.go:85: return inst.State.Name
ditto

https://codereview.appspot.com/40860048/diff/120001/provider/openstack/provider.go
File provider/openstack/provider.go (right):

https://codereview.appspot.com/4...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go#newcode163
environs/cloudinit/cloudinit.go:163: c.AddFile(path.Join(cfg.DataDir,
NonceFile), cfg.MachineNonce, 0644)
On 2013/12/18 09:01:05, rog wrote:
> Is this really the last thing that happens in cloudinit?
> What if there are some scripts in the cloudinit.Config?

It really is. ConfigureBasic is the only thing called to set up
cloud-init for the bootstrap node.

> I think this should probably be an added script.

Added where though? All of the providers call environs.ComposeUserData,
and so there's no nice place to do that.

I could move environs.ComposeUserData to provider/common and add the
script in that function, but I don't think we'll have gained anything.

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

https://codereview.appspot.com/40860048/diff/120001/provider/common/bootstrap.go#newcode237
provider/common/bootstrap.go:237: time.Sleep(hc.checkDelay)
On 2013/12/18 09:01:05, rog wrote:
> This should probably allow itself to be stopped when closed.j

Done.

https://codereview.appspot.com/40860048/diff/120001/provider/common/bootstrap.go#newcode288
provider/common/bootstrap.go:288: for _, addr := range addresses {
On 2013/12/18 09:01:05, rog wrote:
> I was wondering if some of this code might look nicer in a separate
type that
> wraps the Try.

> type parallelHostChecker struct {
> active make(map[instance.Address]chan struct{}
> *parallel.Try
> }

> func newParallelHostChecker(checkDelay, checkHostScript)
*parallelHostChecker

> func (g *parallelHostChecker) SetAddresses(addrs []instance.Address)

> func (g *parallelHostChecker) Close() error {
> g.Try.Close()
> for _, ch := range active {
> close(ch)
> }
> return nil
> }

Done.

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

https://codereview.appspot.com/40860048/diff/120001/provider/common/bootstrap_test.go#newcode364
provider/common/bootstrap_test.go:364: buf := &bytes.Buffer{}
On 2013/12/18 09:01:05, rog wrote:
> Might be worth running go test -race just to make sure that it's ok to
use this,
> assuming you haven't already.

The printing to stderr is all done within one goroutine (the one that
calls WaitSSH), so there's no potential for race.

However, go test -race has shown up something else :)
In bootstrap.go, hostChecker.loop spawns a goroutine that may outlive
loop (and everything that calls that). That goroutine calls connectSSH,
a function var that is replaced in the test setup/teardown.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go
File provider/ec2/ec2.go (right):

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode71
provider/ec2/ec2.go:71: instanceMu sync.Mutex
On 2013/12/18 09:01:05, rog wrote:
> just "mu" should be fine here.
> conventionally it's situated above the fields tha...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Roger Peppe (rogpeppe)]

LGTM, thanks, except I'd like to know a bit more about the AddFile vs
AddRunCmd issue.

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go#newcode163
environs/cloudinit/cloudinit.go:163: c.AddFile(path.Join(cfg.DataDir,
NonceFile), cfg.MachineNonce, 0644)
On 2013/12/18 10:22:10, axw wrote:
> On 2013/12/18 09:01:05, rog wrote:
> > Is this really the last thing that happens in cloudinit?
> > What if there are some scripts in the cloudinit.Config?

> It really is. ConfigureBasic is the only thing called to set up
cloud-init for
> the bootstrap node.

> > I think this should probably be an added script.

> Added where though? All of the providers call
environs.ComposeUserData, and so
> there's no nice place to do that.

I was thinking that instead of this AddFile we'd just
call c.AddRunCmd(scriptToCreateMachineNonceFile).

If we do things as above and one of the providers uses the
additionalScripts argument to ComposeUserData, won't we
write the nonce file before running any of those scripts?

But I'm probably misunderstanding the issues around this.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go
File provider/ec2/ec2.go (right):

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode75
provider/ec2/ec2.go:75: return inst.InstanceId
On 2013/12/18 10:22:10, axw wrote:
> On 2013/12/18 09:01:05, rog wrote:
> > shouldn't this use the mutex?

> No, the mutex only guards ec2Instance.Instance (i.e. the embedded
> *ec2.Instance). I've put the mutex above that field and separated them
from "e"
> to make it more obvious.

Ahem. Discussed online.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode110
provider/ec2/ec2.go:110: err := inst.Refresh()
On 2013/12/18 10:22:10, axw wrote:
> Oops, there's a potential double lock on the mutex here. Fixed that.

Since that evidently wasn't covered by the tests, perhaps we could have
a test that did cover that?

https://codereview.appspot.com/40860048/diff/120001/provider/openstack/provider.go
File provider/openstack/provider.go (right):

https://codereview.appspot.com/40860048/diff/120001/provider/openstack/provider.go#newcode321
provider/openstack/provider.go:321: func (inst *openstackInstance)
idUnlocked() instance.Id {
On 2013/12/18 10:22:10, axw wrote:
> On 2013/12/18 09:01:05, rog wrote:
> > I'm not sure this pulls its weight.
> >
> > How about just:
> >
> > func idFromDetail(d *nova.ServerDetail) instance.Id {
> > return instance.Id(inst.serverDetail.Id)
> > }
> > then Id can just become:
> >
> > func (inst *openstackInstance) Id() instance.Id {
> > return idFromDetail(inst.getServerDetail())
> > }

> Good point. I went a step further and inlined the use if
inst.serverDetail.Id in
> Refresh, and then:

> func (inst *openstackInstance) Id() instance.Id {
> return instance.Id(inst.getServerDetail().Id)
> }

Cool - I almost suggested that.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

On 2013/12/18 12:49:08, rog wrote:
> LGTM, thanks, except I'd like to know a bit more about the AddFile vs
AddRunCmd
> issue.

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go
> File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go#newcode163
> environs/cloudinit/cloudinit.go:163: c.AddFile(path.Join(cfg.DataDir,
> NonceFile), cfg.MachineNonce, 0644)
> On 2013/12/18 10:22:10, axw wrote:
> > On 2013/12/18 09:01:05, rog wrote:
> > > Is this really the last thing that happens in cloudinit?
> > > What if there are some scripts in the cloudinit.Config?
> >
> > It really is. ConfigureBasic is the only thing called to set up
cloud-init for
> > the bootstrap node.
> >
> > > I think this should probably be an added script.
> >
> > Added where though? All of the providers call
environs.ComposeUserData, and so
> > there's no nice place to do that.

> I was thinking that instead of this AddFile we'd just
> call c.AddRunCmd(scriptToCreateMachineNonceFile).

> If we do things as above and one of the providers uses the
additionalScripts
> argument to ComposeUserData, won't we
> write the nonce file before running any of those scripts?

> But I'm probably misunderstanding the issues around this.

Ah, I see what you mean. Well, AddFile is actually a runcmd underneath,
so it's not a problem. The additionalScripts are added before
ConfigureBasic is run, so the AddFile runcmd comes last.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go
> File provider/ec2/ec2.go (right):

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode75
> provider/ec2/ec2.go:75: return inst.InstanceId
> On 2013/12/18 10:22:10, axw wrote:
> > On 2013/12/18 09:01:05, rog wrote:
> > > shouldn't this use the mutex?
> >
> > No, the mutex only guards ec2Instance.Instance (i.e. the embedded
> > *ec2.Instance). I've put the mutex above that field and separated
them from
> "e"
> > to make it more obvious.

> Ahem. Discussed online.

https://codereview.appspot.com/40860048/diff/120001/provider/ec2/ec2.go#newcode110
> provider/ec2/ec2.go:110: err := inst.Refresh()
> On 2013/12/18 10:22:10, axw wrote:
> > Oops, there's a potential double lock on the mutex here. Fixed that.

> Since that evidently wasn't covered by the tests, perhaps we could
have a test
> that did cover that?

Yes, I've just modified goamz/ec2/ec2test to set DNSName to "" the first
time it returns an instance. This showed up a bug. I'll repropose the
fix here, then propose the change to goamz later.

https://codereview.appspot.com/40860048/diff/120001/provider/openstack/provider.go
> File provider/openstack/provider.go (right):

https://codereview.appspot.com/40860048/diff/120001/provider/openstack/provider.go#newcode321
> provider/openstack/provider.go:321: func (inst *openstackInstance)
idUnlocked()
> instance.Id {
> On 2013/12/18 10:22:10, axw wrote:
> > On 2013/12/18 09:01:05, rog wrote:
> > > I'm not sure this pulls its weight.
> > >
> > > How about just:
> > >
> > > func idFromDetail(d *nova.ServerDetail) instance.Id {
> > > return instance.Id(inst.serverDet...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/40860048/diff/120001/environs/cloudinit/cloudinit.go#newcode163
environs/cloudinit/cloudinit.go:163: c.AddFile(path.Join(cfg.DataDir,
NonceFile), cfg.MachineNonce, 0644)
On 2013/12/18 12:49:09, rog wrote:
> On 2013/12/18 10:22:10, axw wrote:
> > On 2013/12/18 09:01:05, rog wrote:
> > > Is this really the last thing that happens in cloudinit?
> > > What if there are some scripts in the cloudinit.Config?
> >
> > It really is. ConfigureBasic is the only thing called to set up
cloud-init for
> > the bootstrap node.
> >
> > > I think this should probably be an added script.
> >
> > Added where though? All of the providers call
environs.ComposeUserData, and so
> > there's no nice place to do that.

> I was thinking that instead of this AddFile we'd just
> call c.AddRunCmd(scriptToCreateMachineNonceFile).

> If we do things as above and one of the providers uses the
additionalScripts
> argument to ComposeUserData, won't we
> write the nonce file before running any of those scripts?

> But I'm probably misunderstanding the issues around this.

As discussed on IRC, I've made this explicitly use AddScripts (which
generates multiple runcmds) in place of AddFile, in the event AddFile
gets changed.

https://codereview.appspot.com/40860048/

=== modified file 'cloudinit/cloudinit_test.go'
--- cloudinit/cloudinit_test.go	2013-12-12 23:00:22 +0000
+++ cloudinit/cloudinit_test.go	2013-12-18 15:16:49 +0000
@@ -244,7 +244,7 @@
 const (
 	header          = "#cloud-config\n"
 	addFileExpected = `runcmd:
-- install -m 644 /dev/null '/etc/apt/apt.conf.d/99proxy'
+- install -D -m 644 /dev/null '/etc/apt/apt.conf.d/99proxy'
 - printf '%s\n' '"Acquire::http::Proxy "http://10.0.3.1:3142";' > '/etc/apt/apt.conf.d/99proxy'
 `
 )

=== modified file 'cloudinit/options.go'
--- cloudinit/options.go	2013-12-11 07:18:02 +0000
+++ cloudinit/options.go	2013-12-18 15:16:49 +0000
@@ -316,7 +316,7 @@
 	// of escape sequences differs between shells, namely bash and
 	// dash. Instead, we use printf (or we could use /bin/echo).
 	cfg.AddScripts(
-		fmt.Sprintf("install -m %o /dev/null %s", mode, p),
+		fmt.Sprintf("install -D -m %o /dev/null %s", mode, p),
 		fmt.Sprintf(`printf '%%s\n' %s > %s`, shquote(data), p),
 	)
 }

=== modified file 'cloudinit/sshinit/configure.go'
--- cloudinit/sshinit/configure.go	2013-12-10 14:43:21 +0000
+++ cloudinit/sshinit/configure.go	2013-12-18 15:16:49 +0000
@@ -75,17 +75,6 @@
 	// but added here to avoid relying on that to be
 	// invariant.
 	script := []string{"#!/bin/bash", "set -e"}
-	// Wait for cloud-init to complete. pkill will return non-zero
-	// if there are no matching processes.
-	script = append(script, `
-    if pkill -0 cloud-init; then
-        printf "Waiting for cloud-init to finish"
-        while pkill -0 cloud-init; do
-            printf "."
-            sleep 2s
-        done
-        printf "\n"
-    fi`)
 	// We must initialise progress reporting before entering
 	// the subshell and redirecting stderr.
 	script = append(script, cloudinit.InitProgressCmd())

=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2013-12-18 13:33:12 +0000
+++ cmd/jujud/machine_test.go	2013-12-18 15:16:49 +0000
@@ -26,8 +26,8 @@
 	"launchpad.net/juju-core/state/api/params"
 	statetesting "launchpad.net/juju-core/state/testing"
 	"launchpad.net/juju-core/state/watcher"
+	"launchpad.net/juju-core/testing"
 	coretesting "launchpad.net/juju-core/testing"
-	"launchpad.net/juju-core/testing"
 	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/testing/testbase"
 	"launchpad.net/juju-core/tools"

=== modified file 'container/kvm/instance.go'
--- container/kvm/instance.go	2013-11-12 21:59:49 +0000
+++ container/kvm/instance.go	2013-12-18 15:16:49 +0000
@@ -29,6 +29,10 @@
 	return "stopped"
 }
 
+func (*kvmInstance) Refresh() error {
+	return nil
+}
+
 func (kvm *kvmInstance) Addresses() ([]instance.Address, error) {
 	logger.Errorf("kvmInstance.Addresses not implemented")
 	return nil, nil

=== modified file 'container/lxc/instance.go'
--- container/lxc/instance.go	2013-10-03 12:08:15 +0000
+++ container/lxc/instance.go	2013-12-18 15:16:49 +0000
@@ -31,6 +31,10 @@
 	return string(state)
 }
 
+func (*lxcInstance) Refresh() error {
+	return nil
+}
+
 func (lxc *lxcInstance) Addresses() ([]instance.Address, error) {
 	return nil, errors.NewNotImplementedError("lxcInstance.Addresses")
 }

=== modified file 'container/lxc/lxc_test.go'
--- container/lxc/lxc_test.go	2013-12-04 04:28:17 +0000
+++ container/lxc/lxc_test.go	2013-12-18 15:16:49 +0000
@@ -88,7 +88,7 @@
 
 	c.Assert(scripts[len(scripts)-4:], gc.DeepEquals, []string{
 		"start jujud-machine-1-lxc-0",
-		"install -m 644 /dev/null '/etc/apt/apt.conf.d/99proxy-extra'",
+		"install -D -m 644 /dev/null '/etc/apt/apt.conf.d/99proxy-extra'",
 		fmt.Sprintf(`printf '%%s\n' '%s' > '/etc/apt/apt.conf.d/99proxy-extra'`, configProxyExtra),
 		"ifconfig",
 	})

=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2013-12-13 04:40:41 +0000
+++ environs/cloudinit/cloudinit.go	2013-12-18 15:16:49 +0000
@@ -140,6 +140,11 @@
 
 const cloudInitOutputLog = "/var/log/cloud-init-output.log"
 
+// NonceFile is written by cloud-init as the last thing it does.
+// The file will contain the machine's nonce. The filename is
+// relative to the Juju data-dir.
+const NonceFile = "nonce.txt"
+
 // ConfigureBasic updates the provided cloudinit.Config with
 // basic configuration to initialise an OS image, such that it can
 // be connected to via SSH, and log to a standard location.
@@ -155,6 +160,18 @@
 func ConfigureBasic(cfg *MachineConfig, c *cloudinit.Config) error {
 	c.AddSSHAuthorizedKeys(cfg.AuthorizedKeys)
 	c.SetOutput(cloudinit.OutAll, "| tee -a "+cloudInitOutputLog, "")
+	// Create a file in a well-defined location containing the machine's
+	// nonce. The presence and contents of this file will be verified
+	// during bootstrap.
+	//
+	// Note: this must be the last runcmd we do in ConfigureBasic, as
+	// the presence of the nonce file is used to gate the remainder
+	// of synchronous bootstrap.
+	noncefile := shquote(path.Join(cfg.DataDir, NonceFile))
+	c.AddScripts(
+		fmt.Sprintf("install -D -m %o /dev/null %s", 0644, noncefile),
+		fmt.Sprintf(`printf '%%s\n' %s > %s`, shquote(cfg.MachineNonce), noncefile),
+	)
 	return nil
 }
 

=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go	2013-12-13 03:45:46 +0000
+++ environs/cloudinit/cloudinit_test.go	2013-12-18 15:16:49 +0000
@@ -88,6 +88,8 @@
 		setEnvConfig: true,
 		expectScripts: `
 echo ENABLE_MONGODB="no" > /etc/default/mongodb
+install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'
+printf '%s\\n' 'FAKE_NONCE' > '/var/lib/juju/nonce.txt'
 test -e /proc/self/fd/9 \|\| exec 9>&2
 set -xe
 mkdir -p /var/lib/juju
@@ -101,7 +103,7 @@
 tar zxf \$bin/tools.tar.gz -C \$bin
 rm \$bin/tools\.tar\.gz && rm \$bin/juju1\.2\.3-precise-amd64\.sha256
 printf %s '{"version":"1\.2\.3-precise-amd64","url":"http://foo\.com/tools/releases/juju1\.2\.3-precise-amd64\.tgz","sha256":"1234","size":10}' > \$bin/downloaded-tools\.txt
-install -m 600 /dev/null '/etc/rsyslog\.d/25-juju\.conf'
+install -D -m 600 /dev/null '/etc/rsyslog\.d/25-juju\.conf'
 printf '%s\\n' '.*' > '/etc/rsyslog.d/25-juju.conf'
 restart rsyslog
 mkdir -p '/var/lib/juju/agents/machine-0'
@@ -110,7 +112,7 @@
 install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
 printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
 ln -s /var/lib/juju/tools/machine-0/jujud /usr/local/bin/juju-run
-install -m 600 /dev/null '/var/lib/juju/server\.pem'
+install -D -m 600 /dev/null '/var/lib/juju/server\.pem'
 printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
 mkdir -p /var/lib/juju/db/journal
 chmod 0700 /var/lib/juju/db
@@ -199,6 +201,8 @@
 			SyslogPort: 514,
 		},
 		expectScripts: `
+install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'
+printf '%s\\n' 'FAKE_NONCE' > '/var/lib/juju/nonce.txt'
 test -e /proc/self/fd/9 \|\| exec 9>&2
 set -xe
 mkdir -p /var/lib/juju
@@ -212,7 +216,7 @@
 tar zxf \$bin/tools.tar.gz -C \$bin
 rm \$bin/tools\.tar\.gz && rm \$bin/juju1\.2\.3-linux-amd64\.sha256
 printf %s '{"version":"1\.2\.3-linux-amd64","url":"http://foo\.com/tools/releases/juju1\.2\.3-linux-amd64\.tgz","sha256":"1234","size":10}' > \$bin/downloaded-tools\.txt
-install -m 600 /dev/null '/etc/rsyslog\.d/25-juju\.conf'
+install -D -m 600 /dev/null '/etc/rsyslog\.d/25-juju\.conf'
 printf '%s\\n' '.*' > '/etc/rsyslog\.d/25-juju\.conf'
 restart rsyslog
 mkdir -p '/var/lib/juju/agents/machine-99'

=== modified file 'environs/cloudinit_test.go'
--- environs/cloudinit_test.go	2013-12-04 10:18:57 +0000
+++ environs/cloudinit_test.go	2013-12-18 15:16:49 +0000
@@ -214,7 +214,11 @@
 			"output": map[interface{}]interface{}{
 				"all": "| tee -a /var/log/cloud-init-output.log",
 			},
-			"runcmd":              []interface{}{"script1", "script2"},
+			"runcmd": []interface{}{
+				"script1", "script2",
+				"install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'",
+				"printf '%s\\n' '5432' > '/var/lib/juju/nonce.txt'",
+			},
 			"ssh_authorized_keys": []interface{}{"wheredidileavemykeys"},
 		})
 	} else {

=== modified file 'instance/instance.go'
--- instance/instance.go	2013-09-27 18:08:05 +0000
+++ instance/instance.go	2013-12-18 15:16:49 +0000
@@ -35,6 +35,9 @@
 	// Status returns the provider-specific status for the instance.
 	Status() string
 
+	// Refresh refreshes local knowledge of the instance from the provider.
+	Refresh() error
+
 	// Addresses returns a list of hostnames or ip addresses
 	// associated with the instance. This will supercede DNSName
 	// which can be implemented by selecting a preferred address.

=== modified file 'provider/azure/instance.go'
--- provider/azure/instance.go	2013-09-30 19:40:06 +0000
+++ provider/azure/instance.go	2013-12-18 15:16:49 +0000
@@ -36,6 +36,13 @@
 
 var AZURE_DOMAIN_NAME = "cloudapp.net"
 
+// Refresh is specified in the Instance interface.
+func (azInstance *azureInstance) Refresh() error {
+	// TODO(axw) 2013-12-16 #1261324
+	// Cache Addresses/netInfo, refresh here.
+	return nil
+}
+
 // Addresses is specified in the Instance interface.
 func (azInstance *azureInstance) Addresses() ([]instance.Address, error) {
 	addrs := []instance.Address{}

=== modified file 'provider/common/bootstrap.go'
--- provider/common/bootstrap.go	2013-12-12 12:39:20 +0000
+++ provider/common/bootstrap.go	2013-12-18 15:16:49 +0000
@@ -6,9 +6,10 @@
 import (
 	"fmt"
 	"io"
-	"net"
 	"os"
 	"os/signal"
+	"path"
+	"strings"
 	"sync"
 	"time"
 
@@ -23,6 +24,9 @@
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/instance"
 	coretools "launchpad.net/juju-core/tools"
+	"launchpad.net/juju-core/utils"
+	"launchpad.net/juju-core/utils/parallel"
+	"launchpad.net/juju-core/utils/ssh"
 )
 
 var logger = loggo.GetLogger("juju.provider.common")
@@ -116,10 +120,28 @@
 var FinishBootstrap = func(ctx *BootstrapContext, inst instance.Instance, machineConfig *cloudinit.MachineConfig) error {
 	var t tomb.Tomb
 	ctx.SetInterruptHandler(func() { t.Killf("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
+	// until cloud-init has completed, which is necessary to ensure apt
+	// invocations don't trample each other.
+	nonceFile := utils.ShQuote(path.Join(machineConfig.DataDir, cloudinit.NonceFile))
+	checkNonceCommand := fmt.Sprintf(`
+	noncefile=%s
+	if [ ! -e "$noncefile" ]; then
+		echo "$noncefile does not exist" >&2
+		exit 1
+	fi
+	content=$(cat $noncefile)
+	if [ "$content" != %s ]; then
+		echo "$noncefile contents do not match machine nonce" >&2
+		exit 1
+	fi
+	`, nonceFile, utils.ShQuote(machineConfig.MachineNonce))
 	// 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.
-	dnsName, err := waitSSH(ctx, inst, &t, DefaultBootstrapSSHTimeout())
+	addr, err := waitSSH(ctx, checkNonceCommand, inst, &t, DefaultBootstrapSSHTimeout())
 	if err != nil {
 		return err
 	}
@@ -133,7 +155,7 @@
 	if err := cloudinit.ConfigureJuju(machineConfig, cloudcfg); err != nil {
 		return err
 	}
-	return sshinit.Configure("ubuntu@"+dnsName, cloudcfg)
+	return sshinit.Configure("ubuntu@"+addr, cloudcfg)
 }
 
 // SSHTimeoutOpts lists the amount of time we will wait for various parts of
@@ -144,81 +166,209 @@
 	// a state server.
 	Timeout time.Duration
 
-	// DNSNameDelay is the amount of time between refreshing the DNS name
-	DNSNameDelay time.Duration
+	// ConnectDelay is the amount of time between attempts to connect to an address.
+	ConnectDelay time.Duration
+
+	// AddressesDelay is the amount of time between refreshing the addresses.
+	AddressesDelay time.Duration
 }
 
 // DefaultBootstrapSSHTimeout is the time we'll wait for SSH to come up on the bootstrap node
 func DefaultBootstrapSSHTimeout() SSHTimeoutOpts {
 	return SSHTimeoutOpts{
-		Timeout:      10 * time.Minute,
-		DNSNameDelay: 1 * time.Second,
-	}
-}
-
-type dnsNamer interface {
-	// DNSName returns the DNS name for the instance.
-	// If the name is not yet allocated, it will return
-	// an ErrNoDNSName error.
-	DNSName() (string, error)
-}
-
-// waitSSH waits for the instance to be assigned a DNS
-// entry, then waits until we can connect to it via SSH.
-func waitSSH(ctx *BootstrapContext, inst dnsNamer, t *tomb.Tomb, timeout SSHTimeoutOpts) (dnsName string, err error) {
+		Timeout: 10 * time.Minute,
+
+		ConnectDelay: 5 * time.Second,
+
+		// Not too frequent, as we refresh addresses from the provider each time.
+		AddressesDelay: 10 * time.Second,
+	}
+}
+
+type addresser interface {
+	// Refresh refreshes the addresses for the instance.
+	Refresh() error
+
+	// Addresses returns the addresses for the instance.
+	// To ensure that the results are up to date, call
+	// Refresh first.
+	Addresses() ([]instance.Address, error)
+}
+
+type hostChecker struct {
+	addr instance.Address
+
+	// checkDelay is the amount of time to wait between retries.
+	checkDelay time.Duration
+
+	// checkHostScript is executed on the host via SSH.
+	// hostChecker.loop will return once the script
+	// runs without error.
+	checkHostScript string
+
+	// closed is closed to indicate that the host checker should
+	// return, without waiting for the result of any ongoing
+	// attempts.
+	closed <-chan struct{}
+}
+
+// Close implements io.Closer, as required by parallel.Try.
+func (*hostChecker) Close() error {
+	return nil
+}
+
+func (hc *hostChecker) loop(dying <-chan struct{}) (io.Closer, error) {
+	// The value of connectSSH is taken outside the goroutine that may outlive
+	// hostChecker.loop, or we evoke the wrath of the race detector.
+	connectSSH := connectSSH
+	done := make(chan error, 1)
+	var lastErr error
+	for {
+		go func() {
+			done <- connectSSH(hc.addr.Value, hc.checkHostScript)
+		}()
+		select {
+		case <-hc.closed:
+			return hc, lastErr
+		case <-dying:
+			return hc, lastErr
+		case lastErr = <-done:
+			if lastErr == nil {
+				return hc, nil
+			}
+		}
+		select {
+		case <-hc.closed:
+		case <-dying:
+		case <-time.After(hc.checkDelay):
+		}
+	}
+}
+
+type parallelHostChecker struct {
+	*parallel.Try
+
+	stderr io.Writer
+
+	// active is a map of adresses to channels for addresses actively
+	// being tested. The goroutine testing the address will continue
+	// to attempt connecting to the address until it succeeds, the Try
+	// is killed, or the corresponding channel in this map is closed.
+	active map[instance.Address]chan struct{}
+
+	// checkDelay is how long each hostChecker waits between attempts.
+	checkDelay time.Duration
+
+	// checkHostScript is the script to run on each host to check that
+	// it is the host we expect.
+	checkHostScript string
+}
+
+func (p *parallelHostChecker) UpdateAddresses(addrs []instance.Address) {
+	for _, addr := range addrs {
+		if _, ok := p.active[addr]; ok {
+			continue
+		}
+		fmt.Fprintf(p.stderr, "Attempting to connect to %s:22\n", addr.Value)
+		closed := make(chan struct{})
+		hc := &hostChecker{
+			addr:            addr,
+			checkDelay:      p.checkDelay,
+			checkHostScript: p.checkHostScript,
+			closed:          closed,
+		}
+		p.active[addr] = closed
+		p.Start(hc.loop)
+	}
+}
+
+// Close prevents additional functions from being added to
+// the Try, and tells each active hostChecker to exit.
+func (p *parallelHostChecker) Close() error {
+	// We signal each checker to stop and wait for them
+	// each to complete; this allows us to get the error,
+	// as opposed to when using try.Kill which does not
+	// wait for the functions to complete.
+	p.Try.Close()
+	for _, ch := range p.active {
+		close(ch)
+	}
+	return nil
+}
+
+// connectSSH is called to connect to the specified host and
+// execute the "checkHostScript" bash script on it.
+var connectSSH = func(host, checkHostScript string) error {
+	cmd := ssh.Command("ubuntu@"+host, []string{"/bin/bash", "-c", utils.ShQuote(checkHostScript)})
+	output, err := cmd.CombinedOutput()
+	if err != nil && len(output) > 0 {
+		err = fmt.Errorf("%s", strings.TrimSpace(string(output)))
+	}
+	return err
+}
+
+// waitSSH waits for the instance to be assigned a routable
+// address, then waits until we can connect to it via SSH.
+//
+// waitSSH attempts on all addresses returned by the instance
+// in parallel; the first succeeding one wins. We ensure that
+// private addresses are for the correct machine by checking
+// 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()
 	globalTimeout := time.After(timeout.Timeout)
-	pollDNS := time.NewTimer(0)
-	fmt.Fprintln(ctx.Stderr, "Waiting for DNS name")
-	var dialResultChan chan error
-	var lastErr error
+	pollAddresses := time.NewTimer(0)
+
+	// checker checks each address in a loop, in parallel,
+	// until one succeeds, the global timeout is reached,
+	// or the tomb is killed.
+	checker := parallelHostChecker{
+		Try:             parallel.NewTry(0, nil),
+		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")
 	for {
-		if dnsName != "" && dialResultChan == nil {
-			addr := dnsName + ":22"
-			fmt.Fprintf(ctx.Stderr, "Attempting to connect to %s\n", addr)
-			dialResultChan = make(chan error, 1)
-			go func() {
-				c, err := net.Dial("tcp", addr)
-				if err == nil {
-					c.Close()
-				}
-				dialResultChan <- err
-			}()
-		}
 		select {
-		case <-pollDNS.C:
-			pollDNS.Reset(timeout.DNSNameDelay)
-			newDNSName, err := inst.DNSName()
-			if err != nil && err != instance.ErrNoDNSName {
-				return "", t.Killf("getting DNS name: %v", err)
-			} else if err != nil {
-				lastErr = err
-			} else if newDNSName != dnsName {
-				dnsName = newDNSName
-				dialResultChan = nil
-			}
-		case lastErr = <-dialResultChan:
-			if lastErr == nil {
-				return dnsName, nil
-			}
-			logger.Debugf("connection failed: %v", lastErr)
-			dialResultChan = nil // retry
+		case <-pollAddresses.C:
+			pollAddresses.Reset(timeout.AddressesDelay)
+			if err := inst.Refresh(); err != nil {
+				return "", t.Killf("refreshing addresses: %v", err)
+			}
+			addresses, err := inst.Addresses()
+			if err != nil {
+				return "", t.Killf("getting addresses: %v", err)
+			}
+			checker.UpdateAddresses(addresses)
 		case <-globalTimeout:
+			checker.Close()
+			lastErr := checker.Wait()
 			format := "waited for %v "
 			args := []interface{}{timeout.Timeout}
-			if dnsName == "" {
-				format += "without getting a DNS name"
+			if len(checker.active) == 0 {
+				format += "without getting any addresses"
 			} else {
-				format += "without being able to connect to %q"
-				args = append(args, dnsName)
+				format += "without being able to connect"
 			}
-			if lastErr != nil {
+			if lastErr != nil && lastErr != parallel.ErrStopped {
 				format += ": %v"
 				args = append(args, lastErr)
 			}
-			return "", t.Killf(format, args...)
+			return "", fmt.Errorf(format, args...)
 		case <-t.Dying():
 			return "", t.Err()
+		case <-checker.Dead():
+			result, err := checker.Result()
+			if err != nil {
+				return "", err
+			}
+			return result.(*hostChecker).addr.Value, nil
 		}
 	}
 }

=== modified file 'provider/common/bootstrap_test.go'
--- provider/common/bootstrap_test.go	2013-12-13 02:19:17 +0000
+++ provider/common/bootstrap_test.go	2013-12-18 15:16:49 +0000
@@ -41,6 +41,9 @@
 func (s *BootstrapSuite) SetUpTest(c *gc.C) {
 	s.LoggingSuite.SetUpTest(c)
 	s.ToolsFixture.SetUpTest(c)
+	s.PatchValue(common.ConnectSSH, func(host, checkHostScript string) error {
+		return fmt.Errorf("mock connection failure to %s", host)
+	})
 }
 
 func (s *BootstrapSuite) TearDownTest(c *gc.C) {
@@ -219,47 +222,57 @@
 	})
 }
 
-type neverDNSName struct {
-}
-
-func (neverDNSName) DNSName() (string, error) {
-	return "", instance.ErrNoDNSName
+type neverRefreshes struct {
+}
+
+func (neverRefreshes) Refresh() error {
+	return nil
+}
+
+type neverAddresses struct {
+	neverRefreshes
+}
+
+func (neverAddresses) Addresses() ([]instance.Address, error) {
+	return nil, nil
 }
 
 var testSSHTimeout = common.SSHTimeoutOpts{
-	Timeout:      10 * time.Millisecond,
-	DNSNameDelay: 1 * time.Millisecond,
+	Timeout:        10 * time.Millisecond,
+	ConnectDelay:   1 * time.Millisecond,
+	AddressesDelay: 1 * time.Millisecond,
 }
 
-func (s *BootstrapSuite) TestWaitSSHTimesOutWaitingForDNSName(c *gc.C) {
+func (s *BootstrapSuite) TestWaitSSHTimesOutWaitingForAddresses(c *gc.C) {
 	ctx := &common.BootstrapContext{}
 	buf := &bytes.Buffer{}
 	ctx.Stderr = buf
 	var t tomb.Tomb
-	_, err := common.WaitSSH(ctx, neverDNSName{}, &t, testSSHTimeout)
-	c.Check(err, gc.ErrorMatches, "waited for 10ms without getting a DNS name: DNS name not allocated")
-	c.Check(buf.String(), gc.Matches, "Waiting for DNS name\n")
+	_, err := common.WaitSSH(ctx, "/bin/true", neverAddresses{}, &t, testSSHTimeout)
+	c.Check(err, gc.ErrorMatches, "waited for 10ms without getting any addresses")
+	c.Check(buf.String(), gc.Matches, "Waiting for address\n")
 }
 
-func (s *BootstrapSuite) TestWaitSSHKilledWaitingForDNSName(c *gc.C) {
+func (s *BootstrapSuite) TestWaitSSHKilledWaitingForAddresses(c *gc.C) {
 	ctx := &common.BootstrapContext{}
 	buf := &bytes.Buffer{}
 	ctx.Stderr = buf
 	var t tomb.Tomb
 	go func() {
 		<-time.After(2 * time.Millisecond)
-		t.Killf("stopping WaitSSH during DNSName")
+		t.Killf("stopping WaitSSH during Addresses")
 	}()
-	_, err := common.WaitSSH(ctx, neverDNSName{}, &t, testSSHTimeout)
-	c.Check(err, gc.ErrorMatches, "stopping WaitSSH during DNSName")
-	c.Check(buf.String(), gc.Matches, "Waiting for DNS name\n")
-}
-
-type brokenDNSName struct {
-}
-
-func (brokenDNSName) DNSName() (string, error) {
-	return "", fmt.Errorf("DNSName will never work")
+	_, 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")
+}
+
+type brokenAddresses struct {
+	neverRefreshes
+}
+
+func (brokenAddresses) Addresses() ([]instance.Address, error) {
+	return nil, fmt.Errorf("Addresses will never work")
 }
 
 func (s *BootstrapSuite) TestWaitSSHStopsOnBadError(c *gc.C) {
@@ -267,17 +280,18 @@
 	buf := &bytes.Buffer{}
 	ctx.Stderr = buf
 	var t tomb.Tomb
-	_, err := common.WaitSSH(ctx, brokenDNSName{}, &t, testSSHTimeout)
-	c.Check(err, gc.ErrorMatches, "getting DNS name: DNSName will never work")
-	c.Check(buf.String(), gc.Equals, "Waiting for DNS name\n")
+	_, err := common.WaitSSH(ctx, "/bin/true", brokenAddresses{}, &t, testSSHTimeout)
+	c.Check(err, gc.ErrorMatches, "getting addresses: Addresses will never work")
+	c.Check(buf.String(), gc.Equals, "Waiting for address\n")
 }
 
 type neverOpensPort struct {
-	name string
+	neverRefreshes
+	addr string
 }
 
-func (n *neverOpensPort) DNSName() (string, error) {
-	return n.name, nil
+func (n *neverOpensPort) Addresses() ([]instance.Address, error) {
+	return []instance.Address{instance.NewAddress(n.addr)}, nil
 }
 
 func (s *BootstrapSuite) TestWaitSSHTimesOutWaitingForDial(c *gc.C) {
@@ -286,28 +300,29 @@
 	ctx.Stderr = buf
 	var t tomb.Tomb
 	// 0.x.y.z addresses are always invalid
-	_, err := common.WaitSSH(ctx, &neverOpensPort{"0.1.2.3"}, &t, testSSHTimeout)
+	_, err := common.WaitSSH(ctx, "/bin/true", &neverOpensPort{addr: "0.1.2.3"}, &t, testSSHTimeout)
 	c.Check(err, gc.ErrorMatches,
-		`waited for 10ms without being able to connect to "0.1.2.3": dial tcp 0.1.2.3:22: invalid argument`)
+		`waited for 10ms without being able to connect: mock connection failure to 0.1.2.3`)
 	c.Check(buf.String(), gc.Matches,
-		"Waiting for DNS name\n"+
+		"Waiting for address\n"+
 			"(Attempting to connect to 0.1.2.3:22\n)+")
 }
 
 type killOnDial struct {
+	neverRefreshes
 	name     string
 	tomb     *tomb.Tomb
 	returned bool
 }
 
-func (k *killOnDial) DNSName() (string, error) {
-	// kill the tomb the second time DNSName is called
+func (k *killOnDial) Addresses() ([]instance.Address, error) {
+	// kill the tomb the second time Addresses is called
 	if !k.returned {
 		k.returned = true
 	} else {
 		k.tomb.Killf("stopping WaitSSH during Dial")
 	}
-	return k.name, nil
+	return []instance.Address{instance.NewAddress(k.name)}, nil
 }
 
 func (s *BootstrapSuite) TestWaitSSHKilledWaitingForDial(c *gc.C) {
@@ -317,10 +332,53 @@
 	var t tomb.Tomb
 	timeout := testSSHTimeout
 	timeout.Timeout = 1 * time.Minute
-	_, err := common.WaitSSH(ctx, &killOnDial{name: "0.1.2.3", tomb: &t}, &t, timeout)
+	_, err := common.WaitSSH(ctx, "", &killOnDial{name: "0.1.2.3", tomb: &t}, &t, timeout)
 	c.Check(err, gc.ErrorMatches, "stopping WaitSSH during Dial")
 	// Exact timing is imprecise but it should have tried a few times before being killed
 	c.Check(buf.String(), gc.Matches,
-		"Waiting for DNS name\n"+
+		"Waiting for address\n"+
 			"(Attempting to connect to 0.1.2.3:22\n)+")
 }
+
+type addressesChange struct {
+	addrs [][]string
+}
+
+func (ac *addressesChange) Refresh() error {
+	if len(ac.addrs) > 1 {
+		ac.addrs = ac.addrs[1:]
+	}
+	return nil
+}
+
+func (ac *addressesChange) Addresses() ([]instance.Address, error) {
+	var addrs []instance.Address
+	for _, addr := range ac.addrs[0] {
+		addrs = append(addrs, instance.NewAddress(addr))
+	}
+	return addrs, nil
+}
+
+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{
+		nil,
+		nil,
+		[]string{"0.1.2.3"},
+		[]string{"0.1.2.3"},
+		nil,
+		[]string{"0.1.2.4"},
+	}}, &t, 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,
+		"Waiting for address\n"+
+			"(.|\n)*(Attempting to connect to 0.1.2.3:22\n)+(.|\n)*")
+	c.Check(buf.String(), gc.Matches,
+		"Waiting for address\n"+
+			"(.|\n)*(Attempting to connect to 0.1.2.4:22\n)+(.|\n)*")
+}

=== modified file 'provider/common/export_test.go'
--- provider/common/export_test.go	2013-12-04 10:28:12 +0000
+++ provider/common/export_test.go	2013-12-18 15:16:49 +0000
@@ -7,5 +7,6 @@
 	GetDNSNames      = getDNSNames
 	GetStateInfo     = getStateInfo
 	ComposeAddresses = composeAddresses
+	ConnectSSH       = &connectSSH
 	WaitSSH          = waitSSH
 )

=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go	2013-12-11 09:14:12 +0000
+++ provider/dummy/environs.go	2013-12-18 15:16:49 +0000
@@ -900,6 +900,10 @@
 	return string(inst.id) + ".dns", nil
 }
 
+func (*dummyInstance) Refresh() error {
+	return nil
+}
+
 func (inst *dummyInstance) Addresses() ([]instance.Address, error) {
 	inst.mu.Lock()
 	defer inst.mu.Unlock()

=== modified file 'provider/ec2/ec2.go'
--- provider/ec2/ec2.go	2013-11-18 05:41:36 +0000
+++ provider/ec2/ec2.go	2013-12-18 15:16:49 +0000
@@ -67,64 +67,84 @@
 
 type ec2Instance struct {
 	e *environ
+
+	mu sync.Mutex
 	*ec2.Instance
 }
 
 func (inst *ec2Instance) String() string {
-	return inst.InstanceId
+	return string(inst.Id())
 }
 
 var _ instance.Instance = (*ec2Instance)(nil)
 
+func (inst *ec2Instance) getInstance() *ec2.Instance {
+	inst.mu.Lock()
+	defer inst.mu.Unlock()
+	return inst.Instance
+}
+
 func (inst *ec2Instance) Id() instance.Id {
-	return instance.Id(inst.InstanceId)
+	return instance.Id(inst.getInstance().InstanceId)
 }
 
 func (inst *ec2Instance) Status() string {
-	return inst.State.Name
-}
-
-// refreshInstance requeries the Instance details over the ec2 api
-func (inst *ec2Instance) refreshInstance() error {
-	insts, err := inst.e.Instances([]instance.Id{inst.Id()})
+	return inst.getInstance().State.Name
+}
+
+// Refresh implements instance.Refresh(), requerying the
+// Instance details over the ec2 api
+func (inst *ec2Instance) Refresh() error {
+	_, err := inst.refresh()
+	return err
+}
+
+// refresh requeries Instance details over the ec2 api.
+func (inst *ec2Instance) refresh() (*ec2.Instance, error) {
+	id := inst.Id()
+	insts, err := inst.e.Instances([]instance.Id{id})
 	if err != nil {
-		return err
+		return nil, err
 	}
+	inst.mu.Lock()
+	defer inst.mu.Unlock()
 	inst.Instance = insts[0].(*ec2Instance).Instance
-	return nil
+	return inst.Instance, nil
 }
 
 // Addresses implements instance.Addresses() returning generic address
 // details for the instance, and requerying the ec2 api if required.
 func (inst *ec2Instance) Addresses() ([]instance.Address, error) {
-	var addresses []instance.Address
 	// TODO(gz): Stop relying on this requerying logic, maybe remove error
-	if inst.Instance.DNSName == "" {
+	instInstance := inst.getInstance()
+	if instInstance.DNSName == "" {
 		// Fetch the instance information again, in case
 		// the DNS information has become available.
-		err := inst.refreshInstance()
+		var err error
+		instInstance, err = inst.refresh()
 		if err != nil {
 			return nil, err
 		}
 	}
+	var addresses []instance.Address
 	possibleAddresses := []instance.Address{
 		{
-			Value:        inst.Instance.DNSName,
+			Value:        instInstance.DNSName,
 			Type:         instance.HostName,
 			NetworkScope: instance.NetworkPublic,
 		},
 		{
-			Value:        inst.Instance.PrivateDNSName,
+			Value:        instInstance.PrivateDNSName,
 			Type:         instance.HostName,
 			NetworkScope: instance.NetworkCloudLocal,
 		},
 		{
-			Value:        inst.Instance.IPAddress,
+			Value:        instInstance.IPAddress,
 			Type:         instance.Ipv4Address,
 			NetworkScope: instance.NetworkPublic,
 		},
 		{
-			Value:        inst.Instance.PrivateIPAddress,
+			Value:        instInstance.PrivateIPAddress,
 			Type:         instance.Ipv4Address,
 			NetworkScope: instance.NetworkCloudLocal,
 		},

=== modified file 'provider/ec2/local_test.go'
--- provider/ec2/local_test.go	2013-12-03 05:20:25 +0000
+++ provider/ec2/local_test.go	2013-12-18 15:16:49 +0000
@@ -241,6 +241,10 @@
 			"all": "| tee -a /var/log/cloud-init-output.log",
 		},
 		"ssh_authorized_keys": []interface{}{"my-keys"},
+		"runcmd": []interface{}{
+			"install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'",
+			"printf '%s\\n' 'user-admin:bootstrap' > '/var/lib/juju/nonce.txt'",
+		},
 	})
 
 	// check that a new instance will be started with a machine agent

=== modified file 'provider/joyent/instance.go'
--- provider/joyent/instance.go	2013-10-07 10:57:04 +0000
+++ provider/joyent/instance.go	2013-12-18 15:16:49 +0000
@@ -24,6 +24,10 @@
 	return "unknown (not implemented)"
 }
 
+func (inst *environInstance) Refresh() error {
+	return nil
+}
+
 func (inst *environInstance) Addresses() ([]instance.Address, error) {
 	_ = inst.env.getSnapshot()
 	return nil, errNotImplemented

=== modified file 'provider/local/instance.go'
--- provider/local/instance.go	2013-10-03 12:08:15 +0000
+++ provider/local/instance.go	2013-12-18 15:16:49 +0000
@@ -28,6 +28,10 @@
 	return ""
 }
 
+func (*localInstance) Refresh() error {
+	return nil
+}
+
 func (inst *localInstance) Addresses() ([]instance.Address, error) {
 	return nil, errors.NewNotImplementedError("localInstance.Addresses")
 }

=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go	2013-11-26 12:24:48 +0000
+++ provider/maas/environ.go	2013-12-18 15:16:49 +0000
@@ -238,7 +238,7 @@
 	if node, tools, err := environ.acquireNode(cons, possibleTools); err != nil {
 		return nil, nil, fmt.Errorf("cannot run instances: %v", err)
 	} else {
-		inst = &maasInstance{&node, environ}
+		inst = &maasInstance{maasObject: &node, environ: environ}
 		machineConfig.Tools = tools
 	}
 	defer func() {

=== modified file 'provider/maas/environ_whitebox_test.go'
--- provider/maas/environ_whitebox_test.go	2013-12-10 04:25:06 +0000
+++ provider/maas/environ_whitebox_test.go	2013-12-18 15:16:49 +0000
@@ -285,7 +285,7 @@
 func (suite *environSuite) getInstance(systemId string) *maasInstance {
 	input := `{"system_id": "` + systemId + `"}`
 	node := suite.testMAASObject.TestServer.NewNode(input)
-	return &maasInstance{&node, suite.makeEnviron()}
+	return &maasInstance{maasObject: &node, environ: suite.makeEnviron()}
 }
 
 func (suite *environSuite) TestStopInstancesReturnsIfParameterEmpty(c *gc.C) {
@@ -316,7 +316,7 @@
 	hostname := "test"
 	input := `{"system_id": "system_id", "hostname": "` + hostname + `"}`
 	node := suite.testMAASObject.TestServer.NewNode(input)
-	testInstance := &maasInstance{&node, suite.makeEnviron()}
+	testInstance := &maasInstance{maasObject: &node, environ: suite.makeEnviron()}
 	err := bootstrap.SaveState(
 		env.Storage(),
 		&bootstrap.BootstrapState{StateInstances: []instance.Id{testInstance.Id()}})

=== modified file 'provider/maas/instance.go'
--- provider/maas/instance.go	2013-10-17 18:36:49 +0000
+++ provider/maas/instance.go	2013-12-18 15:16:49 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"fmt"
+	"sync"
 
 	"launchpad.net/gomaasapi"
 
@@ -13,8 +14,10 @@
 )
 
 type maasInstance struct {
+	environ *maasEnviron
+
+	mu         sync.Mutex
 	maasObject *gomaasapi.MAASObject
-	environ    *maasEnviron
 }
 
 var _ instance.Instance = (*maasInstance)(nil)
@@ -29,8 +32,12 @@
 }
 
 func (mi *maasInstance) Id() instance.Id {
+	return maasObjectId(mi.getMaasObject())
+}
+
+func maasObjectId(maasObject *gomaasapi.MAASObject) instance.Id {
 	// Use the node's 'resource_uri' value.
-	return instance.Id(mi.maasObject.URI().String())
+	return instance.Id(maasObject.URI().String())
 }
 
 func (mi *maasInstance) Status() string {
@@ -42,10 +49,12 @@
 	return ""
 }
 
-// refreshInstance refreshes the instance with the most up-to-date information
+// Refresh refreshes the instance with the most up-to-date information
 // from the MAAS server.
-func (mi *maasInstance) refreshInstance() error {
-	insts, err := mi.environ.Instances([]instance.Id{mi.Id()})
+func (mi *maasInstance) Refresh() error {
+	mi.mu.Lock()
+	defer mi.mu.Unlock()
+	insts, err := mi.environ.Instances([]instance.Id{maasObjectId(mi.maasObject)})
 	if err != nil {
 		return err
 	}
@@ -53,6 +62,12 @@
 	return nil
 }
 
+func (mi *maasInstance) getMaasObject() *gomaasapi.MAASObject {
+	mi.mu.Lock()
+	defer mi.mu.Unlock()
+	return mi.maasObject
+}
+
 func (mi *maasInstance) Addresses() ([]instance.Address, error) {
 	name, err := mi.DNSName()
 	if err != nil {
@@ -76,7 +91,7 @@
 
 func (mi *maasInstance) ipAddresses() ([]string, error) {
 	// we have to do this the hard way, since maasObject doesn't have this built-in yet
-	addressArray := mi.maasObject.GetMap()["ip_addresses"]
+	addressArray := mi.getMaasObject().GetMap()["ip_addresses"]
 	if addressArray.IsNil() {
 		// Older MAAS versions do not return ip_addresses.
 		return nil, nil
@@ -98,7 +113,7 @@
 
 func (mi *maasInstance) DNSName() (string, error) {
 	// A MAAS instance has its DNS name immediately.
-	return mi.maasObject.GetField("hostname")
+	return mi.getMaasObject().GetField("hostname")
 }
 
 func (mi *maasInstance) WaitDNSName() (string, error) {

=== modified file 'provider/maas/instance_test.go'
--- provider/maas/instance_test.go	2013-11-26 12:24:48 +0000
+++ provider/maas/instance_test.go	2013-12-18 15:16:49 +0000
@@ -21,7 +21,7 @@
 	jsonValue := `{"system_id": "system_id", "test": "test"}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
 	resourceURI, _ := obj.GetField("resource_uri")
-	instance := maasInstance{&obj, s.makeEnviron()}
+	instance := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
 	c.Check(string(instance.Id()), gc.Equals, resourceURI)
 }
@@ -29,7 +29,7 @@
 func (s *instanceTest) TestString(c *gc.C) {
 	jsonValue := `{"hostname": "thethingintheplace", "system_id": "system_id", "test": "test"}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	instance := &maasInstance{&obj, s.makeEnviron()}
+	instance := &maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 	hostname, err := instance.DNSName()
 	c.Assert(err, gc.IsNil)
 	expected := hostname + ":" + string(instance.Id())
@@ -40,7 +40,7 @@
 	// For good measure, test what happens if we don't have a hostname.
 	jsonValue := `{"system_id": "system_id", "test": "test"}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	instance := &maasInstance{&obj, s.makeEnviron()}
+	instance := &maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 	_, err := instance.DNSName()
 	c.Assert(err, gc.NotNil)
 	expected := fmt.Sprintf("<DNSName failed: %q>", err) + ":" + string(instance.Id())
@@ -51,9 +51,9 @@
 	jsonValue := `{"system_id": "system_id", "test": "test"}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
 	s.testMAASObject.TestServer.ChangeNode("system_id", "test2", "test2")
-	instance := maasInstance{&obj, s.makeEnviron()}
+	instance := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
-	err := instance.refreshInstance()
+	err := instance.Refresh()
 
 	c.Check(err, gc.IsNil)
 	testField, err := (*instance.maasObject).GetField("test2")
@@ -64,7 +64,7 @@
 func (s *instanceTest) TestDNSName(c *gc.C) {
 	jsonValue := `{"hostname": "DNS name", "system_id": "system_id"}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	instance := maasInstance{&obj, s.makeEnviron()}
+	instance := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
 	dnsName, err := instance.DNSName()
 
@@ -85,7 +85,7 @@
 			"ip_addresses": [ "1.2.3.4", "fe80::d806:dbff:fe23:1199" ]
 		}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	inst := maasInstance{&obj, s.makeEnviron()}
+	inst := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
 	expected := []instance.Address{
 		{Value: "testing.invalid", Type: instance.HostName, NetworkScope: instance.NetworkPublic},
@@ -107,7 +107,7 @@
 		"system_id": "system_id"
 		}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	inst := maasInstance{&obj, s.makeEnviron()}
+	inst := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
 	addr, err := inst.Addresses()
 	c.Assert(err, gc.IsNil)
@@ -123,7 +123,7 @@
 		"ip_addresses": "incompatible"
 		}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	inst := maasInstance{&obj, s.makeEnviron()}
+	inst := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
 	_, err := inst.Addresses()
 	c.Assert(err, gc.NotNil)
@@ -136,7 +136,7 @@
 		"ip_addresses": [42]
 		}`
 	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
-	inst := maasInstance{&obj, s.makeEnviron()}
+	inst := maasInstance{maasObject: &obj, environ: s.makeEnviron()}
 
 	_, err := inst.Addresses()
 	c.Assert(err, gc.NotNil)

=== modified file 'provider/null/instance.go'
--- provider/null/instance.go	2013-11-26 14:52:02 +0000
+++ provider/null/instance.go	2013-12-18 15:16:49 +0000
@@ -12,15 +12,19 @@
 	host string
 }
 
-func (_ nullBootstrapInstance) Id() instance.Id {
+func (nullBootstrapInstance) Id() instance.Id {
 	// The only way to bootrap is via manual bootstrap.
 	return manual.BootstrapInstanceId
 }
 
-func (_ nullBootstrapInstance) Status() string {
+func (nullBootstrapInstance) Status() string {
 	return ""
 }
 
+func (nullBootstrapInstance) Refresh() error {
+	return nil
+}
+
 func (inst nullBootstrapInstance) Addresses() (addresses []instance.Address, err error) {
 	host, err := inst.DNSName()
 	if err != nil {
@@ -43,14 +47,14 @@
 	return i.DNSName()
 }
 
-func (_ nullBootstrapInstance) OpenPorts(machineId string, ports []instance.Port) error {
-	return nil
-}
-
-func (_ nullBootstrapInstance) ClosePorts(machineId string, ports []instance.Port) error {
-	return nil
-}
-
-func (_ nullBootstrapInstance) Ports(machineId string) ([]instance.Port, error) {
+func (nullBootstrapInstance) OpenPorts(machineId string, ports []instance.Port) error {
+	return nil
+}
+
+func (nullBootstrapInstance) ClosePorts(machineId string, ports []instance.Port) error {
+	return nil
+}
+
+func (nullBootstrapInstance) Ports(machineId string) ([]instance.Port, error) {
 	return []instance.Port{}, nil
 }

=== modified file 'provider/openstack/provider.go'
--- provider/openstack/provider.go	2013-12-10 04:25:06 +0000
+++ provider/openstack/provider.go	2013-12-18 15:16:49 +0000
@@ -282,24 +282,43 @@
 var _ simplestreams.HasRegion = (*environ)(nil)
 
 type openstackInstance struct {
-	*nova.ServerDetail
 	e        *environ
 	instType *instances.InstanceType
 	arch     *string
+
+	mu           sync.Mutex
+	serverDetail *nova.ServerDetail
 }
 
 func (inst *openstackInstance) String() string {
-	return inst.ServerDetail.Id
+	return string(inst.Id())
 }
 
 var _ instance.Instance = (*openstackInstance)(nil)
 
+func (inst *openstackInstance) Refresh() error {
+	inst.mu.Lock()
+	defer inst.mu.Unlock()
+	server, err := inst.e.nova().GetServer(inst.serverDetail.Id)
+	if err != nil {
+		return err
+	}
+	inst.serverDetail = server
+	return nil
+}
+
+func (inst *openstackInstance) getServerDetail() *nova.ServerDetail {
+	inst.mu.Lock()
+	defer inst.mu.Unlock()
+	return inst.serverDetail
+}
+
 func (inst *openstackInstance) Id() instance.Id {
-	return instance.Id(inst.ServerDetail.Id)
+	return instance.Id(inst.getServerDetail().Id)
 }
 
 func (inst *openstackInstance) Status() string {
-	return inst.ServerDetail.Status
+	return inst.getServerDetail().Status
 }
 
 func (inst *openstackInstance) hardwareCharacteristics() *instance.HardwareCharacteristics {
@@ -326,7 +345,7 @@
 // getAddress returns the existing server information on addresses,
 // but fetches the details over the api again if no addresses exist.
 func (inst *openstackInstance) getAddresses() (map[string][]nova.IPAddress, error) {
-	addrs := inst.ServerDetail.Addresses
+	addrs := inst.getServerDetail().Addresses
 	if len(addrs) == 0 {
 		server, err := inst.e.nova().GetServer(string(inst.Id()))
 		if err != nil {
@@ -726,7 +745,7 @@
 	}
 	inst := &openstackInstance{
 		e:            e,
-		ServerDetail: detail,
+		serverDetail: detail,
 		arch:         &spec.Image.Arch,
 		instType:     &spec.InstanceType,
 	}
@@ -787,7 +806,7 @@
 			switch server.Status {
 			case nova.StatusActive, nova.StatusBuild, nova.StatusBuildSpawning:
 				// TODO(wallyworld): lookup the flavor details to fill in the instance type data
-				out[id] = &openstackInstance{e: e, ServerDetail: &server}
+				out[id] = &openstackInstance{e: e, serverDetail: &server}
 				continue
 			}
 		}
@@ -836,7 +855,7 @@
 			// TODO(wallyworld): lookup the flavor details to fill in the instance type data
 			insts = append(insts, &openstackInstance{
 				e:            e,
-				ServerDetail: &s,
+				serverDetail: &s,
 			})
 		}
 	}