Merge lp:~themue/juju-core/go-provisioning-test-fix into lp:~juju/juju-core/trunk

[Frank Mueller]

provisioning: fixed a race condition in testing

The test if the provisioner and the firewaller are running
had a race condition. And additional test for nil prevents
this.

https://codereview.appspot.com/6462057/

[lp:~themue/juju-core/go-provisioning-test-fix]

381. By Frank Mueller on 2012-08-14

provisioning: made the test fix a bit more safe

[William Reade (fwereade)]

https://codereview.appspot.com/6462057/diff/3001/cmd/jujud/provisioning_test.go
File cmd/jujud/provisioning_test.go (right):

https://codereview.appspot.com/6462057/diff/3001/cmd/jujud/provisioning_test.go#newcode35
cmd/jujud/provisioning_test.go:35: }
else ...?

https://codereview.appspot.com/6462057/

[Roger Peppe (rogpeppe)]

https://codereview.appspot.com/6462057/diff/3001/cmd/jujud/provisioning_test.go
File cmd/jujud/provisioning_test.go (right):

https://codereview.appspot.com/6462057/diff/3001/cmd/jujud/provisioning_test.go#newcode20
cmd/jujud/provisioning_test.go:20: if p != nil && fw != nil {
I don't think this is sufficient, as discussed online. There is no
guarantee that this goroutine will see the changes made to the
ProvisioningAgent by the goroutine that called Run, as there's no
synchronisation between them.

https://codereview.appspot.com/6462057/

Unmerged revisions

381. By Frank Mueller on 2012-08-14

provisioning: made the test fix a bit more safe

380. By Frank Mueller on 2012-08-14

provisioning: fix race condition in test

379. By Dave Cheney on 2012-08-13

cmd/juju: add unexpose

Unexpose unexposes exposed services previously exposed with juju expose.

R=niemeyer
CC=
https://codereview.appspot.com/6450103

378. By Roger Peppe on 2012-08-13

jujutest: speed up live tests

There's no need for TestBootstrap to connect
to the state (the slowest part of the test), as
this is also tested in TestBootstrapProvisioner.

R=dfc, niemeyer
CC=
https://codereview.appspot.com/6442098

377. By Roger Peppe on 2012-08-08

worker/uniter: simplify EnsureTools and make it concurrent-safe

If someone breaks the symlinks, the user "gets everything they deserve".
Avoiding the link reading logic makes it trivial for EnsureTools
to be idempotent, even when run concurrently.

Also fix the agent name to match our upstart convention
which prefixes the tools directory with the agent kind.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6461061

376. By William Reade on 2012-08-08

add uniter.EnsureTools

...which will write jujuc symlinks into the agent's tools dir for every
known hook command, and bail if it cannot do so for any reason.

R=niemeyer
CC=
https://codereview.appspot.com/6445089

375. By Dave Cheney on 2012-08-08

cmd/juju: add expose

Adds juju expose command, also normalises the format of
commadn descriptions (no capital letters, no trailing period).

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6458085

374. By Dave Cheney on 2012-08-08

cmd/juju: add add-unit command

add-unit adds units.

R=fwereade, rog, niemeyer
CC=
https://codereview.appspot.com/6449093

373. By William Reade on 2012-08-07

less spam

(help is only printed on --help, not on any cmd parse error)

R=niemeyer
CC=
https://codereview.appspot.com/6458094

372. By William Reade on 2012-08-07

add uniter.Relationer

This is a slightly unsatisfying type that emerged fairly naturally from a
sketch of a Uniter type that seemed to be going in a nice direction but
getting big. I don't think this type is complete yet (lifecycle handling
will, I think, force some reasonably significant changes, in time) but it
is a useful and self-contained chunk of functionality and IMO worth the
propose.

Better name suggestions would be appreciated, though.

R=dfc, niemeyer
CC=
https://codereview.appspot.com/6442088

=== modified file 'cmd/jujud/provisioning_test.go'
--- cmd/jujud/provisioning_test.go	2012-08-07 09:46:39 +0000
+++ cmd/jujud/provisioning_test.go	2012-08-14 09:18:03 +0000
@@ -15,18 +15,22 @@
 	start := time.Now()
 	for time.Since(start) < 500*time.Millisecond {
 		time.Sleep(50 * time.Millisecond)
-		perr := a.provisioner.Err()
-		fwerr := a.firewaller.Err()
-		if alive {
-			// Provisioner and firewaller have to be alive.
-			if perr == tomb.ErrStillAlive && fwerr == tomb.ErrStillAlive {
-				return
-			}
-		} else {
-			// Provisioner and firewaller have to be stopped.
-			if perr != tomb.ErrStillAlive && fwerr != tomb.ErrStillAlive {
-				c.Succeed()
-				return
+		p := a.provisioner
+		fw := a.firewaller
+		if p != nil && fw != nil {
+			perr := p.Err()
+			fwerr := fw.Err()
+			if alive {
+				// Provisioner and firewaller have to be alive.
+				if perr == tomb.ErrStillAlive && fwerr == tomb.ErrStillAlive {
+					return
+				}
+			} else {
+				// Provisioner and firewaller have to be stopped.
+				if perr != tomb.ErrStillAlive && fwerr != tomb.ErrStillAlive {
+					c.Succeed()
+					return
+				}
 			}
 		}
 	}