Merge lp:~rogpeppe/juju-core/391-1.14-avoid-allFatal into lp:juju-core/1.14

[Roger Peppe]

cmd/jujud: avoid using allFatal

This is a backport of the https://code.launchpad.net/~rogpeppe/juju-core/384-avoid-allfatal/+merge/184743
now proposed for 1.14, with selected other patches from other revisions
to make it compile.

https://codereview.appspot.com/13696043/

cmd/jujud: avoid using allFatal

This is a backport of the https://code.launchpad.net/~rogpeppe/juju-core/384-avoid-allfatal/+merge/184743
now proposed for 1.14, with selected other patches from other revisions
to make it compile.

https://codereview.appspot.com/13696043/

[Roger Peppe (rogpeppe)]

Reviewers: mp+185445_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/jujud: avoid using allFatal

This is a backport of the
https://code.launchpad.net/~rogpeppe/juju-core/384-avoid-allfatal/+merge/184743
now proposed for 1.14, with selected other patches from other revisions
to make it compile.

https://code.launchpad.net/~rogpeppe/juju-core/391-1.14-avoid-allFatal/+merge/185445

(do not edit description out of merge proposal)

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

Affected files (+315, -65 lines):
   A [revision details]
   M cmd/jujud/agent.go
   M cmd/jujud/agent_test.go
   M cmd/jujud/machine.go
   M cmd/jujud/machine_test.go
   M cmd/jujud/unit.go
   M juju/testing/conn.go
   M state/api/apiclient.go
   M state/apiserver/pinger_test.go
   M state/state.go
   M state/state_test.go
   A testing/checkers/set.go
   A testing/checkers/set_test.go
   M testing/mgo.go

[Dimiter Naydenov (dimitern)]

LGTM

https://codereview.appspot.com/13696043/

[William Reade (fwereade)]

LGTM

https://codereview.appspot.com/13696043/

=== modified file 'cmd/jujud/agent.go'
--- cmd/jujud/agent.go	2013-09-11 06:27:05 +0000
+++ cmd/jujud/agent.go	2013-09-13 09:26:01 +0000
@@ -111,6 +111,27 @@
 	return ok
 }
 
+type pinger interface {
+	Ping() error
+}
+
+// connectionIsFatal returns a function suitable for passing
+// as the isFatal argument to worker.NewRunner,
+// that diagnoses an error as fatal if the connection
+// has failed or if the error is otherwise fatal.
+func connectionIsFatal(conn pinger) func(err error) bool {
+	return func(err error) bool {
+		if isFatal(err) {
+			return true
+		}
+		if err := conn.Ping(); err != nil {
+			log.Infof("error pinging %T: %v", conn, err)
+			return true
+		}
+		return false
+	}
+}
+
 // isleep waits for the given duration or until it receives a value on
 // stop.  It returns whether the full duration was slept without being
 // stopped.

=== modified file 'cmd/jujud/agent_test.go'
--- cmd/jujud/agent_test.go	2013-08-24 13:50:05 +0000
+++ cmd/jujud/agent_test.go	2013-09-13 09:26:01 +0000
@@ -20,7 +20,9 @@
 	envtools "launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api/params"
 	coretesting "launchpad.net/juju-core/testing"
+	jc "launchpad.net/juju-core/testing/checkers"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/version"
 	"launchpad.net/juju-core/worker"
@@ -48,6 +50,70 @@
 	}
 }
 
