Code review comment for lp:~axwalk/juju-core/lp1225825-netlookupip-ip

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):
   A [revision details]
   M environs/manual/provisioner.go
   M instance/address.go

Index: [revision details]
=== 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-20130917082542-j1hu82fubd89ombo
+New revision: <email address hidden>

Index: instance/address.go
=== modified file 'instance/address.go'
--- 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
  // 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{hostAddr}, nil
+ }
   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
=== modified file 'environs/manual/provisioner.go'
--- environs/manual/provisioner.go 2013-09-03 01:56:25 +0000
+++ environs/manual/provisioner.go 2013-09-18 02:36:52 +0000
@@ -6,6 +6,7 @@
  import (
   "errors"
   "fmt"
+ "net"
   "strings"

   "launchpad.net/loggo"
@@ -71,10 +72,22 @@
   if at := strings.Index(sshHostWithoutUser, "@"); at != -1 {
    sshHostWithoutUser = sshHostWithoutUser[at+1:]
   }
+ if ip := net.ParseIP(sshHostWithoutUser); ip != nil {
+ // 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(ip.String())
+ if err != nil {
+ logger.Warningf("failed to resolve %v: %v", ip, err)
+ } else {
+ logger.Infof("resolved %v to %v", ip, names)
+ sshHostWithoutUser = names[0]
+ }
+ }
   addrs, err := instance.HostAddresses(sshHostWithoutUser)
   if err != nil {
    return nil, err
   }
+ logger.Infof("addresses for %v: %v", sshHostWithoutUser, addrs)

   provisioned, err := checkProvisioned(args.Host)
   if err != nil {