Code review comment for lp://staging/~rogpeppe/juju-core/311-juju-bootstrap-state-change-password-1.5

Please take a look.

https://codereview.appspot.com/9738043/diff/5001/cmd/jujud/bootstrap.go
File cmd/jujud/bootstrap.go (right):

https://codereview.appspot.com/9738043/diff/5001/cmd/jujud/bootstrap.go#newcode140
cmd/jujud/bootstrap.go:140: }
On 2013/05/27 20:13:26, fwereade wrote:
> I guess if we fail here, we have a borked environment regardless. So,
given the
> failure modes we've already embraced, ok, I guess... but it's a bit
surprising
> nonetheless. Comment perhaps?

i'm not sure what kind of comment you're thinking here.
how is failing to write the configuration any different
from any of the above failure modes?

https://codereview.appspot.com/9738043/diff/5001/cmd/jujud/bootstrap_test.go
File cmd/jujud/bootstrap_test.go (right):

https://codereview.appspot.com/9738043/diff/5001/cmd/jujud/bootstrap_test.go#newcode198
cmd/jujud/bootstrap_test.go:198: info.Tag, info.Password = "",
testPasswordHash
On 2013/05/27 20:13:26, fwereade wrote:
> wait, why can user-admin log in as ""?

the empty string means "admin" when logging into the state. see the docs
on state.Info.Tag. i disclaim all responsibility for that decision :-)

https://codereview.appspot.com/9738043/diff/5001/cmd/jujud/bootstrap_test.go#newcode220
cmd/jujud/bootstrap_test.go:220:
c.Assert(machineConf1.StateInfo.Password, Equals,
machineConf1.APIInfo.Password)
On 2013/05/27 20:13:26, fwereade wrote:
> Are we duplicating tag/password for the same entities everywhere? If
so, why?

we use the same password for the state that we do for the API.
is that bad?

https://codereview.appspot.com/9738043/diff/5001/environs/agent/agent.go
File environs/agent/agent.go (right):

https://codereview.appspot.com/9738043/diff/5001/environs/agent/agent.go#newcode208
environs/agent/agent.go:208: func (c *Conf) OpenAPI() (st *api.State,
newPassword string, err error) {
On 2013/05/27 20:13:26, fwereade wrote:
> Why are we duplicating this back-to-front newPassword-juggling
behaviour?

what do you suggest instead? we still need to do exactly the same thing
AFAICS. i wasn't thinking of redesigning this logic at this point, but i
can do if you'd like it to change.

https://codereview.appspot.com/9738043/diff/5001/environs/dummy/environs.go
File environs/dummy/environs.go (left):

https://codereview.appspot.com/9738043/diff/5001/environs/dummy/environs.go#oldcode465
environs/dummy/environs.go:465: // logic is done.
On 2013/05/27 20:13:26, fwereade wrote:
> "TODO: this whole test-only parallel implementation of bootstrap-state
is an
> open invitation to bugs and insanity", perhaps?

that's not really a TODO. i've added something slightly different.

https://codereview.appspot.com/9738043/