Please take a look.


https://codereview.appspot.com/101760046/diff/1/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/101760046/diff/1/environs/configstore/disk.go#newcode144
environs/configstore/disk.go:144: EnvironTag: info.EnvInfo.EnvironTag,
On 2014/05/27 07:27:57, fwereade wrote:
> I think we want to *store* the uuid, not the tag -- the tags really
are just for
> the API, and I would prefer that we not let them leak into users'
worldviews.
> Tags coming back from the API are fine and good, but no further.

Fine with me. I wasn't sure if bare UUIDs were better to store than
tags. I realized that in the API things are tags, and in State they are
IDs, but wasn't sure about user level stuff.

Done.

I ended up sticking with the api.State object has an EnvironTag()
method, and api.Info thinks of tags, but configstore.EnvironInfo uses
EnvironUUID.

https://codereview.appspot.com/101760046/diff/1/environs/configstore/interface_test.go
File environs/configstore/interface_test.go (right):

https://codereview.appspot.com/101760046/diff/1/environs/configstore/interface_test.go#newcode51
environs/configstore/interface_test.go:51: EnvironTag:
"environ-dead-beef",
On 2014/05/27 07:27:57, fwereade wrote:
> this choice of "uuid" makes me unreasonably happy :)

:)

https://codereview.appspot.com/101760046/diff/1/juju/api.go
File juju/api.go (right):

https://codereview.appspot.com/101760046/diff/1/juju/api.go#newcode358
juju/api.go:358: EnvironTag: apiInfo.EnvironTag,
On 2014/05/27 07:27:57, fwereade wrote:
> I won't keep saying it, but this is the place to just extract the uuid
and store
> it.

Done.

https://codereview.appspot.com/101760046/diff/1/juju/api.go#newcode358
juju/api.go:358: EnvironTag: apiInfo.EnvironTag,
On 2014/05/27 07:27:57, fwereade wrote:
> I won't keep saying it, but this is the place to just extract the uuid
and store
> it.

Done.

https://codereview.appspot.com/101760046/diff/1/juju/api.go#newcode387
juju/api.go:387: _ = newEnvironTag
On 2014/05/27 07:27:57, fwereade wrote:
> ?

temporary workaround for golang not allowing unused vars while
incrementally building code. removed.

https://codereview.appspot.com/101760046/diff/1/state/apiserver/admin.go
File state/apiserver/admin.go (right):

https://codereview.appspot.com/101760046/diff/1/state/apiserver/admin.go#newcode142
state/apiserver/admin.go:142: // trigger an error for the Environment()
call?
On 2014/05/27 07:27:58, fwereade wrote:
> Heh. You could, inside state, slip in and delete the document; but
Idon't think
> there's a way to do it here. (Stop the mongo? ;p)

Yeah, it is just one of those things where you "have" to propagate
errors, but I really don't like touching the stuff without actually
checking what it does.
Should this actually be returning some other error for the API? Is it
sane to just blat out whatever internal failure occured?

We don't really have concrete decisions around this sort of thing (that
I can tell).

https://codereview.appspot.com/101760046/diff/1/state/apiserver/login_test.go
File state/apiserver/login_test.go (right):

https://codereview.appspot.com/101760046/diff/1/state/apiserver/login_test.go#newcode530
state/apiserver/login_test.go:530: }
On 2014/05/27 07:27:58, fwereade wrote:
> I guess we have enough tests that it accepts a missing environ tag
already?

We do, but I'm much happier to do this sort of thing explicitly rather
than assume that there is a side effect of some other test somewhere.
This is actually tested by the LoginReturnsEnvironTag code path, but
I'll make it more explicit.

It is one of those cases where yes, params.Creds{...} will pass in
EnvironTag: "" if I don't specify it, but it actually makes the test
clearer that we are explicitly testing the case where it is not set.


> I'm wondering whether we should be fixing this stuff for the agents as
well.
> It's less pressing, but still.

> (FWIW, I feel that all this code to connect and store changes etc etc
really
> should be exactly the same on agent and client -- the only interesting
> difference is the extra stuff that agents need to do to change their
passwords,
> and the client will need the same functionality once we expose
> pass-word-changing there.)

I might as well look into it now since I'm here.
The both go via api.Open which has all the logic to accept an
environment tag and return one in the State object that is opened.
It would be interesting if agent/config.go could be unified with
environs/configstore. Would we be trying to change agents to use a .jenv
formatted file as well?

I think that is out of scope for what I'm doing, though.

I do think that agent.conf should probably come pre-seeded with the
environment UUID for all cases that we actually care about. I'm a little
concerned about how the upgrade process would do migration, though,
since it isn't information that we otherwise just have on hand.

I can do the same "upgrade on API Login" that we do for clients, or we
could try to find a way to do it via the actual upgrade logic. (Upgrade
itself has an API connection, right? so it should be able to run an
Upgrade step that pulls it out of there. I'll at least dig into it some
more.)

https://codereview.appspot.com/101760046/