Merge lp:~axwalk/juju-core/lp1225825-netlookupip-ip into lp:~go-bot/juju-core/trunk
- lp1225825-netlookupip-ip
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | John A Meinel |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1831 |
Proposed branch: | lp:~axwalk/juju-core/lp1225825-netlookupip-ip |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
320 lines (+116/-49) 3 files modified
environs/manual/provisioner.go (+13/-0) instance/address.go (+20/-6) instance/address_test.go (+83/-43) |
To merge this branch: | bzr merge lp:~axwalk/juju-core/lp1225825-netlookupip-ip |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email:
|
Commit message
environs/manual: reverse lookup target IP
Thus we may resolve additional addresses for the
machine. Also, we should get a consistent machine ID,
whether a name or an IP is specified.
Also, modify instance/
an IP address into net.LookupIP. This causes an error
if built with CGO_ENABLED=0.
Fixes #1225825
Description of the change
environs/manual: reverse lookup target IP
Thus we may resolve additional addresses for the
machine. Also, we should get a consistent machine ID,
whether a name or an IP is specified.
Also, modify instance/
an IP address into net.LookupIP. This causes an error
if built with CGO_ENABLED=0.
Fixes #1225825
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Tim Penhey (thumper) wrote : | # |
LGTM with a test added, also consider the logging severity.
https:/
File environs/
https:/
environs/
%v: %v", ip, err)
Is it really a warning if we can't do a reverse lookup? Is it something
that we should be fixing later?
Perhaps just log with info or debug, certainly not really a big deal is
it?
https:/
File instance/address.go (right):
https:/
instance/
[]Address, err error) {
How about a test for this?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
https:/
File environs/
https:/
environs/
%v: %v", ip, err)
On 2013/09/18 03:52:12, thumper wrote:
> Is it really a warning if we can't do a reverse lookup? Is it
something that we
> should be fixing later?
Hmm, yeah, it's not necessarily a problem.
> Perhaps just log with info or debug, certainly not really a big deal
is it?
I'll log it as INFO.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Go Bot (go-bot) wrote : | # |
The attempt to merge lp:~axwalk/juju-core/lp1225825-netlookupip-ip into lp:juju-core failed. Below is the output from the failed tests.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
? launchpad.
-------
PANIC: addmachine.go:0: AddUnitSuite.
... Panic: Couldn't create temporary directory: mkdir /tmp/gocheck-
/usr/lib/
in panic
/home/tarmac/
in tempDir.newPath
/home/tarmac/
in C.MkDir
/home/tarmac/
in JujuConnSuite.
/home/tarmac/
in RepoSuite.SetUpTest
-------
PANIC: addmachine.go:0: AddUnitSuite.
... Panic: runtime error: invalid memory address or nil pointer dereference (PC=0x414321)
/usr/lib/
in panic
/usr/lib/
in panicstring
/usr/lib/
in sigpanic
/home/tarmac/
in State.SetAdminM
/home/tarmac/
in JujuConnSuite.
/home/tarmac/
in JujuConnSuite.
/home/tarmac/
in RepoSuite.
-------
PANIC: addunit_test.go:60: AddUnitSuite.
... Panic: Fixture has panicked (see related PANIC)
listing available tools
found 4 tools
found 4 recent tools (version 1.2.0)
listing target bucket
found 3 tools in target; 4 tools to be copied
copying 1.2.0-precise-amd64 from http://
copying tools/releases/
downloaded tools/releases/
download 0kB, uploading
copying 1.2.0-quantal-amd64 from http://
copying tools/releases/
downloaded tools/releases/
download 0kB, uploading
copying 1.2.0-quantal-i386 from http://
copying tools/releases/
downloaded tools/releases/
download 0kB, uploading
copying 1.2.0-raring-amd64 from http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
John A Meinel (jameinel) wrote : | # |
The bot failure is because go check doesn't seed its random-directory generator, and we hit 100 leaked directories.
I think we have a bug somewhere that is causing us to not clean up a dir during testing, but unfortunately I thought of that right after I nuked all the dirs, so it will be hard to figure out where until this happens again.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
Thanks jam. When I have my MPs in order, I'll run some tests locally to see if I can find the culprits.
Preview Diff
1 | === modified file 'environs/manual/provisioner.go' | |||
2 | --- environs/manual/provisioner.go 2013-09-03 01:56:25 +0000 | |||
3 | +++ environs/manual/provisioner.go 2013-09-18 04:44:17 +0000 | |||
4 | @@ -6,6 +6,7 @@ | |||
5 | 6 | import ( | 6 | import ( |
6 | 7 | "errors" | 7 | "errors" |
7 | 8 | "fmt" | 8 | "fmt" |
8 | 9 | "net" | ||
9 | 9 | "strings" | 10 | "strings" |
10 | 10 | 11 | ||
11 | 11 | "launchpad.net/loggo" | 12 | "launchpad.net/loggo" |
12 | @@ -71,10 +72,22 @@ | |||
13 | 71 | if at := strings.Index(sshHostWithoutUser, "@"); at != -1 { | 72 | if at := strings.Index(sshHostWithoutUser, "@"); at != -1 { |
14 | 72 | sshHostWithoutUser = sshHostWithoutUser[at+1:] | 73 | sshHostWithoutUser = sshHostWithoutUser[at+1:] |
15 | 73 | } | 74 | } |
16 | 75 | if ip := net.ParseIP(sshHostWithoutUser); ip != nil { | ||
17 | 76 | // Do a reverse-lookup on the IP. The IP may not have | ||
18 | 77 | // a DNS entry, so just log a warning if this fails. | ||
19 | 78 | names, err := net.LookupAddr(ip.String()) | ||
20 | 79 | if err != nil { | ||
21 | 80 | logger.Infof("failed to resolve %v: %v", ip, err) | ||
22 | 81 | } else { | ||
23 | 82 | logger.Infof("resolved %v to %v", ip, names) | ||
24 | 83 | sshHostWithoutUser = names[0] | ||
25 | 84 | } | ||
26 | 85 | } | ||
27 | 74 | addrs, err := instance.HostAddresses(sshHostWithoutUser) | 86 | addrs, err := instance.HostAddresses(sshHostWithoutUser) |
28 | 75 | if err != nil { | 87 | if err != nil { |
29 | 76 | return nil, err | 88 | return nil, err |
30 | 77 | } | 89 | } |
31 | 90 | logger.Infof("addresses for %v: %v", sshHostWithoutUser, addrs) | ||
32 | 78 | 91 | ||
33 | 79 | provisioned, err := checkProvisioned(args.Host) | 92 | provisioned, err := checkProvisioned(args.Host) |
34 | 80 | if err != nil { | 93 | if err != nil { |
35 | 81 | 94 | ||
36 | === modified file 'instance/address.go' | |||
37 | --- instance/address.go 2013-09-04 15:11:51 +0000 | |||
38 | +++ instance/address.go 2013-09-18 04:44:17 +0000 | |||
39 | @@ -59,24 +59,38 @@ | |||
40 | 59 | return Address{value, addresstype, "", NetworkUnknown} | 59 | return Address{value, addresstype, "", NetworkUnknown} |
41 | 60 | } | 60 | } |
42 | 61 | 61 | ||
43 | 62 | // netLookupIP is a var for testing. | ||
44 | 63 | var netLookupIP = net.LookupIP | ||
45 | 64 | |||
46 | 62 | // HostAddresses looks up the IP addresses of the specified | 65 | // HostAddresses looks up the IP addresses of the specified |
47 | 63 | // host, and translates them into instance.Address values. | 66 | // host, and translates them into instance.Address values. |
50 | 64 | func HostAddresses(host string) ([]Address, error) { | 67 | func HostAddresses(host string) (addrs []Address, err error) { |
51 | 65 | ipaddrs, err := net.LookupIP(host) | 68 | hostAddr := NewAddress(host) |
52 | 69 | if hostAddr.Type != HostName { | ||
53 | 70 | // IPs shouldn't be fed into LookupIP. | ||
54 | 71 | return []Address{hostAddr}, nil | ||
55 | 72 | } | ||
56 | 73 | ipaddrs, err := netLookupIP(host) | ||
57 | 66 | if err != nil { | 74 | if err != nil { |
58 | 67 | return nil, err | 75 | return nil, err |
59 | 68 | } | 76 | } |
61 | 69 | addrs := make([]Address, len(ipaddrs)) | 77 | addrs = make([]Address, len(ipaddrs)+1) |
62 | 70 | for i, ipaddr := range ipaddrs { | 78 | for i, ipaddr := range ipaddrs { |
63 | 71 | switch len(ipaddr) { | 79 | switch len(ipaddr) { |
65 | 72 | case 4: | 80 | case net.IPv4len: |
66 | 73 | addrs[i].Type = Ipv4Address | 81 | addrs[i].Type = Ipv4Address |
67 | 74 | addrs[i].Value = ipaddr.String() | 82 | addrs[i].Value = ipaddr.String() |
70 | 75 | case 16: | 83 | case net.IPv6len: |
71 | 76 | addrs[i].Type = Ipv6Address | 84 | if ipaddr.To4() != nil { |
72 | 85 | // ipaddr is an IPv4 address represented in 16 bytes. | ||
73 | 86 | addrs[i].Type = Ipv4Address | ||
74 | 87 | } else { | ||
75 | 88 | addrs[i].Type = Ipv6Address | ||
76 | 89 | } | ||
77 | 77 | addrs[i].Value = ipaddr.String() | 90 | addrs[i].Value = ipaddr.String() |
78 | 78 | } | 91 | } |
79 | 79 | } | 92 | } |
80 | 93 | addrs[len(addrs)-1] = hostAddr | ||
81 | 80 | return addrs, err | 94 | return addrs, err |
82 | 81 | } | 95 | } |
83 | 82 | 96 | ||
84 | 83 | 97 | ||
85 | === modified file 'instance/address_test.go' | |||
86 | --- instance/address_test.go 2013-09-05 16:02:35 +0000 | |||
87 | +++ instance/address_test.go 2013-09-18 04:44:17 +0000 | |||
88 | @@ -1,12 +1,15 @@ | |||
89 | 1 | // Copyright 2013 Canonical Ltd. | 1 | // Copyright 2013 Canonical Ltd. |
90 | 2 | // Licensed under the AGPLv3, see LICENCE file for details. | 2 | // Licensed under the AGPLv3, see LICENCE file for details. |
91 | 3 | 3 | ||
93 | 4 | package instance_test | 4 | package instance |
94 | 5 | 5 | ||
95 | 6 | import ( | 6 | import ( |
96 | 7 | "errors" | ||
97 | 8 | "net" | ||
98 | 9 | |||
99 | 7 | gc "launchpad.net/gocheck" | 10 | gc "launchpad.net/gocheck" |
100 | 8 | 11 | ||
102 | 9 | "launchpad.net/juju-core/instance" | 12 | jc "launchpad.net/juju-core/testing/checkers" |
103 | 10 | ) | 13 | ) |
104 | 11 | 14 | ||
105 | 12 | type AddressSuite struct{} | 15 | type AddressSuite struct{} |
106 | @@ -14,64 +17,64 @@ | |||
107 | 14 | var _ = gc.Suite(&AddressSuite{}) | 17 | var _ = gc.Suite(&AddressSuite{}) |
108 | 15 | 18 | ||
109 | 16 | func (s *AddressSuite) TestNewAddressIpv4(c *gc.C) { | 19 | func (s *AddressSuite) TestNewAddressIpv4(c *gc.C) { |
111 | 17 | addr := instance.NewAddress("127.0.0.1") | 20 | addr := NewAddress("127.0.0.1") |
112 | 18 | c.Check(addr.Value, gc.Equals, "127.0.0.1") | 21 | c.Check(addr.Value, gc.Equals, "127.0.0.1") |
114 | 19 | c.Check(addr.Type, gc.Equals, instance.Ipv4Address) | 22 | c.Check(addr.Type, gc.Equals, Ipv4Address) |
115 | 20 | } | 23 | } |
116 | 21 | 24 | ||
117 | 22 | func (s *AddressSuite) TestNewAddressIpv6(c *gc.C) { | 25 | func (s *AddressSuite) TestNewAddressIpv6(c *gc.C) { |
119 | 23 | addr := instance.NewAddress("::1") | 26 | addr := NewAddress("::1") |
120 | 24 | c.Check(addr.Value, gc.Equals, "::1") | 27 | c.Check(addr.Value, gc.Equals, "::1") |
122 | 25 | c.Check(addr.Type, gc.Equals, instance.Ipv6Address) | 28 | c.Check(addr.Type, gc.Equals, Ipv6Address) |
123 | 26 | } | 29 | } |
124 | 27 | 30 | ||
125 | 28 | func (s *AddressSuite) TestNewAddressHostname(c *gc.C) { | 31 | func (s *AddressSuite) TestNewAddressHostname(c *gc.C) { |
127 | 29 | addr := instance.NewAddress("localhost") | 32 | addr := NewAddress("localhost") |
128 | 30 | c.Check(addr.Value, gc.Equals, "localhost") | 33 | c.Check(addr.Value, gc.Equals, "localhost") |
130 | 31 | c.Check(addr.Type, gc.Equals, instance.HostName) | 34 | c.Check(addr.Type, gc.Equals, HostName) |
131 | 32 | } | 35 | } |
132 | 33 | 36 | ||
133 | 34 | type selectTests struct { | 37 | type selectTests struct { |
134 | 35 | about string | 38 | about string |
136 | 36 | addresses []instance.Address | 39 | addresses []Address |
137 | 37 | expected string | 40 | expected string |
138 | 38 | } | 41 | } |
139 | 39 | 42 | ||
140 | 40 | var selectPublicTests = []selectTests{{ | 43 | var selectPublicTests = []selectTests{{ |
141 | 41 | "no addresses gives empty string result", | 44 | "no addresses gives empty string result", |
143 | 42 | []instance.Address{}, | 45 | []Address{}, |
144 | 43 | "", | 46 | "", |
145 | 44 | }, { | 47 | }, { |
146 | 45 | "a public address is selected", | 48 | "a public address is selected", |
149 | 46 | []instance.Address{ | 49 | []Address{ |
150 | 47 | {"8.8.8.8", instance.Ipv4Address, "public", instance.NetworkPublic}, | 50 | {"8.8.8.8", Ipv4Address, "public", NetworkPublic}, |
151 | 48 | }, | 51 | }, |
152 | 49 | "8.8.8.8", | 52 | "8.8.8.8", |
153 | 50 | }, { | 53 | }, { |
154 | 51 | "a machine local address is not selected", | 54 | "a machine local address is not selected", |
157 | 52 | []instance.Address{ | 55 | []Address{ |
158 | 53 | {"127.0.0.1", instance.Ipv4Address, "machine", instance.NetworkMachineLocal}, | 56 | {"127.0.0.1", Ipv4Address, "machine", NetworkMachineLocal}, |
159 | 54 | }, | 57 | }, |
160 | 55 | "", | 58 | "", |
161 | 56 | }, { | 59 | }, { |
162 | 57 | "an ipv6 address is not selected", | 60 | "an ipv6 address is not selected", |
165 | 58 | []instance.Address{ | 61 | []Address{ |
166 | 59 | {"2001:DB8::1", instance.Ipv6Address, "", instance.NetworkPublic}, | 62 | {"2001:DB8::1", Ipv6Address, "", NetworkPublic}, |
167 | 60 | }, | 63 | }, |
168 | 61 | "", | 64 | "", |
169 | 62 | }, { | 65 | }, { |
170 | 63 | "a public name is preferred to an unknown or cloud local address", | 66 | "a public name is preferred to an unknown or cloud local address", |
175 | 64 | []instance.Address{ | 67 | []Address{ |
176 | 65 | {"127.0.0.1", instance.Ipv4Address, "local", instance.NetworkUnknown}, | 68 | {"127.0.0.1", Ipv4Address, "local", NetworkUnknown}, |
177 | 66 | {"10.0.0.1", instance.Ipv4Address, "cloud", instance.NetworkCloudLocal}, | 69 | {"10.0.0.1", Ipv4Address, "cloud", NetworkCloudLocal}, |
178 | 67 | {"public.invalid.testing", instance.HostName, "public", instance.NetworkPublic}, | 70 | {"public.invalid.testing", HostName, "public", NetworkPublic}, |
179 | 68 | }, | 71 | }, |
180 | 69 | "public.invalid.testing", | 72 | "public.invalid.testing", |
181 | 70 | }, { | 73 | }, { |
182 | 71 | "last unknown address selected", | 74 | "last unknown address selected", |
186 | 72 | []instance.Address{ | 75 | []Address{ |
187 | 73 | {"10.0.0.1", instance.Ipv4Address, "cloud", instance.NetworkUnknown}, | 76 | {"10.0.0.1", Ipv4Address, "cloud", NetworkUnknown}, |
188 | 74 | {"8.8.8.8", instance.Ipv4Address, "floating", instance.NetworkUnknown}, | 77 | {"8.8.8.8", Ipv4Address, "floating", NetworkUnknown}, |
189 | 75 | }, | 78 | }, |
190 | 76 | "8.8.8.8", | 79 | "8.8.8.8", |
191 | 77 | }} | 80 | }} |
192 | @@ -79,43 +82,43 @@ | |||
193 | 79 | func (s *AddressSuite) TestSelectPublicAddressEmpty(c *gc.C) { | 82 | func (s *AddressSuite) TestSelectPublicAddressEmpty(c *gc.C) { |
194 | 80 | for i, t := range selectPublicTests { | 83 | for i, t := range selectPublicTests { |
195 | 81 | c.Logf("test %d. %s", i, t.about) | 84 | c.Logf("test %d. %s", i, t.about) |
197 | 82 | c.Check(instance.SelectPublicAddress(t.addresses), gc.Equals, t.expected) | 85 | c.Check(SelectPublicAddress(t.addresses), gc.Equals, t.expected) |
198 | 83 | } | 86 | } |
199 | 84 | } | 87 | } |
200 | 85 | 88 | ||
201 | 86 | var selectInternalTests = []selectTests{{ | 89 | var selectInternalTests = []selectTests{{ |
202 | 87 | "no addresses gives empty string result", | 90 | "no addresses gives empty string result", |
204 | 88 | []instance.Address{}, | 91 | []Address{}, |
205 | 89 | "", | 92 | "", |
206 | 90 | }, { | 93 | }, { |
207 | 91 | "a public address is selected", | 94 | "a public address is selected", |
210 | 92 | []instance.Address{ | 95 | []Address{ |
211 | 93 | {"8.8.8.8", instance.Ipv4Address, "public", instance.NetworkPublic}, | 96 | {"8.8.8.8", Ipv4Address, "public", NetworkPublic}, |
212 | 94 | }, | 97 | }, |
213 | 95 | "8.8.8.8", | 98 | "8.8.8.8", |
214 | 96 | }, { | 99 | }, { |
215 | 97 | "a cloud local address is selected", | 100 | "a cloud local address is selected", |
218 | 98 | []instance.Address{ | 101 | []Address{ |
219 | 99 | {"10.0.0.1", instance.Ipv4Address, "private", instance.NetworkCloudLocal}, | 102 | {"10.0.0.1", Ipv4Address, "private", NetworkCloudLocal}, |
220 | 100 | }, | 103 | }, |
221 | 101 | "10.0.0.1", | 104 | "10.0.0.1", |
222 | 102 | }, { | 105 | }, { |
223 | 103 | "a machine local address is not selected", | 106 | "a machine local address is not selected", |
226 | 104 | []instance.Address{ | 107 | []Address{ |
227 | 105 | {"127.0.0.1", instance.Ipv4Address, "machine", instance.NetworkMachineLocal}, | 108 | {"127.0.0.1", Ipv4Address, "machine", NetworkMachineLocal}, |
228 | 106 | }, | 109 | }, |
229 | 107 | "", | 110 | "", |
230 | 108 | }, { | 111 | }, { |
231 | 109 | "ipv6 addresses are not selected", | 112 | "ipv6 addresses are not selected", |
234 | 110 | []instance.Address{ | 113 | []Address{ |
235 | 111 | {"::1", instance.Ipv6Address, "", instance.NetworkCloudLocal}, | 114 | {"::1", Ipv6Address, "", NetworkCloudLocal}, |
236 | 112 | }, | 115 | }, |
237 | 113 | "", | 116 | "", |
238 | 114 | }, { | 117 | }, { |
239 | 115 | "a cloud local address is preferred to a public address", | 118 | "a cloud local address is preferred to a public address", |
243 | 116 | []instance.Address{ | 119 | []Address{ |
244 | 117 | {"10.0.0.1", instance.Ipv4Address, "cloud", instance.NetworkCloudLocal}, | 120 | {"10.0.0.1", Ipv4Address, "cloud", NetworkCloudLocal}, |
245 | 118 | {"8.8.8.8", instance.Ipv4Address, "public", instance.NetworkPublic}, | 121 | {"8.8.8.8", Ipv4Address, "public", NetworkPublic}, |
246 | 119 | }, | 122 | }, |
247 | 120 | "10.0.0.1", | 123 | "10.0.0.1", |
248 | 121 | }} | 124 | }} |
249 | @@ -123,20 +126,20 @@ | |||
250 | 123 | func (s *AddressSuite) TestSelectInternalAddress(c *gc.C) { | 126 | func (s *AddressSuite) TestSelectInternalAddress(c *gc.C) { |
251 | 124 | for i, t := range selectInternalTests { | 127 | for i, t := range selectInternalTests { |
252 | 125 | c.Logf("test %d. %s", i, t.about) | 128 | c.Logf("test %d. %s", i, t.about) |
254 | 126 | c.Check(instance.SelectInternalAddress(t.addresses, false), gc.Equals, t.expected) | 129 | c.Check(SelectInternalAddress(t.addresses, false), gc.Equals, t.expected) |
255 | 127 | } | 130 | } |
256 | 128 | } | 131 | } |
257 | 129 | 132 | ||
258 | 130 | var selectInternalMachineTests = []selectTests{{ | 133 | var selectInternalMachineTests = []selectTests{{ |
259 | 131 | "a cloud local address is selected", | 134 | "a cloud local address is selected", |
262 | 132 | []instance.Address{ | 135 | []Address{ |
263 | 133 | {"10.0.0.1", instance.Ipv4Address, "cloud", instance.NetworkCloudLocal}, | 136 | {"10.0.0.1", Ipv4Address, "cloud", NetworkCloudLocal}, |
264 | 134 | }, | 137 | }, |
265 | 135 | "10.0.0.1", | 138 | "10.0.0.1", |
266 | 136 | }, { | 139 | }, { |
267 | 137 | "a machine local address is selected", | 140 | "a machine local address is selected", |
270 | 138 | []instance.Address{ | 141 | []Address{ |
271 | 139 | {"127.0.0.1", instance.Ipv4Address, "container", instance.NetworkMachineLocal}, | 142 | {"127.0.0.1", Ipv4Address, "container", NetworkMachineLocal}, |
272 | 140 | }, | 143 | }, |
273 | 141 | "127.0.0.1", | 144 | "127.0.0.1", |
274 | 142 | }} | 145 | }} |
275 | @@ -144,6 +147,43 @@ | |||
276 | 144 | func (s *AddressSuite) TestSelectInternalMachineAddress(c *gc.C) { | 147 | func (s *AddressSuite) TestSelectInternalMachineAddress(c *gc.C) { |
277 | 145 | for i, t := range selectInternalMachineTests { | 148 | for i, t := range selectInternalMachineTests { |
278 | 146 | c.Logf("test %d. %s", i, t.about) | 149 | c.Logf("test %d. %s", i, t.about) |
281 | 147 | c.Check(instance.SelectInternalAddress(t.addresses, true), gc.Equals, t.expected) | 150 | c.Check(SelectInternalAddress(t.addresses, true), gc.Equals, t.expected) |
282 | 148 | } | 151 | } |
283 | 152 | } | ||
284 | 153 | |||
285 | 154 | func (*AddressSuite) TestHostAddresses(c *gc.C) { | ||
286 | 155 | // Mock the call to net.LookupIP made from HostAddresses. | ||
287 | 156 | var lookupIPs []net.IP | ||
288 | 157 | var lookupErr error | ||
289 | 158 | lookupIP := func(addr string) ([]net.IP, error) { | ||
290 | 159 | return append([]net.IP{}, lookupIPs...), lookupErr | ||
291 | 160 | } | ||
292 | 161 | defer jc.Set(&netLookupIP, lookupIP).Restore() | ||
293 | 162 | |||
294 | 163 | // err is only non-nil if net.LookupIP fails. | ||
295 | 164 | addrs, err := HostAddresses("") | ||
296 | 165 | c.Assert(err, gc.IsNil) | ||
297 | 166 | // addrs always contains the input address. | ||
298 | 167 | c.Assert(addrs, gc.HasLen, 1) | ||
299 | 168 | c.Assert(addrs[0], gc.Equals, NewAddress("")) | ||
300 | 169 | |||
301 | 170 | loopback := net.ParseIP("127.0.0.1").To4() | ||
302 | 171 | lookupIPs = []net.IP{net.IPv6loopback, net.IPv4zero, loopback} | ||
303 | 172 | addrs, err = HostAddresses("localhost") | ||
304 | 173 | c.Assert(err, gc.IsNil) | ||
305 | 174 | c.Assert(addrs, gc.HasLen, 4) | ||
306 | 175 | c.Assert(addrs[0], gc.Equals, NewAddress(net.IPv6loopback.String())) | ||
307 | 176 | c.Assert(addrs[1], gc.Equals, NewAddress(net.IPv4zero.String())) | ||
308 | 177 | c.Assert(addrs[2], gc.Equals, NewAddress(loopback.String())) | ||
309 | 178 | c.Assert(addrs[3], gc.Equals, NewAddress("localhost")) | ||
310 | 179 | |||
311 | 180 | lookupErr = errors.New("what happened?") | ||
312 | 181 | addrs, err = HostAddresses("localhost") | ||
313 | 182 | c.Assert(err, gc.Equals, lookupErr) | ||
314 | 183 | |||
315 | 184 | // If the input address is an IP, the call to net.LookupIP is elided. | ||
316 | 185 | addrs, err = HostAddresses("127.0.0.1") | ||
317 | 186 | c.Assert(err, gc.IsNil) | ||
318 | 187 | c.Assert(addrs, gc.HasLen, 1) | ||
319 | 188 | c.Assert(addrs[0], gc.Equals, NewAddress("127.0.0.1")) | ||
320 | 149 | } | 189 | } |
Reviewers: mp+186229_ code.launchpad. net,
Message:
Please take a look.
Description:
environs/manual: reverse lookup target IP
Thus we may resolve additional addresses for the
machine. Also, we should get a consistent machine ID,
whether a name or an IP is specified.
Also, modify instance/ HostAddresses to avoid passing
an IP address into net.LookupIP. This causes an error
if built with CGO_ENABLED=0.
Fixes #1225825
https:/ /code.launchpad .net/~axwalk/ juju-core/ lp1225825- netlookupip- ip/+merge/ 186229
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/13504044/
Affected files (+23, -2 lines): manual/ provisioner. go
A [revision details]
M environs/
M instance/address.go
Index: [revision details] 20130917082542- j1hu82fubd89omb o
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-
+New revision: <email address hidden>
Index: instance/address.go address. go'
=== modified file 'instance/
--- instance/address.go 2013-09-04 15:11:51 +0000
+++ instance/address.go 2013-09-18 02:36:52 +0000
@@ -61,12 +61,17 @@
// HostAddresses looks up the IP addresses of the specified hostAddr} , nil
// host, and translates them into instance.Address values.
-func HostAddresses(host string) ([]Address, error) {
+func HostAddresses(host string) (addrs []Address, err error) {
+ hostAddr := NewAddress(host)
+ if hostAddr.Type != HostName {
+ // IPs shouldn't be fed into LookupIP.
+ return []Address{
+ }
ipaddrs, err := net.LookupIP(host)
if err != nil {
return nil, err
}
- addrs := make([]Address, len(ipaddrs))
+ addrs = make([]Address, len(ipaddrs)+1)
for i, ipaddr := range ipaddrs {
switch len(ipaddr) {
case 4:
@@ -77,6 +82,7 @@
addrs[i].Value = ipaddr.String()
}
}
+ addrs[len(addrs)-1] = hostAddr
return addrs, err
}
Index: environs/ manual/ provisioner. go manual/ provisioner. go' manual/ provisioner. go 2013-09-03 01:56:25 +0000 manual/ provisioner. go 2013-09-18 02:36:52 +0000
=== modified file 'environs/
--- environs/
+++ environs/
@@ -6,6 +6,7 @@
import (
"errors"
"fmt"
+ "net"
"strings"
"launchpad. net/loggo" Index(sshHostWi thoutUser, "@"); at != -1 { outUser = sshHostWithoutU ser[at+ 1:] sshHostWithoutU ser); ip != nil { ip.String( )) Warningf( "failed to resolve %v: %v", ip, err) Infof(" resolved %v to %v", ip, names) HostAddresses( sshHostWithoutU ser) Infof(" addresses for %v: %v", sshHostWithoutUser, addrs)
@@ -71,10 +72,22 @@
if at := strings.
sshHostWith
}
+ if ip := net.ParseIP(
+ // Do a reverse-lookup on the IP. The IP may not have
+ // a DNS entry, so just log a warning if this fails.
+ names, err := net.LookupAddr(
+ if err != nil {
+ logger.
+ } else {
+ logger.
+ sshHostWithoutUser = names[0]
+ }
+ }
addrs, err := instance.
if err != nil {
return nil, err
}
+ logger.
provisioned, err := checkProvisione d(args. Host)
if err != nil {