state/apiserver/uniter: use cached APIAddresses

state/apiserver/uniter: use cached APIAddresses

When Roger changed the common.Addresser code to use the cached
addresses, he missed the fact that Uniter had its own implementation.
This was because we weren't exposing StateAddress or CACert to the
Uniter. So this change moves the code around to share the common
APIAddresses method and uses it in the Uniter facade.

https://code.launchpad.net/~jameinel/juju-core/uniter-cached-addresses/+merge/193650

(do not edit description out of merge proposal)

[jameinel, 2013-11-01 19:21:51]

Please take a look.

[rog, 2013-11-01 20:00:07]

LGTM with a couple of passing thoughts.

https://codereview.appspot.com/20940043/diff/1/state/apiserver/common/address...
File state/apiserver/common/addresses.go (right):

https://codereview.appspot.com/20940043/diff/1/state/apiserver/common/address...
state/apiserver/common/addresses.go:19: type APIAddresser struct {
Any particular reason this doesn't include CACert too?

https://codereview.appspot.com/20940043/diff/1/state/apiserver/common/address...
state/apiserver/common/addresses.go:44: APIAddresser
Rather than embedding APIAddresser, I wonder if it might be conceptually nicer
to have the two types completely separate, though I guess that would imply more
memory overhead.

[jameinel, 2013-11-01 21:12:58]

Please take a look.

[jameinel, 2013-11-04 10:41:40]

Please take a look.

[dimitern, 2013-11-04 10:48:57]

https://codereview.appspot.com/20940043/diff/40001/state/apiserver/common/add...
File state/apiserver/common/addresses.go (left):

https://codereview.appspot.com/20940043/diff/40001/state/apiserver/common/add...
state/apiserver/common/addresses.go:33: // TODO(dimitern): Remove this once we
have a way to get state/API
Please, assign this bug to you and link this branch?

[fwereade, 2013-11-06 10:18:27]

LGTM, but I thought we'd need a CACerter (as it were). Do we always just get the
cert from where it's stored locally? I guess that if you don't already have the
CACert you have no reason to trust what a CACert method would tell you, but it
still seems a bit strange.

https://codereview.appspot.com/20940043/diff/40001/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/20940043/diff/40001/worker/uniter/uniter_test....
worker/uniter/uniter_test.go:1109: func (s ensureStateWorker) step(c *gc.C, ctx
*context) {
Can't we just jam this into createUniter?

[jameinel, 2013-11-06 11:38:54]

On 2013/11/06 10:18:27, fwereade wrote:
> LGTM, but I thought we'd need a CACerter (as it were). Do we always just get
the
> cert from where it's stored locally? I guess that if you don't already have
the
> CACert you have no reason to trust what a CACert method would tell you, but it
> still seems a bit strange.


So there is a bug that the Uniter puts APIAddress into the charm context but not
the CACert. When someone comes along to fix that bug they can expose it here.

Yes we need it, but it is out of scope for this change.

[jameinel, 2013-11-06 11:40:55]

Please take a look.

https://codereview.appspot.com/20940043/diff/40001/state/apiserver/common/add...
File state/apiserver/common/addresses.go (left):

https://codereview.appspot.com/20940043/diff/40001/state/apiserver/common/add...
state/apiserver/common/addresses.go:33: // TODO(dimitern): Remove this once we
have a way to get state/API
On 2013/11/04 10:48:58, dimitern wrote:
> Please, assign this bug to you and link this branch?

Done.

https://codereview.appspot.com/20940043/diff/40001/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/20940043/diff/40001/worker/uniter/uniter_test....
worker/uniter/uniter_test.go:1109: func (s ensureStateWorker) step(c *gc.C, ctx
*context) {
On 2013/11/06 10:18:28, fwereade wrote:
> Can't we just jam this into createUniter?

Done.

There is still 1 path that doesn't call createUniter because it didn't want to
have the wait, but it does reduce how many things need it.
I had originally done quickStart, but actually quite a few tests don't call that
one. createUniter is much more prevalent.