nova: get/set server addresses

nova: get/set server addresses

Implemented GetServerAddresses() Nova API call to retrieve a server's
private and public addresses. Did the same for the service double - only
reading supported through HTTP, but that's what we need.

This is needed in order to implement DNSName() and WaitDNSName() in the
juju-core/environs/openstack provider.

https://code.launchpad.net/~dimitern/goose/nova-get-server-addresses/+merge/144537

(do not edit description out of merge proposal)

[dimitern, 2013-01-23 17:19:34]

Please take a look.

[rog, 2013-01-23 18:03:27]

LGTM with one minor thought below, although i can't say anything about how
appropriate the actual heuristics are.

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
testservices/novaservice/service_http.go:634: serverId := ""
how about:
prefix := path.Dir(r.URL.Path)

then below, you can do:
serverId = path.Base(prefix)

and avoid the Replace?

[gz, 2013-01-23 18:29:31]

Strictly, a using a new api should not be needed, as juju will be using
/servers/{UUID} and /servers/detail which already give currently assigned
addresses for servers.

You noted on IRC that /ips seems to return something useful earlier during BUILD
though, and overall adding support for that is reasonable. However, I think a
good chunk of the code here should actually live in juju-core, and the api call
should just return a struct that reflects the json openstack gives.

https://codereview.appspot.com/7195046/diff/1/nova/nova.go
File nova/nova.go (right):

https://codereview.appspot.com/7195046/diff/1/nova/nova.go#newcode218
nova/nova.go:218: func (c *Client) GetServerAddresses(serverId string) (private,
public string, err error) {
This logic should not be baked into goose. It's juju's problem that it expects a
exactly two, unchanging, addresses 'public' and 'private', we don't want to
hobble what information this api can provide.

Having a separate function in juju that translates from the Openstack view of
addresses to the current juju view would also mean you can straight port the
nice (actual unit) tests from python juju that verify the sanity of the
translation logic.

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
File testservices/novaservice/service.go (right):

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
testservices/novaservice/service.go:285: func (n *Nova)
setServerAddresses(serverId, private, public string) error {
This needs to accept a list of structs, not two strings.

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
testservices/novaservice/service.go:295: func (n *Nova)
getServerAddresses(serverId string) (private, public string, err error) {
This needs to return a list of structs, not two strings.

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
testservices/novaservice/service_http.go:581: if err := n.setServerAddresses(id,
"10.99.88.77", "212.33.44.55"); err != nil {
Wants updating with the other changes. I'd use something well-known and ideally
unassigned for the non-10.* address so if it's accidentally resolved in a test,
we don't send packets to a random server.

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
testservices/novaservice/service_http.go:613: private, public, err :=
n.getServerAddresses(serverId)
Simplify to just return the response (list of structs) as properly encoded json.

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
File testservices/novaservice/service_http_test.go (right):

https://codereview.appspot.com/7195046/diff/1/testservices/novaservice/servic...
testservices/novaservice/service_http_test.go:605: err =
s.service.setServerAddresses(server.Id, "1.2.3.4", "5.6.7.8")
Same note about being careful about what random IPs you pick to use in tests.

[dimitern, 2013-01-24 14:05:31]

As discussed, I'm abandoning this approach and will implement a similar behavior
using the servers/detail API, parsing the addresses as needed. Juju will handle
the logic of selecting private and public address. So I'm closing this.