juju-core

Merge lp:~axwalk/juju-core/lp1303735-fix-address-logic into lp:~go-bot/juju-core/trunk

  1. lp1303735-fix-address-logic
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Martin Packman
Approved revision: no longer in the source branch.
Merged at revision: 2607
Proposed branch: lp:~axwalk/juju-core/lp1303735-fix-address-logic
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 837 lines (+282/-198)
16 files modified
cmd/juju/status_test.go (+3/-3)
instance/address.go (+133/-90)
instance/address_test.go (+49/-49)
provider/common/state.go (+0/-1)
provider/maas/instance.go (+1/-1)
provider/openstack/provider.go (+4/-6)
provider/openstack/provider_test.go (+8/-8)
state/api/machiner/machiner_test.go (+2/-1)
state/api/state.go (+2/-6)
state/apiserver/machine/machiner.go (+1/-1)
state/machine.go (+15/-13)
state/machine_test.go (+30/-1)
state/unit_test.go (+25/-0)
worker/machiner/machiner.go (+1/-8)
worker/machiner/machiner_test.go (+6/-4)
worker/peergrouper/worker_test.go (+2/-6)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1303735-fix-address-logic
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+215085@code.launchpad.net

Commit message

Various address logic fixes

- Unified logic for Select{Public,Internal}Address.
  Now they both return the first public/private address,
  or the first address with a usable scope.
- instance.NewAddress will now derive the scope of
  IPv4 address from the network range. Loopback addresses
  are machine-local, private network addresses are cloud-
  local, all others are public.
- Now using instance.NewAddress in provider/openstack and
  worker/machiner so scope derivation logic is used.
  Live-tested on HP Cloud and Canonistack.
- Dropped instance.HostAddresses; it is no longer used.
- Fixed state.mergedAddresses to maintain order.

Fixes lp:1303735

https://codereview.appspot.com/85590044/

Description of the change

Various address logic fixes

- Unified logic for Select{Public,Internal}Address.
  Now they both return the first public/private address,
  or the first address with a usable scope.
- instance.NewAddress will now derive the scope of
  IPv4 address from the network range. Loopback addresses
  are machine-local, private network addresses are cloud-
  local, all others are public.
- Now using instance.NewAddress in provider/openstack and
  worker/machiner so scope derivation logic is used.
  Live-tested on HP Cloud and Canonistack.
- Dropped instance.HostAddresses; it is no longer used.
- Fixed state.mergedAddresses to maintain order.

Fixes lp:1303735

https://codereview.appspot.com/85590044/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+215085_code.launchpad.net,

Message:
Please take a look.

Description:
Various address logic fixes

- Unified logic for Select{Public,Internal}Address.
   Now they both return the first public/private address,
   or the first address with a usable scope.
- instance.NewAddress will now derive the scope of
   IPv4 address from the network range. Loopback addresses
   are machine-local, private network addresses are cloud-
   local, all others are public.
- Now using instance.NewAddress in provider/openstack and
   worker/machiner so scope derivation logic is used.
   Live-tested on HP Cloud and Canonistack.
- Dropped instance.HostAddresses; it is no longer used.
- Fixed state.mergedAddresses to maintain order.

Fixes lp:1303735

https://code.launchpad.net/~axwalk/juju-core/lp1303735-fix-address-logic/+merge/215085

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/85590044/

Affected files (+273, -188 lines):
   A [revision details]
   M cmd/juju/status_test.go
   M instance/address.go
   M instance/address_test.go
   M provider/common/state.go
   M provider/maas/instance.go
   M provider/openstack/provider.go
   M provider/openstack/provider_test.go
   M state/api/machiner/machiner_test.go
   M state/api/state.go
   M state/apiserver/machine/machiner.go
   M state/machine.go
   M state/machine_test.go
   M state/unit_test.go
   M worker/machiner/machiner.go
   M worker/machiner/machiner_test.go
   M worker/peergrouper/worker_test.go

Revision history for this message
Roger Peppe (rogpeppe) wrote :

Nice, LGTM.

