openstack: (Wait)DNSName() implemented

openstack: (Wait)DNSName() implemented

This is a follow up of https://codereview.appspot.com/7228058/.
Implements WaitDNSName() and DNSName() calls in the OpenStack provider, using the
map of networks to IP addresses, correctly extracting the correct IP address from it.
See getInstanceAddress() - logic ported from py-juju's openstack provider, along with
the needed tests to check all real-world cases of OS. Modified the behavior, as discussed
to use the public address if available.

https://code.launchpad.net/~dimitern/juju-core/openstack-server-addresses/+merge/145340

(do not edit description out of merge proposal)

[dimitern, 2013-01-29 10:04:31]

Please take a look.

[rog, 2013-01-29 10:44:42]

LGTM with one trivial remark below, but i can't speak more than superficially.

https://codereview.appspot.com/7241045/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/7241045/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:117: func GetPrivateAddress(addresses
map[string][]nova.IPAddress) (string, error) {
should this really be exported?

https://codereview.appspot.com/7241045/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:120: if address.Version == 4 {
if address.Version == 4 && network != "public" {

?

or, perhaps, just after the "for network" line:

if network == "public" {
   continue
}

and lose the test in the inner loop.

[dimitern, 2013-01-29 15:08:31]

Please take a look.

https://codereview.appspot.com/7241045/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/7241045/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:117: func GetPrivateAddress(addresses
map[string][]nova.IPAddress) (string, error) {
On 2013/01/29 10:44:42, rog wrote:
> should this really be exported?

Done, as agreed - through export_test.

https://codereview.appspot.com/7241045/diff/1/environs/openstack/provider.go#...
environs/openstack/provider.go:120: if address.Version == 4 {
On 2013/01/29 10:44:42, rog wrote:
> if address.Version == 4 && network != "public" {
> 
> ?
> 
> or, perhaps, just after the "for network" line:
> 
> if network == "public" {
>    continue
> }
> 
> and lose the test in the inner loop.

Changed to reflect what we discussed - also returns public if possible, private
if not.

[dimitern, 2013-01-29 15:08:34]

Changes done as discussed.

[gz, 2013-01-29 15:53:10]

LGTM, bar my whining about the tests. :)

https://codereview.appspot.com/7241045/diff/4001/environs/openstack/local_tes...
File environs/openstack/local_test.go (right):

https://codereview.appspot.com/7241045/diff/4001/environs/openstack/local_tes...
environs/openstack/local_test.go:153: private:  []nova.IPAddress{{6,
"::dead:beef:f00d"}},
Use ::1 here, it's valid and safe.

https://codereview.appspot.com/7241045/diff/4001/environs/openstack/local_tes...
environs/openstack/local_test.go:172: if len(t.networks) < 2 {
This encodes specific logic from the implementation in the test, I'd prefer a
straight "this is the result from the function, which matches what the test data
expects" approach, but it's not too bad I guess.

[dimitern, 2013-01-29 16:13:32]

*** Submitted:

openstack: (Wait)DNSName() implemented

This is a follow up of https://codereview.appspot.com/7228058/.
Implements WaitDNSName() and DNSName() calls in the OpenStack provider, using
the
map of networks to IP addresses, correctly extracting the correct IP address
from it.
See getInstanceAddress() - logic ported from py-juju's openstack provider, along
with
the needed tests to check all real-world cases of OS. Modified the behavior, as
discussed
to use the public address if available.

R=rog, gz
CC=
https://codereview.appspot.com/7241045