+var isFatalTests = []struct {
+	err     error
+	isFatal bool
+}{{
+	err:     worker.ErrTerminateAgent,
+	isFatal: true,
+}, {
+	err:     &upgrader.UpgradeReadyError{},
+	isFatal: true,
+}, {
+	err: &params.Error{
+		Message: "blah",
+		Code:    params.CodeNotProvisioned,
+	},
+	isFatal: true,
+}, {
+	err:     &fatalError{"some fatal error"},
+	isFatal: true,
+}, {
+	err:     stderrors.New("foo"),
+	isFatal: false,
+}, {
+	err: &params.Error{
+		Message: "blah",
+		Code:    params.CodeNotFound,
+	},
+	isFatal: false,
+}}
+
+func (*toolSuite) TestIsFatal(c *gc.C) {
+	for i, test := range isFatalTests {
+		c.Logf("test %d: %s", i, test.err)
+		c.Assert(isFatal(test.err), gc.Equals, test.isFatal)
+	}
+}
+
+type testPinger func() error
+
+func (f testPinger) Ping() error {
+	return f()
+}
+
+func (s *toolSuite) TestConnectionIsFatal(c *gc.C) {
+	var (
+		errPinger testPinger = func() error {
+			return stderrors.New("ping error")
+		}
+		okPinger testPinger = func() error {
+			return nil
+		}
+	)
+	for i, pinger := range []testPinger{errPinger, okPinger} {
+		for j, test := range isFatalTests {
+			c.Logf("test %d.%d: %s", i, j, test.err)
+			fatal := connectionIsFatal(pinger)(test.err)
+			if test.isFatal {
+				c.Check(fatal, jc.IsTrue)
+			} else {
+				c.Check(fatal, gc.Equals, i == 0)
+			}
+		}
+	}
+}
+
 func mkTools(s string) *coretools.Tools {
 	return &coretools.Tools{
 		Version: version.MustParseBinary(s + "-foo-bar"),

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-09-11 06:27:05 +0000
+++ cmd/jujud/machine.go	2013-09-13 09:26:01 +0000
@@ -34,6 +34,16 @@
 	"launchpad.net/juju-core/worker/upgrader"
 )
 
+type workerRunner interface {
+	worker.Worker
+	StartWorker(id string, startFunc func() (worker.Worker, error)) error
+	StopWorker(id string) error
+}
+
+var newRunner = func(isFatal func(error) bool, moreImportant func(e0, e1 error) bool) workerRunner {
+	return worker.NewRunner(isFatal, moreImportant)
+}
+
 const bootstrapMachineId = "0"
 
 var retryDelay = 3 * time.Second
@@ -44,7 +54,7 @@
 	tomb      tomb.Tomb
 	Conf      AgentConf
 	MachineId string
-	runner    *worker.Runner
+	runner    workerRunner
 }
 
 // Info returns usage information for the command.
@@ -68,7 +78,7 @@
 	if err := a.Conf.checkArgs(args); err != nil {
 		return err
 	}
-	a.runner = worker.NewRunner(isFatal, moreImportant)
+	a.runner = newRunner(isFatal, moreImportant)
 	return nil
 }
 
@@ -127,10 +137,6 @@
 	return err
 }
 
-func allFatal(error) bool {
-	return true
-}
-
 var stateJobs = map[params.MachineJob]bool{
 	params.JobHostUnits:     true,
 	params.JobManageEnviron: true,
@@ -164,7 +170,7 @@
 	if needsStateWorker {
 		ensureStateWorker()
 	}
-	runner := worker.NewRunner(allFatal, moreImportant)
+	runner := newRunner(connectionIsFatal(st), moreImportant)
 	// Only the machiner currently connects to the API.
 	// Add other workers here as they are ready.
 	runner.StartWorker("machiner", func() (worker.Worker, error) {
@@ -208,7 +214,7 @@
 	// TODO(rog) use more discriminating test for errors
 	// rather than taking everything down indiscriminately.
 	dataDir := a.Conf.dataDir
-	runner := worker.NewRunner(allFatal, moreImportant)
+	runner := newRunner(connectionIsFatal(st), moreImportant)
 	// At this stage, since we don't embed lxc containers, just start an lxc
 	// provisioner task for non-lxc containers.  Since we have only LXC
 	// containers and normal machines, this effectively means that we only

=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2013-09-11 06:27:05 +0000
+++ cmd/jujud/machine_test.go	2013-09-13 09:26:01 +0000
@@ -24,7 +24,7 @@
 	"launchpad.net/juju-core/state/api/params"
 	"launchpad.net/juju-core/state/watcher"
 	"launchpad.net/juju-core/testing"
-	"launchpad.net/juju-core/testing/checkers"
+	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/version"
 	"launchpad.net/juju-core/worker/deployer"
@@ -33,7 +33,7 @@
 type MachineSuite struct {
 	agentSuite
 	lxc.TestSuite
-	oldCacheDir string
+	restoreCacheDir jc.Restorer
 }
 
 var _ = gc.Suite(&MachineSuite{})
@@ -41,11 +41,11 @@
 func (s *MachineSuite) SetUpSuite(c *gc.C) {
 	s.agentSuite.SetUpSuite(c)
 	s.TestSuite.SetUpSuite(c)
-	s.oldCacheDir = charm.CacheDir
+	s.restoreCacheDir = jc.Set(&charm.CacheDir, c.MkDir())
 }
 
 func (s *MachineSuite) TearDownSuite(c *gc.C) {
-	charm.CacheDir = s.oldCacheDir
+	s.restoreCacheDir()
 	s.TestSuite.TearDownSuite(c)
 	s.agentSuite.TearDownSuite(c)
 }
@@ -231,7 +231,7 @@
 		if err == nil && attempt.HasNext() {
 			continue
 		}
-		c.Assert(err, checkers.Satisfies, errors.IsNotFoundError)
+		c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
 	}
 
 	// short-circuit-remove u1 after it's been deployed; check it's recalled
@@ -239,7 +239,7 @@
 	err = u1.Destroy()
 	c.Assert(err, gc.IsNil)
 	err = u1.Refresh()
-	c.Assert(err, checkers.Satisfies, errors.IsNotFoundError)
+	c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
 	ctx.waitDeployed(c)
 }
 

=== modified file 'cmd/jujud/unit.go'
--- cmd/jujud/unit.go	2013-09-11 06:27:05 +0000
+++ cmd/jujud/unit.go	2013-09-13 09:26:01 +0000
@@ -85,7 +85,7 @@
 	}
 	unit := entity.(*state.Unit)
 	dataDir := a.Conf.dataDir