https://codereview.appspot.com/85590044/diff/1/instance/address.go
File instance/address.go (right):

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode34
instance/address.go:34: Ipv4Address AddressType = "ipv4"
It would be great if we could fix the spelling of these to
conform with Go convention (IPv4Address, IPv6Address).
No particular need to do it now though - it's just
been bugging me occasionally when I see it.

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode119
instance/address.go:119: if ip != nil {
if ip == nil {
     return HostName
}

then lose an indent level below?
or even:

switch {
case ip == nil:
     return HostName
case ip.To4() != nil:
     return Ipv4Address
case ip.To6() != nil:
     return Ipv6Address
default:
     panic("Unknown form of IP address"
}

oops, wrote that, then realised it wasn't new code.
feel free to ignore.

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode132
instance/address.go:132: func isIpv4PrivateNetworkAddress(ip net.IP)
bool {
s/Ip/IP/

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode170
instance/address.go:170: func NewAddress(value string, scope
NetworkScope) Address {
Perhaps move scope to the first argument here, so that if we add
the same argument to NewAddresses, the two functions can be consistent?

https://codereview.appspot.com/85590044/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/85590044/diff/1/instance/address.go
File instance/address.go (right):

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode34
instance/address.go:34: Ipv4Address AddressType = "ipv4"
On 2014/04/10 07:31:08, rog wrote:
> It would be great if we could fix the spelling of these to
> conform with Go convention (IPv4Address, IPv6Address).
> No particular need to do it now though - it's just
> been bugging me occasionally when I see it.

Agreed, but not now as discussed.

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode119
instance/address.go:119: if ip != nil {
On 2014/04/10 07:31:08, rog wrote:
> if ip == nil {
> return HostName
> }

> then lose an indent level below?
> or even:

> switch {
> case ip == nil:
> return HostName
> case ip.To4() != nil:
> return Ipv4Address
> case ip.To6() != nil:
> return Ipv6Address
> default:
> panic("Unknown form of IP address"
> }

> oops, wrote that, then realised it wasn't new code.
> feel free to ignore.

Easy win, done.

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode132
instance/address.go:132: func isIpv4PrivateNetworkAddress(ip net.IP)
bool {
On 2014/04/10 07:31:08, rog wrote:
> s/Ip/IP/

Done.

https://codereview.appspot.com/85590044/diff/1/instance/address.go#newcode170
instance/address.go:170: func NewAddress(value string, scope
NetworkScope) Address {
On 2014/04/10 07:31:08, rog wrote:
> Perhaps move scope to the first argument here, so that if we add
> the same argument to NewAddresses, the two functions can be
 consistent?

Maybe another time, but this is going to create a lot of noise I'd
rather not have to backport.

https://codereview.appspot.com/85590044/

Revision history for this message
Martin Packman (gz) wrote :
Download full text (3.3 KiB)

LGTM. Only one real thing to fix, and some other general comments.

https://codereview.appspot.com/85590044/diff/20001/instance/address.go
File instance/address.go (left):

https://codereview.appspot.com/85590044/diff/20001/instance/address.go#oldcode196
instance/address.go:196: mostPublicIndex = i
We're losing a subtle hack here, which is public addresses fallback to
the last, not the first, non-public addr. This is to support the HP
style case where adding a floating ip goes on the end of the list of
addresses. Obviously, that was pretty fragile anyway. With the
public/private bits being auto-filled for openstack anyway, this is
probably okay.

https://codereview.appspot.com/85590044/diff/20001/instance/address.go
File instance/address.go (right):

https://codereview.appspot.com/85590044/diff/20001/instance/address.go#newcode159
instance/address.go:159: return NetworkPublic
I'm not super-happy with this statement, but I think it will work for
all the cases we care about.

https://codereview.appspot.com/85590044/diff/20001/instance/address_test.go
File instance/address_test.go (right):

https://codereview.appspot.com/85590044/diff/20001/instance/address_test.go#newcode165
instance/address_test.go:165: "first unknown address selected",
Okay, and here's the test change to go with the logic flip.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider.go
File provider/openstack/provider.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider.go#oldcode419
provider/openstack/provider.go:419: NetworkName: network,
FIX: You've lost the recording of NetworkName - we really do want this
when it's available.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go
File provider/openstack/provider_test.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go#oldcode63
provider/openstack/provider_test.go:63: private: []nova.IPAddress{{4,
"127.0.0.4"}, {4, "8.8.4.4"}},
And this is the real world example (mostly, not sure why I used a 127.
address not a 10. address) we expect to work... I'm not sure about your
changes to this test? The code should still pick the 8. address if a
non-public one is first, due to the private address range matching?

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go
File provider/openstack/provider_test.go (right):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go#newcode82
provider/openstack/provider_test.go:82: private: []nova.IPAddress{{4,
"127.0.0.4"}, {4, "192.168.0.1"}},
Hm, okay, as I'm not actually doing the network name checking any more,
making these 'private' labelled addresses all cloud-local is reasonable.

https://codereview.appspot.com/85590044/diff/20001/worker/machiner/machiner.go
File worker/machiner/machiner.go (right):

https://codereview.appspot.com/85590044/diff/20001/worker/machiner/machiner.go#newcode80
worker/machiner/machiner.go:80: address :=
instance.NewAddress(ip.String(), instance.NetworkUnknown)
Good change. We probably still don't want to use this function too
widely, as using machine-re...

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider.go
File provider/openstack/provider.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider.go#oldcode419
provider/openstack/provider.go:419: NetworkName: network,
On 2014/04/10 11:23:27, gz wrote:
> FIX: You've lost the recording of NetworkName - we really do want this
 when it's
> available.

Good catch! Thanks, done.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go
File provider/openstack/provider_test.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go#oldcode63
provider/openstack/provider_test.go:63: private: []nova.IPAddress{{4,
"127.0.0.4"}, {4, "8.8.4.4"}},
On 2014/04/10 11:23:27, gz wrote:
> And this is the real world example (mostly, not sure why I used a 127.
 address
> not a 10. address) we expect to work... I'm not sure about your
 changes to this
> test? The code should still pick the 8. address if a non-public one is
 first,
> due to the private address range matching?

I changed it because 127.0.0.4 is machine-local, and hence will never
match. Likewise for the other test changes.

https://codereview.appspot.com/85590044/

Revision history for this message
Martin Packman (gz) wrote :

LGTM.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go
File provider/openstack/provider_test.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go#oldcode63
provider/openstack/provider_test.go:63: private: []nova.IPAddress{{4,
"127.0.0.4"}, {4, "8.8.4.4"}},
On 2014/04/10 11:57:54, axw wrote:
> On 2014/04/10 11:23:27, gz wrote:
> > And this is the real world example (mostly, not sure why I used a
 127. address
> > not a 10. address) we expect to work... I'm not sure about your
 changes to
> this
> > test? The code should still pick the 8. address if a non-public one
 is first,
> > due to the private address range matching?

> I changed it because 127.0.0.4 is machine-local, and hence will never
match.
> Likewise for the other test changes.

So, specifically, the test should be (and should work as) {"10.0.0.1",
"8.8.4.4"} -> "8.8.4.4".

https://codereview.appspot.com/85590044/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (64.1 KiB)

The attempt to merge lp:~axwalk/juju-core/lp1303735-fix-address-logic into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.641s
ok launchpad.net/juju-core/agent/mongo 1.099s
ok launchpad.net/juju-core/agent/tools 0.215s
ok launchpad.net/juju-core/bzr 4.985s
ok launchpad.net/juju-core/cert 2.670s
ok launchpad.net/juju-core/charm 0.414s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cloudinit/sshinit 0.734s
ok launchpad.net/juju-core/cmd 0.161s
ok launchpad.net/juju-core/cmd/charm-admin 0.799s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.168s
ok launchpad.net/juju-core/cmd/juju 218.659s
ok launchpad.net/juju-core/cmd/jujud 78.811s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 9.461s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.178s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.023s
ok launchpad.net/juju-core/container 0.036s
ok launchpad.net/juju-core/container/factory 0.040s
ok launchpad.net/juju-core/container/kvm 0.212s
ok launchpad.net/juju-core/container/kvm/mock 0.033s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.320s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.226s
ok launchpad.net/juju-core/environs 2.313s
ok launchpad.net/juju-core/environs/bootstrap 11.478s
ok launchpad.net/juju-core/environs/cloudinit 0.533s
ok launchpad.net/juju-core/environs/config 1.908s
ok launchpad.net/juju-core/environs/configstore 0.032s
ok launchpad.net/juju-core/environs/filestorage 0.029s
ok launchpad.net/juju-core/environs/httpstorage 0.653s
ok launchpad.net/juju-core/environs/imagemetadata 0.461s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.036s
ok launchpad.net/juju-core/environs/jujutest 0.149s
ok launchpad.net/juju-core/environs/manual 14.835s
ok launchpad.net/juju-core/environs/simplestreams 0.334s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.008s
ok launchpad.net/juju-core/environs/storage 1.025s
ok launchpad.net/juju-core/environs/sync 48.055s
ok launchpad.net/juju-core/environs/testing 0.141s
ok launchpad.net/juju-core/environs/tools 4.680s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.012s
ok launchpad.net/juju-core/instance 0.020s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 2...

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go
File provider/openstack/provider_test.go (left):

https://codereview.appspot.com/85590044/diff/20001/provider/openstack/provider_test.go#oldcode63
provider/openstack/provider_test.go:63: private: []nova.IPAddress{{4,
"127.0.0.4"}, {4, "8.8.4.4"}},
On 2014/04/10 12:20:04, gz wrote:
> On 2014/04/10 11:57:54, axw wrote:
> > On 2014/04/10 11:23:27, gz wrote:
> > > And this is the real world example (mostly, not sure why I used a
 127.
> address
> > > not a 10. address) we expect to work... I'm not sure about your
 changes to
> > this
> > > test? The code should still pick the 8. address if a non-public
 one is
> first,
> > > due to the private address range matching?
> >
> > I changed it because 127.0.0.4 is machine-local, and hence will
 never match.
> > Likewise for the other test changes.

> So, specifically, the test should be (and should work as) {"10.0.0.1",
> "8.8.4.4"} -> "8.8.4.4".

Done.

https://codereview.appspot.com/85590044/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (102.4 KiB)

The attempt to merge lp:~axwalk/juju-core/lp1303735-fix-address-logic into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.013s
ok launchpad.net/juju-core/agent 1.337s
ok launchpad.net/juju-core/agent/mongo 1.324s
ok launchpad.net/juju-core/agent/tools 0.173s
ok launchpad.net/juju-core/bzr 5.132s
ok launchpad.net/juju-core/cert 2.615s
ok launchpad.net/juju-core/charm 0.409s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.030s
ok launchpad.net/juju-core/cloudinit/sshinit 0.847s
ok launchpad.net/juju-core/cmd 0.202s
ok launchpad.net/juju-core/cmd/charm-admin 0.300s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.140s
ok launchpad.net/juju-core/cmd/juju 216.235s
ok launchpad.net/juju-core/cmd/jujud 79.369s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 9.921s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.217s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.026s
ok launchpad.net/juju-core/container 0.039s
ok launchpad.net/juju-core/container/factory 0.041s
ok launchpad.net/juju-core/container/kvm 0.273s
ok launchpad.net/juju-core/container/kvm/mock 0.041s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.350s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.234s
ok launchpad.net/juju-core/environs 2.601s
ok launchpad.net/juju-core/environs/bootstrap 10.598s
ok launchpad.net/juju-core/environs/cloudinit 0.430s
ok launchpad.net/juju-core/environs/config 1.803s
ok launchpad.net/juju-core/environs/configstore 0.031s
ok launchpad.net/juju-core/environs/filestorage 0.027s
ok launchpad.net/juju-core/environs/httpstorage 0.710s
ok launchpad.net/juju-core/environs/imagemetadata 0.421s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.035s
ok launchpad.net/juju-core/environs/jujutest 0.213s
ok launchpad.net/juju-core/environs/manual 11.097s
ok launchpad.net/juju-core/environs/simplestreams 0.237s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.029s
ok launchpad.net/juju-core/environs/storage 0.892s
ok launchpad.net/juju-core/environs/sync 48.830s
ok launchpad.net/juju-core/environs/testing 0.178s
ok launchpad.net/juju-core/environs/tools 4.867s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.011s
ok launchpad.net/juju-core/instance 0.018s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 2...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/juju/status_test.go'
2--- cmd/juju/status_test.go 2014-04-02 11:35:49 +0000
3+++ cmd/juju/status_test.go 2014-04-10 13:19:09 +0000
4@@ -250,7 +250,7 @@
5 startAliveMachine{"0"},
6 setAddresses{"0", []instance.Address{
7 instance.NewAddress("10.0.0.1", instance.NetworkUnknown),
8- instance.NewAddress("dummyenv-0.dns", instance.NetworkUnknown),
9+ instance.NewAddress("dummyenv-0.dns", instance.NetworkPublic),
10 }},
11 expect{
12 "simulate the PA starting an instance in response to the state change",
13@@ -306,7 +306,7 @@
14 setMachineStatus{"0", params.StatusStarted, ""},
15 setAddresses{"0", []instance.Address{
16 instance.NewAddress("10.0.0.1", instance.NetworkUnknown),
17- instance.NewAddress("dummyenv-0.dns", instance.NetworkUnknown),
18+ instance.NewAddress("dummyenv-0.dns", instance.NetworkPublic),
19 }},
20 addCharm{"dummy"},
21 addService{
22@@ -352,7 +352,7 @@
23 addMachine{machineId: "0", cons: machineCons, job: state.JobManageEnviron},
24 setAddresses{"0", []instance.Address{
25 instance.NewAddress("10.0.0.1", instance.NetworkUnknown),
26- instance.NewAddress("dummyenv-0.dns", instance.NetworkUnknown),
27+ instance.NewAddress("dummyenv-0.dns", instance.NetworkPublic),
28 }},
29 startAliveMachine{"0"},
30 setMachineStatus{"0", params.StatusStarted, ""},
31
32=== modified file 'instance/address.go'
33--- instance/address.go 2014-04-07 00:36:36 +0000
34+++ instance/address.go 2014-04-10 13:19:09 +0000
35@@ -9,6 +9,22 @@
36 "strconv"
37 )
38
39+// Private network ranges for IPv4.
40+// See: http://tools.ietf.org/html/rfc1918
41+var (
42+ classAPrivate = mustParseCIDR("10.0.0.0/8")
43+ classBPrivate = mustParseCIDR("172.16.0.0/12")
44+ classCPrivate = mustParseCIDR("192.168.0.0/16")
45+)
46+
47+func mustParseCIDR(s string) *net.IPNet {
48+ _, net, err := net.ParseCIDR(s)
49+ if err != nil {
50+ panic(err)
51+ }
52+ return net
53+}
54+
55 // AddressType represents the possible ways of specifying a machine location by
56 // either a hostname resolvable by dns lookup, or ipv4 or ipv6 address.
57 type AddressType string
58@@ -100,72 +116,76 @@
59
60 func DeriveAddressType(value string) AddressType {
61 ip := net.ParseIP(value)
62- if ip != nil {
63- if ip.To4() != nil {
64- return Ipv4Address
65- }
66- if ip.To16() != nil {
67- return Ipv6Address
68- }
69+ switch {
70+ case ip == nil:
71+ // TODO(gz): Check value is a valid hostname
72+ return HostName
73+ case ip.To4() != nil:
74+ return Ipv4Address
75+ case ip.To16() != nil:
76+ return Ipv6Address
77+ default:
78 panic("Unknown form of IP address")
79 }
80- // TODO(gz): Check value is a valid hostname
81- return HostName
82-}
83-
84+}
85+
86+func isIPv4PrivateNetworkAddress(ip net.IP) bool {
87+ return classAPrivate.Contains(ip) ||
88+ classBPrivate.Contains(ip) ||
89+ classCPrivate.Contains(ip)
90+}
91+
92+// deriveNetworkScope attempts to derive the network scope from an address's
93+// type and value, returning the original network scope if no deduction can
94+// be made.
95+func deriveNetworkScope(addr Address) NetworkScope {
96+ if addr.Type == HostName {
97+ return addr.NetworkScope
98+ }
99+ ip := net.ParseIP(addr.Value)
100+ if ip == nil {
101+ return addr.NetworkScope
102+ }
103+ if ip.IsLoopback() {
104+ return NetworkMachineLocal
105+ }
106+ switch addr.Type {
107+ case Ipv4Address:
108+ if isIPv4PrivateNetworkAddress(ip) {
109+ return NetworkCloudLocal
110+ }
111+ // If it's not loopback, and it's not a private
112+ // network address, then it's publicly routable.
113+ return NetworkPublic
114+ case Ipv6Address:
115+ // TODO(axw) check for IPv6 unique local address, if/when we care.
116+ }
117+ return addr.NetworkScope
118+}
119+
120+// NewAddress creates a new Address, deriving its type from the value.
121+//
122+// If the specified scope is NetworkUnknown, then NewAddress will
123+// attempt derive the scope based on reserved IP address ranges.
124 func NewAddress(value string, scope NetworkScope) Address {
125- return Address{
126+ addr := Address{
127 Value: value,
128 Type: DeriveAddressType(value),
129 NetworkScope: scope,
130 }
131-}
132-
133-// netLookupIP is a var for testing.
134-var netLookupIP = net.LookupIP
135-
136-// HostAddresses looks up the IP addresses of the specified
137-// host, and translates them into instance.Address values.
138-//
139-// The argument passed in is always added ast the final
140-// address in the resulting slice.
141-func HostAddresses(host string) (addrs []Address, err error) {
142- hostAddr := NewAddress(host, NetworkUnknown)
143- if hostAddr.Type != HostName {
144- // IPs shouldn't be fed into LookupIP.
145- return []Address{hostAddr}, nil
146- }
147- ipaddrs, err := netLookupIP(host)
148- if err != nil {
149- return nil, err
150- }
151- addrs = make([]Address, len(ipaddrs)+1)
152- for i, ipaddr := range ipaddrs {
153- switch len(ipaddr) {
154- case net.IPv4len:
155- addrs[i].Type = Ipv4Address
156- addrs[i].Value = ipaddr.String()
157- case net.IPv6len:
158- if ipaddr.To4() != nil {
159- // ipaddr is an IPv4 address represented in 16 bytes.
160- addrs[i].Type = Ipv4Address
161- } else {
162- addrs[i].Type = Ipv6Address
163- }
164- addrs[i].Value = ipaddr.String()
165- }
166- }
167- addrs[len(addrs)-1] = hostAddr
168- return addrs, err
169+ if scope == NetworkUnknown {
170+ addr.NetworkScope = deriveNetworkScope(addr)
171+ }
172+ return addr
173 }
174
175 // SelectPublicAddress picks one address from a slice that would
176 // be appropriate to display as a publicly accessible endpoint.
177 // If there are no suitable addresses, the empty string is returned.
178 func SelectPublicAddress(addresses []Address) string {
179- index := publicAddressIndex(len(addresses), func(i int) Address {
180+ index := bestAddressIndex(len(addresses), func(i int) Address {
181 return addresses[i]
182- })
183+ }, publicMatch)
184 if index < 0 {
185 return ""
186 }
187@@ -173,40 +193,22 @@
188 }
189
190 func SelectPublicHostPort(hps []HostPort) string {
191- index := publicAddressIndex(len(hps), func(i int) Address {
192+ index := bestAddressIndex(len(hps), func(i int) Address {
193 return hps[i].Address
194- })
195+ }, publicMatch)
196 if index < 0 {
197 return ""
198 }
199 return hps[index].NetAddr()
200 }
201
202-// publicAddressIndex is the internal version of SelectPublicAddress.
203-// It returns the index the selected address, or -1 if not found.
204-func publicAddressIndex(numAddr int, getAddr func(i int) Address) int {
205- mostPublicIndex := -1
206- for i := 0; i < numAddr; i++ {
207- addr := getAddr(i)
208- if addr.Type != Ipv6Address {
209- switch addr.NetworkScope {
210- case NetworkPublic:
211- return i
212- case NetworkCloudLocal, NetworkUnknown:
213- mostPublicIndex = i
214- }
215- }
216- }
217- return mostPublicIndex
218-}
219-
220 // SelectInternalAddress picks one address from a slice that can be
221 // used as an endpoint for juju internal communication.
222 // If there are no suitable addresses, the empty string is returned.
223 func SelectInternalAddress(addresses []Address, machineLocal bool) string {
224- index := internalAddressIndex(len(addresses), func(i int) Address {
225+ index := bestAddressIndex(len(addresses), func(i int) Address {
226 return addresses[i]
227- }, machineLocal)
228+ }, internalAddressMatcher(machineLocal))
229 if index < 0 {
230 return ""
231 }
232@@ -218,35 +220,76 @@
233 // and returns it in its NetAddr form.
234 // If there are no suitable addresses, the empty string is returned.
235 func SelectInternalHostPort(hps []HostPort, machineLocal bool) string {
236- index := internalAddressIndex(len(hps), func(i int) Address {
237+ index := bestAddressIndex(len(hps), func(i int) Address {
238 return hps[i].Address
239- }, machineLocal)
240+ }, internalAddressMatcher(machineLocal))
241 if index < 0 {
242 return ""
243 }
244 return hps[index].NetAddr()
245 }
246
247-// internalAddressIndex is the internal version of SelectInternalAddress.
248-// It returns the index the selected address, or -1 if not found.
249-func internalAddressIndex(numAddr int, getAddr func(i int) Address, machineLocal bool) int {
250- usableAddressIndex := -1
251+func publicMatch(addr Address) scopeMatch {
252+ switch addr.NetworkScope {
253+ case NetworkPublic:
254+ return exactScope
255+ case NetworkCloudLocal, NetworkUnknown:
256+ return fallbackScope
257+ }
258+ return invalidScope
259+}
260+
261+func internalAddressMatcher(machineLocal bool) func(Address) scopeMatch {
262+ if machineLocal {
263+ return cloudOrMachineLocalMatch
264+ }
265+ return cloudLocalMatch
266+}
267+
268+func cloudLocalMatch(addr Address) scopeMatch {
269+ switch addr.NetworkScope {
270+ case NetworkCloudLocal:
271+ return exactScope
272+ case NetworkPublic, NetworkUnknown:
273+ return fallbackScope
274+ }
275+ return invalidScope
276+}
277+
278+func cloudOrMachineLocalMatch(addr Address) scopeMatch {
279+ if addr.NetworkScope == NetworkMachineLocal {
280+ return exactScope
281+ }
282+ return cloudLocalMatch(addr)
283+}
284+
285+type scopeMatch int
286+
287+const (
288+ invalidScope scopeMatch = iota
289+ exactScope
290+ fallbackScope
291+)
292+
293+// bestAddressIndex returns the index of the first address
294+// with an exactly matching scope, or the first address with
295+// a matching fallback scope if there are no exact matches.
296+// If there are no suitable addresses, -1 is returned.
297+func bestAddressIndex(numAddr int, getAddr func(i int) Address, match func(addr Address) scopeMatch) int {
298+ fallbackAddressIndex := -1
299 for i := 0; i < numAddr; i++ {
300 addr := getAddr(i)
301 if addr.Type != Ipv6Address {
302- switch addr.NetworkScope {
303- case NetworkCloudLocal:
304+ switch match(addr) {
305+ case exactScope:
306 return i
307- case NetworkMachineLocal:
308- if machineLocal {
309- return i
310- }
311- case NetworkPublic, NetworkUnknown:
312- if usableAddressIndex == -1 {
313- usableAddressIndex = i
314+ case fallbackScope:
315+ // Use the first fallback address if there are no exact matches.
316+ if fallbackAddressIndex == -1 {
317+ fallbackAddressIndex = i
318 }
319 }
320 }
321 }
322- return usableAddressIndex
323+ return fallbackAddressIndex
324 }
325
326=== modified file 'instance/address_test.go'
327--- instance/address_test.go 2014-04-01 00:12:08 +0000
328+++ instance/address_test.go 2014-04-10 13:19:09 +0000
329@@ -4,9 +4,6 @@
330 package instance
331
332 import (
333- "errors"
334- "net"
335-
336 jc "github.com/juju/testing/checkers"
337 gc "launchpad.net/gocheck"
338
339@@ -20,16 +17,56 @@
340 var _ = gc.Suite(&AddressSuite{})
341
342 func (s *AddressSuite) TestNewAddressIpv4(c *gc.C) {
343- addr := NewAddress("127.0.0.1", NetworkUnknown)
344- c.Check(addr.Value, gc.Equals, "127.0.0.1")
345- c.Check(addr.Type, gc.Equals, Ipv4Address)
346- c.Check(addr.NetworkScope, gc.Equals, NetworkUnknown)
347+ type test struct {
348+ value string
349+ scope NetworkScope
350+ expectedScope NetworkScope
351+ }
352+
353+ tests := []test{{
354+ value: "127.0.0.1",
355+ scope: NetworkUnknown,
356+ expectedScope: NetworkMachineLocal,
357+ }, {
358+ value: "127.0.0.1",
359+ scope: NetworkPublic,
360+ expectedScope: NetworkPublic, // don't second guess != Unknown
361+ }, {
362+ value: "10.0.3.1",
363+ scope: NetworkUnknown,
364+ expectedScope: NetworkCloudLocal,
365+ }, {
366+ value: "172.16.15.14",
367+ scope: NetworkUnknown,
368+ expectedScope: NetworkCloudLocal,
369+ }, {
370+ value: "192.168.0.1",
371+ scope: NetworkUnknown,
372+ expectedScope: NetworkCloudLocal,
373+ }, {
374+ value: "8.8.8.8",
375+ scope: NetworkUnknown,
376+ expectedScope: NetworkPublic,
377+ }}
378+
379+ for _, t := range tests {
380+ c.Logf("test %s %s", t.value, t.scope)
381+ addr := NewAddress(t.value, t.scope)
382+ c.Check(addr.Value, gc.Equals, t.value)
383+ c.Check(addr.Type, gc.Equals, Ipv4Address)
384+ c.Check(addr.NetworkScope, gc.Equals, t.expectedScope)
385+ }
386 }
387
388 func (s *AddressSuite) TestNewAddressIpv6(c *gc.C) {
389 addr := NewAddress("::1", NetworkUnknown)
390 c.Check(addr.Value, gc.Equals, "::1")
391 c.Check(addr.Type, gc.Equals, Ipv6Address)
392+ c.Check(addr.NetworkScope, gc.Equals, NetworkMachineLocal)
393+
394+ addr = NewAddress("2001:DB8::1", NetworkUnknown)
395+ c.Check(addr.Value, gc.Equals, "2001:DB8::1")
396+ c.Check(addr.Type, gc.Equals, Ipv6Address)
397 c.Check(addr.NetworkScope, gc.Equals, NetworkUnknown)
398 }
399
400@@ -37,11 +74,11 @@
401 addresses := NewAddresses("127.0.0.1", "192.168.1.1", "192.168.178.255")
402 c.Assert(len(addresses), gc.Equals, 3)
403 c.Assert(addresses[0].Value, gc.Equals, "127.0.0.1")
404- c.Assert(addresses[0].NetworkScope, gc.Equals, NetworkUnknown)
405+ c.Assert(addresses[0].NetworkScope, gc.Equals, NetworkMachineLocal)
406 c.Assert(addresses[1].Value, gc.Equals, "192.168.1.1")
407- c.Assert(addresses[1].NetworkScope, gc.Equals, NetworkUnknown)
408+ c.Assert(addresses[1].NetworkScope, gc.Equals, NetworkCloudLocal)
409 c.Assert(addresses[2].Value, gc.Equals, "192.168.178.255")
410- c.Assert(addresses[2].NetworkScope, gc.Equals, NetworkUnknown)
411+ c.Assert(addresses[2].NetworkScope, gc.Equals, NetworkCloudLocal)
412 }
413
414 func (s *AddressSuite) TestNewAddressHostname(c *gc.C) {
415@@ -125,12 +162,12 @@
416 },
417 2,
418 }, {
419- "last unknown address selected",
420+ "first unknown address selected",
421 []Address{
422 {"10.0.0.1", Ipv4Address, "cloud", NetworkUnknown},
423 {"8.8.8.8", Ipv4Address, "floating", NetworkUnknown},
424 },
425- 1,
426+ 0,
427 }}
428
429 func (s *AddressSuite) TestSelectPublicAddress(c *gc.C) {
430@@ -229,43 +266,6 @@
431 }
432 }
433
434-func (s *AddressSuite) TestHostAddresses(c *gc.C) {
435- // Mock the call to net.LookupIP made from HostAddresses.
436- var lookupIPs []net.IP
437- var lookupErr error
438- lookupIP := func(addr string) ([]net.IP, error) {
439- return append([]net.IP{}, lookupIPs...), lookupErr
440- }
441- s.PatchValue(&netLookupIP, lookupIP)
442-
443- // err is only non-nil if net.LookupIP fails.
444- addrs, err := HostAddresses("")
445- c.Assert(err, gc.IsNil)
446- // addrs always contains the input address.
447- c.Assert(addrs, gc.HasLen, 1)
448- c.Assert(addrs[0], gc.Equals, NewAddress("", NetworkUnknown))
449-
450- loopback := net.ParseIP("127.0.0.1").To4()
451- lookupIPs = []net.IP{net.IPv6loopback, net.IPv4zero, loopback}
452- addrs, err = HostAddresses("localhost")
453- c.Assert(err, gc.IsNil)
454- c.Assert(addrs, gc.HasLen, 4)
455- c.Assert(addrs[0], gc.Equals, NewAddress(net.IPv6loopback.String(), NetworkUnknown))
456- c.Assert(addrs[1], gc.Equals, NewAddress(net.IPv4zero.String(), NetworkUnknown))
457- c.Assert(addrs[2], gc.Equals, NewAddress(loopback.String(), NetworkUnknown))
458- c.Assert(addrs[3], gc.Equals, NewAddress("localhost", NetworkUnknown))
459-
460- lookupErr = errors.New("what happened?")
461- addrs, err = HostAddresses("localhost")
462- c.Assert(err, gc.Equals, lookupErr)
463-
464- // If the input address is an IP, the call to net.LookupIP is elided.
465- addrs, err = HostAddresses("127.0.0.1")
466- c.Assert(err, gc.IsNil)
467- c.Assert(addrs, gc.HasLen, 1)
468- c.Assert(addrs[0], gc.Equals, NewAddress("127.0.0.1", NetworkUnknown))
469-}
470-
471 var stringTests = []struct {
472 addr Address
473 str string
474
475=== modified file 'provider/common/state.go'
476--- provider/common/state.go 2014-04-09 15:23:12 +0000
477+++ provider/common/state.go 2014-04-10 13:19:09 +0000
478@@ -23,7 +23,6 @@
479 for _, inst := range instances {
480 if inst != nil {
481 name, err := inst.DNSName()
482- logger.Debugf("Couldn't get DNSName from instance %v: %v", inst.Id(), err)
483 // If that fails, just keep looking.
484 if err == nil && name != "" {
485 names = append(names, name)
486
487=== modified file 'provider/maas/instance.go'
488--- provider/maas/instance.go 2013-12-18 10:18:18 +0000
489+++ provider/maas/instance.go 2014-04-10 13:19:09 +0000
490@@ -82,7 +82,7 @@
491 }
492
493 for _, ip := range ips {
494- a := instance.Address{ip, instance.DeriveAddressType(ip), "", instance.NetworkUnknown}
495+ a := instance.NewAddress(ip, instance.NetworkUnknown)
496 addrs = append(addrs, a)
497 }
498
499
500=== modified file 'provider/openstack/provider.go'
501--- provider/openstack/provider.go 2014-04-09 16:36:12 +0000
502+++ provider/openstack/provider.go 2014-04-10 13:19:09 +0000
503@@ -412,12 +412,10 @@
504 if address.Version == 6 {
505 addrtype = instance.Ipv6Address
506 }
507- // TODO(gz): Use NewAddress... with sanity checking
508- machineAddr := instance.Address{
509- Value: address.Address,
510- Type: addrtype,
511- NetworkName: network,
512- NetworkScope: networkscope,
513+ machineAddr := instance.NewAddress(address.Address, networkscope)
514+ machineAddr.NetworkName = network
515+ if machineAddr.Type != addrtype {
516+ logger.Warningf("derived address type %v, nova reports %v", machineAddr.Type, addrtype)
517 }
518 machineAddresses = append(machineAddresses, machineAddr)
519 }
520
521=== modified file 'provider/openstack/provider_test.go'
522--- provider/openstack/provider_test.go 2013-08-21 18:38:22 +0000
523+++ provider/openstack/provider_test.go 2014-04-10 13:19:09 +0000
524@@ -54,13 +54,13 @@
525 },
526 {
527 summary: "private only",
528- private: []nova.IPAddress{{4, "127.0.0.4"}},
529+ private: []nova.IPAddress{{4, "192.168.0.1"}},
530 networks: []string{"private"},
531- expected: "127.0.0.4",
532+ expected: "192.168.0.1",
533 },
534 {
535 summary: "private plus (HP cloud)",
536- private: []nova.IPAddress{{4, "127.0.0.4"}, {4, "8.8.4.4"}},
537+ private: []nova.IPAddress{{4, "10.0.0.1"}, {4, "8.8.4.4"}},
538 networks: []string{"private"},
539 expected: "8.8.4.4",
540 },
541@@ -72,27 +72,27 @@
542 },
543 {
544 summary: "public and private",
545- private: []nova.IPAddress{{4, "127.0.0.4"}},
546+ private: []nova.IPAddress{{4, "10.0.0.4"}},
547 public: []nova.IPAddress{{4, "8.8.4.4"}},
548 networks: []string{"private", "public"},
549 expected: "8.8.4.4",
550 },
551 {
552 summary: "public private plus",
553- private: []nova.IPAddress{{4, "127.0.0.4"}, {4, "8.8.4.4"}},
554+ private: []nova.IPAddress{{4, "127.0.0.4"}, {4, "192.168.0.1"}},
555 public: []nova.IPAddress{{4, "8.8.8.8"}},
556 networks: []string{"private", "public"},
557 expected: "8.8.8.8",
558 },
559 {
560 summary: "custom only",
561- private: []nova.IPAddress{{4, "127.0.0.2"}},
562+ private: []nova.IPAddress{{4, "192.168.0.1"}},
563 networks: []string{"special"},
564- expected: "127.0.0.2",
565+ expected: "192.168.0.1",
566 },
567 {
568 summary: "custom and public",
569- private: []nova.IPAddress{{4, "127.0.0.2"}},
570+ private: []nova.IPAddress{{4, "172.16.0.1"}},
571 public: []nova.IPAddress{{4, "8.8.8.8"}},
572 networks: []string{"special", "public"},
573 expected: "8.8.8.8",
574
575=== modified file 'state/api/machiner/machiner_test.go'
576--- state/api/machiner/machiner_test.go 2014-04-01 03:37:13 +0000
577+++ state/api/machiner/machiner_test.go 2014-04-10 13:19:09 +0000
578@@ -41,7 +41,7 @@
579 s.JujuConnSuite.SetUpTest(c)
580 m, err := s.State.AddMachine("quantal", state.JobManageEnviron)
581 c.Assert(err, gc.IsNil)
582- err = m.SetAddresses(instance.NewAddress("127.0.0.1", instance.NetworkUnknown))
583+ err = m.SetAddresses(instance.NewAddress("10.0.0.1", instance.NetworkUnknown))
584 c.Assert(err, gc.IsNil)
585
586 s.st, s.machine = s.OpenAPIAsNewMachine(c)
587@@ -134,6 +134,7 @@
588
589 addresses := []instance.Address{
590 instance.NewAddress("127.0.0.1", instance.NetworkUnknown),
591+ instance.NewAddress("10.0.0.1", instance.NetworkUnknown),
592 instance.NewAddress("8.8.8.8", instance.NetworkUnknown),
593 }
594 err = machine.SetMachineAddresses(addresses)
595
596=== modified file 'state/api/state.go'
597--- state/api/state.go 2014-03-31 14:43:24 +0000
598+++ state/api/state.go 2014-04-10 13:19:09 +0000
599@@ -66,12 +66,8 @@
600 return nil, err
601 }
602 hostPort := instance.HostPort{
603- Address: instance.Address{
604- Value: host,
605- Type: instance.DeriveAddressType(host),
606- NetworkScope: instance.NetworkUnknown,
607- },
608- Port: port,
609+ Address: instance.NewAddress(host, instance.NetworkUnknown),
610+ Port: port,
611 }
612 return append(servers, []instance.HostPort{hostPort}), nil
613 }
614
615=== modified file 'state/apiserver/machine/machiner.go'
616--- state/apiserver/machine/machiner.go 2014-03-21 16:44:10 +0000
617+++ state/apiserver/machine/machiner.go 2014-04-10 13:19:09 +0000
618@@ -71,7 +71,7 @@
619 var m *state.Machine
620 m, err = api.getMachine(arg.Tag)
621 if err == nil {
622- err = m.SetMachineAddresses(arg.Addresses)
623+ err = m.SetMachineAddresses(arg.Addresses...)
624 } else if errors.IsNotFoundError(err) {
625 err = common.ErrPerm
626 }
627
628=== modified file 'state/machine.go'
629--- state/machine.go 2014-04-09 09:11:31 +0000
630+++ state/machine.go 2014-04-10 13:19:09 +0000
631@@ -21,6 +21,7 @@
632 "launchpad.net/juju-core/state/presence"
633 "launchpad.net/juju-core/tools"
634 "launchpad.net/juju-core/utils"
635+ "launchpad.net/juju-core/utils/set"
636 "launchpad.net/juju-core/version"
637 )
638
639@@ -880,25 +881,26 @@
640 }
641
642 func mergedAddresses(machineAddresses, providerAddresses []address) []instance.Address {
643- merged := make(map[string]instance.Address)
644+ merged := make([]instance.Address, len(providerAddresses), len(providerAddresses)+len(machineAddresses))
645+ var providerValues set.Strings
646+ for i, address := range providerAddresses {
647+ providerValues.Add(address.Value)
648+ merged[i] = address.InstanceAddress()
649+ }
650 for _, address := range machineAddresses {
651- merged[address.Value] = address.InstanceAddress()
652- }
653- for _, address := range providerAddresses {
654- merged[address.Value] = address.InstanceAddress()
655- }
656- addresses := make([]instance.Address, 0, len(merged))
657- for _, address := range merged {
658- addresses = append(addresses, address)
659- }
660- return addresses
661+ if !providerValues.Contains(address.Value) {
662+ merged = append(merged, address.InstanceAddress())
663+ }
664+ }
665+ return merged
666 }
667
668 // Addresses returns any hostnames and ips associated with a machine,
669 // determined both by the machine itself, and by asking the provider.
670 //
671 // The addresses returned by the provider shadow any of the addresses
672-// that the machine reported with the same address value.
673+// that the machine reported with the same address value. Provider-reported
674+// addresses always come before machine-reported addresses.
675 func (m *Machine) Addresses() (addresses []instance.Address) {
676 return mergedAddresses(m.doc.MachineAddresses, m.doc.Addresses)
677 }
678@@ -934,7 +936,7 @@
679
680 // SetMachineAddresses records any addresses related to the machine, sourced
681 // by asking the machine.
682-func (m *Machine) SetMachineAddresses(addresses []instance.Address) (err error) {
683+func (m *Machine) SetMachineAddresses(addresses ...instance.Address) (err error) {
684 stateAddresses := instanceAddressesToAddresses(addresses)
685 ops := []txn.Op{
686 {
687
688=== modified file 'state/machine_test.go'
689--- state/machine_test.go 2014-04-09 09:11:31 +0000
690+++ state/machine_test.go 2014-04-10 13:19:09 +0000
691@@ -1412,13 +1412,42 @@
692 instance.NewAddress("127.0.0.1", instance.NetworkUnknown),
693 instance.NewAddress("8.8.8.8", instance.NetworkUnknown),
694 }
695- err = machine.SetMachineAddresses(addresses)
696+ err = machine.SetMachineAddresses(addresses...)
697 c.Assert(err, gc.IsNil)
698 err = machine.Refresh()
699 c.Assert(err, gc.IsNil)
700 c.Assert(machine.MachineAddresses(), gc.DeepEquals, addresses)
701 }
702
703+func (s *MachineSuite) TestMergedAddresses(c *gc.C) {
704+ machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
705+ c.Assert(err, gc.IsNil)
706+ c.Assert(machine.Addresses(), gc.HasLen, 0)
707+
708+ addresses := []instance.Address{
709+ instance.NewAddress("127.0.0.1", instance.NetworkUnknown),
710+ instance.NewAddress("8.8.8.8", instance.NetworkUnknown),
711+ }
712+ addresses[0].NetworkName = "loopback"
713+ err = machine.SetAddresses(addresses...)
714+ c.Assert(err, gc.IsNil)
715+
716+ machineAddresses := []instance.Address{
717+ instance.NewAddress("127.0.0.1", instance.NetworkUnknown),
718+ instance.NewAddress("192.168.0.1", instance.NetworkUnknown),
719+ }
720+ err = machine.SetMachineAddresses(machineAddresses...)
721+ c.Assert(err, gc.IsNil)
722+ err = machine.Refresh()
723+ c.Assert(err, gc.IsNil)
724+
725+ c.Assert(machine.Addresses(), gc.DeepEquals, []instance.Address{
726+ addresses[0],
727+ addresses[1],
728+ machineAddresses[1],
729+ })
730+}
731+
732 func (s *MachineSuite) addMachineWithSupportedContainer(c *gc.C, container instance.ContainerType) *state.Machine {
733 machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
734 c.Assert(err, gc.IsNil)
735
736=== modified file 'state/unit_test.go'
737--- state/unit_test.go 2014-04-07 00:36:36 +0000
738+++ state/unit_test.go 2014-04-10 13:19:09 +0000
739@@ -221,6 +221,31 @@
740 c.Assert(ok, gc.Equals, true)
741 }
742
743+func (s *UnitSuite) TestPublicAddressMachineAddresses(c *gc.C) {
744+ machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
745+ c.Assert(err, gc.IsNil)
746+ err = s.unit.AssignToMachine(machine)
747+ c.Assert(err, gc.IsNil)
748+
749+ publicProvider := instance.NewAddress("8.8.8.8", instance.NetworkPublic)
750+ privateProvider := instance.NewAddress("127.0.0.1", instance.NetworkCloudLocal)
751+ privateMachine := instance.NewAddress("127.0.0.2", instance.NetworkUnknown)
752+
753+ err = machine.SetAddresses(privateProvider)
754+ c.Assert(err, gc.IsNil)
755+ err = machine.SetMachineAddresses(privateMachine)
756+ c.Assert(err, gc.IsNil)
757+ address, ok := s.unit.PublicAddress()
758+ c.Check(address, gc.Equals, "127.0.0.1")
759+ c.Assert(ok, gc.Equals, true)
760+
761+ err = machine.SetAddresses(publicProvider, privateProvider)
762+ c.Assert(err, gc.IsNil)
763+ address, ok = s.unit.PublicAddress()
764+ c.Check(address, gc.Equals, "8.8.8.8")
765+ c.Assert(ok, gc.Equals, true)
766+}
767+
768 func (s *UnitSuite) TestPrivateAddressSubordinate(c *gc.C) {
769 subUnit := s.addSubordinateUnit(c)
770 _, ok := subUnit.PrivateAddress()
771
772=== modified file 'worker/machiner/machiner.go'
773--- worker/machiner/machiner.go 2014-04-01 03:37:13 +0000
774+++ worker/machiner/machiner.go 2014-04-10 13:19:09 +0000
775@@ -77,14 +77,7 @@
776 default:
777 continue
778 }
779- if ip.IsLoopback() {
780- continue
781- }
782- address := instance.Address{
783- Value: ip.String(),
784- Type: instance.DeriveAddressType(ip.String()),
785- NetworkScope: instance.NetworkUnknown,
786- }
787+ address := instance.NewAddress(ip.String(), instance.NetworkUnknown)
788 hostAddresses = append(hostAddresses, address)
789 }
790 if len(hostAddresses) == 0 {
791
792=== modified file 'worker/machiner/machiner_test.go'
793--- worker/machiner/machiner_test.go 2014-03-28 06:56:55 +0000
794+++ worker/machiner/machiner_test.go 2014-04-10 13:19:09 +0000
795@@ -140,9 +140,9 @@
796 s.PatchValue(machiner.InterfaceAddrs, func() ([]net.Addr, error) {
797 addrs := []net.Addr{
798 &net.IPAddr{IP: net.IPv4(10, 0, 0, 1)},
799- &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)}, // loopback, ignored
800- &net.IPAddr{IP: net.IPv6loopback}, // loopback, ignored
801- &net.UnixAddr{}, // not IP, ignored
802+ &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)},
803+ &net.IPAddr{IP: net.IPv6loopback},
804+ &net.UnixAddr{}, // not IP, ignored
805 &net.IPNet{IP: net.ParseIP("2001:db8::1")},
806 }
807 return addrs, nil
808@@ -154,7 +154,9 @@
809 c.Assert(mr.Wait(), gc.Equals, worker.ErrTerminateAgent)
810 c.Assert(s.machine.Refresh(), gc.IsNil)
811 c.Assert(s.machine.MachineAddresses(), gc.DeepEquals, []instance.Address{
812- instance.NewAddress("10.0.0.1", instance.NetworkUnknown),
813+ instance.NewAddress("10.0.0.1", instance.NetworkCloudLocal),
814+ instance.NewAddress("127.0.0.1", instance.NetworkMachineLocal),
815+ instance.NewAddress("::1", instance.NetworkMachineLocal),
816 instance.NewAddress("2001:db8::1", instance.NetworkUnknown),
817 })
818 }
819
820=== modified file 'worker/peergrouper/worker_test.go'
821--- worker/peergrouper/worker_test.go 2014-04-09 22:29:37 +0000
822+++ worker/peergrouper/worker_test.go 2014-04-10 13:19:09 +0000
823@@ -101,12 +101,8 @@
824 servers := make([][]instance.HostPort, n)
825 for i := range servers {
826 servers[i] = []instance.HostPort{{
827- Address: instance.Address{
828- Value: fmt.Sprintf("0.1.2.%d", i+10),
829- NetworkScope: instance.NetworkUnknown,
830- Type: instance.Ipv4Address,
831- },
832- Port: apiPort,
833+ Address: instance.NewAddress(fmt.Sprintf("0.1.2.%d", i+10), instance.NetworkUnknown),
834+ Port: apiPort,
835 }}
836 }
837 return servers

Subscribers

People subscribed via source and target branches

to status/vote changes: