Code review comment for lp:~axwalk/juju-core/lp1306902-mongo-ensureadminuser

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/