Merge lp:~axwalk/juju-core/lp1306902-mongo-ensureadminuser into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Ensure state server has Mongo admin rights

The machine agent is updated so that if it
fails to open state with an unauthorized error,
then the agent will attempt to give itself
admin rights by temporarily restarting mongo
with --noauth.

There will be follow-ups to perform replset
initiation, and to upgrade the upstart config.

https://codereview.appspot.com/88030043/

Ensure state server has Mongo admin rights

The machine agent is updated so that if it
fails to open state with an unauthorized error,
then the agent will attempt to give itself
admin rights by temporarily restarting mongo
with --noauth.

There will be follow-ups to perform replset
initiation, and to upgrade the upstart config.

https://codereview.appspot.com/88030043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+215821_code.launchpad.net,

Message:
Please take a look.

Description:
Ensure state server has Mongo admin rights

The machine agent is updated so that if it
fails to open state with an unauthorized error,
then the agent will attempt to give itself
admin rights by temporarily restarting mongo
with --noauth.

There will be follow-ups to perform replset
initiation, and to upgrade the upstart config.

https://code.launchpad.net/~axwalk/juju-core/lp1306902-mongo-ensureadminuser/+merge/215821

(do not edit description out of merge proposal)

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

Affected files (+255, -48 lines):
   A [revision details]
   M agent/mongo/mongo.go
   M agent/mongo/mongo_test.go
   A agent/mongo/upgrade.go
   M cmd/jujud/machine.go
   M cmd/jujud/machine_test.go
   M state/compat_test.go
   M state/open.go

[Roger Peppe (rogpeppe)]

LGTM assuming it's been tested live (both upgrade and normal bootstrap).

Good stuff.

https://codereview.appspot.com/88030043/diff/1/agent/mongo/mongo_test.go
File agent/mongo/mongo_test.go (right):

https://codereview.appspot.com/88030043/diff/1/agent/mongo/mongo_test.go#newcode156
agent/mongo/mongo_test.go:156: defer inst.Destroy()
Hmm, I wonder if that was the source of some problems.

https://codereview.appspot.com/88030043/diff/1/agent/mongo/upgrade.go
File agent/mongo/upgrade.go (right):

