API should report all non-machine-local addresses

API should report all non-machine-local addresses

To support the manual provider, we need to report all addresses that are not machine-local for the API servers.  Previously, we were only reporting cloud-local addresses, which often will not work if you're using the manual provider.

https://code.launchpad.net/~natefinch/juju-core/051-agent-addresses/+merge/218878

(do not edit description out of merge proposal)

[natefinch, 2014-05-08 20:25:14]

Please take a look.

[fwereade, 2014-05-08 21:07:17]

LGTM.

Only quibble is that it might be worth extracting that new logic to a selection
func that can live next to the original one.

https://codereview.appspot.com/91270044/diff/1/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/91270044/diff/1/agent/agent.go#newcode444
agent/agent.go:444: if hp.NetworkScope != instance.NetworkMachineLocal {
is there any possibility this will fail on the local provider?

https://codereview.appspot.com/91270044/diff/1/agent/agent_test.go
File agent/agent_test.go (left):

https://codereview.appspot.com/91270044/diff/1/agent/agent_test.go#oldcode516
agent/agent_test.go:516: // at all, then it will be excluded.
ok, I guess if machine-local was definitely already excluded we're fine :)

[jameinel, 2014-05-09 09:15:52]

On 2014/05/08 21:07:17, fwereade wrote:
> LGTM.
> 
> Only quibble is that it might be worth extracting that new logic to a
selection
> func that can live next to the original one.
> 
> https://codereview.appspot.com/91270044/diff/1/agent/agent.go
> File agent/agent.go (right):
> 
> https://codereview.appspot.com/91270044/diff/1/agent/agent.go#newcode444
> agent/agent.go:444: if hp.NetworkScope != instance.NetworkMachineLocal {
> is there any possibility this will fail on the local provider?
> 
> https://codereview.appspot.com/91270044/diff/1/agent/agent_test.go
> File agent/agent_test.go (left):
> 
> https://codereview.appspot.com/91270044/diff/1/agent/agent_test.go#oldcode516
> agent/agent_test.go:516: // at all, then it will be excluded.
> ok, I guess if machine-local was definitely already excluded we're fine :)


I'd be happier if we were doing HostPorts storage on this, so that we could see
the list of ones to try first, and then what to try second (cloud-local first,
etc).
Are there plans to push all the way to that point? This could be an ok
bridge-the-gap, I just worry that it will be good-enough-for-now and we don't
actually fix the underlying issues.

[natefinch, 2014-05-09 20:21:12]

On 2014/05/09 09:15:52, jameinel wrote:
> I'd be happier if we were doing HostPorts storage on this, so that we could
see
> the list of ones to try first, and then what to try second (cloud-local first,
> etc).
> Are there plans to push all the way to that point? This could be an ok
> bridge-the-gap, I just worry that it will be good-enough-for-now and we don't
> actually fix the underlying issues.

I hadn't had plans to do that, though I do agree it's a better approach.  I can
do that if we think it's worth a little extra work.

[fwereade, 2014-05-15 08:59:57]

OK, let's go with hostport storage for consistency's sake as well.