-	runner := worker.NewRunner(allFatal, moreImportant)
+	runner := worker.NewRunner(connectionIsFatal(st), moreImportant)
 	runner.StartWorker("uniter", func() (worker.Worker, error) {
 		return uniter.NewUniter(st, unit.Name(), dataDir), nil
 	})
@@ -98,7 +98,7 @@
 		return nil, err
 	}
 	dataDir := a.Conf.dataDir
-	runner := worker.NewRunner(allFatal, moreImportant)
+	runner := worker.NewRunner(connectionIsFatal(st), moreImportant)
 	runner.StartWorker("upgrader", func() (worker.Worker, error) {
 		return upgrader.New(st.Upgrader(), entity.Tag(), dataDir), nil
 	})

=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go	2013-09-11 06:27:05 +0000
+++ juju/testing/conn.go	2013-09-13 09:26:01 +0000
@@ -9,7 +9,7 @@
 	"os"
 	"path/filepath"
 
-	. "launchpad.net/gocheck"
+	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/constraints"
@@ -85,13 +85,13 @@
 
 // StartInstance is a test helper function that starts an instance on the
 // environment using the current series and invalid info states.
-func StartInstance(c *C, env environs.Environ, machineId string) (instance.Instance, *instance.HardwareCharacteristics) {
+func StartInstance(c *gc.C, env environs.Environ, machineId string) (instance.Instance, *instance.HardwareCharacteristics) {
 	return StartInstanceWithConstraints(c, env, machineId, constraints.Value{})
 }
 
 // StartInstanceWithConstraints is a test helper function that starts an instance on the
 // environment with the specified constraints, using the current series and invalid info states.
-func StartInstanceWithConstraints(c *C, env environs.Environ, machineId string,
+func StartInstanceWithConstraints(c *gc.C, env environs.Environ, machineId string,
 	cons constraints.Value) (instance.Instance, *instance.HardwareCharacteristics) {
 	series := config.DefaultSeries
 	inst, metadata, err := provider.StartInstance(
@@ -103,7 +103,7 @@
 		FakeStateInfo(machineId),
 		FakeAPIInfo(machineId),
 	)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	return inst, metadata
 }
 
@@ -119,24 +119,24 @@
         agent-version: %s
 `
 
-func (s *JujuConnSuite) SetUpSuite(c *C) {
+func (s *JujuConnSuite) SetUpSuite(c *gc.C) {
 	s.LoggingSuite.SetUpSuite(c)
 	s.MgoSuite.SetUpSuite(c)
 }
 
-func (s *JujuConnSuite) TearDownSuite(c *C) {
+func (s *JujuConnSuite) TearDownSuite(c *gc.C) {
 	s.MgoSuite.TearDownSuite(c)
 	s.LoggingSuite.TearDownSuite(c)
 }
 
-func (s *JujuConnSuite) SetUpTest(c *C) {
+func (s *JujuConnSuite) SetUpTest(c *gc.C) {
 	s.oldJujuHome = config.SetJujuHome(c.MkDir())
 	s.LoggingSuite.SetUpTest(c)
 	s.MgoSuite.SetUpTest(c)
 	s.setUpConn(c)
 }
 
-func (s *JujuConnSuite) TearDownTest(c *C) {
+func (s *JujuConnSuite) TearDownTest(c *gc.C) {
 	s.tearDownConn(c)
 	s.MgoSuite.TearDownTest(c)
 	s.LoggingSuite.TearDownTest(c)
@@ -145,51 +145,63 @@
 
 // Reset returns environment state to that which existed at the start of
 // the test.
-func (s *JujuConnSuite) Reset(c *C) {
+func (s *JujuConnSuite) Reset(c *gc.C) {
 	s.tearDownConn(c)
 	s.setUpConn(c)
 }
 
-func (s *JujuConnSuite) StateInfo(c *C) *state.Info {
+func (s *JujuConnSuite) StateInfo(c *gc.C) *state.Info {
 	info, _, err := s.Conn.Environ.StateInfo()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	info.Password = "dummy-secret"
 	return info
 }
 
-func (s *JujuConnSuite) APIInfo(c *C) *api.Info {
+func (s *JujuConnSuite) APIInfo(c *gc.C) *api.Info {
 	_, apiInfo, err := s.APIConn.Environ.StateInfo()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	apiInfo.Tag = "user-admin"
 	apiInfo.Password = "dummy-secret"
 	return apiInfo
 }
 
-func (s *JujuConnSuite) openAPIAs(c *C, tag, password, nonce string) *api.State {
+func (s *JujuConnSuite) openAPIAs(c *gc.C, tag, password, nonce string) *api.State {
 	_, info, err := s.APIConn.Environ.StateInfo()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	info.Tag = tag
 	info.Password = password
 	info.Nonce = nonce
 	st, err := api.Open(info, api.DialOpts{})
-	c.Assert(err, IsNil)
-	c.Assert(st, NotNil)
+	c.Assert(err, gc.IsNil)
+	c.Assert(st, gc.NotNil)
 	return st
 }
 
 // OpenAPIAs opens the API using the given identity tag
 // and password for authentication.
-func (s *JujuConnSuite) OpenAPIAs(c *C, tag, password string) *api.State {
+func (s *JujuConnSuite) OpenAPIAs(c *gc.C, tag, password string) *api.State {
 	return s.openAPIAs(c, tag, password, "")
 }
 
 // OpenAPIAsMachine opens the API using the given machine tag,
 // password and nonce for authentication.
-func (s *JujuConnSuite) OpenAPIAsMachine(c *C, tag, password, nonce string) *api.State {
+func (s *JujuConnSuite) OpenAPIAsMachine(c *gc.C, tag, password, nonce string) *api.State {
 	return s.openAPIAs(c, tag, password, nonce)
 }
 
-func (s *JujuConnSuite) setUpConn(c *C) {
+// OpenAPIAsNewMachine creates a new machine entry that lives in system state, and
+// then uses that to open the API.
+func (s *JujuConnSuite) OpenAPIAsNewMachine(c *gc.C) (*api.State, *state.Machine) {
+	machine, err := s.State.AddMachine("series", state.JobHostUnits)
+	c.Assert(err, gc.IsNil)
+	err = machine.SetPassword("test-password")
+	c.Assert(err, gc.IsNil)
+	err = machine.SetProvisioned("foo", "fake_nonce", nil)
+	c.Assert(err, gc.IsNil)
+	return s.openAPIAs(c, machine.Tag(), "test-password", "fake_nonce"), machine
+}
+
+func (s *JujuConnSuite) setUpConn(c *gc.C) {
 	if s.RootDir != "" {
 		panic("JujuConnSuite.setUpConn without teardown")
 	}
@@ -197,38 +209,38 @@
 	s.oldHome = os.Getenv("HOME")
 	home := filepath.Join(s.RootDir, "/home/ubuntu")
 	err := os.MkdirAll(home, 0777)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	os.Setenv("HOME", home)
 
 	dataDir := filepath.Join(s.RootDir, "/var/lib/juju")
 	err = os.MkdirAll(dataDir, 0777)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 
 	yaml := []byte(fmt.Sprintf(envConfig, version.Current.Number))
 	err = ioutil.WriteFile(config.JujuHomePath("environments.yaml"), yaml, 0600)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 
 	err = ioutil.WriteFile(config.JujuHomePath("dummyenv-cert.pem"), []byte(testing.CACert), 0666)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 
 	err = ioutil.WriteFile(config.JujuHomePath("dummyenv-private-key.pem"), []byte(testing.CAKey), 0600)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 
 	environ, err := environs.PrepareFromName("dummyenv")
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	// sanity check we've got the correct environment.
-	c.Assert(environ.Name(), Equals, "dummyenv")
-	c.Assert(bootstrap.Bootstrap(environ, constraints.Value{}), IsNil)
+	c.Assert(environ.Name(), gc.Equals, "dummyenv")
+	c.Assert(bootstrap.Bootstrap(environ, constraints.Value{}), gc.IsNil)
 
 	s.BackingState = environ.(GetStater).GetStateInAPIServer()
 
 	conn, err := juju.NewConn(environ)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	s.Conn = conn
 	s.State = conn.State
 
 	apiConn, err := juju.NewAPIConn(environ, api.DialOpts{})
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	s.APIConn = apiConn
 	s.APIState = apiConn.State
 	s.environ = environ
@@ -238,7 +250,7 @@
 	GetStateInAPIServer() *state.State
 }
 
-func (s *JujuConnSuite) tearDownConn(c *C) {
+func (s *JujuConnSuite) tearDownConn(c *gc.C) {
 	// Bootstrap will set the admin password, and render non-authorized use
 	// impossible. s.State may still hold the right password, so try to reset
 	// the password so that the MgoSuite soft-resetting works. If that fails,
@@ -247,8 +259,8 @@
 	if err := s.State.SetAdminMongoPassword(""); err != nil {
 		c.Logf("cannot reset admin password: %v", err)
 	}
-	c.Assert(s.Conn.Close(), IsNil)
-	c.Assert(s.APIConn.Close(), IsNil)
+	c.Assert(s.Conn.Close(), gc.IsNil)
+	c.Assert(s.APIConn.Close(), gc.IsNil)
 	dummy.Reset()
 	s.Conn = nil
 	s.State = nil
@@ -276,13 +288,13 @@
 	}
 }
 
-func (s *JujuConnSuite) AddTestingCharm(c *C, name string) *state.Charm {
+func (s *JujuConnSuite) AddTestingCharm(c *gc.C, name string) *state.Charm {
 	ch := testing.Charms.Dir(name)
 	ident := fmt.Sprintf("%s-%d", ch.Meta().Name, ch.Revision())
 	curl := charm.MustParseURL("local:series/" + ident)
 	repo, err := charm.InferRepository(curl, testing.Charms.Path)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	sch, err := s.Conn.PutCharm(curl, repo, false)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	return sch
 }

=== modified file 'state/api/apiclient.go'
--- state/api/apiclient.go	2013-09-11 06:27:05 +0000
+++ state/api/apiclient.go	2013-09-13 09:26:01 +0000
@@ -133,11 +133,8 @@
 }
 
 func (s *State) heartbeatMonitor() {
-	ping := func() error {
-		return s.Call("Pinger", "", "Ping", nil, nil)
-	}
 	for {
-		if err := ping(); err != nil {
+		if err := s.Ping(); err != nil {
 			close(s.broken)
 			return
 		}
@@ -145,6 +142,10 @@
 	}
 }
 
+func (s *State) Ping() error {
+	return s.Call("Pinger", "", "Ping", nil, nil)
+}
+
 // Call invokes a low-level RPC method of the given objType, id, and
 // request, passing the given parameters and filling in the response
 // results. This should not be used directly by clients.

=== modified file 'state/apiserver/pinger_test.go'
--- state/apiserver/pinger_test.go	2013-09-11 06:27:05 +0000
+++ state/apiserver/pinger_test.go	2013-09-13 09:26:01 +0000
@@ -6,6 +6,7 @@
 import (
 	gc "launchpad.net/gocheck"
 	"launchpad.net/juju-core/juju/testing"
+	"launchpad.net/juju-core/rpc"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"time"
@@ -54,3 +55,14 @@
 		return
 	}
 }
+
+func (s *stateSuite) TestPing(c *gc.C) {
+	st, _ := s.OpenAPIAsNewMachine(c)
+	defer st.Close()
+	err := st.Ping()
+	c.Assert(err, gc.IsNil)
+	err = st.Close()
+	c.Assert(err, gc.IsNil)
+	err = st.Ping()
+	c.Assert(err, gc.Equals, rpc.ErrShutdown)
+}

=== modified file 'state/state.go'
--- state/state.go	2013-09-11 06:27:05 +0000
+++ state/state.go	2013-09-13 09:26:01 +0000
@@ -102,6 +102,12 @@
 	return st.runner.Run(ops, "", nil)
 }
 
+// Ping probes the state's database connection to ensure
+// that it is still alive.
+func (st *State) Ping() error {
+	return st.db.Session.Ping()
+}
+
 func (st *State) Watch() *multiwatcher.Watcher {
 	st.mu.Lock()
 	if st.allManager == nil {

=== modified file 'state/state_test.go'
--- state/state_test.go	2013-09-11 06:27:05 +0000
+++ state/state_test.go	2013-09-13 09:26:01 +0000
@@ -63,6 +63,12 @@
 	c.Assert(s.State.CACert(), gc.DeepEquals, info.CACert)
 }
 
+func (s *StateSuite) TestPing(c *gc.C) {
+	c.Assert(s.State.Ping(), gc.IsNil)
+	testing.MgoRestart()
+	c.Assert(s.State.Ping(), gc.NotNil)
+}
+
 func (s *StateSuite) TestAPIAddresses(c *gc.C) {
 	config, err := s.State.EnvironConfig()
 	c.Assert(err, gc.IsNil)

=== added file 'testing/checkers/set.go'
--- testing/checkers/set.go	1970-01-01 00:00:00 +0000
+++ testing/checkers/set.go	2013-09-13 09:26:01 +0000
@@ -0,0 +1,35 @@
+package checkers
+
+import (
+	"reflect"
+)
+
+// Restorer holds a function that can be used
+// to restore some previous state.
+type Restorer func()
+
+// Restore restores some previous state.
+func (r Restorer) Restore() {
+	r()
+}
+
+// Set sets the value pointed to by the given
+// destination to the given value, and returns
+// a function to restore it to its original value.
+// The value must be assignable to the element
+// type of the destination.
+func Set(dest, value interface{}) Restorer {
+	destv := reflect.ValueOf(dest).Elem()
+	oldv := reflect.New(destv.Type()).Elem()
+	oldv.Set(destv)
+	valuev := reflect.ValueOf(value)
+	if !valuev.IsValid() {
+		// This isn't quite right when the destination type is not
+		// nilable, but it's better than the complex alternative.
+		valuev = reflect.Zero(destv.Type())
+	}
+	destv.Set(valuev)
+	return func() {
+		destv.Set(oldv)
+	}
+}

=== added file 'testing/checkers/set_test.go'
--- testing/checkers/set_test.go	1970-01-01 00:00:00 +0000
+++ testing/checkers/set_test.go	2013-09-13 09:26:01 +0000
@@ -0,0 +1,54 @@
+package checkers_test
+
+import (
+	"errors"
+
+	gc "launchpad.net/gocheck"
+	jc "launchpad.net/juju-core/testing/checkers"
+)
+
+type SetSuite struct{}
+
+var _ = gc.Suite(&SetSuite{})
+
+func (*SetSuite) TestSetInt(c *gc.C) {
+	i := 99
+	restore := jc.Set(&i, 88)
+	c.Assert(i, gc.Equals, 88)
+	restore()
+	c.Assert(i, gc.Equals, 99)
+}
+
+func (*SetSuite) TestSetError(c *gc.C) {
+	oldErr := errors.New("foo")
+	newErr := errors.New("bar")
+	err := oldErr
+	restore := jc.Set(&err, newErr)
+	c.Assert(err, gc.Equals, newErr)
+	restore()
+	c.Assert(err, gc.Equals, oldErr)
+}
+
+func (*SetSuite) TestSetErrorToNil(c *gc.C) {
+	oldErr := errors.New("foo")
+	err := oldErr
+	restore := jc.Set(&err, nil)
+	c.Assert(err, gc.Equals, nil)
+	restore()
+	c.Assert(err, gc.Equals, oldErr)
+}
+
+func (*SetSuite) TestSetMapToNil(c *gc.C) {
+	oldMap := map[string]int{"foo": 1234}
+	m := oldMap
+	restore := jc.Set(&m, nil)
+	c.Assert(m, gc.IsNil)
+	restore()
+	c.Assert(m, gc.DeepEquals, oldMap)
+}
+
+func (*SetSuite) TestSetPanicsWhenNotAssignable(c *gc.C) {
+	i := 99
+	type otherInt int
+	c.Assert(func() { jc.Set(&i, otherInt(88)) }, gc.PanicMatches, `reflect\.Set: value of type checkers_test\.otherInt is not assignable to type int`)
+}

=== modified file 'testing/mgo.go'
--- testing/mgo.go	2013-08-27 07:28:22 +0000
+++ testing/mgo.go	2013-09-13 09:26:01 +0000
@@ -57,7 +57,6 @@
 }
 
 // startMgoServer starts a MongoDB server in a temporary directory.
-// It panics if it encounters an error.
 func startMgoServer() error {
 	dbdir, err := ioutil.TempDir("", "test-mgo")
 	if err != nil {
@@ -69,12 +68,30 @@
 		return fmt.Errorf("cannot write cert/key PEM: %v", err)
 	}
 	MgoPort = FindTCPPort()
+	MgoAddr = fmt.Sprintf("localhost:%d", MgoPort)
+	mgoDir = dbdir
+	if err := runMgoServer(); err != nil {
+		MgoAddr = ""
+		MgoPort = 0
+		os.RemoveAll(mgoDir)
+		mgoDir = ""
+		return err
+	}
+	return nil
+}
+
+// runMgoServer runs the MongoDB server at the
+// address and directory already configured.
+func runMgoServer() error {
+	if mgoServer != nil {
+		panic("mongo server is already running")
+	}
 	mgoport := strconv.Itoa(MgoPort)
 	mgoargs := []string{
 		"--auth",
-		"--dbpath", dbdir,
+		"--dbpath", mgoDir,
 		"--sslOnNormalPorts",
-		"--sslPEMKeyFile", pemPath,
+		"--sslPEMKeyFile", filepath.Join(mgoDir, "server.pem"),
 		"--sslPEMKeyPassword", "ignored",
 		"--bind_ip", "localhost",
 		"--port", mgoport,
@@ -106,21 +123,33 @@
 	}()
 	mgoExited = exited
 	if err := server.Start(); err != nil {
-		os.RemoveAll(dbdir)
 		return err
 	}
-	MgoAddr = "localhost:" + mgoport
 	mgoServer = server
-	mgoDir = dbdir
 	return nil
 }
 
+func mgoKill() {
+	mgoServer.Process.Kill()
+	<-mgoExited
+	mgoServer = nil
+	mgoExited = nil
+}
+
 func destroyMgoServer() {
 	if mgoServer != nil {
-		mgoServer.Process.Kill()
-		<-mgoExited
+		mgoKill()
 		os.RemoveAll(mgoDir)
-		MgoAddr, mgoServer, mgoExited, mgoDir = "", nil, nil, ""
+		MgoAddr, mgoDir = "", ""
+	}
+}
+
+// MgoRestart restarts the mongo server, useful for
+// testing what happens when a state server goes down.
+func MgoRestart() {
+	mgoKill()
+	if err := startMgoServer(); err != nil {
+		panic(err)
 	}
 }