https://codereview.appspot.com/88030043/diff/1/agent/mongo/upgrade.go#newcode92
agent/mongo/upgrade.go:92: return false, fmt.Errorf("mongod did not
cleanly terminate: %v", err)
"cannot kill mongod: %v"
?

https://codereview.appspot.com/88030043/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/88030043/diff/1/cmd/jujud/machine.go#newcode431
cmd/jujud/machine.go:431: if errors.IsUnauthorizedError(err) {
// TODO remove this when we no longer need
// compatibility with pre-HA environments.

?

https://codereview.appspot.com/88030043/diff/1/cmd/jujud/machine.go#newcode433
cmd/jujud/machine.go:433: added, ensureErr :=
a.ensureMongoAdminUser(agentConfig, info.StatePort, namespace)
Perhaps slightly easier to follow the logic if we return
early when possible?

added, ensureErr := a.ensureMongoAdminUser(agentConfig, info.StatePort,
namespace)
if ensureErr != nil {
     return nil, fmt.Errorf("cannot ensure admin user: %v", ensureErr)
}
if !added {
     // No user added, so it's probably a genuine unauthorized error.
     return nil, err
}
st, m, err = openState(agentConfig)

https://codereview.appspot.com/88030043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/88030043/diff/1/agent/mongo/upgrade.go
File agent/mongo/upgrade.go (right):

https://codereview.appspot.com/88030043/diff/1/agent/mongo/upgrade.go#newcode92
agent/mongo/upgrade.go:92: return false, fmt.Errorf("mongod did not
cleanly terminate: %v", err)
On 2014/04/15 08:33:38, rog wrote:
> "cannot kill mongod: %v"
> ?

Done.

https://codereview.appspot.com/88030043/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/88030043/diff/1/cmd/jujud/machine.go#newcode431
cmd/jujud/machine.go:431: if errors.IsUnauthorizedError(err) {
On 2014/04/15 08:33:38, rog wrote:
> // TODO remove this when we no longer need
> // compatibility with pre-HA environments.

> ?

Done.

https://codereview.appspot.com/88030043/diff/1/cmd/jujud/machine.go#newcode433
cmd/jujud/machine.go:433: added, ensureErr :=
a.ensureMongoAdminUser(agentConfig, info.StatePort, namespace)
On 2014/04/15 08:33:38, rog wrote:
> Perhaps slightly easier to follow the logic if we return
> early when possible?

> added, ensureErr := a.ensureMongoAdminUser(agentConfig,
info.StatePort,
> namespace)
> if ensureErr != nil {
> return nil, fmt.Errorf("cannot ensure admin user: %v", ensureErr)
> }
> if !added {
> // No user added, so it's probably a genuine unauthorized error.
> return nil, err
> }
> st, m, err = openState(agentConfig)

Done.

https://codereview.appspot.com/88030043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/88030043/

[Go Bot (go-bot)]

The attempt to merge lp:~axwalk/juju-core/lp1306902-mongo-ensureadminuser into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.013s
ok launchpad.net/juju-core/agent 1.716s
ok launchpad.net/juju-core/agent/mongo 2.285s
ok launchpad.net/juju-core/agent/tools 0.176s
ok launchpad.net/juju-core/bzr 5.267s
ok launchpad.net/juju-core/cert 2.738s
ok launchpad.net/juju-core/charm 0.453s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.030s
ok launchpad.net/juju-core/cloudinit/sshinit 0.748s
ok launchpad.net/juju-core/cmd 0.217s
ok launchpad.net/juju-core/cmd/charm-admin 0.724s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.184s
ok launchpad.net/juju-core/cmd/juju 221.673s
panic: Session already closed

goroutine 1955 [running]:
labix.org/v2/mgo.(*Session).cluster(0xc20033f000, 0x0)
 /home/tarmac/trees/src/labix.org/v2/mgo/session.go:1037 +0x64
labix.org/v2/mgo.(*Session).acquireSocket(0xc20033f000, 0xc200000001, 0x0, 0x0, 0x0, ...)
 /home/tarmac/trees/src/labix.org/v2/mgo/session.go:3099 +0x212
labix.org/v2/mgo.(*Query).Iter(0xc200ede0e0, 0x7f8754dbde78)
 /home/tarmac/trees/src/labix.org/v2/mgo/session.go:2249 +0x18a
launchpad.net/juju-core/state/watcher.(*Watcher).sync(0xc20077ae10, 0xc200ef1a80, 0x7f8754dbdf70)
 /home/tarmac/trees/src/launchpad.net/juju-core/state/watcher/watcher.go:350 +0xe8
launchpad.net/juju-core/state/watcher.(*Watcher).loop(0xc20077ae10, 0x417ed0, 0xc2003266c0)
 /home/tarmac/trees/src/launchpad.net/juju-core/state/watcher/watcher.go:220 +0x87
launchpad.net/juju-core/state/watcher.func·001()
 /home/tarmac/trees/src/launchpad.net/juju-core/state/watcher/watcher.go:118 +0x2c
created by launchpad.net/juju-core/state/watcher.New
 /home/tarmac/trees/src/launchpad.net/juju-core/state/watcher/watcher.go:120 +0x100

goroutine 1 [chan receive]:
testing.RunTests(0x10a1328, 0x17282e0, 0x2, 0x2, 0xe2a901, ...)
 /usr/lib/go/src/pkg/testing/testing.go:434 +0x88e
testing.Main(0x10a1328, 0x17282e0, 0x2, 0x2, 0x1734d40, ...)
 /usr/lib/go/src/pkg/testing/testing.go:365 +0x8a
main.main()
 launchpad.net/juju-core/cmd/jujud/_test/_testmain.go:45 +0x9a

goroutine 2 [syscall]:

goroutine 4 [syscall]:
os/signal.loop()
 /usr/lib/go/src/pkg/os/signal/signal_unix.go:21 +0x1c
created by os/signal.init·1
 /usr/lib/go/src/pkg/os/signal/signal_unix.go:27 +0x2f

goroutine 6 [chan receive]:
launchpad.net/juju-core/provider/dummy.func·001()
 /home/tarmac/trees/src/launchpad.net/juju-core/provider/dummy/environs.go:208 +0x3a
created by launchpad.net/juju-core/provider/dummy.init·1
 /home/tarmac/trees/src/launchpad.net/juju-core/provider/dummy/environs.go:210 +0xcf

goroutine 9 [chan receive]:
launchpad.net/gocheck.(*suiteRunner).runTest(0xc200804070, 0xc200783000, 0xc20037a820)
 /home/tarmac/trees/src/launchpad.net/gocheck/gocheck.go:771 +0x4f
launchpad.net/gocheck.(*suiteRunner).run(0xc200804070, 0xc200395600)
 /home/tarmac/trees/src/launchpad.net/gocheck/gocheck.go:584 +0x1c5
laun...

=== modified file 'agent/mongo/mongo.go'
--- agent/mongo/mongo.go	2014-04-15 03:40:01 +0000
+++ agent/mongo/mongo.go	2014-04-15 09:07:40 +0000
@@ -302,3 +302,27 @@
 	}
 	return conf, nil
 }
+
+// mongoNoauthCommand returns an os/exec.Cmd that may be executed to
+// run mongod without security.
+func mongoNoauthCommand(dataDir string, port int) (*exec.Cmd, error) {
+	sslKeyFile := path.Join(dataDir, "server.pem")
+	dbDir := filepath.Join(dataDir, "db")
+	mongopath, err := MongodPath()
+	if err != nil {
+		return nil, err
+	}
+	cmd := exec.Command(mongopath,
+		"--noauth",
+		"--dbpath", dbDir,
+		"--sslOnNormalPorts",
+		"--sslPEMKeyFile", sslKeyFile,
+		"--sslPEMKeyPassword", "ignored",
+		"--bind_ip", "127.0.0.1",
+		"--port", fmt.Sprint(port),
+		"--noprealloc",
+		"--syslog",
+		"--smallfiles",
+	)
+	return cmd, nil
+}

=== modified file 'agent/mongo/mongo_test.go'
--- agent/mongo/mongo_test.go	2014-04-15 03:40:01 +0000
+++ agent/mongo/mongo_test.go	2014-04-15 09:07:40 +0000
@@ -3,13 +3,16 @@
 import (
 	"encoding/base64"
 	"io/ioutil"
+	"net"
 	"os"
 	"path"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 
 	jc "github.com/juju/testing/checkers"
+	"labix.org/v2/mgo"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/instance"
@@ -165,6 +168,7 @@
 	inst := &coretesting.MgoInstance{Params: []string{"--replSet", "juju"}}
 	err = inst.Start(true)
 	c.Assert(err, gc.IsNil)
+	defer inst.Destroy()
 
 	info := inst.DialInfo()
 
@@ -230,6 +234,57 @@
 	c.Assert(address, gc.Equals, "10.0.0.1:37017")
 }
 
+func (s *MongoSuite) TestEnsureAdminUser(c *gc.C) {
+	inst := &coretesting.MgoInstance{}
+	err := inst.Start(true)
+	c.Assert(err, gc.IsNil)
+	defer inst.Destroy()
+	dialInfo := inst.DialInfo()
+	// First call succeeds, as there are no users yet.
+	added, err := s.ensureAdminUser(c, dialInfo, "whomever", "whatever")
+	c.Assert(err, gc.IsNil)
+	c.Assert(added, jc.IsTrue)
+	// Second call succeeds, as the admin user is already there.
+	added, err = s.ensureAdminUser(c, dialInfo, "whomever", "whatever")
+	c.Assert(err, gc.IsNil)
+	c.Assert(added, jc.IsFalse)
+}
+
+func (s *MongoSuite) TestEnsureAdminUserError(c *gc.C) {
+	inst := &coretesting.MgoInstance{}
+	err := inst.Start(true)
+	c.Assert(err, gc.IsNil)
+	defer inst.Destroy()
+	dialInfo := inst.DialInfo()
+
+	// First call succeeds, as there are no users yet (mimics --noauth).
+	added, err := s.ensureAdminUser(c, dialInfo, "whomever", "whatever")
+	c.Assert(err, gc.IsNil)
+	c.Assert(added, jc.IsTrue)
+
+	// Second call fails, as there is another user and the database doesn't
+	// actually get reopened with --noauth in the test; mimics AddUser failure
+	_, err = s.ensureAdminUser(c, dialInfo, "whomeverelse", "whateverelse")
+	c.Assert(err, gc.ErrorMatches, `failed to add "whomeverelse" to admin database: not authorized for upsert on admin.system.users`)
+}
+
+func (s *MongoSuite) ensureAdminUser(c *gc.C, dialInfo *mgo.DialInfo, user, password string) (added bool, err error) {
+	_, portString, err := net.SplitHostPort(dialInfo.Addrs[0])
+	c.Assert(err, gc.IsNil)
+	port, err := strconv.Atoi(portString)
+	c.Assert(err, gc.IsNil)
+	s.PatchValue(&JujuMongodPath, "/bin/true")
+	s.PatchValue(&processSignal, func(*os.Process, os.Signal) error {
+		return nil
+	})
+	return EnsureAdminUser(EnsureAdminUserParams{
+		DialInfo: dialInfo,
+		Port:     port,
+		User:     user,
+		Password: password,
+	})
+}
+
 func (s *MongoSuite) TestGenerateSharedSecret(c *gc.C) {
 	secret, err := GenerateSharedSecret()
 	c.Assert(err, gc.IsNil)

=== added file 'agent/mongo/upgrade.go'
--- agent/mongo/upgrade.go	1970-01-01 00:00:00 +0000
+++ agent/mongo/upgrade.go	2014-04-15 09:07:40 +0000
@@ -0,0 +1,101 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package mongo
+
+import (
+	"fmt"
+	"os"
+	"syscall"
+
+	"labix.org/v2/mgo"
+
+	"launchpad.net/juju-core/upstart"
+)
+
+var (
+	processSignal = (*os.Process).Signal
+)
+
+type EnsureAdminUserParams struct {
+	// DialInfo specifies how to connect to the mongo server.
+	DialInfo *mgo.DialInfo
+	// Namespace is the agent namespace, used to derive the Mongo service name.
+	Namespace string
+	// DataDir is the Juju data directory, used to start a --noauth server.
+	DataDir string
+	// Port is the listening port of the Mongo server.
+	Port int
+	// User holds the user to log in to the mongo server as.
+	User string
+	// Password holds the password for the user to log in as.
+	Password string
+}
+
+// EnsureAdminUser ensures that the specified user and password
+// are added to the admin database.
+//
+// This function will stop the Mongo service if it needs to add
+// the admin user, as it must restart Mongo in --noauth mode.
+func EnsureAdminUser(p EnsureAdminUserParams) (added bool, err error) {
+	if len(p.DialInfo.Addrs) > 1 {
+		logger.Infof("more than one state server; admin user must exist")
+		return false, nil
+	}
+	p.DialInfo.Addrs = []string{fmt.Sprintf("127.0.0.1:%d", p.Port)}
+	p.DialInfo.Direct = true
+
+	// Attempt to login to the admin database first.
+	session, err := mgo.DialWithInfo(p.DialInfo)
+	if err != nil {
+		return false, fmt.Errorf("can't dial mongo to ensure admin user: %v", err)
+	}
+	err = session.DB("admin").Login(p.User, p.Password)
+	session.Close()
+	if err == nil {
+		return false, nil
+	}
+
+	// Login failed, so we need to add the user.
+	// Stop mongo, so we can start it in --noauth mode.
+	mongoServiceName := ServiceName(p.Namespace)
+	mongoService := upstart.NewService(mongoServiceName)
+	if err := mongoService.Stop(); err != nil {
+		return false, fmt.Errorf("failed to stop %v: %v", mongoServiceName, err)
+	}
+
+	// Start mongod in --noauth mode.
+	logger.Debugf("starting mongo with --noauth")
+	cmd, err := mongoNoauthCommand(p.DataDir, p.Port)
+	if err != nil {
+		return false, fmt.Errorf("failed to prepare mongod command: %v", err)
+	}
+	if err := cmd.Start(); err != nil {
+		return false, fmt.Errorf("failed to start mongod: %v", err)
+	}
+	defer cmd.Process.Kill()
+
+	// Add the user to the admin database.
+	logger.Debugf("setting admin password")
+	if session, err = mgo.DialWithInfo(p.DialInfo); err != nil {
+		return false, fmt.Errorf("can't dial mongo to ensure admin user: %v", err)
+	}
+	err = session.DB("admin").AddUser(p.User, p.Password, false)
+	session.Close()
+	if err != nil {
+		return false, fmt.Errorf("failed to add %q to admin database: %v", p.User, err)
+	}
+	logger.Infof("added %q to admin database", p.User)
+
+	// Restart mongo using upstart.
+	if err := processSignal(cmd.Process, syscall.SIGTERM); err != nil {
+		return false, fmt.Errorf("cannot kill mongod: %v", err)
+	}
+	if err := cmd.Wait(); err != nil {
+		return false, fmt.Errorf("mongod did not cleanly terminate: %v", err)
+	}
+	if err := mongoService.Start(); err != nil {
+		return false, err
+	}
+	return true, nil
+}

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2014-04-15 03:40:01 +0000
+++ cmd/jujud/machine.go	2014-04-15 09:07:40 +0000
@@ -74,6 +74,7 @@
 	// allow the tests to intercept calls to the functions.
 	ensureMongoServer        = mongo.EnsureMongoServer
 	maybeInitiateMongoServer = mongo.MaybeInitiateMongoServer
+	ensureMongoAdminUser     = mongo.EnsureAdminUser
 	newSingularRunner        = singular.New
 
 	// reportOpenedAPI is exposed for tests to know when
@@ -390,6 +391,26 @@
 	return nil
 }
 
+func (a *MachineAgent) ensureMongoAdminUser(agentConfig agent.Config, port int, namespace string) (added bool, err error) {
+	stateInfo := agentConfig.StateInfo()
+	dialInfo, err := state.DialInfo(stateInfo, state.DefaultDialOpts())
+	if err != nil {
+		return false, err
+	}
+	if len(dialInfo.Addrs) > 1 {
+		logger.Infof("more than one state server; admin user must exist")
+		return false, nil
+	}
+	return ensureMongoAdminUser(mongo.EnsureAdminUserParams{
+		DialInfo:  dialInfo,
+		Namespace: namespace,
+		DataDir:   agentConfig.DataDir(),
+		Port:      port,
+		User:      stateInfo.Tag,
+		Password:  stateInfo.Password,
+	})
+}
+
 // StateJobs returns a worker running all the workers that require
 // a *state.State connection.
 func (a *MachineAgent) StateWorker() (worker.Worker, error) {
@@ -408,16 +429,31 @@
 	if err != nil {
 		return nil, err
 	}
+
+	st, m, err := openState(agentConfig)
+	if errors.IsUnauthorizedError(err) {
+		// TODO(axw) remove this when we no longer need
+		// to upgrade from pre-HA-capable environments.
+		logger.Debugf("failed to open state, reattempt after ensuring admin user exists: %v", err)
+		added, ensureErr := a.ensureMongoAdminUser(agentConfig, info.StatePort, namespace)
+		if ensureErr != nil {
+			err = ensureErr
+		}
+		if !added {
+			// No user added, so it's probably a genuine unauthorized error.
+			return nil, err
+		}
+		st, m, err = openState(agentConfig)
+	}
+	if err != nil {
+		return nil, err
+	}
+	reportOpenedState(st)
+
 	// TODO(rog) call maybeInitiateMongoServer to upgrade mongo
 	// from old environments. We'll need to acquire a non-localhost
 	// address for the current instance before we do.
 
-	st, m, err := openState(agentConfig)
-	if err != nil {
-		return nil, err
-	}
-	reportOpenedState(st)
-
 	singularStateConn := singularStateConn{st.MongoSession(), m}
 	runner := newRunner(connectionIsFatal(st), moreImportant)
 	singularRunner, err := newSingularRunner(runner, singularStateConn)

=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2014-04-15 03:40:01 +0000
+++ cmd/jujud/machine_test.go	2014-04-15 09:07:40 +0000
@@ -17,6 +17,7 @@
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/agent"
+	"launchpad.net/juju-core/agent/mongo"
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/cmd"
 	lxctesting "launchpad.net/juju-core/container/lxc/testing"
@@ -895,6 +896,39 @@
 	c.Fatalf("timeout while waiting for agent config to change")
 }
 
+func (s *MachineSuite) TestMachineAgentEnsureAdminUser(c *gc.C) {
+	m, _, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
+	err := s.State.MongoSession().DB("admin").RemoveUser(m.Tag())
+	c.Assert(err, gc.IsNil)
+
+	s.PatchValue(&ensureMongoAdminUser, func(p mongo.EnsureAdminUserParams) (bool, error) {
+		err := s.State.MongoSession().DB("admin").AddUser(p.User, p.Password, false)
+		c.Assert(err, gc.IsNil)
+		return true, nil
+	})
+
+	stateOpened := make(chan eitherState, 1)
+	s.PatchValue(&reportOpenedState, func(st eitherState) {
+		select {
+		case stateOpened <- st:
+		default:
+		}
+	})
+
+	// Start the machine agent, and wait for state to be opened.
+	a := s.newAgent(c, m)
+	done := make(chan error)
+	go func() { done <- a.Run(nil) }()
+	defer a.Stop() // in case of failure
+	select {
+	case st := <-stateOpened:
+		c.Assert(st, gc.NotNil)
+	case <-time.After(coretesting.LongWait):
+		c.Fatalf("state not opened")
+	}
+	s.waitStopped(c, state.JobManageEnviron, a, done)
+}
+
 // MachineWithCharmsSuite provides infrastructure for tests which need to
 // work with charms.
 type MachineWithCharmsSuite struct {

=== modified file 'state/compat_test.go'
--- state/compat_test.go	2014-04-14 19:17:58 +0000
+++ state/compat_test.go	2014-04-15 09:07:40 +0000
@@ -10,7 +10,6 @@
 
 	"launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/testing/testbase"
-	"launchpad.net/juju-core/utils"
 )
 
 // compatSuite contains backwards compatibility tests,
@@ -105,34 +104,3 @@
 	c.Assert(include, gc.HasLen, 0)
 	c.Assert(exclude, gc.HasLen, 0)
 }
-
-func (s *compatSuite) TestOpenStateWithoutAdmin(c *gc.C) {
-	// https://launchpad.net/bugs/1306902
-	// In 1.18, machine-0 did not have access to the "admin" database. In
-	// newer versions we need access in order to do replicaSet mutations.
-	// However, we have not added the ability during upgrade to add
-	// machine-0 to the admin db, so we should still continue even when it
-	// doesn't have rights.
-	machine, err := s.state.AddMachine("quantal", JobManageEnviron)
-	c.Assert(err, gc.IsNil)
-	machinePassword, err := utils.RandomPassword()
-	c.Assert(err, gc.IsNil)
-	err = machine.SetPassword(machinePassword)
-	c.Assert(err, gc.IsNil)
-	err = machine.SetMongoPassword(machinePassword)
-	c.Assert(err, gc.IsNil)
-	// (jam) The only way I've found to actually ensure "machine-0" is
-	// removed from the Admin database is to actually login *as* the
-	// machine agent we just gave admin rights to, and then remove it.
-	adminDB := s.state.db.Session.DB("admin")
-	err = adminDB.Login(machine.Tag(), machinePassword)
-	c.Assert(err, gc.IsNil)
-	err = adminDB.RemoveUser(machine.Tag())
-	c.Assert(err, gc.IsNil)
-	info := TestingStateInfo()
-	info.Tag = machine.Tag()
-	info.Password = machinePassword
-	machineState, err := Open(info, TestingDialOpts(), Policy(nil))
-	c.Assert(err, gc.IsNil)
-	machineState.Close()
-}

=== modified file 'state/open.go'
--- state/open.go	2014-04-14 19:30:01 +0000
+++ state/open.go	2014-04-15 09:07:40 +0000
@@ -251,6 +251,7 @@
 func newState(session *mgo.Session, info *Info, policy Policy) (*State, error) {
 	db := session.DB("juju")
 	pdb := session.DB("presence")
+	admin := session.DB("admin")
 	if info.Tag != "" {
 		if err := db.Login(info.Tag, info.Password); err != nil {
 			return nil, maybeUnauthorized(err, fmt.Sprintf("cannot log in to juju database as %q", info.Tag))
@@ -258,19 +259,10 @@
 		if err := pdb.Login(info.Tag, info.Password); err != nil {
 			return nil, maybeUnauthorized(err, fmt.Sprintf("cannot log in to presence database as %q", info.Tag))
 		}
-		admin := session.DB("admin")
 		if err := admin.Login(info.Tag, info.Password); err != nil {
-			if isUnauthorized(err) {
-				// TODO(jam) https://launchpad.net/bugs/1306902 in juju <=1.18,
-				// machine-0 was not an admin, so it cannot login to the "admin"
-				// database until bug #1306902 is fixed.
-				logger.Infof("ignoring failure to login to \"admin\" database as %q (bug #1306902): %v", info.Tag, err)
-			} else {
-				return nil, maybeUnauthorized(err, fmt.Sprintf("cannot log in to admin database as %q", info.Tag))
-			}
+			return nil, maybeUnauthorized(err, fmt.Sprintf("cannot log in to admin database as %q", info.Tag))
 		}
 	} else if info.Password != "" {
-		admin := session.DB("admin")
 		if err := admin.Login(AdminUser, info.Password); err != nil {
 			return nil, maybeUnauthorized(err, "cannot log in to admin database")
 		}