api/state: allow multiple addresses; add APIInfo

api/state: allow multiple addresses; add APIInfo

We don't use them yet, but as we only ever have
one server currently, that's shouldn't be an issue.

We also add APIInfo to cloudinit.Config, which
probably shouldn't have got mixed into this CL,
but hopefully it's ok here.

https://code.launchpad.net/~rogpeppe/juju-core/203-api-multi-addr/+merge/144534

Requires: https://code.launchpad.net/~rogpeppe/juju-core/202-environs-apiinfo/+merge/144452

(do not edit description out of merge proposal)

[rog, 2013-01-23 16:53:38]

Please take a look.

[dimitern, 2013-01-23 17:26:21]

LGTM, only one question - why do we use Addrs[0] only? There's a TODO about
randomizing the addresses or trying multiple before failing. I assume when this
happens, we won't use only the first address?

[rog, 2013-01-23 17:39:07]

On 2013/01/23 17:26:21, dimitern wrote:
> LGTM, only one question - why do we use Addrs[0] only? There's a TODO about
> randomizing the addresses or trying multiple before failing. I assume when
this
> happens, we won't use only the first address?

that's right.

we may even make connections to more than one server at once and multiplex
requests, but for the time being, a single connection
is fine.

the answer is slightly different if you're talking
about the call to NewServer in cmd/jujud. there, the
address is actually the address to listen on, which is
probably not entirely right (if you've got more
than one address, how do you know which one to listen
on?), but that's how the mongo setup works, so we go with
that for the time being, because we've only got one
server, so it works ok.

dave cheney's HA stuff should hopefully sort all this out.

[niemeyer, 2013-01-24 13:39:38]

I don't understand the relationship of some of the changes with the CL
description. Otherwise, LGTM.

https://codereview.appspot.com/7197049/diff/2001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/7197049/diff/2001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:22: const apiPort = 37018
As per the previous review, it'd be best to have this in a range by itself.

https://codereview.appspot.com/7197049/diff/2001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/7197049/diff/2001/environs/ec2/ec2.go#newcode265
environs/ec2/ec2.go:265: apiInfo: &api.Info{
This looks unrelated to the multiple addresses change that this CL claims in its
description.

https://codereview.appspot.com/7197049/diff/2001/environs/ec2/ec2.go#newcode384
environs/ec2/ec2.go:384: apiInfo         *api.Info
Same.

[rog, 2013-01-24 14:06:22]

*** Submitted:

api/state: allow multiple addresses; add APIInfo

We don't use them yet, but as we only ever have
one server currently, that's shouldn't be an issue.

We also add APIInfo to cloudinit.Config, which
probably shouldn't have got mixed into this CL,
but hopefully it's ok here.

R=dimitern, niemeyer
CC=
https://codereview.appspot.com/7197049

https://codereview.appspot.com/7197049/diff/2001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/7197049/diff/2001/environs/ec2/ec2.go#newcode265
environs/ec2/ec2.go:265: apiInfo: &api.Info{
On 2013/01/24 13:39:38, niemeyer wrote:
> This looks unrelated to the multiple addresses change that this CL claims in
its
> description.

yes. it got dragged in somehow, and i did change the CL description, but
couldn't fit it all in the top 50 characters.

i hope you're ok with it all going in in one lump.